Contrib.social

saveIdToken should accept arrays too?

Open on Drupal.org →
Created on 12 June 2023, about 1 year ago

Problem/Motivation

When trying to connect via the remote SSO of one of our clients, module openid_connect fails with a 500 fatal error when querying the token:

TypeError: Drupal\openid_connect\OpenIDConnectSession::saveIdToken(): Argument #1 ($token) must be of type string, array given, called in /var/www/public/modules/contrib/openid_connect/src/OpenIDConnect.php on line 429 in Drupal\openid_connect\OpenIDConnectSession->saveIdToken() (line 170 of /var/www/public/modules/contrib/openid_connect/src/OpenIDConnectSession.php).

Steps to reproduce

To reproduce the issue, you need openid_connect to be configured to talk to a remote SSO server that returns the following structure when querying the "Token endpoint" (for instance):

{
    "access_token": "XXX",
    "token_type": "Bearer",
    "refresh_token": "XXX",
    "expires_in": 3600,
    "id_token":
    {
        "iss": "https://id.example.com",
        "sub": "012345678901",
        "aud": "messages",
        "exp": 1523632701.51226,
        "iat": "2018-04-13T14:18:21Z",
        "given_name": "John",
        "family_name": "JOHNSON",
        "email": "jjohnson@example.com",
        "phone_number": "",
        "group": "60",
        "photo": "https://static.example.com/api/get-photo.aspx?id=012345678901&s=100x100"
    }
}

The important part is the "id_token" because some servers do return it as a key/value map instead of an encoded string, as shown above.

Proposed resolution

Currently, the openid_connect module assumes that property "id_token" is necessarily a string when saving it in session via \Drupal\openid_connect\OpenIDConnectSessionInterface::saveAccessToken(). It is hardcoded as a PHP type.

Although, if I'm not mistaken it's been developed to handle both cases in other parts of the openid_connect module apparently. For instance, function \Drupal\openid_connect\OpenIDConnect::buildContext() does check whether "id_token" is a string or not before retrieving info from it.

So it feels like we should accept the token to be a string or an array when saving it in session?

🐛 Bug report
Status

Needs review

Version

3.0

Component

Code

Created by

🇫🇷France pacproduct

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @pacproduct
  • Comment about 1 year ago →
    🇫🇷France pacproduct

    Here is a simple patch attached that extends the hardcoded "string" type so it now accepts "string" and "array".
    It seems to solve the issue in my case.

    As a side note, I'm not sure how the following code works out if id_token is an array, in \Drupal\openid_connect\Controller\OpenIDConnectRedirectController::redirectLogout():

                  $url_options = [
                'query' => ['id_token_hint' => $this->session->retrieveIdToken()],
              ];
              [...]
              $redirect = Url::fromUri($endpoints['end_session'], $url_options)->toString(TRUE);
  • Status changed to Needs review about 1 year ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.0.7 + Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    100 pass
  • Comment about 1 year ago →
    🇫🇷France Simon Georges Rouen, France

    Changing status to "Needs review", considering a patch is included.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024