Details

    • Type: Bug
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.9.1
    • Fix Version/s: None
    • Component/s: Security
    • Labels:

      Description

      The implementation using OIDC has some issues (see the class StandardOidcIdentityProvider for all issues):

      • when accessing an OIDC endpoint that doesn't provide any scopes, you'll get a NullPointerException
      • when accessing an OIDC endpoint that doesn't provide the email scope, you'll never have the chance to login at all

      The first issue is just a wrong implementation of the check (line 151).

      The complete implementation is not correct in my opinion. The OpenID spec for the discovery endpoint states that it is RECOMMENDED to send the scopes_supported within the provider metadata. Therefore it is not assured to have those scopes. The implementation of the StandardOidcIdentityProvider want's to throw an exception within the constructor if neither the scope OPENID nor EMAIL is provided (there is an error in the implementation, see line 151).

      On the other side in the overwritten function getScopes() (line 250), the openid scope is always added, the email scope is only added when the metadata contains this scope. Otherwise the function lookupEmail() (line 336) is called to get the email out of the UserInfo endpoint using the Bearer token. This also will never work, because the Bearer token doesn't contain the email scope, thus it will never be returned.

      Therefore I would remove the check in the constructor as well as the function (lookupEmail()) completely, add the email scope to every request and throw an exception, if the email address is not provided.

      This can all be tested and simulated by connecting to Google OIDC, but commenting the code in the getScopes() function so that the email scope is not sent (line 258).

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                Delmol Simon Linder
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 2h 20m
                  2h 20m