gatsby-theme-auth0 icon indicating copy to clipboard operation
gatsby-theme-auth0 copied to clipboard

Add signup method to auth service

Open devuxer opened this issue 4 years ago • 9 comments

  • New signup method brings up auth page defaulted to signup rather than login scenario.
  • Uses screen_hint option of auth0.authorize method (see universal login docs).

devuxer avatar May 20 '20 01:05 devuxer

Deploy request for gatsby-theme-auth0 pending review.

Review with commit f64d7fefaa56ef90d36a5e89adae6d811eb49b4b

https://app.netlify.com/sites/gatsby-theme-auth0/deploys

netlify[bot] avatar May 20 '20 01:05 netlify[bot]

Deploy request for gatsby-theme-auth0-custom pending review.

Review with commit f64d7fefaa56ef90d36a5e89adae6d811eb49b4b

https://app.netlify.com/sites/gatsby-theme-auth0-custom/deploys

netlify[bot] avatar May 20 '20 01:05 netlify[bot]

Hey @devuxer thanks for adding this.

I'm wondering instead of adding a new method, what if we updated the login method to take in args and pass that through auth0.authorize?

Then the usage would be:

<button onClick={() => AuthService.login({ screen_hint: "signup" })}>Login</button>

Let me know your thoughts.

epilande avatar May 26 '20 17:05 epilande

Hey @epilande ,

Passing the entire options as args would definitely be a more flexible solution. I went with a dedicated method because I thought it would be a lot more discoverable. The Auth0 Authentication API docs don't even mention screen_hint. I was really lucky to discover it in the New Universal Login Experience docs.

As I type this, though, it occurs to me that my signup method may not even work for people using the "old" login method. I actually have no idea what would happen in that case.

So yes, I think you're right that just allowing options to be passed to login would be the better way to go, but I think it would be pretty important to document how to do a signup and provide good typings for the options object. (It looks like @types/auth0-js has AuthorizeOptions so the typings might be trivial.)

Anyway, I'm happy to take a crack at this, but I'm a little concerned about messing something up. So, it might be just as much work to review what I do as implement yourself. Let me know which you prefer!

devuxer avatar May 26 '20 19:05 devuxer

A non intrusive way to handle this would be

-  public login = () => {
+  public login = (options?: auth0.AuthorizeOptions) => {
     if (!isBrowser) return;
     // Save postLoginUrl so we can redirect user back to where they left off after login screen
     localStorage.setItem("postLoginUrl", window.location.pathname);
-    this.auth0 && this.auth0.authorize();
+    this.auth0 && this.auth0.authorize(options);
   };

I agree this package needs better documentation. I created a new issue to track this https://github.com/epilande/gatsby-theme-auth0/issues/202

epilande avatar May 26 '20 20:05 epilande

@epilande,

Just tested your proposed changes. Worked as expected, so I commited it to patch-1. Should be able to pull now.

devuxer avatar May 27 '20 00:05 devuxer

(also took a shot at documenting a signup scenario, which should show up as a separate pull request)

devuxer avatar May 27 '20 01:05 devuxer

@epilande ,

Just wanted to check in and see if you needed anything else done before you could merge this. Thanks.

devuxer avatar Jun 21 '20 00:06 devuxer

@devuxer sorry for the delayed response, will get this merged in and cut a new release soon

epilande avatar Jul 07 '20 06:07 epilande