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

[53040] Mocked Authentication authorize route setup #11867

Merged
merged 0 commits into from
Mar 8, 2023

Conversation

bramleyjl
Copy link
Contributor

@bramleyjl bramleyjl commented Feb 22, 2023

Summary

  • This PR adds the authorize route to the new Mocked Authentication module. The route currently takes (and validates) a CSP type param as a URI-encoded JSON string through the credential_info URL param. If the request passes this validation the JSON hash will be stored with a generated code as its key, this code is then returned as the response:
    {"credential_info_code":"13d1ffc6bb2e99897a746523fb69a1e5"}
    
  • To test, go to the following route:
    http://localhost:3000/mocked_authentication/authorize?credential_info=%7B%22type%22:%22logingov%22%7D
    
    You should receive a credential_info_code, which you can then use in a Rails console to look up the corresponding Redis data:
    2.7.0 :002 > MockedAuthentication::MockCredentialInfo.find('13d1ffc6bb2e99897a746523fb69a1e5')
     => #<MockedAuthentication::MockCredentialInfo:0x000055594f78a7f0 @credential_info_code="13d1ffc6bb2e99897a746523fb69a1e5", @credential_info={"type"=>"logingov"}, @persisted=true, @validation_context=nil, @errors=#<ActiveModel::Errors:0x000055594f5ffae8 @base=# <MockedAuthentication::MockCredentialInfo:0x000055594f78a7f0 ...>, @errors=[]>> 
    

Related issue(s)

Testing done

  • Manual testing of feature, safety authentication tests, unit specs added to cover new feature.

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected

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?

@bramleyjl bramleyjl self-assigned this Feb 22, 2023
@va-vfs-bot va-vfs-bot temporarily deployed to 53040_vamock_route/main/main February 22, 2023 19:10 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 53040_vamock_route/main/main February 22, 2023 23:45 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 53040_vamock_route/main/main February 24, 2023 16:26 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 53040_vamock_route/main/main February 24, 2023 16:58 Inactive
@bramleyjl bramleyjl added identity identity-backend Identity team backend label labels Feb 24, 2023
@va-vfs-bot va-vfs-bot temporarily deployed to 53040_vamock_route/main/main February 24, 2023 21:33 Inactive
@bramleyjl bramleyjl marked this pull request as ready for review February 27, 2023 16:26
@bramleyjl bramleyjl requested review from a team as code owners February 27, 2023 16:26
rileyanderson
rileyanderson previously approved these changes Feb 27, 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! Tested and found MockCredentialInfo in redis. Just a comment about adding mount specs that doesn't necessarily need to be done in this PR

config/routes.rb Outdated Show resolved Hide resolved
rjohnson2011
rjohnson2011 previously approved these changes Feb 27, 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.

I just have a request for a couple small changes. For functionality, tested hitting the route and confirmed that a redis record was created as expected with a matching code to reference it downstream

@bramleyjl bramleyjl dismissed stale reviews from rjohnson2011 and rileyanderson via 24e1028 February 27, 2023 22:26
@va-vfs-bot va-vfs-bot temporarily deployed to 53040_vamock_route/main/main February 28, 2023 03:33 Inactive
@github-actions
Copy link

github-actions bot commented Feb 28, 2023

1 Warning
⚠️ This PR changes 205 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

  • .rubocop_todo.yml (+1/-0)

  • config/redis.yml (+3/-0)

  • config/routes.rb (+1/-1)

  • modules/mocked_authentication/app/controllers/mocked_authentication/credential_providers_controller.rb (+14/-0)

  • modules/mocked_authentication/app/models/mocked_authentication/mock_credential_info.rb (+14/-0)

  • modules/mocked_authentication/app/services/mocked_authentication/mock_credential_info_creator.rb (+35/-0)

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

  • modules/mocked_authentication/lib/mocked_authentication/engine.rb (+4/-0)

  • modules/mocked_authentication/spec/factories/factories.rb (+8/-0)

  • modules/mocked_authentication/spec/models/mock_credential_info_spec.rb (+24/-0)

  • modules/mocked_authentication/spec/requests/credential_providers_request_spec.rb (+60/-0)

  • modules/mocked_authentication/spec/services/mocked_authentication/mock_credential_info_creator_spec.rb (+39/-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

@bramleyjl bramleyjl requested a review from bosawt February 28, 2023 17:41
@bramleyjl bramleyjl changed the title [53040] mock_authorization authorize route setup [53040] Mock Authentication authorize route setup Feb 28, 2023
@bramleyjl bramleyjl changed the title [53040] Mock Authentication authorize route setup [53040] Mocked Authentication authorize route setup Feb 28, 2023
@va-vfs-bot va-vfs-bot temporarily deployed to 53040_vamock_route/main/main February 28, 2023 21:35 Inactive
bosawt
bosawt previously approved these changes Feb 28, 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.

Thanks for making those changes. Looks good!

@bramleyjl bramleyjl enabled auto-merge (squash) February 28, 2023 23:15
@bramleyjl bramleyjl force-pushed the 53040_vamock_route branch 3 times, most recently from d69de11 to 3c2029d Compare March 7, 2023 19:53
@va-vfs-bot va-vfs-bot temporarily deployed to 53040_vamock_route/main/main March 7, 2023 21:34 Inactive
@bramleyjl bramleyjl force-pushed the 53040_vamock_route branch from 3c2029d to ba0e06e Compare March 8, 2023 02:57
config/routes.rb Outdated Show resolved Hide resolved
@bramleyjl bramleyjl force-pushed the 53040_vamock_route branch from 12e6c25 to 6bb25c4 Compare March 8, 2023 17:30
@bramleyjl bramleyjl merged commit 7c86018 into master Mar 8, 2023
@bramleyjl bramleyjl deleted the 53040_vamock_route branch March 8, 2023 18:09
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.

5 participants