Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PKCE support #421

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 73 additions & 2 deletions includes/openid-connect-generic-client-wrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ public static function register( OpenID_Connect_Generic_Client $client, OpenID_C
add_action( 'wp_loaded', array( $client_wrapper, 'ensure_tokens_still_fresh' ) );
}

// Modify authentication-token request to include PKCE code verifier.
if ( true === (bool) $settings->enable_pkce ) {
add_filter( 'openid-connect-generic-alter-request', array( $client_wrapper, 'alter_authentication_token_request' ), 15, 2 );
}

return $client_wrapper;
}

Expand Down Expand Up @@ -236,15 +241,24 @@ public function get_authentication_url( $atts = array() ) {
$url_format .= '&acr_values=%7$s';
}

if ( true === (bool) $this->settings->enable_pkce ) {
$pkce_data = $this->pkce_code_generator();
if ( false !== $pkce_data ) {
$url_format .= '&code_challenge=%8$s&code_challenge_method=%9$s';
}
}

$url = sprintf(
$url_format,
$atts['endpoint_login'],
$separator,
rawurlencode( $atts['scope'] ),
rawurlencode( $atts['client_id'] ),
$this->client->new_state( $atts['redirect_to'] ),
$this->client->new_state( $atts['redirect_to'], $pkce_data['code_verifier'] ?? '' ),
rawurlencode( $atts['redirect_uri'] ),
rawurlencode( $atts['acr_values'] )
rawurlencode( $atts['acr_values'] ),
rawurlencode( $pkce_data['code_challenge'] ?? '' ),
rawurlencode( $pkce_data['code_challenge_method'] ?? '' )
);

$url = apply_filters( 'openid-connect-generic-auth-url', $url );
Expand Down Expand Up @@ -427,6 +441,31 @@ public function alter_request( $request, $operation ) {
return $request;
}

/**
* Include PKCE code verifier in authentication token request.
*
* @param array<mixed> $request The outgoing request array.
* @param string $operation The request operation name.
*
* @return mixed
*/
public function alter_authentication_token_request( $request, $operation ) {
if ( 'get-authentication-token' !== $operation ) {
return $request;
}

$code_verifier = '';
$state = $_GET['state'] ?? ''; //phpcs:ignore WordPress.Security.ValidatedSanitizedInput -- Sanitized later if not empty.
if ( ! empty( $state ) ) {
$state_object = get_transient( 'openid-connect-generic-state--' . sanitize_text_field( $state ) );
$code_verifier = $state_object[ $state ]['code_verifier'] ?? '';
}

$request['body']['code_verifier'] = $code_verifier;

return $request;
}

/**
* Control the authentication and subsequent authorization of the user when
* returning from the IDP.
Expand Down Expand Up @@ -1177,4 +1216,36 @@ public function update_existing_user( $uid, $subject_identity ) {
// Return our updated user.
return get_user_by( 'id', $uid );
}

/**
* Generate PKCE code for OAuth flow.
*
* @see : https://help.aweber.com/hc/en-us/articles/360036524474-How-do-I-use-Proof-Key-for-Code-Exchange-PKCE-
*
* @return array<string, mixed>|bool Code challenge array on success, false on error.
*/
private function pkce_code_generator() {
try {
$verifier_bytes = random_bytes( 64 );
} catch ( \Exception $e ) {
$this->logger->log(
sprintf( 'Fail to generate PKCE code challenge : %s', $e->getMessage() ),
'pkce_code_generator'
);

return false;
}

$verifier = rtrim( strtr( base64_encode( $verifier_bytes ), '+/', '-_' ), '=' );

// Very important, "raw_output" must be set to true or the challenge will not match the verifier.
$challenge_bytes = hash( 'sha256', $verifier, true );
$challenge = rtrim( strtr( base64_encode( $challenge_bytes ), '+/', '-_' ), '=' );

return array(
'code_verifier' => $verifier,
'code_challenge' => $challenge,
'code_challenge_method' => 'S256',
);
}
}
8 changes: 5 additions & 3 deletions includes/openid-connect-generic-client.php
Original file line number Diff line number Diff line change
Expand Up @@ -355,16 +355,18 @@ public function request_userinfo( $access_token ) {
/**
* Generate a new state, save it as a transient, and return the state hash.
*
* @param string $redirect_to The redirect URL to be used after IDP authentication.
* @param string $redirect_to The redirect URL to be used after IDP authentication.
* @param string $pkce_code_verifier The PKCE code verifier to be sent during the authorization code exchange request.
*
* @return string
*/
public function new_state( $redirect_to ) {
public function new_state( $redirect_to, $pkce_code_verifier = '' ) {
// New state w/ timestamp.
$state = md5( mt_rand() . microtime( true ) );
$state_value = array(
$state => array(
'redirect_to' => $redirect_to,
'redirect_to' => $redirect_to,
'code_verifier' => $pkce_code_verifier,
),
);
set_transient( 'openid-connect-generic-state--' . $state, $state_value, $this->state_time_limit );
Expand Down
2 changes: 2 additions & 0 deletions includes/openid-connect-generic-option-settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
* @property string $endpoint_token The IDP token validation endpoint URL.
* @property string $endpoint_end_session The IDP logout endpoint URL.
* @property string $acr_values The Authentication contract as defined on the IDP.
* @property bool $enable_pkce The flag to enable/disable PKCE support.
*
* Non-standard Settings:
*
Expand Down Expand Up @@ -103,6 +104,7 @@ class OpenID_Connect_Generic_Option_Settings {
'acr_values' => 'OIDC_ACR_VALUES',
'enable_logging' => 'OIDC_ENABLE_LOGGING',
'log_limit' => 'OIDC_LOG_LIMIT',
'enable_pkce' => 'OIDC_ENABLE_PKCE',
);

/**
Expand Down
7 changes: 7 additions & 0 deletions includes/openid-connect-generic-settings-page.php
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,13 @@ private function get_settings_fields() {
'disabled' => defined( 'OIDC_ACR_VALUES' ),
'section' => 'client_settings',
),
'enable_pkce' => array(
'title' => __( 'Enable PKCE support', 'daggerhart-openid-connect-generic' ),
'description' => __( 'If checked, add PKCE challenge during authentication requests.', 'daggerhart-openid-connect-generic' ),
'type' => 'checkbox',
'disabled' => defined( 'OIDC_ENABLE_PKCE' ),
'section' => 'client_settings',
),
'identity_key' => array(
'title' => __( 'Identity Key', 'daggerhart-openid-connect-generic' ),
'description' => __( 'Where in the user claim array to find the user\'s identification data. Possible standard values: preferred_username, name, or sub. If you\'re having trouble, use "sub".', 'daggerhart-openid-connect-generic' ),
Expand Down
1 change: 1 addition & 0 deletions openid-connect-generic.php
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ public static function bootstrap() {
'endpoint_token' => defined( 'OIDC_ENDPOINT_TOKEN_URL' ) ? OIDC_ENDPOINT_TOKEN_URL : '',
'endpoint_end_session' => defined( 'OIDC_ENDPOINT_LOGOUT_URL' ) ? OIDC_ENDPOINT_LOGOUT_URL : '',
'acr_values' => defined( 'OIDC_ACR_VALUES' ) ? OIDC_ACR_VALUES : '',
'enable_pkce' => defined( 'OIDC_ENABLE_PKCE' ) ? OIDC_ENABLE_PKCE : false,

// Non-standard settings.
'no_sslverify' => 0,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
<?php
/**
* Class OpenID_Connect_Generic_Client_Wrapper_Pkce_Test
*
* @package OpenID_Connect_Generic
*/

/**
* Plugin OIDC/oAuth client wrapper class test case.
*/
class OpenID_Connect_Generic_Client_Wrapper_Pkce_Test extends WP_UnitTestCase {

protected $openid_client_pkce;

/**
* Test case setup method.
*
* @return void
*/
public function setUp(): void {
parent::setUp();

$settings = new OpenID_Connect_Generic_Option_Settings(
// Default settings values.
array(
// OAuth client settings.
'login_type' => defined( 'OIDC_LOGIN_TYPE' ) ? OIDC_LOGIN_TYPE : 'button',
'client_id' => defined( 'OIDC_CLIENT_ID' ) ? OIDC_CLIENT_ID : '',
'client_secret' => defined( 'OIDC_CLIENT_SECRET' ) ? OIDC_CLIENT_SECRET : '',
'scope' => defined( 'OIDC_CLIENT_SCOPE' ) ? OIDC_CLIENT_SCOPE : '',
'endpoint_login' => defined( 'OIDC_ENDPOINT_LOGIN_URL' ) ? OIDC_ENDPOINT_LOGIN_URL : '',
'endpoint_userinfo' => defined( 'OIDC_ENDPOINT_USERINFO_URL' ) ? OIDC_ENDPOINT_USERINFO_URL : '',
'endpoint_token' => defined( 'OIDC_ENDPOINT_TOKEN_URL' ) ? OIDC_ENDPOINT_TOKEN_URL : '',
'endpoint_end_session' => defined( 'OIDC_ENDPOINT_LOGOUT_URL' ) ? OIDC_ENDPOINT_LOGOUT_URL : '',
'acr_values' => defined( 'OIDC_ACR_VALUES' ) ? OIDC_ACR_VALUES : '',
'enable_pkce' => true,

// Non-standard settings.
'no_sslverify' => 0,
'http_request_timeout' => 5,
'identity_key' => 'preferred_username',
'nickname_key' => 'preferred_username',
'email_format' => '{email}',
'displayname_format' => '',
'identify_with_username' => false,
'state_time_limit' => 180,

// Plugin settings.
'enforce_privacy' => defined( 'OIDC_ENFORCE_PRIVACY' ) ? intval( OIDC_ENFORCE_PRIVACY ) : 0,
'alternate_redirect_uri' => 0,
'token_refresh_enable' => 1,
'link_existing_users' => defined( 'OIDC_LINK_EXISTING_USERS' ) ? intval( OIDC_LINK_EXISTING_USERS ) : 0,
'create_if_does_not_exist' => defined( 'OIDC_CREATE_IF_DOES_NOT_EXIST' ) ? intval( OIDC_CREATE_IF_DOES_NOT_EXIST ) : 1,
'redirect_user_back' => defined( 'OIDC_REDIRECT_USER_BACK' ) ? intval( OIDC_REDIRECT_USER_BACK ) : 0,
'redirect_on_logout' => defined( 'OIDC_REDIRECT_ON_LOGOUT' ) ? intval( OIDC_REDIRECT_ON_LOGOUT ) : 1,
'enable_logging' => defined( 'OIDC_ENABLE_LOGGING' ) ? intval( OIDC_ENABLE_LOGGING ) : 0,
'log_limit' => defined( 'OIDC_LOG_LIMIT' ) ? intval( OIDC_LOG_LIMIT ) : 1000,
)
);

$logger = new OpenID_Connect_Generic_Option_Logger( 'error', $settings->enable_logging, $settings->log_limit );

$this->openid_client_pkce = new OpenID_Connect_Generic( $settings, $logger );
$this->openid_client_pkce->init();
}

/**
* Test case cleanup method.
*
* @return void
*/
public function tearDown(): void {
unset( $this->openid_client_pkce );

parent::tearDown();
}

/**
* @covers OpenID_Connect_Generic_Client_Wrapper::get_authentication_url
*/
public function test_plugin_client_wrapper_authentication_url_contain_pkce_parameters() {
// Generate an authentication URL.
$authentication_url = $this->openid_client_pkce->client_wrapper->get_authentication_url();

// Extract the URL querystring and fill the `$params` array with the parameters.
parse_str(
parse_url( $authentication_url, PHP_URL_QUERY ),
$params
);

// Asserts required parameters are present.
$this->assertArrayHasKey( 'code_challenge', $params, 'check for PKCE parameter "code_challenge" in the authentication URL when PKCE option is enable.' );
$this->assertArrayHasKey( 'code_challenge_method', $params, 'check for PKCE parameter "code_challenge_method" in the authentication URL when PKCE option is enable.' );

// Assert state contain the required code
$state = $params['state'];
$state_data = get_transient( 'openid-connect-generic-state--' . sanitize_text_field( $state ) );
$this->assertNotEmpty( $state_data[ $state ]['code_verifier'], 'check for non-empty "code_verifier" in the state.' );
}

/**
* @covers OpenID_Connect_Generic_Client_Wrapper::alter_authentication_token_request
*/
public function test_plugin_client_wrapper_filter_for_get_authentication_token_request_exist() {
$this->assertNotFalse(
has_filter(
'openid-connect-generic-alter-request',
array( $this->openid_client_pkce->client_wrapper, 'alter_authentication_token_request')
)
);
}

/**
* @covers OpenID_Connect_Generic_Client_Wrapper::alter_authentication_token_request
*/
public function test_plugin_client_wrapper_code_verifier_is_included_in_get_authentication_token_request() {
// Generate an authentication URL.
$authentication_url = $this->openid_client_pkce->client_wrapper->get_authentication_url();

// Extract the URL querystring and fill the `$params` array with the parameters.
parse_str(
parse_url( $authentication_url, PHP_URL_QUERY ),
$params
);

// Assert the request body include the `code_verifier`.
$state = $params['state'];
$state_data = get_transient( 'openid-connect-generic-state--' . sanitize_text_field( $state ) );
$request = [
'body' => [],
];
$_GET['state'] = $state;
$request = $this->openid_client_pkce->client_wrapper->alter_authentication_token_request( $request, 'get-authentication-token' );
$this->assertEquals( $state_data[ $state ]['code_verifier'], $request['body']['code_verifier'] );
unset( $_GET['state'] );
}
}