Patch gem for id.me. Fix unit, system and integration tests to align with new data model changes.#3018
Merged
Jose-verdance merged 140 commits intoJun 10, 2026
Merged
Conversation
… into jd/dpc-5127-multiple-oidc
manojwadhwa81
changed the title
manojwadhwa81
requested review from
Jose-verdance,
MEspositoE14s,
ashley-weaver and
lukey-luke
| @@ -100,8 +100,9 @@ def renew | |||
| end | |||
|
|
|||
| def set_idp_token | |||
Contributor
There was a problem hiding this comment.
should this method take the csp as a param?
| csp_user = CspUser.find_or_create_by!(user: @user, csp: csp, uuid: user_info['sub']) | ||
|
|
||
| # Update emails based upon the latest information in user info. | ||
| new_emails = user_info['all_emails'] || user_info['emails'] || user_info['emails_confirmed'] |
Contributor
There was a problem hiding this comment.
can this be a separate method? like
def fetch_user_emails(user_info)
user_info['all_emails'] || user_info['emails'] || user_info['emails_confirmed']
end
Author
There was a problem hiding this comment.
Yes, but i'd like to take a step further and normalize responses into one structure e.g DPCUserInfo, so as to centralize translation and consumer access. This would be an extension of the work we discussed in today's call for creating DPCSession/CSPSession.
| def ial_2_actions(user, auth) | ||
| data = auth.extra.raw_info | ||
|
|
||
| return unless data.ial == 'http://idmanagement.gov/ns/assurance/ial/2' |
Contributor
There was a problem hiding this comment.
are the only possible values 1 and 2?
Author
There was a problem hiding this comment.
Yes, based upon what i know. IAL1 is primarily self attestation, and with the right configuration, we can ask only IAL2 responses to be sent back to us and others will land up in the error queue.
Comment on lines
81
to
90
| csp_config = CspConfig.for(:id_me) | ||
| url = URI::HTTPS.build(host: csp_config.host, | ||
| path: '/oauth/authorize', | ||
| query: { client_id: csp_config.identifier, | ||
| redirect_uri: "#{my_protocol_host}/auth/id_me/callback", | ||
| response_type: 'code', | ||
| scope: 'openid email all_emails profile social_security_number', | ||
| scope: 'openid http://idmanagement.gov/ns/assurance/ial/2/aal/2', | ||
| nonce: @nonce, | ||
| state: @state }.to_query) | ||
| redirect_to url, allow_other_host: true |
Contributor
There was a problem hiding this comment.
@manojwadhwa81 how come we are defaulting this to id.me instead of the existing login.gov?
Author
There was a problem hiding this comment.
The initial approach taken was to make changes to switch to id.me and enable logging with id.me. As we add new buttons to show in invitation flow, next step will be enhance this to redirect the user to the user selected csp.
Contributor
There was a problem hiding this comment.
This is fixed in my PR to link across all CSP's:
#3021
| private | ||
|
|
||
| def check_user_verification | ||
| # puts current_user.inspect |
Contributor
There was a problem hiding this comment.
SonarQube might flag commented out lines as a code smell. On the other hand, it might not be smart enough to find them in Ruby, so 🤷
| if response.content_type.to_s.strip.downcase == 'application/jwt' || looks_like_jwt?(body) | ||
| decode_jwt(body) | ||
| else | ||
| JSON.parse(body).with_indifferent_access |
Contributor
There was a problem hiding this comment.
I didn't know about with_indifferent_access, that's handy!
|
|
||
| # within('#verify-modal') do | ||
| # click_button 'Yes, I acknowledge' | ||
| # end |
Contributor
There was a problem hiding this comment.
I've been playing with this test since we talked about it, and I think I've got a fix to turn it back on and get it working.
-
In
config/environments/test.rb, remove the lineconfig.action_controller.asset_host = "file://#{::Rails.root}/public" if ENV['ACCESSIBILITY'] == 'true'. -
In
dpc-portal/docker/accessibility-test.shremove theACCESSIBILITY=truebit on line 7.
I think this config was originally put in to help an old version of Axe find our static assets. Today, though, CORS is preventing us from loading our javascript from a "file://" url and it breaks the test. If you remove it everything passes.
Renamed method. Co-authored-by: MEspositoE14s <133133846+MEspositoE14s@users.noreply.github.com>
) ## 🎫 Ticket [DPC-5500](https://jira.cms.gov/browse/DPC-5500) ## 🛠 Changes <!-- What was added, updated, or removed in this PR? --> - fixed login.gov redirect failing on localhost - updated text on sign in page to reflect figma spec - updated buttons on verify invitation page to match sign in - updated links for sign in and verify buttons to map to all CSP's ## ℹ️ Context <!-- Why were these changes made? Add background context suitable for a non-technical audience. --> <!-- If any of the following security implications apply, this PR must not be merged without Stephen Walter's approval. Explain in this section and add @SJWalter11 as a reviewer. - Adds a new software dependency or dependencies. - Modifies or invalidates one or more of our security controls. - Stores or transmits data that was not stored or transmitted before. - Requires additional review of security implications for other reasons. --> - figma for sign in page here: https://www.figma.com/design/cvJ8ru6pzsVTS5zAmMwIAC/DPC-Production-Portal?node-id=1764-259&p=f&m=dev - ## 🧪 Validation <!-- How were the changes verified? Did you fully test the acceptance criteria in the ticket? Provide reproducible testing instructions and screenshots if applicable. --> <img width="1296" height="894" alt="Screenshot 2026-06-03 at 8 50 49 AM" src="https://github.com/user-attachments/assets/e47a88db-92ab-4111-ba83-fde9853eaa10" /> <img width="1290" height="976" alt="Screenshot 2026-06-03 at 8 51 26 AM" src="https://github.com/user-attachments/assets/7e73159d-2fa6-41d3-b30e-e81f87557a37" />
## 🎫 Ticket https://jira.cms.gov/browse/DPC-5454 ## 🛠 Changes <!-- What was added, updated, or removed in this PR? --> This change resolves an error when successfully authenticating to login.gov. ## ℹ️ Context <!-- Why were these changes made? Add background context suitable for a non-technical audience. --> <!-- If any of the following security implications apply, this PR must not be merged without Stephen Walter's approval. Explain in this section and add @SJWalter11 as a reviewer. - Adds a new software dependency or dependencies. - Modifies or invalidates one or more of our security controls. - Stores or transmits data that was not stored or transmitted before. - Requires additional review of security implications for other reasons. --> A follow up PR will be created to make additional changes to allow login.gov to use IAL2 and additional tests. ## 🧪 Validation <!-- How were the changes verified? Did you fully test the acceptance criteria in the ticket? Provide reproducible testing instructions and screenshots if applicable. --> Successfully ran test locally and make ci-portal.
## 🎫 Ticket This PR depends on changes covered in DPC-5371: #3018 [DPC-5442](https://jira.cms.gov/browse/DPC-5442) ## 🛠 Changes <!-- What was added, updated, or removed in this PR? --> - add additional logging to include CSP being used for controller logs - update spec files to check for CSP in logs ## ℹ️ Context <!-- Why were these changes made? Add background context suitable for a non-technical audience. --> <!-- If any of the following security implications apply, this PR must not be merged without Stephen Walter's approval. Explain in this section and add @SJWalter11 as a reviewer. - Adds a new software dependency or dependencies. - Modifies or invalidates one or more of our security controls. - Stores or transmits data that was not stored or transmitted before. - Requires additional review of security implications for other reasons. --> - In order to prepare for multi-csp integration for DPC portal, it would be helpful to capture errors across CSP's - additional logging is required to determine if a user is encountering onboarding errors with a specific CSP - Dashboards for portal metrics already exist on Splunk: [DPC Production Portal](https://splunk.cloud.cms.gov/en-US/app/Office_of_Enterprise_Data_Analytics/production_portal?form.environment=test&form.timeframe_token.earliest=-24h%40h&form.timeframe_token.latest=now) and [DPC Portal Metrics](https://splunk.cloud.cms.gov/en-US/app/Office_of_Enterprise_Data_Analytics/dpc_portal_metrics?form.PortalDataTimeRange.earliest=%40y&form.PortalDataTimeRange.latest=now) - these are built on a structured logging format that includes `actionContext` and `actionType` in [logging.rb](https://github.com/CMSgov/dpc-app/blob/26b2037cefd8e90d9eac404aa305a703b0b61ed2/dpc-portal/config/initializers/logging.rb) - this adds the CSP from user's session - splunk widgets will be updated to account for this ## 🧪 Validation <!-- How were the changes verified? Did you fully test the acceptance criteria in the ticket? Provide reproducible testing instructions and screenshots if applicable. --> - update spec file - manual review logs locally --------- Co-authored-by: jose-verdance <jose@verdance.co> Co-authored-by: MEspositoE14s <133133846+MEspositoE14s@users.noreply.github.com> Co-authored-by: manojwadhwa81 <manojwadhwa@navapbc.com>
Jose-verdance
merged commit c3f7743
into
multi-csp-feature-branch-1
17 of 19 checks passed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
🎫 Ticket
https://jira.cms.gov/browse/DPC-5371
🛠 Changes
Added new query methods to User model to centralize logic sprinkled elsewhere in code
New helper methods added to LoginSupport
Update test cases to a) Use new helper methods b) Set-up data as needed to synch up with new changes in InvitationController and LoginDotGov Controller
Add email / deactivate email in registration flow.
ℹ️ Context
This PR covers the area at the intersection of multiple stores e.g. DPC-5371, DPC-5368, DPC-5372.
It updates all the test cases that were previously left untouched due to overlaps of the work done by different team members.
🧪 Validation
Updated all the test cases to a) synch with new models and b) test new conditions