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

Add new options for WordPress Login Enabled #642

Merged
merged 5 commits into from
Mar 29, 2019
Merged

Conversation

joshcanhelp
Copy link
Contributor

@joshcanhelp joshcanhelp commented Mar 26, 2019

Changes

  • Add new options to allow more flexibility and security around when the WP login form is shown
  • Change how core login form display CSS is handled

UI change:

Screenshot 2019-03-29 12 34 13

References

Testing

Checklist

  • All existing and new tests complete without errors
  • All code quality tools/guidelines in the Contribution guide have been run/followed
  • All active GitHub CI checks have passed

@joshcanhelp joshcanhelp added this to the 3.10.0 milestone Mar 26, 2019
@auth0 auth0 deleted a comment from codecov-io Mar 26, 2019
@@ -682,9 +676,6 @@ public static function get_authorize_params( $connection = null, $redirect_to =
$params['connection'] = $connection;
}

// Get the telemetry header.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unused from previous PR.


if ( isset( $_GET['wle'] ) ) {
if ( $this->options->can_show_wp_login_form() ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Relevant change here.

@@ -310,10 +369,6 @@ public function basic_validation( $old_options, $input ) {
$input['client_id'] = sanitize_text_field( $input['client_id'] );
$input['cache_expiration'] = absint( $input['cache_expiration'] );

$input['wordpress_login_enabled'] = ( isset( $input['wordpress_login_enabled'] )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to wle_validation

@@ -373,6 +373,7 @@ public function basic_validation( $old_options, $input ) {
// Turn SLO off if SSO is off.
$input['singlelogout'] = empty( $input['singlelogout'] ) || empty( $input['sso'] ) ? 0 : 1;

$input['auto_login'] = empty( $input['auto_login'] ) ? 0 : 1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missing settings validation causing ULP setting to malfunction

@@ -118,12 +118,6 @@ public function login_auto() {
return false;
}

// Do not redirect Auth0 login processing.
// TODO: This should be removed as this page is no longer used for callbacks.
if ( null !== $this->query_vars( 'auth0' ) ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Callback is not actually processed on this page so this just closes a hole.

Reason for removed tests in tests/testLoginManager.php.

/**
* Test that the ULP redirect does not happen if wp-login.php is used as a callback.
*/
public function testThatUlpRedirectIsSkippedForCallback() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Explained above.

@joshcanhelp joshcanhelp marked this pull request as ready for review March 26, 2019 23:15
@joshcanhelp joshcanhelp changed the title Failing tests for new WLE setting field Add new options for WordPress Login Enabled Mar 26, 2019
@damieng
Copy link
Contributor

damieng commented Mar 27, 2019

I'm finding this UI a little confusing. Might it be better to put the descriptions next to the various checkboxes so you can see all the details at a glance? wle enabled, wle present etc. is hard to understand at a glance.

@joshcanhelp
Copy link
Contributor Author

@damieng - I think you're right. I'll take another pass at this, thank you.

@auth0 auth0 deleted a comment from codecov-io Mar 29, 2019
@auth0 auth0 deleted a comment from codecov-io Mar 29, 2019
damieng
damieng previously approved these changes Mar 29, 2019
@codecov-io
Copy link

codecov-io commented Mar 29, 2019

Codecov Report

Merging #642 into master will increase coverage by 1.13%.
The diff coverage is 89.39%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #642      +/-   ##
============================================
+ Coverage     40.71%   41.85%   +1.13%     
- Complexity     1231     1244      +13     
============================================
  Files            51       51              
  Lines          3753     3799      +46     
============================================
+ Hits           1528     1590      +62     
+ Misses         2225     2209      -16
Impacted Files Coverage Δ Complexity Δ
WP_Auth0.php 22.84% <0%> (-0.3%) 62 <0> (ø)
lib/WP_Auth0_WooCommerceOverrides.php 0% <0%> (ø) 8 <0> (ø) ⬇️
lib/WP_Auth0_DBManager.php 37.5% <100%> (+1.13%) 69 <0> (+1) ⬆️
lib/admin/WP_Auth0_Admin_Features.php 26.94% <100%> (+0.44%) 34 <0> (+1) ⬆️
lib/WP_Auth0_Options.php 74.28% <100%> (+3.95%) 37 <6> (+6) ⬆️
lib/admin/WP_Auth0_Admin_Generic.php 72.03% <100%> (+14.64%) 41 <9> (+3) ⬆️
lib/WP_Auth0_LoginManager.php 17.37% <100%> (-2.4%) 120 <0> (-1)
lib/admin/WP_Auth0_Admin_Basic.php 34.33% <97.05%> (+18.02%) 30 <3> (+3) ⬆️

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 dfbf49e...46d00a4. Read the comment docs.

damieng
damieng previously approved these changes Mar 29, 2019
Copy link
Contributor

@damieng damieng left a comment

Choose a reason for hiding this comment

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

It's groundhog day... again.

@joshcanhelp joshcanhelp merged commit 5557b0e into master Mar 29, 2019
@joshcanhelp joshcanhelp deleted the add-more-wle-options branch March 29, 2019 20:53
@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