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

Fix Passwordless handling; update Lock instantiation #434

Merged
merged 6 commits into from
Apr 19, 2018

Conversation

joshcanhelp
Copy link
Contributor

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

  • Combined JS file to initiate Lock; removed JS from PHP template files
  • Removing unused template files
  • README support block, whitespace

Fixes #400, #430

@@ -27,8 +27,7 @@ We're happy to review and approve new filters and actions that help you integrat
**Thank you in advance!**
Copy link
Contributor Author

@joshcanhelp joshcanhelp Apr 16, 2018

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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() ) { ?>
Copy link
Contributor Author

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

Copy link
Contributor

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'] ) ): ?>
Copy link
Contributor Author

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.

Copy link
Contributor

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?

Copy link
Contributor Author

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 );
Copy link
Contributor Author

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 ),
Copy link
Contributor Author

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? =
Copy link
Contributor Author

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' ) {
Copy link
Contributor Author

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.

Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

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.
Copy link
Contributor

@lbalmaceda lbalmaceda left a 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
Copy link
Contributor

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
Copy link
Contributor

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/).
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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() ) { ?>
Copy link
Contributor

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' ) {
Copy link
Contributor

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'] ) ): ?>
Copy link
Contributor

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?

Copy link
Contributor

@lbalmaceda lbalmaceda left a comment

Choose a reason for hiding this comment

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

LG

This was referenced May 3, 2018
@joshcanhelp joshcanhelp mentioned this pull request Jun 5, 2018
@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.

None yet

3 participants