-
Notifications
You must be signed in to change notification settings - Fork 96
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
Fix Passwordless handling; update Lock instantiation #434
Conversation
@@ -27,8 +27,7 @@ We're happy to review and approve new filters and actions that help you integrat | |||
**Thank you in advance!** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only line break changes here. Surfaced during a rebase.
@@ -1,98 +0,0 @@ | |||
<?php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Combined with templates/auth0-login-form.php
@@ -1,212 +1,27 @@ | |||
<?php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified to handle standard and Passwordless
var callback = null; | ||
|
||
|
||
<?php if ( $lock_options->get_auth0_implicit_workflow() ) { ?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All this moved into a separate file in #426
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the new time, please try to keep the "refactor->move" actions in the same PR for diff tracking.
@@ -1,34 +0,0 @@ | |||
<?php if ( isset( $_GET['message'] ) ): ?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked with @glena, this is likely not used anymore. No references in the plugin anymore and duplicates the wp_die()
catches during login.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is likely not used anymore
Are you tracking usage somehow? can this be safely removed or would it be considered a breaking change in your opinion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it can be safely removed, yes. No other references to this and this functionality (minus the URL param) is part of the main login flow. There's nothing in the plugin that uses this functionality and I've never seen it during any testing thus far.
) | ||
); | ||
|
||
$login_tpl = apply_filters( 'auth0_login_form_tpl', 'auth0-login-form.php', $lock_options, $canShowLegacyLogin ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a note for myself to document this hook when released
"params" => array("state" => $state ), | ||
"params" => array( | ||
"state" => $this->get_state_obj( $redirect_to ), | ||
"scope" => apply_filters( 'auth0_auth_param_scopes', $this->_scopes ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a note for myself to document this hook when released
@@ -128,6 +128,19 @@ All is not lost! | |||
* If you have questions about how to use Auth0 or the plugin, please [post on our community site](https://community.auth0.com/) or create a [support forum request here](https://wordpress.org/support/plugin/auth0). | |||
* You can also see additional documentation and answers on our [support site](https://support.auth0.com/). Customers on a paid Auth0 plan can [submit a trouble ticket](https://support.auth0.com/tickets) for a fast response. | |||
|
|||
= My question is not covered here; what do I do? = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surfaced during a rebase.
|
||
if (lock.on) { | ||
lock.on('ready', function(){ | ||
if ( lock.options['$client'].subscription === 'free' ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was not working properly in all cases. Made a note to address this in a later release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK we don't rely on this client info value since it's easy to change on the fly (@luisrudge). If that's the case then Josh can you do the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct. we don't use that anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
e3046c6
to
4a4261e
Compare
4a4261e
to
87aad83
Compare
Documentation for the plugin resides in mulitple places - auth0.com docs, wp.org readme, GitHub readme - and this commit is part of an on- going effort to consolidate and clarify. This commit removes the installation instructions on wp.org, pointing to the docs site instead. It also updates the screenshots and adds information about support to the FAQs. This commit also clearly points the GitHub readme to docs in a few cases and updates the dev instructions.
State generation, specifically cookie storage, needs to happen before any output. The previous arch was ok for login page generation but failed with a "headers already set" error if used in the shortcode. This addresses the issue by storing state earlier and getting the value later.
Passwordless (PWL) was not working with new tenants and used the old, separate PWL JS library. This commit updates the Lock library used to 11.5, moves and re-writes the JS used to show Lock, loads required JS in one place for widget + shortcode + wp-login, and revises state handling to work better for the shortcode and widget.
87aad83
to
313a1bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly text format issues
readme.txt
Outdated
|
||
All is not lost! | ||
|
||
* If you're setting up the plugin for the first time or having issues after an upgrade, please review the [configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new line after [configuration
readme.txt
Outdated
|
||
* If you're setting up the plugin for the first time or having issues after an upgrade, please review the [configuration | ||
page at auth0.com/docs](https://auth0.com/docs/cms/wordpress/configuration) | ||
* If you found a bug in the plugin code [submit an issue](https://github.com/auth0/wp-auth0/issues) or [create a pull |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new line after pull
readme.txt
Outdated
* If you're setting up the plugin for the first time or having issues after an upgrade, please review the [configuration | ||
page at auth0.com/docs](https://auth0.com/docs/cms/wordpress/configuration) | ||
* If you found a bug in the plugin code [submit an issue](https://github.com/auth0/wp-auth0/issues) or [create a pull | ||
request](https://github.com/auth0/wp-auth0/pulls) on [GitHub](https://github.com/auth0/wp-auth0/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO the last github link is overkill. It's already on the pull requests one
readme.txt
Outdated
page at auth0.com/docs](https://auth0.com/docs/cms/wordpress/configuration) | ||
* If you found a bug in the plugin code [submit an issue](https://github.com/auth0/wp-auth0/issues) or [create a pull | ||
request](https://github.com/auth0/wp-auth0/pulls) on [GitHub](https://github.com/auth0/wp-auth0/). | ||
* If you have questions about how to use Auth0 or the plugin, please [post on our community site](https://community |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new line after community
readme.txt
Outdated
request](https://github.com/auth0/wp-auth0/pulls) on [GitHub](https://github.com/auth0/wp-auth0/). | ||
* If you have questions about how to use Auth0 or the plugin, please [post on our community site](https://community | ||
.auth0.com/) or create a [support forum request here](https://wordpress.org/support/plugin/auth0). | ||
* You can also see additional documentation and answers on our [support site](https://support.auth0.com/). Customers on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new line after on
var callback = null; | ||
|
||
|
||
<?php if ( $lock_options->get_auth0_implicit_workflow() ) { ?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the new time, please try to keep the "refactor->move" actions in the same PR for diff tracking.
|
||
if (lock.on) { | ||
lock.on('ready', function(){ | ||
if ( lock.options['$client'].subscription === 'free' ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK we don't rely on this client info value since it's easy to change on the fly (@luisrudge). If that's the case then Josh can you do the same?
@@ -1,34 +0,0 @@ | |||
<?php if ( isset( $_GET['message'] ) ): ?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is likely not used anymore
Are you tracking usage somehow? can this be safely removed or would it be considered a breaking change in your opinion?
313a1bf
to
95e1cea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG
Main focus here was to update Lock Passwordless to use the combined library and move all instantiation JS to an external file, populated via
wpAuth0LockGlobal
Fixes #400, #430