oidc-wp / openid-connect-generic

WordPress plugin to provide an OpenID Connect Generic client
https://wordpress.org/plugins/daggerhart-openid-connect-generic/
261 stars 155 forks source link

$session[$this->cookie_token_refresh_key] issue -- token variable mismatch #535

Closed ghost closed 6 months ago

ghost commented 7 months ago

Undefined array key "expires_in" in /var/www/wordpress/wp-content/plugins/daggerhart-openid-connect-generic/includes/openid-connect-generic-client-wrapper.php on line 742

The cookie refresh token needs to be changed to :

            $session[ $this->cookie_token_refresh_key ] = array(
                    'next_access_token_refresh_time' => $token_response['id_token_expires_in'] + $now,
                    'refresh_token' => isset( $token_response['refresh_token'] ) ? $token_response['refresh_token'] : false,
                    'refresh_expires' => false,
            );

From :

            $session[ $this->cookie_token_refresh_key ] = array(
                    'next_access_token_refresh_time' => $token_response['expires_in'] + $now,
                    'refresh_token' => isset( $token_response['refresh_token'] ) ? $token_response['refresh_token'] : false,
                    'refresh_expires' => false,
            );

My IDP sends back a token with 'id_token_expires_in' not 'expires_in'.

Could this be an option or just search for '*expires_in' as a key?

Expected behavior User is never logged in because the cookie timeout doesn't get a value.

Isolating the problem (mark completed items with an [x]):

WordPress Environment

timnolte commented 7 months ago

@chadsterBAM hmm, I can't find that as a valid implementation of OIDC. We do have an issue open already because expires_in is actually an optional item.

https://openid.net/specs/openid-connect-core-1_0.html

ghost commented 7 months ago

{"id_token":"","token_type":"Bearer","not_before":1714407443,"id_token_expires_in":3600,"profile_info":"","scope":"openid"}

This is what comes back from B2C. I haven't altered the output of it at all except to obfuscate the id_token and profile_info.

timnolte commented 7 months ago

OK, so the real fix on my side is to ensure that the expires_in is truly optional, but then also ensuring that according to spec:

As specified in OAuth 2.0 [RFC6749], Clients SHOULD ignore unrecognized response parameters.

https://openid.net/specs/openid-connect-core-1_0.html#TokenResponse

I won't be changing the code to accommodate the invalid attribute that is being sent that is on the side of the IDP as they have incorrectly implemented the OIDC specs.

timnolte commented 6 months ago

IDPs should use expires_in but that should also be optional. https://github.com/oidc-wp/openid-connect-generic/issues/439