Prevent Verify by mail flow redirect bug#1508
Conversation
gemfarmer
changed the title
There was a problem hiding this comment.
I think we can remove this line. It doesn't seem to be used anywhere.
spec/features/saml/loa3_sso_spec.rb
Outdated
There was a problem hiding this comment.
What do you think about putting all these steps between lines 207 and 218 into a well-named method (the method can live in this file), such as perform_id_verification_with_usps_without_confirming_code_then_sign_out?
spec/features/saml/loa3_sso_spec.rb
Outdated
There was a problem hiding this comment.
oh oops! Good catch
app/decorators/usps_decorator.rb
Outdated
There was a problem hiding this comment.
In a follow-up PR, we should replace all instances of this check with the new one we created here. I just remembered that we're not actually sending the letter anywhere. Before you added the ability to limit the amount of letters a user can send, this session key was the only way to determine if the USPS method had been chosen. However, now that we keep track of when letters are sent in the DB, we can use that instead. Does that make sense?
monfresh
left a comment
monfresh
left a comment
There was a problem hiding this comment.
LGTM. Please squash your commits into one, and please write your commit message following our style guide: https://github.com/18F/identity-idp/blob/master/CONTRIBUTING.md#commit-message-style-guide
**Why** The situation As @andrewhughey had stated in 18F/identity-private#1890 (comment), here is the flow that this PR is fixing: create account (email, password, 2FA) verify identity (person info, financial info) choose to confirm address by mail send letter see profile / flash message that letter was sent sign out sign in asked for code from letter but oh no! I never got a letter choose send new letter expected: send new letter screen actual: restart identity verification **How** This PR adds logic to prevent the user from being redirected back to identity verification, sending them to the "Send another letter" confirmation screen instead, as expected. The faulty redirect was happening because the a request to send more mail was triggering the IdvSession service, which redirects to verification if verification hasn't been completed. To fix this, we added an exception that prevents that redirect if a user has mail already sent mail. For issue: 18F/identity-private#1890
gemfarmer
force-pushed
the
bsh-verify-mail
branch
from
4052630 to
5ab507b
Compare
|
Should we create a feature spec for sending another letter? I didn't see one in |
|
Ah found it right where I though it should be. My bad, wasn't looking hard enough. |
3 participants
For issue: https://github.com/18F/identity-private/issues/1890
The situation
As @andrewhughey had stated in https://github.com/18F/identity-private/issues/1890#issuecomment-307799695, here is the flow that this PR is fixing:
create account (email, password, 2FA)
verify identity (person info, financial info)
choose to confirm address by mail
send letter
see profile / flash message that letter was sent
sign out
sign in
asked for code from letter but oh no! I never got a letter
choose send new letter
expected: send new letter screen
actual: restart identity verification
The solution
This PR adds logic to prevent the user from being redirected back to identity verification, sending them to the "Send another letter" confirmation screen instead, as expected.
The faulty redirect was happening because the a request to send more mail was triggering the
IdvSessionservice, which redirects to verification if verification hasn't been completed. To fix this, we added an exception that prevents that redirect if a user has mail already sent mail.