diff --git a/includes/openid-connect-generic-client-wrapper.php b/includes/openid-connect-generic-client-wrapper.php index 294d1b0..17c1285 100644 --- a/includes/openid-connect-generic-client-wrapper.php +++ b/includes/openid-connect-generic-client-wrapper.php @@ -275,27 +275,14 @@ public function ensure_tokens_still_fresh() { $current_time = time(); $refresh_token_info = $session[ $this->cookie_token_refresh_key ]; - $next_access_token_refresh_time = $refresh_token_info['next_access_token_refresh_time']; + $next_access_token_refresh_time = $refresh_token_info['next_access_token_refresh_time'] ?? 0; if ( $current_time < $next_access_token_refresh_time ) { return; } - $refresh_token = $refresh_token_info['refresh_token']; - $refresh_expires = $refresh_token_info['refresh_expires']; - - if ( ! $refresh_token || ( $refresh_expires && $current_time > $refresh_expires ) ) { - if ( isset( $_SERVER['REQUEST_URI'] ) ) { - do_action( 'openid-connect-generic-session-expired', wp_get_current_user(), esc_url_raw( wp_unslash( $_SERVER['REQUEST_URI'] ) ) ); - wp_logout(); - - if ( $this->settings->redirect_on_logout ) { - $this->error_redirect( new WP_Error( 'access-token-expired', __( 'Session expired. Please login again.', 'daggerhart-openid-connect-generic' ) ) ); - } - - return; - } - } + $refresh_token = $refresh_token_info['refresh_token'] ?? null; + $refresh_expires = $refresh_token_info['refresh_expires'] ?? null; $token_result = $this->client->request_new_tokens( $refresh_token ); @@ -663,6 +650,9 @@ public function refresh_user_claim( $user, $token_response ) { /** * Record user meta data, and provide an authorization cookie. * + * @todo All uses of `expires_in` values to control application session + * length need to be removed as this is not in spec. + * * @param WP_User $user The user object. * @param array $token_response The token response. * @param array $id_token_claim The ID token claim. @@ -671,7 +661,7 @@ public function refresh_user_claim( $user, $token_response ) { * * @return void */ - public function login_user( $user, $token_response, $id_token_claim, $user_claim, $subject_identity ) { + public function login_user( $user, $token_response, $id_token_claim, $user_claim, $subject_identity ): void { // Store the tokens for future reference. update_user_meta( $user->ID, 'openid-connect-generic-last-token-response', $token_response ); update_user_meta( $user->ID, 'openid-connect-generic-last-id-token-claim', $id_token_claim ); @@ -689,9 +679,9 @@ public function login_user( $user, $token_response, $id_token_claim, $user_claim if ( $remember_me && apply_filters( 'openid-connect-generic-use-token-refresh-expiration', false ) - && ( $token_response['refresh_expires_in'] ?? 0 ) + && ( $token_response['expires_in'] ?? 0 ) ) { - $this->openid_token_refresh_expires_in = $token_response['refresh_expires_in']; + $this->openid_token_refresh_expires_in = $token_response['expires_in']; add_filter( 'auth_cookie_expiration', array( $this, 'set_cookie_expiration_to_openid_token_refresh_expiration' ) ); } @@ -716,6 +706,9 @@ public function login_user( $user, $token_response, $id_token_claim, $user_claim * openid token refresh expiration. This is applied both when creating the session * token as well as when wp_set_auth_cookie is called. * + * @todo This method needs to be remove as token refresh expiration is not + * intended for application sesssion expiration. + * * @param integer $expiration_in_seconds The expiration time in seconds. * @return integer */ @@ -731,25 +724,33 @@ public function set_cookie_expiration_to_openid_token_refresh_expiration( $expir * @param string $token The current users session token. * @param array|WP_Error|null $token_response The authentication token response. */ - public function save_refresh_token( $manager, $token, $token_response ) { + public function save_refresh_token( $manager, $token, $token_response ): void { if ( ! $this->settings->token_refresh_enable ) { return; } - $session = $manager->get( $token ); - $now = time(); - $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, - ); - if ( isset( $token_response['refresh_expires_in'] ) ) { - $refresh_expires_in = $token_response['refresh_expires_in']; - if ( $refresh_expires_in > 0 ) { + + $session = $manager->get( $token ); + $now = time(); + $next_refresh_time = null; + $refresh_expires_in = null; + + if ( isset( $token_response['expires_in'] ) ) { + $expires_in = $token_response['expires_in']; + + // Only set a valid expiration time if the token expiration is not instant. + if ( $expires_in > 0 ) { // Leave enough time for the actual refresh request to go through. - $refresh_expires = $now + $refresh_expires_in - 5; - $session[ $this->cookie_token_refresh_key ]['refresh_expires'] = $refresh_expires; + $next_refresh_time = $now + $next_refresh_time - 5; + $refresh_expires_in = $now + $refresh_expires_in; } } + + $session[ $this->cookie_token_refresh_key ] = array( + 'next_access_token_refresh_time' => $next_refresh_time ?? false, + 'refresh_token' => $token_response['refresh_token'] ?? false, + 'refresh_expires' => $refresh_expires_in ?? false, + ); + $manager->update( $token, $session ); return; } diff --git a/tests/phpunit/includes/openid-connect-generic-client-wrapper_test.php b/tests/phpunit/includes/openid-connect-generic-client-wrapper_test.php index c5f12df..09fe297 100644 --- a/tests/phpunit/includes/openid-connect-generic-client-wrapper_test.php +++ b/tests/phpunit/includes/openid-connect-generic-client-wrapper_test.php @@ -10,6 +10,16 @@ */ class OpenID_Connect_Generic_Client_Wrapper_Test extends WP_UnitTestCase { + /** + * @var OpenID_Connect_Generic_Client_Wrapper + */ + private $client_wrapper; + + /** + * @var WP_User_Meta_Session_Tokens + */ + private $manager; + /** * Test case setup method. * @@ -17,10 +27,14 @@ class OpenID_Connect_Generic_Client_Wrapper_Test extends WP_UnitTestCase { */ public function setUp(): void { - $this->client_wrapper = OpenID_Connect_Generic::instance()->client_wrapper; - parent::setUp(); + remove_all_filters( 'session_token_manager' ); + $user_id = self::factory()->user->create(); + $this->manager = WP_Session_Tokens::get_instance( $user_id ); + + $this->client_wrapper = OpenID_Connect_Generic::instance()->client_wrapper; + } /** @@ -30,6 +44,8 @@ public function setUp(): void { */ public function tearDown(): void { + unset( $this->client_wrapper ); + parent::tearDown(); } @@ -108,4 +124,23 @@ public function test_plugin_client_wrapper_token_expiration() { wp_clear_auth_cookie(); } + /** + * Test proper handling of saving refresh tokens. + * + * @group ClientWrapperTests + */ + public function test_save_refresh_token() { + $expiration = time() + DAY_IN_SECONDS; + $token = $this->manager->create( $expiration ); + $session = $this->manager->get( $token ); + $refresh_token_info = $session['openid-connect-generic-refresh']; + $refresh_token = $refresh_token_info['refresh_token']; + $token_result = $this->client->request_new_tokens( $refresh_token ); + $token_response = $this->client->get_token_response( $token_result ); + + $this->client_wrapper->save_refresh_token( $this->manager, $token, $token_response ); + + $this->manager->destroy( $token ); + } + }