Changing home_url() to site_url(), wp_login_url(), and wp_logout_url() #360
Conversation
…xpected rather than a site page; changed home_urL() to wp_login_url() or wp_logout_url() where appropriate
lib/WP_Auth0_Api_Client.php
Outdated
| ), | ||
| "allowed_origins"=>array( | ||
| home_url( '/wp-login.php' ) | ||
| wp_login_url() |
There was a problem hiding this comment.
@cocojoe - Plugin is formatted both ways throughout ... my IDE can be configured whatever way. Any opinion on what we should stick with? I'm impartial.
| $response = wp_remote_post( $endpoint , array( | ||
| 'method' => 'POST', | ||
| 'headers' => $headers, | ||
| 'body' => json_encode( array( | ||
| 'name' => $name, | ||
| 'callbacks' => array( | ||
| home_url( '/index.php?auth0=1' ), | ||
| home_url( '/wp-login.php' ) | ||
| site_url( 'index.php?auth0=1' ), |
There was a problem hiding this comment.
So the site_url always includes the trailing slash?
There was a problem hiding this comment.
@cocojoe - It will correct either way:
# site_url( 'index.php' ):
http://localhost/index.php
# site_url( '/index.php' ):
http://localhost/index.php
# home_url( '/index.php' ):
http://localhost/index.php
# home_url( 'index.php' ):
http://localhost/index.php
lib/WP_Auth0_Api_Operations.php
Outdated
|
|
||
| $get_user_script = str_replace( '{THE_WS_TOKEN}', $migration_token, WP_Auth0_CustomDBLib::$get_user_script ); | ||
| $get_user_script = str_replace( '{THE_WS_URL}', get_home_url() . '/index.php?a0_action=migration-ws-get-user', $get_user_script ); | ||
| $get_user_script = str_replace( '{THE_WS_URL}', site_url(), $get_user_script ); |
There was a problem hiding this comment.
why is this one $get_user_script but the previous one is index.php?a0_action=migration-ws-login even though appears to be a $login_script ?
There was a problem hiding this comment.
@cocojoe - Eeks, big miss on my part. Fixing.
| @@ -91,14 +91,12 @@ public function logout() { | |||
| $auto_login = absint( $this->a0_options->get( 'auto_login' ) ); | |||
|
|
|||
| if ( $slo && isset( $_REQUEST['SLO'] ) ) { | |||
| $redirect_to = $_REQUEST['redirect_to']; | |||
| wp_redirect( $redirect_to ); | |||
| wp_redirect( $_REQUEST['redirect_to'] ); | |||
There was a problem hiding this comment.
does this wp_redirect method validate the redirect_to in some way?
There was a problem hiding this comment.
@cocojoe - Uses wp_sanitize_redirect()
lib/WP_Auth0_Options.php
Outdated
| @@ -138,7 +138,7 @@ protected function defaults() { | |||
| 'auto_provisioning' => false, | |||
| 'default_login_redirection' => home_url(), | |||
|
|
|||
| 'auth0_server_domain' => 'auth0.auth0.com', | |||
| 'auth0_server_domain' => 'auth0.auth0.com', | |||
| @@ -80,14 +80,14 @@ | |||
| return p; | |||
| }, {}); | |||
|
|
|||
| post('<?php echo home_url( '/index.php?auth0=implicit' ); ?>', { | |||
| post('<?php echo site_url( 'index.php?auth0=implicit' ); ?>', { | |||
| token:data.id_token, | |||
| state:data.state | |||
| }, 'POST'); | |||
| } | |||
|
|
|||
| // lock.on("authenticated", function(authResult) { | |||
There was a problem hiding this comment.
maybe time to just remove this commented code?
|
@cocojoe - For your first question ... the values shouldn't be exactly the same as before, that's part of the issue. It was something I flagged in my initial review of the plugin (local notes). |
|
@cocojoe - Ready for re-review |
* Added RS256 JWT Support (Default for New Clients) * Add caching to JWKS fetching * Added Lock 11, Update SSO (#350) * Fixed Admin migration step in Setup Wizard * Added WP_Auth0_Api_Client::signup_user
…xpected rather than a site page; changed home_urL() to wp_login_url() or wp_logout_url() where appropriate
…th0 into changed-home-url-to-site-url
This pull request fixes #339 and #342 as well as a number of other instances of home_url(). The only time home_url() should be used is to display a front-end page of the site. For most installs, these two will be the same but for the ones that aren't this change is is important.