search

GSA / notifications-admin

The UI of Notify.gov
https://notify.gov
Other
11 stars 2 forks source link

Verify NONCE on login. #1966

Closed xlorepdarkhelm closed 1 month ago

xlorepdarkhelm commented 2 months ago

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

Sets up the process to validate that the nonce we receive from login.gov matches what we sent them.

NOTE:

All developers/testers will need to ensure the new environment variable is set in their .env file in order to ensure they can actually log into the system. Put the following line at the end of the list of other login.gov vars:

LOGIN_DOT_GOV_CERTS_URL = "https://idp.int.identitysandbox.gov/api/openid_connect/certs"

Security Considerations

This improves the security of our login process, following the guidelines that were covered in https://github.com/GSA/us-notify-compliance/issues/49

xlorepdarkhelm commented 2 months ago

Tracking down an issue in this, don't merge it until I get it worked out please.

I have it finally working.

It appears that login.gov is sending back our state as the nonce. So I am checking that this is valid.

xlorepdarkhelm commented 2 months ago

Note: there is an additional environment variable needed for this to work, put in the .env file.

LOGIN_DOT_GOV_CERTS_URL = "https://idp.int.identitysandbox.gov/api/openid_connect/certs"

This is needed because login.gov provides public keys which are necessary to get the id_token to be decoded.

terrazoon commented 2 months ago

It looks like you might have to add the CERTS_UTIL env variable to the staging/demo/prod environments as well. Otherwise, looks good!

xlorepdarkhelm commented 2 months ago

It looks like you might have to add the CERTS_UTIL env variable to the staging/demo/prod environments as well. Otherwise, looks good!

I did. And in the sample.env file too.

terrazoon commented 2 months ago

I don't see a change to manifest.yml?