Skip to content

Commit

Permalink
fix(Wrapper): Fixes handling of optional expires_in attribute in Ac…
Browse files Browse the repository at this point in the history 
…cess Token (#539)

- Fixes #439
- Properly handles `expires_in` being OPTIONAl according to spec. (See: 3.2.2.5.  Successful Authentication Response)
  • Loading branch information
@timnolte
timnolte committed Jun 11, 2024
1 parent 0038ce7 commit 1c34384
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 127 deletions.
144 changes: 47 additions & 97 deletions includes/openid-connect-generic-client-wrapper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@
*/
class OpenID_Connect_Generic_Client_Wrapper {

/**
* The user redirect cookie key.
*
* @deprecated Redirection should be done via state transient and not cookies.
*
* @var string
*/
const COOKIE_REDIRECT_KEY = 'openid-connect-generic-redirect';

/**
* The token refresh info cookie key.
*
* @var string
*/
const COOKIE_TOKEN_REFRESH_KEY = 'openid-connect-generic-refresh';

/**
* The client object instance.
*
Expand All @@ -40,22 +56,6 @@ class OpenID_Connect_Generic_Client_Wrapper {
*/
private $logger;

/**
* The token refresh info cookie key.
*
* @var string
*/
private $cookie_token_refresh_key = 'openid-connect-generic-refresh';

/**
* The user redirect cookie key.
*
* @deprecated Redirection should be done via state transient and not cookies.
*
* @var string
*/
public $cookie_redirect_key = 'openid-connect-generic-redirect';

/**
* The return error onject.
*
Expand All @@ -65,13 +65,6 @@ class OpenID_Connect_Generic_Client_Wrapper {
*/
private $error = false;

/**
* Used to pass the openid token refresh expiration time to the auth_cookie_expiration filter.
*
* @var integer
*/
private $openid_token_refresh_expires_in = 0;

/**
* Inject necessary objects and services into the client.
*
Expand Down Expand Up @@ -122,11 +115,6 @@ public static function register( OpenID_Connect_Generic_Client $client, OpenID_C
add_action( 'parse_request', array( $client_wrapper, 'alternate_redirect_uri_parse_request' ) );
}

// Verify token for any logged in user.
if ( is_user_logged_in() ) {
add_action( 'wp_loaded', array( $client_wrapper, 'ensure_tokens_still_fresh' ) );
}

return $client_wrapper;
}

Expand Down Expand Up @@ -263,40 +251,36 @@ public function ensure_tokens_still_fresh() {
}

$user_id = wp_get_current_user()->ID;
$last_token_response = get_user_meta( $user_id, 'openid-connect-generic-last-token-response', true );

if ( ! empty( $last_token_response['expires_in'] ) && ! empty( $last_token_response['time'] ) ) {
/*
* @var int $expiration_time
*/
$expiration_time = intval( $last_token_response['time'] ) + intval( $last_token_response['expires_in'] );
if ( time() < $expiration_time ) {
// Access token is not expired so don't attempt to refresh.
return;
}
}

$manager = WP_Session_Tokens::get_instance( $user_id );
$token = wp_get_session_token();
$session = $manager->get( $token );

if ( ! isset( $session[ $this->cookie_token_refresh_key ] ) ) {
if ( ! isset( $session[ self::COOKIE_TOKEN_REFRESH_KEY ] ) ) {
// Not an OpenID-based session.
return;
}

$current_time = time();
$refresh_token_info = $session[ $this->cookie_token_refresh_key ];
$refresh_token_info = $session[ self::COOKIE_TOKEN_REFRESH_KEY ];

$next_access_token_refresh_time = $refresh_token_info['next_access_token_refresh_time'];

if ( $current_time < $next_access_token_refresh_time ) {
$refresh_token = $refresh_token_info['refresh_token'] ?? null;
if ( empty( $refresh_token ) ) {
// No valid refresh token.
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;
}
}

$token_result = $this->client->request_new_tokens( $refresh_token );

if ( is_wp_error( $token_result ) ) {
Expand All @@ -305,12 +289,14 @@ public function ensure_tokens_still_fresh() {
}

$token_response = $this->client->get_token_response( $token_result );

if ( is_wp_error( $token_response ) ) {
wp_logout();
$this->error_redirect( $token_response );
}

// Capture the time so that access token expiration can be calculated later.
$token_response[] = time();

update_user_meta( $user_id, 'openid-connect-generic-last-token-response', $token_response );
$this->save_refresh_token( $manager, $token, $token_response );
}
Expand Down Expand Up @@ -571,8 +557,8 @@ public function authentication_request_callback() {
}

// Provide backwards compatibility for customization using the deprecated cookie method.
if ( ! empty( $_COOKIE[ $this->cookie_redirect_key ] ) ) {
$redirect_url = esc_url_raw( wp_unslash( $_COOKIE[ $this->cookie_redirect_key ] ) );
if ( ! empty( $_COOKIE[ self::COOKIE_REDIRECT_KEY ] ) ) {
$redirect_url = esc_url_raw( wp_unslash( $_COOKIE[ self::COOKIE_REDIRECT_KEY ] ) );
}

// Only do redirect-user-back action hook when the plugin is configured for it.
Expand Down Expand Up @@ -671,7 +657,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 );
Expand All @@ -683,20 +669,8 @@ public function login_user( $user, $token_response, $id_token_claim, $user_claim
$remember_me = apply_filters( 'openid-connect-generic-remember-me', false, $user, $token_response, $id_token_claim, $user_claim, $subject_identity );
$wp_expiration_days = $remember_me ? 14 : 2;

// If remember-me is enabled, and using token expiration is enabled,
// add a filter to overwrite the default cookie expiration with the
// openid token expiration.
if (
$remember_me
&& apply_filters( 'openid-connect-generic-use-token-refresh-expiration', false )
&& ( $token_response['refresh_expires_in'] ?? 0 )
) {
$this->openid_token_refresh_expires_in = $token_response['refresh_expires_in'];
add_filter( 'auth_cookie_expiration', array( $this, 'set_cookie_expiration_to_openid_token_refresh_expiration' ) );
}

// Create the WP session, so we know its token.
$expiration = time() + apply_filters( 'auth_cookie_expiration', $wp_expiration_days * DAY_IN_SECONDS, $user->ID, false );
$expiration = time() + apply_filters( 'auth_cookie_expiration', $wp_expiration_days * DAY_IN_SECONDS, $user->ID, $remember_me );
$manager = WP_Session_Tokens::get_instance( $user->ID );
$token = $manager->create( $expiration );

Expand All @@ -706,22 +680,6 @@ public function login_user( $user, $token_response, $id_token_claim, $user_claim
// you did great, have a cookie!
wp_set_auth_cookie( $user->ID, $remember_me, '', $token );
do_action( 'wp_login', $user->user_login, $user );

// Remove the filter for the auth cookie expiration after all the auth cookies are set.
remove_filter( 'auth_cookie_expiration', array( $this, 'set_cookie_expiration_to_openid_token_refresh_expiration' ) );
}

/**
* Filter callback to overwrite the default cookie expiration with the
* openid token refresh expiration. This is applied both when creating the session
* token as well as when wp_set_auth_cookie is called.
*
* @param integer $expiration_in_seconds The expiration time in seconds.
* @return integer
*/
public function set_cookie_expiration_to_openid_token_refresh_expiration( $expiration_in_seconds ) {
$expiration_in_seconds = $this->openid_token_refresh_expires_in;
return $expiration_in_seconds;
}

/**
Expand All @@ -731,25 +689,17 @@ 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,

$session[ self::COOKIE_TOKEN_REFRESH_KEY ] = array(
'refresh_token' => $token_response['refresh_token'] ?? false,
);
if ( isset( $token_response['refresh_expires_in'] ) ) {
$refresh_expires_in = $token_response['refresh_expires_in'];
if ( $refresh_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;
}
}

$manager->update( $token, $session );
return;
}
Expand Down
2 changes: 1 addition & 1 deletion phpstan.neon.dist
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ parameters:
# Uses func_get_args()
#- '#^Function apply_filters(_ref_array)? invoked with [34567] parameters, 2 required\.$#'
# Ignore cookie_redirect_key deprecation errors.
- '/^Access to deprecated property \$cookie_redirect_key/'
- '/^Fetching deprecated class constant COOKIE_REDIRECT_KEY of class OpenID_Connect_Generic_Client_Wrapper/'
65 changes: 36 additions & 29 deletions tests/phpunit/includes/openid-connect-generic-client-wrapper_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,31 @@
*/
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.
*
* @return void
*/
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;

}

/**
Expand All @@ -30,6 +44,8 @@ public function setUp(): void {
*/
public function tearDown(): void {

unset( $this->client_wrapper );

parent::tearDown();

}
Expand Down Expand Up @@ -76,36 +92,27 @@ public function test_plugin_client_wrapper_remember_me() {
}

/**
* Test if by using the use-token-expiration, the user session expiration
* is set to the value of the expires_in parameter of the token.
* Test proper handling of saving refresh tokens.
*
* @group ClientWrapperTests
*/
public function test_plugin_client_wrapper_token_expiration() {
// Set the remember me option to true
add_filter( 'openid-connect-generic-remember-me', '__return_true' );
add_filter( 'openid-connect-generic-use-token-refresh-expiration', '__return_true' );

// Create a user and log in using the login function of the client wrapper
$user = $this->factory()->user->create_and_get( array( 'user_login' => 'test-remember-me-user' ) );
$this->client_wrapper->login_user( $user, array(
'expires_in' => 5 * MINUTE_IN_SECONDS,
'refresh_expires_in' => 30 * DAY_IN_SECONDS,
), array(), array(), '' );

// Retrieve the session tokens
$manager = WP_Session_Tokens::get_instance( $user->ID );
$token = $manager->get_all()[0];

// Assert if the token is set to expire in 30 days, with some timing margin
$this->assertGreaterThan( time() + 29 * DAY_IN_SECONDS, $token['expiration'] );
$this->assertLessThan( time() + 31 * DAY_IN_SECONDS, $token['expiration'] );

// Cleanup
remove_filter( 'openid-connect-generic-remember-me', '__return_true' );
remove_filter( 'openid-connect-generic-use-token-refresh-expiration', '__return_true' );
$manager->destroy_all();
wp_clear_auth_cookie();
public function test_save_refresh_token() {
$expiration = time() + DAY_IN_SECONDS;
$token = $this->manager->create( $expiration );
$token_response = array(
"access_token" => "TlBN45jURg",
"token_type" => "Bearer",
"refresh_token" => "9yNOxJtZa5",
"expires_in" => 3600, // Expiration time of the Access Token in seconds since the response was generated. OPTIONAL.
);

$this->client_wrapper->save_refresh_token( $this->manager, $token, $token_response );
$session = $this->manager->get( $token );

$this->assertArrayHasKey( $this->client_wrapper::COOKIE_TOKEN_REFRESH_KEY, $session, "Session token is missing expected key!" );
$this->assertArrayHasKey( 'refresh_token', $session[ $this->client_wrapper::COOKIE_TOKEN_REFRESH_KEY ], "Refresh token is missing key!" );

$this->manager->destroy( $token );
}

}

0 comments on commit 1c34384

Please sign in to comment.