From 13f2f38ae4538d542489b6d3c20505415b3d7c90 Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Fri, 2 Mar 2018 14:37:24 -0800 Subject: [PATCH 1/4] State handling during login process for both types, implicit and redirect; added WP_Auth0_LoginManager->die_on_login() to handle login errors better --- lib/WP_Auth0_Lock10_Options.php | 11 +++-- lib/WP_Auth0_Lock_Options.php | 11 +++-- lib/WP_Auth0_LoginManager.php | 54 ++++++++++++++++------- lib/WP_Auth0_State_Handler.php | 78 +++++++++++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 28 deletions(-) create mode 100644 lib/WP_Auth0_State_Handler.php diff --git a/lib/WP_Auth0_Lock10_Options.php b/lib/WP_Auth0_Lock10_Options.php index 919768e1..9872a947 100644 --- a/lib/WP_Auth0_Lock10_Options.php +++ b/lib/WP_Auth0_Lock10_Options.php @@ -99,13 +99,12 @@ public function modal_button_name() { public function get_state_obj( $redirect_to = null ) { - if ( isset( $_GET['interim-login'] ) && $_GET['interim-login'] == 1 ) { - $interim_login = true; - } else { - $interim_login = false; - } + $stateHandler = new WP_Auth0_State_Handler(); + $stateObj = array( + 'interim' => ( isset( $_GET['interim-login'] ) && $_GET['interim-login'] == 1 ), + 'nonce' => $stateHandler->issue() + ); - $stateObj = array( "interim" => $interim_login, "uuid" =>uniqid() ); if ( !empty( $redirect_to ) ) { $stateObj["redirect_to"] = addslashes( $redirect_to ); } diff --git a/lib/WP_Auth0_Lock_Options.php b/lib/WP_Auth0_Lock_Options.php index dec1a920..236921ba 100644 --- a/lib/WP_Auth0_Lock_Options.php +++ b/lib/WP_Auth0_Lock_Options.php @@ -98,12 +98,11 @@ public function modal_button_name() { } public function get_state_obj( $redirect_to = null ) { - if ( isset( $_GET['interim-login'] ) && $_GET['interim-login'] == 1 ) { - $interim_login = true; - } else { - $interim_login = false; - } - $stateObj = array( "interim" => $interim_login, "uuid" =>uniqid() ); + $stateHandler = new WP_Auth0_State_Handler(); + $stateObj = array( + 'interim' => ( isset( $_GET['interim-login'] ) && $_GET['interim-login'] == 1 ), + 'nonce' => $stateHandler->issue() + ); if ( !empty( $redirect_to ) ) { $stateObj["redirect_to"] = addslashes( $redirect_to ); } diff --git a/lib/WP_Auth0_LoginManager.php b/lib/WP_Auth0_LoginManager.php index ee239e0b..c3fe9475 100755 --- a/lib/WP_Auth0_LoginManager.php +++ b/lib/WP_Auth0_LoginManager.php @@ -162,6 +162,12 @@ public function init_auth0() { return; } + // Check for valid state UUID + $state_decoded = json_decode( base64_decode( $this->query_vars( 'state' ) ), TRUE ); + if ( empty( $state_decoded[ 'nonce' ] ) || ! WP_Auth0_State_Handler::validate( $state_decoded[ 'nonce' ] ) ) { + $this->die_on_login( __( 'Invalid state', 'wp-auth0' ) ); + } + try { if ( $this->query_vars( 'auth0' ) === 'implicit' ) { $this->implicit_login(); @@ -170,19 +176,11 @@ public function init_auth0() { } } catch (WP_Auth0_LoginFlowValidationException $e) { - $msg = __( 'There was a problem with your log in. ', 'wp-auth0' ); - $msg .= ' '. $e->getMessage(); - $msg .= '

'; - $msg .= '' . __( '← Login', 'wp-auth0' ) . ''; - wp_die( $msg ); + $this->die_on_login( $e->getMessage(), $e->getCode() ); } catch (WP_Auth0_BeforeLoginException $e) { - $msg = __( 'You have logged in successfully, but there is a problem accessing this site. ', 'wp-auth0' ); - $msg .= ' '. $e->getMessage(); - $msg .= '

'; - $msg .= '' . __( '← Logout', 'wp-auth0' ) . ''; - wp_die( $msg ); + $this->die_on_login( $e->getMessage(), $e->getCode(), FALSE ); } catch (Exception $e) { @@ -206,9 +204,7 @@ public function redirect_login() { } $code = $this->query_vars( 'code' ); - $state = $this->query_vars( 'state' ); - - $stateFromGet = json_decode( base64_decode( $state ) ); + $state_decoded = json_decode( base64_decode( $this->query_vars( 'state' ) ), TRUE ); $domain = $this->a0_options->get( 'domain' ); @@ -265,12 +261,12 @@ public function redirect_login() { $userinfo = json_decode( $response['body'] ); if ( $this->login_user( $userinfo, $data->id_token, $data->access_token ) ) { - if ( ! empty( $stateFromGet->interim ) ) { + if ( ! empty( $state_decoded->interim ) ) { include WPA0_PLUGIN_DIR . 'templates/login-interim.php'; exit(); } else { - if ( ! empty( $stateFromGet->redirect_to ) && wp_login_url() !== $stateFromGet->redirect_to ) { - $redirectURL = $stateFromGet->redirect_to; + if ( ! empty( $state_decoded->redirect_to ) && wp_login_url() !== $state_decoded->redirect_to ) { + $redirectURL = $state_decoded->redirect_to; } else { $redirectURL = $this->a0_options->get( 'default_login_redirection' ); } @@ -537,6 +533,32 @@ protected function query_vars( $key ) { return null; } + /** + * Die during login process with a message + * + * @param string $msg - translated error message to display + * @param string|int $code - error code, if given + * @param bool $login_link - TRUE for login link, FALSE for logout link + */ + protected function die_on_login( $msg = '', $code = 0, $login_link = TRUE ) { + + wp_die( sprintf( + '%s: %s [%s: %s]

%s', + $login_link + ? __( 'There was a problem with your log in', 'wp-auth0' ) + : __( 'You have logged in successfully, but there is a problem accessing this site', 'wp-auth0' ), + ! empty( $msg ) + ? sanitize_text_field( $msg ) + : __( 'Please see the site administrator', 'wp-auth0' ), + __( 'error code', 'wp-auth0' ), + $code ? sanitize_text_field( $code ) : __( 'unknown', 'wp-auth0' ), + $login_link ? wp_login_url() : wp_logout_url(), + $login_link + ? __( '← Login', 'wp-auth0' ) + : __( '← Logout', 'wp-auth0' ) + ) ); + } + /** * DEPRECATED 3.5.0 * Deprecated to improve the functionality and move to a new class diff --git a/lib/WP_Auth0_State_Handler.php b/lib/WP_Auth0_State_Handler.php new file mode 100644 index 00000000..291022fc --- /dev/null +++ b/lib/WP_Auth0_State_Handler.php @@ -0,0 +1,78 @@ +uniqid = self::generateNonce(); + } + + /** + * Set the unique ID for state and return + * + * @return string + */ + public function issue() { + $this->store(); + return $this->uniqid; + } + + /** + * Set the state cookie value + * + * @return bool + */ + protected function store() { + return setcookie( self::COOKIE_NAME, $this->uniqid, time() + $this->cookieExpiresIn ); + } + + /** + * Check if the stored state matches a specific value + * + * @param $state + * + * @return bool + */ + public static function validate( $state ) { + $valid = isset( $_COOKIE[ self::COOKIE_NAME ] ) ? $_COOKIE[ self::COOKIE_NAME ] === $state : FALSE; + self::reset(); + return $valid; + } + + /** + * Reset the state cookie value + * + * @return bool + */ + public static function reset() { + return setcookie( self::COOKIE_NAME, '', 0 ); + } + + /** + * Generate a pseudo-random ID (not cryptographically secure) + * + * @see https://stackoverflow.com/a/1846229/728480 + * + * @return string + */ + public static function generateNonce() { + return md5( uniqid( rand(), true ) ); + } +} \ No newline at end of file From 32312798b107f0e7def7b9c8526e8bc3b210ee4d Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Mon, 5 Mar 2018 11:04:50 -0800 Subject: [PATCH 2/4] remove stateHandler->issue() from SLO logout footer template to avoid setting a cookie unecessarily --- lib/WP_Auth0_LoginManager.php | 13 ------------ templates/auth0-singlelogout-handler.php | 27 ++++++++++++------------ 2 files changed, 14 insertions(+), 26 deletions(-) diff --git a/lib/WP_Auth0_LoginManager.php b/lib/WP_Auth0_LoginManager.php index c3fe9475..fec02529 100755 --- a/lib/WP_Auth0_LoginManager.php +++ b/lib/WP_Auth0_LoginManager.php @@ -66,19 +66,6 @@ public function auth0_singlelogout_footer( $previous_html ) { return; } - $current_user = get_currentauth0user(); - $user_profile = $current_user->auth0_obj; - - if ( empty( $user_profile ) ) { - return; - } - - $lock_options = new WP_Auth0_Lock10_Options(); - $cdn = $this->a0_options->get('auth0js-cdn'); - $client_id = $this->a0_options->get( 'client_id' ); - $domain = $this->a0_options->get( 'domain' ); - $logout_url = wp_logout_url( get_permalink() ) . '&SLO=1'; - include WPA0_PLUGIN_DIR . 'templates/auth0-singlelogout-handler.php'; } diff --git a/templates/auth0-singlelogout-handler.php b/templates/auth0-singlelogout-handler.php index 562d57b0..113c8e9a 100644 --- a/templates/auth0-singlelogout-handler.php +++ b/templates/auth0-singlelogout-handler.php @@ -1,28 +1,29 @@ - +auth0_obj ) ) { + return; +} +?> + From 0fcf43504d2ff16d6582827b68694aba2ec1dd86 Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Mon, 5 Mar 2018 15:06:24 -0800 Subject: [PATCH 3/4] Review feedback on state handler property --- lib/WP_Auth0_State_Handler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/WP_Auth0_State_Handler.php b/lib/WP_Auth0_State_Handler.php index 291022fc..eed003cf 100644 --- a/lib/WP_Auth0_State_Handler.php +++ b/lib/WP_Auth0_State_Handler.php @@ -5,7 +5,7 @@ class WP_Auth0_State_Handler { /** * @var string */ - protected $uniqid = ''; + protected $uniqid; /** * @var string From 18c574c2e14baebf953c74c0e9912e05d203b696 Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Thu, 8 Mar 2018 15:42:24 -0800 Subject: [PATCH 4/4] Better state getting in WP_Auth0_LoginManager 123456789012345678901234567890123456789012345678901234567890123456789012 Added a get_state method to WP_Auth0_LoginManager to get and parse the state parameter as an object. Added 'state' to allowed query_vars. --- WP_Auth0.php | 1 + lib/WP_Auth0_LoginManager.php | 31 +++++++++++++++++++++++-------- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/WP_Auth0.php b/WP_Auth0.php index 5aabdeb1..c01adb1c 100644 --- a/WP_Auth0.php +++ b/WP_Auth0.php @@ -244,6 +244,7 @@ public function a0_register_query_vars( $qvars ) { $qvars[] = 'a0_action'; $qvars[] = 'auth0'; $qvars[] = 'code'; + $qvars[] = 'state'; return $qvars; } diff --git a/lib/WP_Auth0_LoginManager.php b/lib/WP_Auth0_LoginManager.php index fec02529..c1ee0671 100755 --- a/lib/WP_Auth0_LoginManager.php +++ b/lib/WP_Auth0_LoginManager.php @@ -6,6 +6,7 @@ class WP_Auth0_LoginManager { protected $default_role; protected $ignore_unverified_email; protected $users_repo; + protected $state; public function __construct( WP_Auth0_UsersRepo $users_repo, $a0_options = null, $default_role = null, $ignore_unverified_email = false ) { @@ -149,9 +150,10 @@ public function init_auth0() { return; } - // Check for valid state UUID - $state_decoded = json_decode( base64_decode( $this->query_vars( 'state' ) ), TRUE ); - if ( empty( $state_decoded[ 'nonce' ] ) || ! WP_Auth0_State_Handler::validate( $state_decoded[ 'nonce' ] ) ) { + // Check for valid state nonce + // See https://auth0.com/docs/protocols/oauth2/oauth-state + $state_decoded = $this->get_state(); + if ( empty( $state_decoded->nonce ) || ! WP_Auth0_State_Handler::validate( $state_decoded->nonce ) ) { $this->die_on_login( __( 'Invalid state', 'wp-auth0' ) ); } @@ -191,7 +193,7 @@ public function redirect_login() { } $code = $this->query_vars( 'code' ); - $state_decoded = json_decode( base64_decode( $this->query_vars( 'state' ) ), TRUE ); + $state_decoded = $this->get_state(); $domain = $this->a0_options->get( 'domain' ); @@ -294,7 +296,7 @@ public function redirect_login() { public function implicit_login() { $token = $_POST['token']; - $stateFromGet = json_decode( base64_decode( $_POST['state'] ) ); + $state_decoded = $this->get_state(); $secret = $this->a0_options->get_client_secret_as_key(); @@ -310,12 +312,12 @@ public function implicit_login() { $decodedToken->user_id = $decodedToken->sub; if ( $this->login_user( $decodedToken, $token, null ) ) { - if ( ! empty( $stateFromGet->interim ) ) { + if ( ! empty( $state_decoded->interim ) ) { include WPA0_PLUGIN_DIR . 'templates/login-interim.php'; exit(); } else { - if ( ! empty( $stateFromGet->redirect_to ) && wp_login_url() !== $stateFromGet->redirect_to ) { - $redirectURL = $stateFromGet->redirect_to; + if ( ! empty( $state_decoded->redirect_to ) && wp_login_url() !== $state_decoded->redirect_to ) { + $redirectURL = $state_decoded->redirect_to; } else { $redirectURL = $this->a0_options->get( 'default_login_redirection' ); } @@ -520,6 +522,19 @@ protected function query_vars( $key ) { return null; } + /** + * Get and store state returned from Auth0 + * + * @return object|null + */ + protected function get_state() { + + if ( empty( $this->state ) ) { + $this->state = json_decode( base64_decode( $this->query_vars( 'state' ) ) ); + } + return is_object( $this->state ) ? $this->state : null; + } + /** * Die during login process with a message *