Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
chore(auth): add js passwordless changes #8129
chore(auth): add js passwordless changes #8129
Changes from 8 commits
b55743c
0e9239c
c00a701
b2fab29
ed812d2
1796a30
506e1b1
aba4e40
1beddce
1995190
b546294
8ef54f5
6227f8b
471df92
947a44b
9d5b919
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Indentation is off here.
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.
Also, this info should be on the switching authentication flows page I believe.
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.
Also, be careful with headers, we created 3 new sections on this page. Should only be SMS OTP, Email OTP, WebAuthn Passkeys, and Password or SRP.
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.
Are we restricted to those 3? I think giving details on the new
USER_AUTH
flow is a necessary prerequisite to handle the new passwordless methods.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.
We want to be consistent with the other libraries. We'd provide conceptual introduction to the auth flow on the
Switching Authentication Flows
page.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.
Sounds good, I'll move it there - that section seems like a logical place for it to live in our doc structure (though I would still prefer it together 🙃)
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.
A good compromise might be to add a
<Callout />
mentioningUSER_AUTH
with a link to that pageThere 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.
It might be confusing to have types here that aren't referenced in the examples; the connection between these types and how to pass as options is unclear. Is there a way we can clarify this? Maybe using the
SignInInput
interface which is publicly exposed by the library... or just relying on the examples?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.
nextStep
may not be DONE in all cases. We should provide a partial implementation of thehandleNextSignInStep()
handlerThere 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 think we should avoid including internal types in the documentation; we should show an example of how to use the availableChallenges instead.