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

interactive-auth now handles requesting email tokens #926

Merged
merged 3 commits into from May 22, 2019

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented May 22, 2019

interactive-auth now has a callback to request the email token which
it will call at the appropriate time (now at the start of the
auth process if the chosen flow contain an email auth stage).

This does make this a breaking change, although not sure this is
used outside of Riot. We could make it backwards compatible by
having an option for the new behaviour. It may not be worthwhile
though.

element-hq/element-web#9586

interactive-auth now has a callback to request the email token which
it will call at the appropriate time (now at the start of the
auth process if the chosen flow contain an email auth stage).

This does make this a breaking change, although not sure this is
used outside of Riot. We could make it backwards compatible by
having an option for the new behaviour. It may not be worthwhile
though.

element-hq/element-web#9586
@dbkr dbkr requested a review from a team May 22, 2019 10:54
dbkr added a commit to matrix-org/matrix-react-sdk that referenced this pull request May 22, 2019
We previously sent it in componentWillMount of the email token
auth component which definitely gets us on react's naughtly list.
We now pass the js-sdk a callback it can call at the appropriate
time to send the token (matrix-org/matrix-js-sdk#926).

We should make password reset and adding email addresses work the
same way, but currently they don't even use the interactive-auth
helpers(!) so they're unaffected.

element-hq/element-web#9586
@bwindels bwindels requested review from bwindels and removed request for a team May 22, 2019 11:10
Copy link
Contributor

@bwindels bwindels left a comment

Choose a reason for hiding this comment

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

Thanks for this! Looks good, but some questions about the async flow

src/interactive-auth.js Outdated Show resolved Hide resolved
src/interactive-auth.js Outdated Show resolved Hide resolved
src/interactive-auth.js Show resolved Hide resolved
src/interactive-auth.js Outdated Show resolved Hide resolved
src/interactive-auth.js Outdated Show resolved Hide resolved
src/interactive-auth.js Show resolved Hide resolved
@dbkr dbkr requested a review from bwindels May 22, 2019 12:12
Copy link
Contributor

@bwindels bwindels left a comment

Choose a reason for hiding this comment

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

lgtm!

@dbkr dbkr merged commit 93d51b8 into develop May 22, 2019
@t3chguy t3chguy deleted the dbkr/uiauth_send_email branch May 10, 2022 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants