From e50db3f08388e4f1e57dd7887eee3b96443a3919 Mon Sep 17 00:00:00 2001
From: Maxime Besson <maxime.besson@worteks.com>
Date: Sat, 1 May 2021 20:45:24 +0200
Subject: [PATCH] Skip registration of OIDC RP when config has errors (#2525)

---
 .../Lemonldap/NG/Portal/Lib/OpenIDConnect.pm  | 65 +++++++++++++------
 1 file changed, 44 insertions(+), 21 deletions(-)

diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/OpenIDConnect.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/OpenIDConnect.pm
index e843a390b..a2aa61cb9 100644
--- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/OpenIDConnect.pm
+++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/OpenIDConnect.pm
@@ -105,8 +105,11 @@ sub loadRPs {
             "No OpenID Connect Relying Party found in configuration");
         return 1;
     }
-    $self->oidcRPList( $self->conf->{oidcRPMetaDataOptions} );
-    foreach my $rp ( keys %{ $self->oidcRPList } ) {
+
+    foreach my $rp ( keys %{ $self->conf->{oidcRPMetaDataOptions} || {} } ) {
+        my $valid = 1;
+
+        # Handle attributes
         my $attributes = {
             profile => PROFILE,
             email   => EMAIL,
@@ -125,50 +128,70 @@ sub loadRPs {
                 $attributes->{$claim} = \@extraAttributes;
             }
         }
-        $self->rpAttributes->{$rp} = $attributes;
 
-        my $rule = $self->oidcRPList->{$rp}->{oidcRPMetaDataOptionsRule};
+        # Access rule
+        my $rule = $self->conf->{oidcRPMetaDataOptions}->{$rp}
+          ->{oidcRPMetaDataOptionsRule};
         if ( length $rule ) {
             $rule = $self->p->HANDLER->substitute($rule);
             unless ( $rule = $self->p->HANDLER->buildSub($rule) ) {
-                $self->error( 'OIDC RP rule error: '
+                $self->logger->error( "Unable to build access rule for RP $rp: "
                       . $self->p->HANDLER->tsv->{jail}->error );
-                return 0;
+                $valid = 0;
             }
-            $self->spRules->{$rp} = $rule;
         }
 
         # Load per-RP macros
-        my $macros = $self->conf->{oidcRPMetaDataMacros}->{$rp};
+        my $macros         = $self->conf->{oidcRPMetaDataMacros}->{$rp};
+        my $compiledMacros = {};
         for my $macroAttr ( keys %{$macros} ) {
             my $macroRule = $macros->{$macroAttr};
             if ( length $macroRule ) {
                 $macroRule = $self->p->HANDLER->substitute($macroRule);
-                unless ( $macroRule = $self->p->HANDLER->buildSub($macroRule) )
-                {
-                    $self->error( 'OIDC RP macro error: '
-                          . $self->p->HANDLER->tsv->{jail}->error );
-                    return 0;
+                if ( $macroRule = $self->p->HANDLER->buildSub($macroRule) ) {
+                    $compiledMacros->{$macroAttr} = $macroRule;
+                }
+                else {
+                    $self->logger->error(
+                        "Unable to build macro $macroAttr for RP $rp:"
+                          . $self->p->HANDLER->tsv->{jail}->error );
+                    $valid = 0;
                 }
-                $self->spMacros->{$rp}->{$macroAttr} = $macroRule;
             }
         }
 
         # Load per-RP dynamic scopes
-        my $scopes = $self->conf->{oidcRPMetaDataScopeRules}->{$rp};
+        my $scopes         = $self->conf->{oidcRPMetaDataScopeRules}->{$rp};
+        my $compiledScopes = {};
         for my $scopeName ( keys %{$scopes} ) {
             my $scopeRule = $scopes->{$scopeName};
             if ( length $scopeRule ) {
                 $scopeRule = $self->p->HANDLER->substitute($scopeRule);
-                unless ( $scopeRule = $self->p->HANDLER->buildSub($scopeRule) )
-                {
-                    $self->error( 'OIDC RP dynamic scope rule error: '
-                          . $self->p->HANDLER->tsv->{jail}->error );
-                    return 0;
+                if ( $scopeRule = $self->p->HANDLER->buildSub($scopeRule) ) {
+                    $compiledScopes->{$scopeName} = $scopeRule;
+                }
+                else {
+                    $self->logger->error(
+                        "Unable to build scope $scopeName for RP $rp:"
+                          . $self->p->HANDLER->tsv->{jail}->error );
+                    $valid = 0;
                 }
-                $self->spScopeRules->{$rp}->{$scopeName} = $scopeRule;
             }
         }
+        if ($valid) {
+
+            # Register RP
+            $self->oidcRPList->{$rp} =
+              $self->conf->{oidcRPMetaDataOptions}->{$rp};
+            $self->rpAttributes->{$rp} = $attributes;
+            $self->spMacros->{$rp}     = $compiledMacros;
+            $self->spScopeRules->{$rp} = $compiledScopes;
+            $self->spRules->{$rp}      = $rule;
+        }
+        else {
+            $self->logger->error(
+                "Relaying Party $rp has errors and will be ignored");
+        }
     }
     return 1;
 }