This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
Re-order registration stages to do msisdn & email auth last #5174
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This allows the client to complete the email last which is more natual for the user. Without this stage, if the client would complete the recaptcha (and terms, if enabled) stages and then the registration request would complete because you've now completed a flow, even if you were intending to complete the flow that's the same except has email auth at the end. Adding a dummy auth stage to the recaptcha-only flow means it's always unambiguous which flow the client was trying to complete. Longer term we should think about changing the protocol so the client explicitly says which flow it's trying to complete. element-hq/element-web#9586
It's more natural for the user if the bit that takes them away from the registration flow comes last. Adding the dummy stage allows us to do the stages in this order without the ambiguity.
Codecov Report
@@ Coverage Diff @@
## develop #5174 +/- ##
========================================
Coverage 61.74% 61.74%
========================================
Files 336 336
Lines 34635 34635
Branches 5689 5689
========================================
Hits 21386 21386
Misses 11721 11721
Partials 1528 1528 |
Codecov Report
@@ Coverage Diff @@
## develop #5174 +/- ##
===========================================
+ Coverage 61.75% 61.77% +0.01%
===========================================
Files 336 336
Lines 34639 34646 +7
Branches 5692 5695 +3
===========================================
+ Hits 21390 21401 +11
+ Misses 11721 11719 -2
+ Partials 1528 1526 -2 |
dbkr
changed the title
richvdh
suggested changes
May 14, 2019
| flow.insert(i, LoginType.TERMS) | ||
| inserted = True | ||
| break | ||
| if not inserted: |
There was a problem hiding this comment.
ftr you can do:
for ... :
# ...
else:
# we only get here if we fell off the end of the for
I'm unsure if the gain in not having to mess with inserted outweighs the loss of using a bit of an obscure language feature. up to you.
There was a problem hiding this comment.
Oh yes - I guess whichever is more in-keeping with the code style, but personally I had to stop & think what that syntax does both times I've encountered it so I'd be inclined to keep it as-is.
|
Party like its matrix-doc PR 1999: matrix-org/matrix-spec-proposals#1999 |
turt2live
added a commit
to matrix-org/matrix-spec-proposals
that referenced
this pull request
May 28, 2019
Fixes #1134 Evidence of this being the case is shown here: matrix-org/synapse#5174
turt2live
added a commit
to matrix-org/matrix-spec-proposals
that referenced
this pull request
May 28, 2019
Fixes #1134 Evidence of this being the case is shown here: matrix-org/synapse#5174
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This allows the client to complete the email last which is more
natual for the user. Without this stage, if the client would
complete the recaptcha (and terms, if enabled) stages and then the
registration request would complete because you've now completed a
flow, even if you were intending to complete the flow that's the
same except has email auth at the end.
Adding a dummy auth stage to the recaptcha-only flow means it's
always unambiguous which flow the client was trying to complete.
Longer term we should think about changing the protocol so the
client explicitly says which flow it's trying to complete.
element-hq/element-web#9586