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

Add passwordless endpoints #270

Merged
merged 8 commits into from
Jan 29, 2020
Merged

Add passwordless endpoints #270

merged 8 commits into from
Jan 29, 2020

Conversation

lbalmaceda
Copy link
Contributor

Changes

This PR adds support for invoking the new passwordless endpoints. It also adds a small note on the readme about them and the grant type that must be enabled first.

There are some UX changes on the second request (the one that performs the authentication). The endpoint expects a username parameter, but I'm mapping it from either email or phoneNumber. The same happens with otp, which is getting mapped from code. Please, evaluate if this is desired or if it's only adding noise.

Any formatting change included on the diff is due to the pre-commit hook.

References

SDK-1281

Testing

I've tested the email passwordless manually on a sample app. Unit tests are included as well.

  • This change adds unit test coverage
  • This change has been tested on the latest version of the platform/language or why not

Checklist

@lbalmaceda lbalmaceda added CH: Added medium This PR may require moderate effort to action, or contains many changes to review labels Jan 22, 2020
@lbalmaceda lbalmaceda added this to the v2-Next milestone Jan 22, 2020
@lbalmaceda lbalmaceda requested a review from a team January 22, 2020 19:26
Copy link
Contributor

@Widcket Widcket left a comment

Choose a reason for hiding this comment

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

Most are non-blocking, except the doc comments regarding the usage of links in sms.

@Widcket
Copy link
Contributor

Widcket commented Jan 23, 2020

There are some UX changes on the second request (the one that performs the authentication). The endpoint expects a username parameter, but I'm mapping it from either email or phoneNumber. The same happens with otp, which is getting mapped from code. Please, evaluate if this is desired or if it's only adding noise.

I think this is good from a UX perspective.

@lbalmaceda lbalmaceda force-pushed the add-passwordless-endpoints branch from 83c7c83 to 5da2e58 Compare January 24, 2020 21:53
@lbalmaceda lbalmaceda requested a review from Widcket January 29, 2020 17:30
Copy link
Contributor

@Widcket Widcket left a comment

Choose a reason for hiding this comment

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

There are a couple of leftover references that should be addressed.

@lbalmaceda lbalmaceda requested a review from Widcket January 29, 2020 17:51
Copy link
Contributor

@Widcket Widcket left a comment

Choose a reason for hiding this comment

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

LGTM

@lbalmaceda lbalmaceda merged commit b061c42 into master Jan 29, 2020
@lbalmaceda lbalmaceda deleted the add-passwordless-endpoints branch January 29, 2020 18:40
@lbalmaceda lbalmaceda modified the milestones: v2-Next, v2.2.0 Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CH: Added medium This PR may require moderate effort to action, or contains many changes to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants