Skip to content

LG-4359 Cannot redirect to nil! error during signup#4848

Merged
stevegsa merged 4 commits intomainfrom
stevegsa-completions-500
Mar 30, 2021
Merged

LG-4359 Cannot redirect to nil! error during signup#4848
stevegsa merged 4 commits intomainfrom
stevegsa-completions-500

Conversation

@stevegsa
Copy link
Contributor

@stevegsa stevegsa commented Mar 27, 2021

Why: There is a race condition (0.04% frequency) where one request overwrites session and deletes the request_url from the sp while the other request consents to the terms but the session has no way back to the sp. In that situation redirect back to the accounts page or the root if the session is no longer valid.

@stevegsa stevegsa marked this pull request as ready for review March 27, 2021 04:49
@mitchellhenke
Copy link
Contributor

Is this a double submit situation?

@stevegsa
Copy link
Contributor Author

stevegsa commented Mar 29, 2021

Is this a double submit situation?

No. It is another request sometimes from another SP and generally comes in through the root. Somehow it is maintaining the session/sign in but deleting the sp.

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM, this fixes the error, but is the user experience that they want to be directed back to the SP? should we be trying to save a copy of the SP URL somewhere else, or trying to resolve the race condition somehow?

@mitchellhenke
Copy link
Contributor

I think this could be fixed further up the chain (and potentially more precisely) by redirecting to account in https://github.com/18F/identity-idp/blob/main/app/controllers/users/authorization_confirmation_controller.rb if there is no SP present

@stevegsa
Copy link
Contributor Author

stevegsa commented Mar 29, 2021

I think this could be fixed further up the chain (and potentially more precisely) by redirecting to account in https://github.com/18F/identity-idp/blob/main/app/controllers/users/authorization_confirmation_controller.rb if there is no SP present

Hiya @solipet :-) On user_authorization_confirmation would it make sense to redirect to account on the show if there is no sp_session?

@stevegsa stevegsa merged commit 770e27b into main Mar 30, 2021
@stevegsa stevegsa deleted the stevegsa-completions-500 branch March 30, 2021 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants