Allow special characters in scope names (#2168)

lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Attributes.pm

@@ -2148,6 +2148,7 @@ qr/^(?:\*\.)?(?:(?:(?:(?:[a-zA-Z0-9][-a-zA-Z0-9]*)?[a-zA-Z0-9])[.])*(?:[a-zA-Z][
         },
         'oidcRPMetaDataOptionsExtraClaims' => {
             'default' => {},
+            'keyTest' => qr/^[\x21\x23-\x5B\x5D-\x7E]+$/,
             'type'    => 'keyTextContainer'
         },
         'oidcRPMetaDataOptionsIcon' => {
@@ -2274,7 +2275,8 @@ qr/^(?:\*\.)?(?:(?:(?:(?:[a-zA-Z0-9][-a-zA-Z0-9]*)?[a-zA-Z0-9])[.])*(?:[a-zA-Z][
             'type' => 'keyTextContainer'
         },
         'oidcServiceDynamicRegistrationExtraClaims' => {
-            'type' => 'keyTextContainer'
+            'keyTest' => qr/^[\x21\x23-\x5B\x5D-\x7E]+$/,
+            'type'    => 'keyTextContainer'
         },
         'oidcServiceIDTokenExpiration' => {
             'default' => 3600,

lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Build/Attributes.pm

@@ -3840,7 +3840,8 @@ m{^(?:ldapi://[^/]*/?|\w[\w\-\.]*(?::\d{1,5})?|ldap(?:s|\+tls)?://\w[\w\-\.]*(?:
               'OpenID Connect exported variables for dynamic registration',
         },
         oidcServiceDynamicRegistrationExtraClaims => {
-            type => 'keyTextContainer',
+            type    => 'keyTextContainer',
+            keyTest => qr/^[\x21\x23-\x5B\x5D-\x7E]+$/,
             documentation =>
               'OpenID Connect extra claims for dynamic registration',
         },
@@ -3960,8 +3961,11 @@ m{^(?:ldapi://[^/]*/?|\w[\w\-\.]*(?::\d{1,5})?|ldap(?:s|\+tls)?://\w[\w\-\.]*(?:
         oidcRPMetaDataOptionsAuthorizationCodeExpiration => { type => 'int' },
         oidcRPMetaDataOptionsOfflineSessionExpiration    => { type => 'int' },
         oidcRPMetaDataOptionsRedirectUris                => { type => 'text', },
-        oidcRPMetaDataOptionsExtraClaims =>
+        oidcRPMetaDataOptionsExtraClaims                 => {
-          { type => 'keyTextContainer', default => {} },
+            type    => 'keyTextContainer',
+            keyTest => qr/^[\x21\x23-\x5B\x5D-\x7E]+$/,
+            default => {}
+        },
         oidcRPMetaDataOptionsBypassConsent => {
             type    => 'bool',
             help    => 'openidconnectclaims.html',

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

@@ -346,14 +346,20 @@ sub run {
             }
 
             # Check scope validity
-            unless ( $oidc_request->{'scope'} =~ /^[a-zA-Z_\-\s]+$/ ) {
+            # We use a slightly more relaxed version of
-                $self->logger->error( "Submitted scope is not valid: "
+            # https://tools.ietf.org/html/rfc6749#appendix-A.4
-                      . $oidc_request->{'scope'} );
+            # To be tolerant of user error (trailing spaces, etc.)
+            # Scope names are restricted to printable ASCII characters,
+            # excluding double quote and backslash
+            unless (
+                $oidc_request->{'scope'} =~ /^[\x20\x21\x23-\x5B\x5D-\x7E]*$/ )
+            {
+                $self->logger->error("Submitted scope is not valid");
                 return PE_ERROR;
             }
 
             # Check openid scope
-            unless ( $oidc_request->{'scope'} =~ /\bopenid\b/ ) {
+            unless ( $self->_hasScope( 'openid', $oidc_request->{'scope'} ) ) {
                 $self->logger->debug("No openid scope found");
 
                 #TODO manage standard OAuth request
@@ -467,7 +473,12 @@ sub run {
                     foreach my $requested_scope (
                         split( /\s+/, $oidc_request->{'scope'} ) )
                     {
-                        if ( $consent_scope =~ /\b$requested_scope\b/ ) {
+                        if (
+                            $self->_hasScope(
+                                $requested_scope, $consent_scope
+                            )
+                          )
+                        {
                             $self->logger->debug(
                                 "Scope $requested_scope already accepted");
                         }
@@ -543,7 +554,8 @@ sub run {
                         my $display_name =
                           $self->conf->{oidcRPMetaDataOptions}->{$rp}
                           ->{oidcRPMetaDataOptionsDisplayName};
-                        my $icon = $self->conf->{oidcRPMetaDataOptions}->{$rp}
+                        my $icon =
+                          $self->conf->{oidcRPMetaDataOptions}->{$rp}
                           ->{oidcRPMetaDataOptionsIcon};
                         my $imgSrc;
 
@@ -564,7 +576,7 @@ sub run {
                         };
                         my @list;
                         foreach my $requested_scope (
-                            split( /\s/, $oidc_request->{'scope'} ) )
+                            split( /\s+/, $oidc_request->{'scope'} ) )
                         {
                             if ( my $message =
                                 $scope_messages->{$requested_scope} )
@@ -620,7 +632,9 @@ sub run {
 
             # WIP: Offline access
             my $offline = 0;
-            if ( $oidc_request->{'scope'} =~ /\boffline_access\b/ ) {
+            if (
+                $self->_hasScope( 'offline_access', $oidc_request->{'scope'} ) )
+            {
                 $offline = 1;
 
                 # MUST ensure that the prompt parameter contains consent unless
@@ -655,8 +669,10 @@ sub run {
                 }
 
                 # Strip offline_access from scopes from now on
-                $oidc_request->{'scope'} = join " ", grep !/^offline_access$/,
+                $oidc_request->{'scope'} = join " ",
-                  split /\s+/, $oidc_request->{'scope'};
+                  grep !/^offline_access$/,
+                  split /\s+/,
+                  $oidc_request->{'scope'};
             }
 
             # Authorization Code Flow
@@ -730,7 +746,8 @@ sub run {
                         "Generated access token: $access_token");
 
                     # Compute hash to store in at_hash
-                    my $alg = $self->conf->{oidcRPMetaDataOptions}->{$rp}
+                    my $alg =
+                      $self->conf->{oidcRPMetaDataOptions}->{$rp}
                       ->{oidcRPMetaDataOptionsIDTokenSignAlg};
                     my ($hash_level) = ( $alg =~ /(?:\w{2})(\d{3})/ );
                     $at_hash = $self->createHash( $access_token, $hash_level )
@@ -1510,8 +1527,8 @@ sub userInfo {
     my $userinfo_response =
       $self->buildUserInfoResponse( $req, $scope, $rp, $session );
     unless ($userinfo_response) {
-        return $self->returnBearerError( 'invalid_request', 'Invalid request',
+        return $self->returnBearerError( 'invalid_request',
-            401 );
+            'Invalid request', 401 );
     }
 
     my $userinfo_sign_alg = $self->conf->{oidcRPMetaDataOptions}->{$rp}
@@ -2020,6 +2037,11 @@ sub exportRequestParameters {
     return PE_OK;
 }
 
+sub _hasScope {
+    my ( $self, $scope, $scopelist ) = @_;
+    return scalar grep { $_ eq $scope } ( split /\s+/, $scopelist );
+}
+
 sub _convertOldFormatConsents {
     my ( $self, $req ) = @_;
     my @oidcConsents = ();

lemonldap-ng-portal/site/templates/bootstrap/oidcConsents.tpl

@@ -12,7 +12,7 @@
    <tr partner="<TMPL_VAR NAME="name">">
     <td><TMPL_VAR NAME="displayName"></td>
     <td class="localeDate" val="<TMPL_VAR NAME="epoch">"></td>
-    <td><TMPL_VAR NAME="scope"></td>
+    <td><TMPL_VAR NAME="scope" ESCAPE=HTML></td>
     <td>
       <a partner="<TMPL_VAR NAME="name">" title="delete" class="oidcConsent">
         <span class="btn btn-danger" role="button">

lemonldap-ng-portal/site/templates/bootstrap/oidcGiveConsent.tpl

@@ -8,7 +8,7 @@
    <li>
     <span trspan="<TMPL_VAR NAME="m">"><TMPL_VAR NAME="m"></span>
     <TMPL_IF NAME="n">
-     <i>(<TMPL_VAR NAME="n">)</i>
+     <i>(<TMPL_VAR NAME="n" ESCAPE=HTML>)</i>
     </TMPL_IF>
    </li>
   </TMPL_LOOP>

lemonldap-ng-portal/t/32-OIDC-Offline-Session.t

@@ -82,8 +82,10 @@ $res = $op->_post(
 );
 my $idpId = expectCookie($res);
 
+# Include a weird scope name, to make sure they work (#2168)
 $query =
-    "response_type=code&scope=openid%20profile%20email%20offline_access&"
+    "response_type=code&scope=openid%20profile%20email%20"
+  . "offline_access%20%21weird%3Ascope.name~&"
   . "client_id=rpid&state=af0ifjsldkj&redirect_uri=http%3A%2F%2Ftest%2F";
 $res = $op->_get(
     "/oauth2/authorize",
@@ -265,7 +267,10 @@ is( $json->{active},    1 );
 is( $json->{client_id}, 'rpid' );
 is( $json->{sub},       'french' );
 
-count(3);
+# #2168
+ok( grep { $_ eq "!weird:scope.name~" } ( split /\s+/, $json->{scope} ),
+    "Scope contains weird scope name" );
+count(4);
 
 clean_sessions();
 done_testing( count() );

lemonldap-ng-portal/t/32-OIDC-Token-Security.t

@@ -102,6 +102,21 @@ ok(
 count(1);
 my $idpId = expectCookie($res);
 
+# Try to get code for RP1 with invalide scope name
+$query =
+"response_type=code&scope=openid%20profile%20email%22&client_id=rpid&state=af0ifjsldkj&redirect_uri=http%3A%2F%2Frp2.com%2F";
+ok(
+    $res = $op->_get(
+        "/oauth2/authorize",
+        query  => "$query",
+        accept => 'text/html',
+        cookie => "lemonldap=$idpId",
+    ),
+    "Get authorization code for rp1"
+);
+count(1);
+expectPortalError( $res, 24, "Invalid scope" );
+#
 # Get code for RP1
 $query =
 "response_type=code&scope=openid%20profile%20email&client_id=rpid&state=af0ifjsldkj&redirect_uri=http%3A%2F%2Frp2.com%2F";