Verify state for login.gov #1386
Merged
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
xlorepdarkhelm
requested review from
ccostino,
jonathanbobel,
stvnrlly,
terrazoon,
heyitsmebev,
A-Shumway42 and
alexjanousekGSA
|
FYI - I am testing that this all works as intended, I believe it will, and will update when I have it confirmed. Please do not merge until I say it all 100% works. |
Signed-off-by: Cliff Hill <Clifford.hill@gsa.gov>
Signed-off-by: Cliff Hill <Clifford.hill@gsa.gov>
Signed-off-by: Cliff Hill <Clifford.hill@gsa.gov>
Signed-off-by: Cliff Hill <Clifford.hill@gsa.gov>
xlorepdarkhelm
force-pushed
the
USN-COMPLY-51-Verify_State
branch
from
da68717 to
017406c
Compare
xlorepdarkhelm
temporarily deployed
to
staging
GitHub Actions
Inactive
Signed-off-by: Cliff Hill <Clifford.hill@gsa.gov>
xlorepdarkhelm
temporarily deployed
to
staging
GitHub Actions
Inactive
Signed-off-by: Cliff Hill <Clifford.hill@gsa.gov>
xlorepdarkhelm
temporarily deployed
to
staging
GitHub Actions
Inactive
Signed-off-by: Cliff Hill <Clifford.hill@gsa.gov>
xlorepdarkhelm
temporarily deployed
to
staging
GitHub Actions
Inactive
Signed-off-by: Cliff Hill <Clifford.hill@gsa.gov>
xlorepdarkhelm
temporarily deployed
to
staging
GitHub Actions
Inactive
This all works now. |
ccostino
approved these changes
Nov 7, 2024
A-Shumway42
approved these changes
Nov 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
A note to PR reviewers: it may be helpful to review our code review documentation to know what to keep in mind while reviewing pull requests.
Description
the state was largely unused by our system, and was seen as a necessity for working with login.gov. Now we are checking that the state is correct on return from login.gov. It is sent as a query parameter back from login.gov, and we are now simply checking that it matches a value we have stored in redis.
This also is now generated from the admin, and then passed into the api endpoints for both creating a new invitation, and resending an expired invitation. As such, everything from login.gov is now checked for both state and nonce (the previous work I had done). This will help eliminate possible ingresses from bad actors.
https://github.com/GSA/us-notify-compliance/issues/51
This must only be merged when both this ticket and the one linked here is finished: GSA/notifications-admin#2075
Security Considerations
Checking state, like nonce, is important for the security of the login process.