Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mhv 65761 reroute unauthorized users #34303

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

KyleCardwell
Copy link
Contributor

@KyleCardwell KyleCardwell commented Jan 27, 2025

Note: Delete the description statements, complete each step. None are optional, but can be justified as to why they cannot be completed as written. Provide known gaps to testing that may raise the risk of merging to production.

Are you removing, renaming or moving a folder in this PR?

  • No, I'm not changing any folders (skip to TeamSites and delete the rest of this section)
  • Yes, I'm removing, renaming or moving a folder

⚠️ TeamSites ⚠️

Examples of a TeamSite: https://va.gov/health and https://benefits.va.gov/benefits/. This scenario is also referred to as the "injected" header and footer. You can reach out in the #sitewide-public-websites Slack channel for questions.

Did you change site-wide styles, platform utilities or other infrastructure?

Summary

  • In order to comply with new guidance from platform, we are redirecting users back to "/my-health" if they meet any of the following requirements:
    • user does not have Secure Messaging service
    • unverified credentials
    • user has no facilities
  • We are including a new wrapper component to make those checks. The component will be available for other teams to implement in their respective services.

Related issue(s)

MHV-65761 - Handle rerouting to /my-health for unauthorized users

Handle rerouting to /my-health for unauthorized users

DEETS: The MHV Account Creation API has been live in production for several weeks and is stable. Cartographers are developing React code to consume the API and display appropriate alerts when there is an error. Each of your applications (SM, MR and Medications) depends on an MHV UUID and therefore will need to implement also. See this github doc for the complete context.Cartographers and POs have also aligned on standardized application behavior for two key user types who should not have access to tools: 1) credential is unverified (LOA1) or 2) user has no facilities in their profile. Documented here. (NOTE: this part of the work will also apply to other applications under /my-health/, we will reach out to them in January.)

https://github.com/department-of-veterans-affairs/va.gov-team/blob/master/products/health-care/digital-health-modernization/mhv-to-va.gov/governance/mhv-account-creation-api.md

https://dsva.slack.com/archives/C04DRS3L9NV/p1734721197844169?thread_ts=1734717582.350219&cid=C04DRS3L9NV

Meeting 1/10/25

User Story: As a SM user, I want to so that I can

GIVEN:

WHEN:

THEN:

Feature Flag Y/N ?

Collab Cycle Y/N ?

Platform Approval T/N ?

DataDog Analytics Y/N ?

Manual Testing Y/N ? Y-Rakesh

Automated Testing Y/N ? Y- Fazil

Accessibility Testing Y/N ?

UCD Validation Y/N

Acceptance Criteria:

AC1

Testing done

  • added new unit tests for new component

Screenshots

Note: This field is mandatory for UI changes (non-component work should NOT have screenshots).

Before After
Mobile
Desktop

What areas of the site does it impact?

(Describe what parts of the site are impacted if code touched other areas)

Acceptance criteria

Quality Assurance & Testing

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Linting warnings have been addressed
  • Documentation has been updated (link to documentation *if necessary)
  • Screenshot of the developed feature is added
  • Accessibility testing has been performed

Error Handling

  • Browser console contains no warnings or errors.
  • Events are being sent to the appropriate logging solution
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)

Authentication

  • Did you login to a local build and verify all authenticated routes work as expected with a test user

Requested Feedback

(OPTIONAL) What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?

@va-vfs-bot va-vfs-bot temporarily deployed to master/mhv-65761-reroute-unauthorized-users/main January 27, 2025 21:52 Inactive
oleksii-morgun
oleksii-morgun previously approved these changes Jan 28, 2025
mmoyer-va
mmoyer-va previously approved these changes Jan 29, 2025
vsaleem
vsaleem previously approved these changes Jan 29, 2025
Copy link
Contributor

@dcloud dcloud left a comment

Choose a reason for hiding this comment

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

Reposting my feedback from Slack so it doesn't get lost: there are a few approaches to "guarding" access in place, and one like useMyHealthAccessGuard does a minimal check, on the client side. I think we'd benefit from calling out the vets-api service/policy checks in this new component, so perhaps a name like MHVServiceRequiredGuard would be clearer.

Also no wondering if we should remove useMyHealthAccessGuard if all MHV teams are using vets-api service/policy checks now…

I'd be open to this or a follow-up PR removing the useMyHealthAccessGuard code, as that seems obsolete.

@va-vfs-bot va-vfs-bot temporarily deployed to master/mhv-65761-reroute-unauthorized-users/main January 29, 2025 21:27 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/mhv-65761-reroute-unauthorized-users/main January 30, 2025 17:38 Inactive
Copy link
Contributor

@dcloud dcloud left a comment

Choose a reason for hiding this comment

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

First, thanks for taking my naming concern seriously. The MhvServiceRequiredGuard looks good to me, and easy to check using the "acceleratedCernerUser" mock user, since they don't have SM as a service.

This looks good to me!

@va-vfs-bot va-vfs-bot temporarily deployed to master/mhv-65761-reroute-unauthorized-users/main February 6, 2025 21:34 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants