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

Mocked auth views #12444

Merged
merged 0 commits into from
Apr 24, 2023
Merged

Mocked auth views #12444

merged 0 commits into from
Apr 24, 2023

Conversation

asg5704
Copy link
Contributor

@asg5704 asg5704 commented Apr 19, 2023

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.

Summary

  • (Summarize the changes that have been made to the platform)
    This PR creates a view for the mocked_authentication module to allow users to use the new Mocked Authentication flow and select mock profiles. (Only available in local and development environments)

  • (What is the solution, why is this the solution)
    The solution was to create a template view for the credential_controller and add a new route /profiles that exists on the /mocked_authentication base. Some other components and tweaks in the mocked_authentication module were required to pass local variables to the template view.

  • (Which team do you work for, does your team own the maintainence of this component?)
    Identity. Yes.

Related issue(s)

Testing done

  • Describe what the old behavior was prior to the change
    No frontend views were available for mocked authentication
  • Describe the steps required to verify your changes are working as expected
    Requirements: vets-website (fe-integrate-mock-auth), vets-api (this branch), vets-api-mockdata (mocked_auth)
  1. Start up vets-website & vets-api
  2. Navigate to localhost:3001/sign-in/mocked-auth > Select ID.me from the Credential Service Provider dropdown
  3. Click the Mocked Authentication button to continue the flow
  4. Select a profile using the dropdown > Confirm credentials JSON > Click Continue signing in
  5. Land back on localhost:3001 in an authenticated state
  • Describe the tests completed and the results
    Manual. Success.

What areas of the site does it impact?

This only impacts localhost & development environments on the /mocked_authentication/profiles route
(Describe what parts of the site are impacted andifcode touched other areas)

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

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?

@github-actions
Copy link

github-actions bot commented Apr 19, 2023

1 Warning
⚠️ This PR changes 295 LoC (not counting whitespace/newlines).

In order to ensure each PR receives the proper attention it deserves, we recommend not exceeding
200. Expect some delays getting reviews.

File Summary

Files

  • config/settings.yml (+1/-1)

  • modules/mocked_authentication/app/controllers/mocked_authentication/credential_controller.rb (+24/-2)

  • modules/mocked_authentication/app/views/layouts/application.html.erb (+4/-0)

  • modules/mocked_authentication/app/views/mocked_authentication/credential/index.html.erb (+188/-0)

  • modules/mocked_authentication/config/routes.rb (+2/-0)

  • modules/mocked_authentication/lib/credential/service.rb (+2/-1)

  • modules/mocked_authentication/spec/lib/credential/service_spec.rb (+5/-0)

  • modules/mocked_authentication/spec/requests/credential_request_spec.rb (+65/-0)

    Note: We exclude files matching the following when considering PR size:

    *.csv, *.json, *.tsv, *.txt, Gemfile.lock, app/swagger, modules/mobile/docs, spec/fixtures/, spec/support/vcr_cassettes/, modules/mobile/spec/support/vcr_cassettes/, db/seeds, modules/vaos/app/docs, modules/meb_api/app/docs, modules/appeals_api/app/swagger/
    

Big PRs are difficult to review, often become stale, and cause delays.

Generated by 🚫 Danger

@asg5704 asg5704 self-assigned this Apr 19, 2023
@va-vfs-bot va-vfs-bot temporarily deployed to mocked_auth_views/main/main April 19, 2023 15:10 Inactive
@asg5704 asg5704 marked this pull request as ready for review April 19, 2023 15:54
@asg5704 asg5704 requested review from a team as code owners April 19, 2023 15:54
@va-vsp-bot va-vsp-bot requested a deployment to mocked_auth_views/main/main April 19, 2023 15:54 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to mocked_auth_views/main/main April 19, 2023 15:55 Inactive
ryan-mcneil
ryan-mcneil previously approved these changes Apr 19, 2023
Copy link
Contributor

@ryan-mcneil ryan-mcneil left a comment

Choose a reason for hiding this comment

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

Looks good!

rileyanderson
rileyanderson previously approved these changes Apr 19, 2023
Copy link
Contributor

@rileyanderson rileyanderson left a comment

Choose a reason for hiding this comment

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

Looks good to me. Tested locally and was able to sign in. Just one comment but take it or leave it 😄

Copy link
Contributor

@bosawt bosawt left a comment

Choose a reason for hiding this comment

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

Just a couple comments from the spec fairy, lemme know if you need help with them. Otherwise looks good

@bosawt
Copy link
Contributor

bosawt commented Apr 20, 2023

One other thing, is there any way to not show the 'encoded credential' part in the credential attribute view? Not a problem at all just unnecessary for the user

@asg5704 asg5704 dismissed stale reviews from rileyanderson and ryan-mcneil via 3d39b0c April 20, 2023 16:15
@va-vsp-bot va-vsp-bot requested a deployment to mocked_auth_views/main/main April 20, 2023 16:16 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to mocked_auth_views/main/main April 20, 2023 16:16 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to mocked_auth_views/main/main April 20, 2023 20:53 In progress
@asg5704 asg5704 requested a review from bosawt April 20, 2023 20:53
@va-vfs-bot va-vfs-bot temporarily deployed to mocked_auth_views/main/main April 20, 2023 20:57 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to mocked_auth_views/main/main April 20, 2023 21:01 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to mocked_auth_views/main/main April 20, 2023 21:02 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to mocked_auth_views/main/main April 20, 2023 21:23 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to mocked_auth_views/main/main April 20, 2023 21:24 Inactive
rileyanderson
rileyanderson previously approved these changes Apr 21, 2023
Copy link
Contributor

@rileyanderson rileyanderson left a comment

Choose a reason for hiding this comment

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

Looks good 🚀

@va-vsp-bot va-vsp-bot requested a deployment to mocked_auth_views/main/main April 21, 2023 15:19 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to mocked_auth_views/main/main April 21, 2023 15:19 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to mocked_auth_views/main/main April 21, 2023 17:05 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to mocked_auth_views/main/main April 21, 2023 17:06 Inactive
bosawt
bosawt previously approved these changes Apr 21, 2023
Copy link
Contributor

@bosawt bosawt left a comment

Choose a reason for hiding this comment

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

👍

@asg5704
Copy link
Contributor Author

asg5704 commented Apr 21, 2023

@ryan-mcneil Can I get a re-approval now that specs are working and Rubocop isn't yelling at me?

@va-vsp-bot va-vsp-bot requested a deployment to mocked_auth_views/main/main April 21, 2023 20:24 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to mocked_auth_views/main/main April 21, 2023 20:24 Inactive
@bosawt bosawt dismissed stale reviews from rileyanderson and themself via 4908b98 April 21, 2023 20:32
@bosawt bosawt self-requested a review April 21, 2023 20:32
@va-vsp-bot va-vsp-bot requested a deployment to mocked_auth_views/main/main April 21, 2023 20:33 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to mocked_auth_views/main/main April 21, 2023 20:34 Inactive
Copy link
Contributor

@ryan-mcneil ryan-mcneil left a comment

Choose a reason for hiding this comment

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

Sorry for the delay!

@asg5704 asg5704 merged commit 03295e5 into master Apr 24, 2023
@asg5704 asg5704 deleted the mocked_auth_views branch April 24, 2023 15:42
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.

6 participants