-
Notifications
You must be signed in to change notification settings - Fork 66
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
55597 mocked auth data profiles #12232
Conversation
Generated by 🚫 Danger |
30285ef
to
df21a42
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly comments around some code quality stuff, also have what (I think) will fix your CodeQL issue
modules/mocked_authentication/app/controllers/mocked_authentication/credential_controller.rb
Outdated
Show resolved
Hide resolved
modules/mocked_authentication/app/controllers/mocked_authentication/credential_controller.rb
Outdated
Show resolved
Hide resolved
modules/mocked_authentication/app/controllers/mocked_authentication/credential_controller.rb
Outdated
Show resolved
Hide resolved
modules/mocked_authentication/app/services/mocked_authentication/mockdata_fetcher.rb
Outdated
Show resolved
Hide resolved
modules/mocked_authentication/spec/factories/mock_credential_infos.rb
Outdated
Show resolved
Hide resolved
6d72db4
to
353b319
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small set of nitpicks this time for code quality. I did confirm it works so behaviorally seems good
modules/mocked_authentication/spec/requests/credential_request_spec.rb
Outdated
Show resolved
Hide resolved
584baf7
to
11b8249
Compare
There was a problem hiding this 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, thanks for making the changes!
validate_index_params(type) | ||
mock_profiles = Mockdata::Reader.find_credentials(credential_type: type) | ||
|
||
render json: { mock_profiles: } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've gotta admit, seeing the hashes like this after the Ruby 3.1 upgrade still catches me off guard for a second before I realize this is how it's supposed to be, ha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm starting to get used to it surprisingly quickly! It really does cut a lot of unnecessary text out.
Summary
credential_list
route to the Mocked Authentication module. This route takes a CSPtype
parameter and searches for all CSP user attribute profiles of that type currently in the vets-api-mockdata directory, controlled bySettings.sign_in.mock_credential_dir
. The mock profile filenames should be theuser_identifier
, which is the user's email stripped of any non-word characters. This filenaming is used by the Mocked Authentication mock profile creator, and any files with non-word characters such as.
will not be read by the reader. The mock data reader then creates and returns amock_profiles
hash that looks like:Each entry is keyed to its
user_identifier
and possesses acredential_payload
of attributes that differs depending on the CSP, as well as that payload cast to JSON and encoded as theencoded_credential
field that can be passed back to the Mocked Authentication/authorize
endpoint.Related issue(s)
Testing done
vets-api-mockdata/credentials/
directory for the CSP type you wish to test. You can create new mock credential data for ID.me, DSLogon, and Login.gov by performing an actual CSP authentication in localhost & enabling response logging.mocked_authentication/credential_list
endpoint with your selected CSP type:Screenshots
What areas of the site does it impact?
(Describe what parts of the site are impacted andifcode touched other areas)
Acceptance criteria