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

Jump to bottom

Fix SLO redirect, SLO on when SSO off, SSO setting not pushed to dashboard #466

Merged
merged 2 commits into from
May 24, 2018
Merged

Fix SLO redirect, SLO on when SSO off, SSO setting not pushed to dashboard #466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3bc2f09
Fixing SLO redirect and SSO setting push to dashboard
joshcanhelp May 22, 2018
9aa938b
Changes based on PR review feedback
joshcanhelp May 24, 2018
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
3 changes: 1 addition & 2 deletions lib/WP_Auth0_Lock10_Options.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,10 @@ protected function build_settings( $settings ) {
public function get_sso_options() {
$options["scope"] = WP_Auth0_LoginManager::get_userinfo_scope( 'sso' );

$options["responseType"] = 'token id_token';
if ( $this->get_auth0_implicit_workflow() ) {
$options["responseType"] = 'id_token';
$options["redirectUri"] = $this->get_implicit_callback_url();
} else {
$options["responseType"] = 'code';
$options["redirectUri"] = $this->get_code_callback_url();
}

Expand Down
45 changes: 38 additions & 7 deletions lib/admin/WP_Auth0_Admin_Features.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ public function render_password_policy( $args = array() ) {

/**
* Render form field and description for the `sso` option.
* If SSO is off, the SLO setting will be hidden and turned off as well.
* IMPORTANT: Internal callback use only, do not call this function directly!
*
* @param array $args - callback args passed in from add_settings_field().
Expand All @@ -140,7 +141,7 @@ public function render_password_policy( $args = array() ) {
* @see add_settings_field()
*/
public function render_sso( $args = array() ) {
$this->render_switch( $args['label_for'], $args['opt_name'] );
$this->render_switch( $args['label_for'], $args['opt_name'], 'wpa0_singlelogout' );
Copy link
Contributor

Choose a reason for hiding this comment

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

since this method is named after sso. What is this slo line doing? I understand that is displaying a "toggle button" for enabling slo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is this slo line doing? I understand that is displaying a "toggle button" for enabling slo

Seems like you're answering your own question here ... can you rephrase?

$this->render_field_description(
__( 'SSO allows users to sign in once to multiple Applications in the same tenant. ', 'wp-auth0' ) .
__( 'Turning this on will attempt to automatically log a user in when they visit wp-login.php. ', 'wp-auth0' ) .
Expand Down Expand Up @@ -291,24 +292,54 @@ public function render_override_wp_avatars( $args = array() ) {
}

public function basic_validation( $old_options, $input ) {
// SLO will be turned off in WP_Auth0_Admin_Features::sso_validation() if SSO is not on.
$input['singlelogout'] = ( isset( $input['singlelogout'] ) ? $input['singlelogout'] : 0 );
$input['override_wp_avatars'] = ( isset( $input['override_wp_avatars'] ) ? $input['override_wp_avatars'] : 0 );

return $input;
}

/**
* Update the Auth0 Application if SSO is turned on and disable SLO if it is turned off.
*
* @param array $old_options - option values before saving.
* @param array $input - new option values being saved.
*
* @return array
*/
public function sso_validation( $old_options, $input ) {
$input['sso'] = ( isset( $input['sso'] ) ? $input['sso'] : 0 );
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it would be nice if this method doesn't modify the "original values" passed as parameters and instead, creates a copy of them or just new vars for the values it's going to use. e.g. don't overwrite $input['sso']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not an option, this is how forms are validated in WP. Overwriting the $input array is necessary to validate what's being saved (security with sanitization and reliability with making sure we expect the correct values later)

$is_sso = ! empty( $input['sso'] );

if ( $old_options['sso'] != $input['sso'] && 1 == $input['sso'] ) {
if ( false === WP_Auth0_Api_Client::update_client( $input['domain'], $input['auth0_app_token'], $input['client_id'], $input['sso'] == 1 ) ) {
// SLO does not function without SSO so turn off SLO if SSO is off.
if ( ! $is_sso ) {
unset( $input['singlelogout'] );
}

$error = __( 'There was an error updating your Auth0 App to enable SSO. To do it manually, turn it ', 'wp-auth0' );
$error .= '<a href="https://auth0.com/docs/sso/single-sign-on#1">HERE</a>.';
$this->add_validation_error( $error );
// If SSO is off or nothing was changed, exit early.
if ( ! $is_sso || $old_options['sso'] === $input['sso'] ) {
return $input;
}

}
$app_update_success = false;
$app_token = WP_Auth0_Api_Client::get_client_token();
if ( $app_token ) {
$update_result = WP_Auth0_Api_Client::update_client(
$input['domain'],
$app_token,
$input['client_id'],
true
);
$app_update_success = (bool) $update_result;
}
if ( ! $app_update_success ) {
$this->add_validation_error(
__( 'The SSO setting for your Application could not be updated automatically. ', 'wp-auth0' ) .
__( 'Check that "Use Auth0 instead of the IdP to do Single Sign On" is turned on in the ', 'wp-auth0' ) .
$this->get_dashboard_link( 'applications/' . $input['client_id'] . '/settings' )
);
}

return $input;
}

Expand Down
12 changes: 9 additions & 3 deletions templates/auth0-singlelogout-handler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
if ( empty( $current_user->auth0_obj ) ) {
return;
}

$logout_url = wp_logout_url();
$logout_url = html_entity_decode( $logout_url );
$logout_url = add_query_arg( 'redirect_to', get_permalink(), $logout_url );
$logout_url = add_query_arg( 'SLO', 1, $logout_url );
?>
<script id="auth0" src="<?php echo esc_url( $this->a0_options->get('auth0js-cdn') ) ?>"></script>
<script type="text/javascript">
Expand All @@ -18,9 +23,10 @@
domain:'<?php echo sanitize_text_field( $this->a0_options->get( 'domain' ) ); ?>'
});

webAuth.checkSession( { 'responseType' : 'token', 'redirectUri' : window.location.href }, function ( err ) {
if ( err && err.error ==='login_required' ) {
window.location = '<?php echo esc_url( wp_logout_url( get_permalink() ) . '&SLO=1' ); ?>';
var sloOptions = { 'responseType' : 'token id_token', 'redirectUri' : '<?php echo home_url() ?>' };
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better if this options are obtained through a method, like the one you already have ...Lock10_Options#get_sso_options()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That method exists to check settings and decide what is passed to checkSession. In this case, we're just checking whether someone is logged in or not, which does not depend on site options. Using that method would be overly complicated, IMHO.

webAuth.checkSession( sloOptions, function ( err ) {
if ( err && err.error && 'login_required' === err.error ) {
window.location = '<?php echo $logout_url ?>';
Copy link
Contributor

Choose a reason for hiding this comment

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

if an error happens, you show the log out page. But if the session is still valid? Should you do anything with the new received tokens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But if the session is still valid? Should you do anything with the new received tokens?

No. We're checking for an Auth0 session and, if one is not found, they get logged out. This only runs if someone is logged in so if this SLO check finds a session, they already have a WP session and nothing should change.

}
}
);
Expand Down
9 changes: 2 additions & 7 deletions templates/auth0-sso-handler-lock10.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,12 @@
});

var options = <?php echo json_encode( $lock_options->get_sso_options() ); ?>;
options.responseType = 'token id_token';
webAuth.checkSession(options
, function (err, authResult) {
webAuth.checkSession(options, function (err, authResult) {
if (typeof(authResult) === 'undefined') {
return;
}

if (typeof(authResult.code) !== 'undefined') {
window.location = '<?php echo WP_Auth0_Options::Instance()->get_wp_auth0_url(); ?>&code='
+ authResult.code + '&state=' + authResult.state;
} else if (typeof(authResult.idToken) !== 'undefined') {
if (authResult.idToken) {
jQuery(document).ready(function($){
var $form=$(document.createElement('form')).css({display:'none'}).attr("method","POST").attr("action","<?php
echo add_query_arg( 'auth0', 'implicit', site_url( 'index.php' ) ); ?>");
Expand Down