Merge branch 'fix-jwks-refresh' into 'v2.0'

Improve JWKS refresh

See merge request !679

lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/OpenIDConnect.pm

@@ -337,7 +337,7 @@ sub refreshJWKSdata {
 }
 
 sub refreshJWKSdataForOp {
-    my ( $self, $op ) = @_;
+    my ( $self, $op, $force ) = @_;
 
     $self->logger->debug("Attempting to refresh JWKS data for $op");
 
@@ -349,22 +349,27 @@ sub refreshJWKSdataForOp {
       $self->opOptions->{$op}->{oidcOPMetaDataOptionsJWKSTimeout};
     my $jwksUri = $self->opMetadata->{$op}->{conf}->{jwks_uri};
 
-    unless ($jwksTimeout) {
-        $self->logger->debug(
-            "No JWKS refresh timeout defined for $op, skipping...");
-        return;
-    }
-
     unless ($jwksUri) {
         $self->logger->debug("No JWKS URI defined for $op, skipping...");
         return;
     }
 
-    if ( $self->opMetadata->{$op}->{jwks}->{time}
-        && ( $self->opMetadata->{$op}->{jwks}->{time} + $jwksTimeout > time ) )
-    {
-        $self->logger->debug("JWKS data still valid for $op, skipping...");
-        return;
+    if ( !$force ) {
+        unless ($jwksTimeout) {
+            $self->logger->debug(
+                "No JWKS refresh timeout defined for $op, skipping...");
+            return;
+        }
+
+        if (
+            $self->opMetadata->{$op}->{jwks}->{time}
+            && (
+                $self->opMetadata->{$op}->{jwks}->{time} + $jwksTimeout > time )
+          )
+        {
+            $self->logger->debug("JWKS data still valid for $op, skipping...");
+            return;
+        }
     }
 
     $self->logger->debug("Refresh JWKS data for $op from $jwksUri");
@@ -1540,7 +1545,25 @@ sub decodeJWT {
 
     my $jwks;
     if ($op) {
+
+        # Always refresh JWKS if timeout has elapsed
         $self->refreshJWKSdataForOp($op);
+
+        my $kid = $jwt_header->{kid};
+
+        # If the JWT is signed by an unknown kid, force a refresh
+        if (
+            $kid
+            and !$self->_kid_found_in_jwks(
+                $kid, $self->opMetadata->{$op}->{jwks}
+            )
+          )
+        {
+            $self->logger->debug(
+                "Key ID $kid not found in current JWKS, forcing JWKS refresh");
+            $self->refreshJWKSdataForOp( $op, 1 );
+        }
+
         $jwks = $self->opMetadata->{$op}->{jwks};
     }
     else {
@@ -1615,6 +1638,18 @@ sub decodeJWT {
     return wantarray ? ( $content, $alg ) : $content;
 }
 
+sub _kid_found_in_jwks {
+    my ( $self, $kid, $jwks ) = @_;
+
+    return 0 if !$kid;
+
+    my @keys = $jwks ? @{ $jwks->{keys} // [] } : ();
+
+    my @found = grep { $_->{kid} and $_->{kid} eq $kid } @keys;
+
+    return @found > 0;
+}
+
 ### HERE
 
 # Check value hash

lemonldap-ng-portal/t/32-Auth-OIDC-JWKS-Refresh.t

@@ -80,6 +80,41 @@ LWP::Protocol::PSGI->register(
     }
 );
 
+sub tryauth {
+    my ($rp) = @_;
+    ok( my $res = $rp->_get( '/', accept => 'text/html' ),
+        'Unauth SP request' );
+    my ($url) =
+      expectRedirection( $res,
+        qr#(https://op.example.com/oauth2/authorize\?.*)# );
+    $url = URI->new($url);
+    is( $url->host, "op.example.com", "Correct host" );
+    my %query = $url->query_form;
+    is( $query{client_id}, 'rpid',                 "Correct client_id" );
+    is( $query{scope},     'openid profile email', "Correct scope" );
+    is(
+        $query{redirect_uri},
+        'http://auth.rp.com/?openidconnectcallback=1',
+        "Correct redirect_uri"
+    );
+    ok( my $state = $query{state}, "Found state" );
+
+    # Post return authorization code
+    ok(
+        $res = $rp->_get(
+            '/',
+            query => {
+                openidconnectcallback => 1,
+                code                  => "aaa",
+                state                 => $state,
+            },
+            accept => 'text/html'
+        ),
+        'Authorization code'
+    );
+    return $res;
+}
+
 my $metadata = <<EOF;
 {
   "authorization_endpoint": "https://op.example.com/oauth2/authorize",
@@ -95,69 +130,31 @@ $main::jwks_show_kid   = 0;
 my $rp = rp($metadata);
 is( $main::jwks_call_count, 1, "JWKS url was called during startup" );
 
-ok( my $res = $rp->_get( '/', accept => 'text/html' ), 'Unauth SP request' );
-
-my ($url) =
-  expectRedirection( $res, qr#(https://op.example.com/oauth2/authorize\?.*)# );
-$url = URI->new($url);
-is( $url->host, "op.example.com", "Correct host" );
-my %query = $url->query_form;
-is( $query{client_id}, 'rpid',                 "Correct client_id" );
-is( $query{scope},     'openid profile email', "Correct scope" );
-is(
-    $query{redirect_uri},
-    'http://auth.rp.com/?openidconnectcallback=1',
-    "Correct redirect_uri"
-);
-ok( my $state = $query{state}, "Found state" );
-
-# Post return authorization code
-ok(
-    $res = $rp->_get(
-        '/',
-        query => {
-            openidconnectcallback => 1,
-            code                  => "aaa",
-            state                 => $state,
-        },
-        accept => 'text/html'
-    ),
-    'Authorization code'
-);
+# Try to authenticate with a token containing a kid that is not found in jwks
+my $res = tryauth($rp);
 expectPortalError( $res, 106 );
+is( $main::jwks_call_count, 2, "JWKS refresh was forced due to wrong kid" );
 
-Time::Fake->offset("+600s");
+# Update OP's JWKS to publish the correct kid
 $main::jwks_show_kid = 1;
 
-ok( $res = $rp->_get( '/', accept => 'text/html' ), 'Unauth SP request' );
-($url) =
-  expectRedirection( $res, qr#(https://op.example.com/oauth2/authorize\?.*)# );
-$url = URI->new($url);
-is( $url->host, "op.example.com", "Correct host" );
-%query = $url->query_form;
-is( $query{client_id}, 'rpid',                 "Correct client_id" );
-is( $query{scope},     'openid profile email', "Correct scope" );
-is(
-    $query{redirect_uri},
-    'http://auth.rp.com/?openidconnectcallback=1',
-    "Correct redirect_uri"
-);
-ok( $state = $query{state}, "Found state" );
-
-ok(
-    $res = $rp->_get(
-        '/',
-        query => {
-            openidconnectcallback => 1,
-            code                  => "aaa",
-            state                 => $state,
-        },
-        accept => 'text/html'
-    ),
-    'Authorization code'
-);
-is( $main::jwks_call_count, 2, "JWKS url was called again" );
+# LemonLDAP immediately refreshes its JWKS
+$res = tryauth($rp);
+expectCookie($res);
+is( $main::jwks_call_count, 3, "JWKS refresh was forced due to wrong kid" );
+
+# The next attempt does not trigger a refresh
+$res = tryauth($rp);
+expectCookie($res);
+is( $main::jwks_call_count, 3, "JWKS url was not called again" );
+
+# After cache expiration, the next attemps triggers a refresh
+Time::Fake->offset("+600s");
+
+$res = tryauth($rp);
 expectCookie($res);
+is( $main::jwks_call_count, 4,
+    "JWKS url was called again due to cache expiration" );
 
 clean_sessions();
 done_testing();