From f8ee867f40452ac6004863cd0c1da5ca96c468f9 Mon Sep 17 00:00:00 2001 From: Maxime Besson Date: Thu, 14 Mar 2024 10:18:13 +0100 Subject: [PATCH 1/2] Unit test for #3123 --- .../Lemonldap/NG/Portal/Lib/OpenIDConnect.pm | 8 +- .../t/32-Auth-OIDC-JWKS-Refresh.t | 206 ++++++++++++++++++ 2 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 lemonldap-ng-portal/t/32-Auth-OIDC-JWKS-Refresh.t 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 ea9253f6e0..dfbe0db2f3 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/OpenIDConnect.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/OpenIDConnect.pm @@ -8,7 +8,7 @@ package Lemonldap::NG::Portal::Lib::OpenIDConnect; use strict; use Crypt::OpenSSL::RSA; use Crypt::OpenSSL::X509; -use Crypt::JWT qw(encode_jwt decode_jwt); +use Crypt::JWT qw(encode_jwt decode_jwt); use Digest::SHA qw/sha1 hmac_sha256_base64 sha256 sha384 sha512 sha256_base64/; use JSON; use Lemonldap::NG::Common::FormEncode; @@ -331,7 +331,11 @@ sub refreshJWKSdata { next; } - if ( $self->opMetadata->{$_}->{jwks}->{time} + $jwksTimeout > time ) { + if ( + $self->opMetadata->{$_}->{jwks}->{time} + && ( $self->opMetadata->{$_}->{jwks}->{time} + $jwksTimeout > time ) + ) + { $self->logger->debug("JWKS data still valid for $_, skipping..."); next; } diff --git a/lemonldap-ng-portal/t/32-Auth-OIDC-JWKS-Refresh.t b/lemonldap-ng-portal/t/32-Auth-OIDC-JWKS-Refresh.t new file mode 100644 index 0000000000..c6d7f48b84 --- /dev/null +++ b/lemonldap-ng-portal/t/32-Auth-OIDC-JWKS-Refresh.t @@ -0,0 +1,206 @@ +use warnings; +use Test::More; +use strict; +use IO::String; +use LWP::UserAgent; +use LWP::Protocol::PSGI; +use MIME::Base64; +use URI::QueryParam; +use Plack::Request; +use Plack::Response; +use JSON; +use URI; +use Crypt::JWT qw(encode_jwt); + +BEGIN { + require 't/test-lib.pm'; + require 't/oidc-lib.pm'; +} + +my $access_token; + +use Lemonldap::NG::Portal::Lib::OpenIDConnect; + +my $jwk = + Lemonldap::NG::Portal::Lib::OpenIDConnect->key2jwks(oidc_key_op_private_sig); + +LWP::Protocol::PSGI->register( + sub { + my $req = Plack::Request->new(@_); + note "Internal request to " . $req->path; + + if ( $req->path eq "/oauth2/token" ) { + is( $req->parameters->{client_id}, "rpid", "expected client_id" ); + is( $req->parameters->{client_secret}, + "rpsecret", "expected client_secret" ); + is( + $req->parameters->{redirect_uri}, + "http://auth.rp.com/?openidconnectcallback=1", + "expected redirect_uri" + ); + is( $req->parameters->{code}, "aaa", "expected code" ); + + my $key = oidc_key_op_private_sig; + my $response = { + token_type => "Bearer", + access_token => "abc", + expired_in => 3600, + id_token => encode_jwt( + payload => { + + iss => "https://op.example.com/", + aud => "rpid", + exp => time + 1000, + sub => "dwho", + at_hash => "ungWv48Bz-pBQUDeXa4iIw", + }, + alg => "RS256", + key => \$key, + extra_headers => { kid => "mykid" } + ), + }; + return Plack::Response->new( "200", + { "Content-Type" => "application/json" }, + encode_json($response) )->finalize; + } + + if ( $req->path eq "/oauth2/jwks" ) { + $main::jwks_call_count += 1; + + my $kid = $main::jwks_show_kid ? "mykid" : "wrongkid"; + my $jwks = { keys => [ { kid => $kid, %$jwk } ] }; + return Plack::Response->new( "200", + { "Content-Type" => "application/json" }, + encode_json($jwks) )->finalize; + } + + my $res = Plack::Response->new; + $res->status(500); + return $res->finalize; + } +); + +my $metadata = <_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' +); +expectPortalError( $res, 106 ); + +Time::Fake->offset("+600s"); +$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" ); +expectCookie($res); + +clean_sessions(); +done_testing(); + +sub rp { + my ($metadata) = @_; + return LLNG::Manager::Test->new( { + ini => { + domain => 'rp.com', + portal => 'http://auth.rp.com/', + authentication => 'OpenIDConnect', + userDB => 'Same', + restSessionServer => 1, + restExportSecretKeys => 1, + oidcOPMetaDataExportedVars => { + op => { + cn => "name", + uid => "sub", + sn => "family_name", + mail => "email", + groups => "groups", + } + }, + oidcOPMetaDataOptions => { + op => { + oidcOPMetaDataOptionsCheckJWTSignature => 1, + oidcOPMetaDataOptionsJWKSTimeout => 100, + oidcOPMetaDataOptionsClientSecret => "rpsecret", + oidcOPMetaDataOptionsScope => "openid profile email", + oidcOPMetaDataOptionsStoreIDToken => 0, + oidcOPMetaDataOptionsDisplay => "", + oidcOPMetaDataOptionsClientID => "rpid", + oidcOPMetaDataOptionsStoreIDToken => 1, + oidcOPMetaDataOptionsUseNonce => 0, + oidcOPMetaDataOptionsConfigurationURI => + "https://auth.op.com/.well-known/openid-configuration" + } + }, + oidcOPMetaDataJSON => { + op => $metadata, + }, + customPlugins => 't::OidcHookPlugin', + } + } + ); +} -- GitLab From b355a4f1447317bdf3e3678f8f6dacc91362ea97 Mon Sep 17 00:00:00 2001 From: Maxime Besson Date: Fri, 15 Mar 2024 14:55:46 +0100 Subject: [PATCH 2/2] Fix #3123 --- .../Lemonldap/NG/Portal/Lib/OpenIDConnect.pm | 90 ++++++++++--------- 1 file changed, 50 insertions(+), 40 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 dfbe0db2f3..2c5e763c87 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/OpenIDConnect.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/OpenIDConnect.pm @@ -311,53 +311,59 @@ sub refreshJWKSdata { } foreach ( keys %{ $self->conf->{oidcOPMetaDataJSON} } ) { + $self->refreshJWKSdataForOp($_); + } + return 1; +} - # Refresh JWKS data if - # 1/ oidcOPMetaDataOptionsJWKSTimeout > 0 - # 2/ jwks_uri defined in metadata +sub refreshJWKSdataForOp { + my ( $self, $op ) = @_; - my $jwksTimeout = - $self->opOptions->{$_}->{oidcOPMetaDataOptionsJWKSTimeout}; - my $jwksUri = $self->opMetadata->{$_}->{conf}->{jwks_uri}; + $self->logger->debug("Attempting to refresh JWKS data for $op"); - unless ($jwksTimeout) { - $self->logger->debug( - "No JWKS refresh timeout defined for $_, skipping..."); - next; - } - - unless ($jwksUri) { - $self->logger->debug("No JWKS URI defined for $_, skipping..."); - next; - } + # Refresh JWKS data if + # 1/ oidcOPMetaDataOptionsJWKSTimeout > 0 + # 2/ jwks_uri defined in metadata - if ( - $self->opMetadata->{$_}->{jwks}->{time} - && ( $self->opMetadata->{$_}->{jwks}->{time} + $jwksTimeout > time ) - ) - { - $self->logger->debug("JWKS data still valid for $_, skipping..."); - next; - } + my $jwksTimeout = + $self->opOptions->{$op}->{oidcOPMetaDataOptionsJWKSTimeout}; + my $jwksUri = $self->opMetadata->{$op}->{conf}->{jwks_uri}; - $self->logger->debug("Refresh JWKS data for $_ from $jwksUri"); + unless ($jwksTimeout) { + $self->logger->debug( + "No JWKS refresh timeout defined for $op, skipping..."); + return; + } - my $response = $self->ua->get($jwksUri); + unless ($jwksUri) { + $self->logger->debug("No JWKS URI defined for $op, skipping..."); + return; + } - if ( $response->is_error ) { - $self->logger->warn( - "Unable to get JWKS data for $_ from $jwksUri: " - . $response->message ); - $self->logger->debug( $response->content ); - next; - } + if ( $self->opMetadata->{$op}->{jwks}->{time} + && ( $self->opMetadata->{$op}->{jwks}->{time} + $jwksTimeout > time ) ) + { + $self->logger->debug("JWKS data still valid for $op, skipping..."); + return; + } - my $content = $self->decodeJSON( $response->decoded_content ); + $self->logger->debug("Refresh JWKS data for $op from $jwksUri"); - $self->opMetadata->{$_}->{jwks} = $content; - $self->opMetadata->{$_}->{jwks}->{time} = time; + my $response = $self->ua->get($jwksUri); + if ( $response->is_error ) { + $self->logger->warn( + "Unable to get JWKS data for $op from $jwksUri: " + . $response->message ); + $self->logger->debug( $response->content ); + return; } + + my $content = $self->decodeJSON( $response->decoded_content ); + + $self->opMetadata->{$op}->{jwks} = $content; + $self->opMetadata->{$op}->{jwks}->{time} = time; + return 1; } @@ -1478,10 +1484,14 @@ sub decodeJWT { return; } - # For now, only OP has JWKS - #my $jwks = - # $op ? $self->opMetadata->{$op}->{jwks} : $self->rpMetadata->{$rp}->{jwks}; - my $jwks = $op ? $self->opMetadata->{$op}->{jwks} : $self->rpSigKey->{$rp}; + my $jwks; + if ($op) { + $self->refreshJWKSdataForOp($op); + $jwks = $self->opMetadata->{$op}->{jwks}; + } + else { + $jwks = $self->rpSigKey->{$rp}; + } unless ( $alg =~ /^HS/ ) { unless ($jwks) { -- GitLab