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.

Sign up for GitHub

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

New class for state handling; set cookie for implicit nonce #458

Merged
merged 1 commit into from
May 10, 2018
Merged

New class for state handling; set cookie for implicit nonce #458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 0 additions & 5 deletions WP_Auth0.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

define( 'WPA0_AUTH0_LOGIN_FORM_ID', 'auth0-login-form' );
define( 'WPA0_CACHE_GROUP', 'wp_auth0' );
define( 'WPA0_STATE_COOKIE_NAME', 'auth0_state' );
define( 'WPA0_JWKS_CACHE_TRANSIENT_NAME', 'WP_Auth0_JWKS_cache' );

define( 'WPA0_LANG', 'wp-auth0' ); // deprecated; do not use for translations
Expand Down Expand Up @@ -127,10 +126,6 @@ public function init() {

$this->check_signup_status();

if ( $this->a0_options->get( 'auto_login' ) ) {
WP_Auth0_Nonce_Handler::getInstance()->setCookie();
}

WP_Auth0_Email_Verification::init();
}

Expand Down
4 changes: 4 additions & 0 deletions assets/js/lock-init.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ jQuery(document).ready(function ($) {
// Set state cookie to verify during callback
Cookies.set( opts.stateCookieName, opts.settings.auth.params.state );

if ( opts.settings.auth.params.nonce ) {
Cookies.set( opts.nonceCookieName, opts.settings.auth.params.nonce );
}

// Set Lock to standard or Passwordless
var Lock = opts.usePasswordless
? new Auth0LockPasswordless( opts.clientId, opts.domain, opts.settings )
Expand Down
6 changes: 3 additions & 3 deletions lib/WP_Auth0_Lock10_Options.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public function get_state_obj( $redirect_to = null ) {

$stateObj = array(
'interim' => ( isset( $_GET['interim-login'] ) && $_GET['interim-login'] == 1 ),
'nonce' => WP_Auth0_Nonce_Handler::getInstance()->get()
'nonce' => WP_Auth0_State_Handler::get_instance()->get_unique()
);

if ( !empty( $redirect_to ) ) {
Expand Down Expand Up @@ -154,7 +154,7 @@ public function get_sso_options() {

unset( $options["authParams"] );
$options["state"] = $this->get_state_obj( $redirect_to );
$options["nonce"] = WP_Auth0_Nonce_Handler::getInstance()->get();
$options["nonce"] = WP_Auth0_Nonce_Handler::get_instance()->get_unique();

return $options;
}
Expand Down Expand Up @@ -182,7 +182,7 @@ public function get_lock_options() {
$extraOptions["auth"]["responseType"] = 'id_token';
$extraOptions["auth"]["redirectUrl"] = $this->get_implicit_callback_url();
$extraOptions["autoParseHash"] = false;
$extraOptions["auth"]["params"]["nonce"] = WP_Auth0_Nonce_Handler::getInstance()->get();
$extraOptions["auth"]["params"]["nonce"] = WP_Auth0_Nonce_Handler::get_instance()->get_unique();
} else {
$extraOptions["auth"]["responseType"] = 'code';
$extraOptions["auth"]["redirectUrl"] = $this->get_code_callback_url();
Expand Down
2 changes: 1 addition & 1 deletion lib/WP_Auth0_Lock_Options.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public function modal_button_name() {
public function get_state_obj( $redirect_to = null ) {
$stateObj = array(
'interim' => ( isset( $_GET['interim-login'] ) && $_GET['interim-login'] == 1 ),
'nonce' => WP_Auth0_Nonce_Handler::getInstance()->get()
'nonce' => WP_Auth0_State_Handler::get_instance()->get_unique()
);
if ( !empty( $redirect_to ) ) {
$stateObj["redirect_to"] = addslashes( $redirect_to );
Expand Down
144 changes: 54 additions & 90 deletions lib/WP_Auth0_LoginManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,6 @@ class WP_Auth0_LoginManager {
*/
protected $users_repo;

/**
* State value returned from successful Auth0 login.
*
* @var string
*
* @see WP_Auth0_Lock10_Options::get_state_obj()
*/
protected $state;

/**
* Decoded version of $this>state.
*
* @var object
*/
protected $state_decoded;

/**
* WP_Auth0_LoginManager constructor.
*
Expand Down Expand Up @@ -103,31 +87,36 @@ public function init() {
*/
public function login_auto() {
if (
// Nothing to do
( ! $this->a0_options->get( 'auto_login', FALSE ) )
// Auth0 is not ready to process logins
// Nothing to do.
( ! $this->a0_options->get( 'auto_login', false ) )
// Auth0 is not ready to process logins.
|| ! WP_Auth0::ready()
// Do not redirect POST requests
// Do not redirect POST requests.
|| strtolower( $_SERVER['REQUEST_METHOD'] ) !== 'get'
// Do not redirect login page override
// Do not redirect login page override.
|| isset( $_GET['wle'] )
// Do not redirect log out action
// Do not redirect log out action.
|| ( isset( $_GET['action'] ) && 'logout' === $_GET['action'] )
// Do not redirect Auth0 login processing
// Do not redirect Auth0 login processing.
|| null !== $this->query_vars( 'auth0' )
// Do not redirect if already authenticated
// Do not redirect if already authenticated.
|| is_user_logged_in()
) {
return;
}

$connection = apply_filters( 'auth0_get_auto_login_connection', $this->a0_options->get( 'auto_login_method' ) );
$connection = apply_filters( 'auth0_get_auto_login_connection', $this->a0_options->get( 'auto_login_method' ) );
$auth_params = self::get_authorize_params( $connection );

$auth_url = 'https://' . $this->a0_options->get( 'domain' ) . '/authorize';
$auth_url = add_query_arg( array_map( 'rawurlencode', $auth_params ), $auth_url );

setcookie( WPA0_STATE_COOKIE_NAME, $auth_params['state'], time() + WP_Auth0_Nonce_Handler::COOKIE_EXPIRES, '/' );
WP_Auth0_State_Handler::get_instance()->set_cookie( $auth_params['state'] );

if ( isset( $auth_params['nonce'] ) ) {
WP_Auth0_Nonce_Handler::get_instance()->set_cookie();
}

wp_redirect( $auth_url );
exit;
}
Expand All @@ -146,15 +135,16 @@ public function init_auth0() {

// Catch any incoming errors and stop the login process.
// See https://auth0.com/docs/libraries/error-messages for more info.
if ( $this->query_vars( 'error' ) || $this->query_vars( 'error_description' ) ) {
$error_msg = sanitize_text_field( rawurldecode( $_REQUEST[ 'error_description' ] ) );
$error_code = sanitize_text_field( rawurldecode( $_REQUEST[ 'error' ] ) );
if ( ! empty( $_REQUEST['error'] ) || ! empty( $_REQUEST['error_description'] ) ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

query_vars was causing problems in the state retrieval so I changed that here as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would an error_description by itself be valid? I guess only when error is present. So 2nd clause could be removed.

$error_msg = sanitize_text_field( rawurldecode( $_REQUEST['error_description'] ) );
$error_code = sanitize_text_field( rawurldecode( $_REQUEST['error'] ) );
$this->die_on_login( $error_msg, $error_code );
}

// Check for valid state nonce, set in WP_Auth0_Lock10_Options::get_state_obj().
// See https://auth0.com/docs/protocols/oauth2/oauth-state for more info.
if ( ! $this->validate_state() ) {
$state_returned = isset( $_REQUEST['state'] ) ? rawurldecode( $_REQUEST['state'] ) : null;
if ( ! $state_returned || ! WP_Auth0_State_Handler::get_instance()->validate( $state_returned ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

! $state_returned is already covered inside the verify method

$this->die_on_login( __( 'Invalid state', 'wp-auth0' ) );
}

Expand Down Expand Up @@ -228,8 +218,7 @@ public function redirect_login() {

// Look for clues as to what went wrong.
$e_message = ! empty( $data->error_description ) ? $data->error_description : __( 'Unknown error', 'wp-auth0' );
$e_code = ! empty( $data->error ) ? $data->error : $exchange_resp_code;
throw new WP_Auth0_LoginFlowValidationException( $e_message, $e_code );
throw new WP_Auth0_LoginFlowValidationException( $e_message, $exchange_resp_code );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't like the string error code ... found during debugging

}

$access_token = $data->access_token;
Expand Down Expand Up @@ -272,7 +261,7 @@ public function redirect_login() {
$userinfo = json_decode( $userinfo_resp_body );

if ( $this->login_user( $userinfo, $id_token, $access_token, $refresh_token ) ) {
$state_decoded = $this->get_state( true );
$state_decoded = $this->get_state();
if ( ! empty( $state_decoded->interim ) ) {
include WPA0_PLUGIN_DIR . 'templates/login-interim.php';
} else {
Expand Down Expand Up @@ -326,7 +315,7 @@ public function implicit_login() {

// Validate the nonce if one was included in the request if using auto-login.
$nonce = isset( $decoded_token->nonce ) ? $decoded_token->nonce : null;
if ( ! WP_Auth0_Nonce_Handler::getInstance()->validate( $nonce ) ) {
if ( ! WP_Auth0_Nonce_Handler::get_instance()->validate( $nonce ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you following the same method naming convention everywhere? Asking since I see a misxture for class names, i.e. WP_Auth0_LoginFlowValidationException

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For new methods, I'm following the WP standard, which is snake-case. There was no naming convention, really, up until now.

throw new WP_Auth0_LoginFlowValidationException(
__( 'Invalid nonce', 'wp-auth0' )
);
Expand All @@ -338,7 +327,7 @@ public function implicit_login() {
if ( $this->login_user( $decoded_token, $id_token ) ) {

// Validated above in $this->init_auth0().
$state_decoded = $this->get_state( true );
$state_decoded = $this->get_state();

if ( ! empty( $state_decoded->interim ) ) {
include WPA0_PLUGIN_DIR . 'templates/login-interim.php';
Expand Down Expand Up @@ -541,7 +530,6 @@ public function logout() {
}

/**
*
* Outputs JS on wp-login.php to log a user in if an Auth0 session is found.
* Hooked to `login_message` filter.
* IMPORTANT: Internal callback use only, do not call this function directly!
Expand Down Expand Up @@ -581,8 +569,8 @@ public function auth0_singlelogout_footer() {

/**
* End the PHP session.
*
* TODO: Deprecate
*
* TODO: Deprecate
*/
public function end_session() {
if ( session_id() ) {
Expand All @@ -591,9 +579,9 @@ public function end_session() {
}

/**
* Get and filter the scope used for access and ID tokens
* Get and filter the scope used for access and ID tokens.
*
* @param string $context - how are the scopes being used?
* @param string $context - how the scopes are being used.
*
* @return string
*/
Expand All @@ -612,30 +600,30 @@ public static function get_userinfo_scope( $context = '' ) {
* @return array
*/
public static function get_authorize_params( $connection = null, $redirect_to = null ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly whitespace changes in this method

$params = array();
$options = WP_Auth0_Options::Instance();
$params = array();
$options = WP_Auth0_Options::Instance();
$lock_options = new WP_Auth0_Lock10_Options();
$is_implicit = (bool) $options->get( 'auth0_implicit_workflow', FALSE );
$nonce = WP_Auth0_Nonce_Handler::getInstance()->get();
$is_implicit = (bool) $options->get( 'auth0_implicit_workflow', false );
$nonce = WP_Auth0_Nonce_Handler::get_instance()->get_unique();

$params[ 'client_id' ] = $options->get( 'client_id' );
$params[ 'scope' ] = self::get_userinfo_scope( 'authorize_url' );
$params[ 'response_type' ] = $is_implicit ? 'id_token': 'code';
$params[ 'redirect_uri' ] = $is_implicit
$params['client_id'] = $options->get( 'client_id' );
$params['scope'] = self::get_userinfo_scope( 'authorize_url' );
$params['response_type'] = $is_implicit ? 'id_token' : 'code';
$params['redirect_uri'] = $is_implicit
? $lock_options->get_implicit_callback_url()
: $options->get_wp_auth0_url( null );

if ( $is_implicit ) {
$params[ 'nonce' ] = $nonce;
$params['nonce'] = $nonce;
}

if ( ! empty( $connection ) ) {
$params[ 'connection' ] = $connection;
$params['connection'] = $connection;
}

// Get the telemetry header.
$telemetry = WP_Auth0_Api_Client::get_info_headers();
$params[ 'auth0Client' ] = $telemetry[ 'Auth0-Client' ];
$telemetry = WP_Auth0_Api_Client::get_info_headers();
$params['auth0Client'] = $telemetry['Auth0-Client'];

// Where should the user be redirected after logging in?
if ( empty( $redirect_to ) && ! empty( $_GET['redirect_to'] ) ) {
Expand All @@ -645,11 +633,15 @@ public static function get_authorize_params( $connection = null, $redirect_to =
}

// State parameter, checked during login callback.
$params[ 'state' ] = base64_encode( json_encode( array(
'interim' => false,
'nonce' => $nonce,
'redirect_to' => filter_var( $redirect_to, FILTER_SANITIZE_URL ),
) ) );
$params['state'] = base64_encode(
json_encode(
array(
'interim' => false,
'nonce' => $nonce,
'redirect_to' => filter_var( $redirect_to, FILTER_SANITIZE_URL ),
)
)
);

return $params;
}
Expand All @@ -675,41 +667,13 @@ protected function query_vars( $key ) {
/**
* Get the state value returned from Auth0 during login processing.
*
* @param bool $decoded - pass `true` to return decoded state, leave blank for raw string.
*
* @return string|object|null
*/
protected function get_state( $decoded = false ) {

if ( empty( $this->state ) ) {
// Get and store base64 encoded state.
$state_val = isset( $_REQUEST['state'] ) ? $_REQUEST['state'] : '';
$state_val = urldecode( $state_val );
$this->state = $state_val;

// Decode and store the state.
$state_val = base64_decode( $state_val );
$this->state_decoded = json_decode( $state_val );
}

if ( $decoded ) {
return is_object( $this->state_decoded ) ? $this->state_decoded : null;
} else {
return $this->state;
}
}

/**
* Check the state send back from Auth0 with the one stored in the user's browser.
*
* @return bool
*/
protected function validate_state() {
$valid = isset( $_COOKIE[ WPA0_STATE_COOKIE_NAME ] )
? $_COOKIE[ WPA0_STATE_COOKIE_NAME ] === $this->get_state()
: false;
setcookie( WPA0_STATE_COOKIE_NAME, '', 0, '/' );
return $valid;
protected function get_state() {
$state_val = rawurldecode( $_REQUEST['state'] );
$state_val = base64_decode( $state_val );
$state_val = json_decode( $state_val );
return $state_val;
}

/**
Expand Down
Loading