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

Improve callback functionality #313

Closed
simenandre opened this issue Oct 24, 2020 · 6 comments
Closed

Improve callback functionality #313

simenandre opened this issue Oct 24, 2020 · 6 comments

Comments

@simenandre
Copy link
Member

simenandre commented Oct 24, 2020

Currently, our context looks for a few parts in the URL (either # or ?). I don't think this is the ultimate solution for handling callback, and I do see some having experience caused by the way we do it (such as #307).

Any thoughts on how we can improve it?

cc: @ryancole

@ryancole
Copy link
Contributor

Previously, we used Microsoft Azure's MSAL library. It's a great library with a great user experience. Unfortunately, it's only compatible with their Azure B2C (AD) services. I wish that were not the case, considering it bills itself as an OpenID / OAuth client library. Although upon recent review it looks like they've taken that verbiage out of their README. Regardless, MSAL operated in the same way - it made you specify an auth redirect handler and then in it you had to handle the token (via hash parameter) yourself. It was up to the developer to handle the login token and redirect as-needed.

I think this is a safe approach and requires the developer to have an understanding of the workflow involved. Not much magic going on.

Perhaps this is a documentation thing that we can improve?

Maybe there's a better automated approach? It'd just end up being a wrapper around a redirect though, I figure.

@simenandre
Copy link
Member Author

Actually, we could provide multiple options easily. We can export a callback component (or otherwise), but keep it clean for those who'd like with a simple boolean (autoCallback={true}-kind of thing)?

I agree that it's much safer to require the developer as well, and if we do multiple options, we should probably default to having a callback component/function instead of doing it for all routes.

@ed-sparkes
Copy link
Contributor

Not really sure what improvements you are looking for here. Have worked a lot with auth0 in the past they have a similar react library to this that wraps their single page app sdk (essentially auth0 specific OIDC). They handle the redirect in much the same way, however there may be some small improvements that can be gleaned from their library https://github.com/auth0/auth0-react/blob/master/src/auth0-provider.tsx

@ed-sparkes
Copy link
Contributor

One specific improvement i can see that we could include from their library and that i have found useful in the past is to catch and export in the useAuth hook any authentication errors, this will give the consumer the opportunity to act on these such as display an error message in their client or initiate a logout flow etc. If i get a chance over the next week i will look at adding something similar here

@simenandre
Copy link
Member Author

@ed-sparkes That sounds great! 🎉

@simenandre
Copy link
Member Author

I'm closing this for now; probably something we should revisit once we close #652.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants