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

Do not update Auth0 Application when SSO is turned on #625

Merged
merged 3 commits into from
Feb 26, 2019

Conversation

joshcanhelp
Copy link
Contributor

@joshcanhelp joshcanhelp commented Feb 16, 2019

Changes

  • Remove Auth0 Application update from SSO validation middleware
  • Add SSO on to Application created process during setup wizard
  • Set Features tab defaults to int to match validation
  • Remove other deprecated middleware

Testing

  • This change adds unit test coverage

'jwt_configuration' => array(
'alg' => self::DEFAULT_CLIENT_ALG,
),

// "Use Auth0 to do Single Sign On"
'sso' => true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This option is default on for new applications so just making sure it's on for admins who want to use SSO in the plugin.

'sso' => false,
'singlelogout' => false,
'override_wp_avatars' => true,
'sso' => 0,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The settings are validated and checked for int 1 or 0. This adjusts the default value (used when the plugin is first installed) to use the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment to say 0=false, 1=true or is it well known in the PHP world?

@@ -12,7 +12,6 @@ class WP_Auth0_Admin_Advanced extends WP_Auth0_Admin_Generic {
protected $actions_middlewares = array(
'basic_validation',
'migration_ws_validation',
'link_accounts_validation',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Validation middleware left over from previous commit (#624)

@codecov-io
Copy link

codecov-io commented Feb 18, 2019

Codecov Report

Merging #625 into master will increase coverage by 0.5%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master     #625     +/-   ##
===========================================
+ Coverage     37.32%   37.82%   +0.5%     
- Complexity     1241     1243      +2     
===========================================
  Files            51       51             
  Lines          3845     3849      +4     
===========================================
+ Hits           1435     1456     +21     
+ Misses         2410     2393     -17
Impacted Files Coverage Δ Complexity Δ
lib/WP_Auth0_Api_Client.php 33.06% <ø> (ø) 81 <0> (ø) ⬇️
lib/admin/WP_Auth0_Admin_Advanced.php 33.45% <ø> (ø) 57 <0> (ø) ⬇️
lib/admin/WP_Auth0_Admin_Features.php 25.88% <100%> (+9.9%) 33 <0> (+2) ⬆️
lib/WP_Auth0_Options.php 84.31% <100%> (+0.47%) 28 <0> (ø) ⬇️
lib/WP_Auth0_Lock10_Options.php 70.53% <0%> (+0.89%) 61% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 190b0ac...7912bd4. Read the comment docs.

@@ -11,12 +11,7 @@ class WP_Auth0_Admin_Features extends WP_Auth0_Admin_Generic {

protected $actions_middlewares = array(
'basic_validation',
'georule_validation',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deprecated SSO validation middleware and others left over from previous commit (#624)

// 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 );
$input['sso'] = ! empty( $input['sso'] ) ? 1 : 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding in validation from removed sso_validation middleware.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe flip the inversion and results to simplify? empty( $input['sso'] ) ? 0 : 1;

$input['sso'] = ! empty( $input['sso'] ) ? 1 : 0;

// Turn SLO off if SSO is off.
$input['singlelogout'] = ! empty( $input['singlelogout'] ) && $input['sso'] ? 1 : 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using empty here because that array key may not be set. Not using empty for $input['sso'] because that is always normalized above.

@joshcanhelp joshcanhelp marked this pull request as ready for review February 18, 2019 16:32
@joshcanhelp joshcanhelp added this to the 3.10.0 milestone Feb 18, 2019
damieng
damieng previously approved these changes Feb 25, 2019
@joshcanhelp joshcanhelp merged commit 68c6f8e into master Feb 26, 2019
@joshcanhelp joshcanhelp deleted the remove-app-change-for-sso branch March 15, 2019 15:21
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants