Mobile: Add guard against users that cannot access preferred name and gender identity

[Tonksthebear]

Summary

We recently found out that only login.gov and id.me users are able to update their preferred name and gender identity. This pr is intended to put policies in place to account for this new knowledge

Related issue(s)

• department-of-veterans-affairs/va.gov-team#5776

Testing done

Updated tests to account for different uuids being present

Screenshots

Note: Optional

What areas of the site does it impact?

Preferred name and gender identity

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

[github-actions]

1 Warning

⚠️

This PR changes 342 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 app/policies/demographics_policy.rb (+11/-0) modules/mobile/app/controllers/mobile/v0/gender_identity_controller.rb (+1/-0) modules/mobile/app/controllers/mobile/v0/preferred_names_controller.rb (+1/-0) modules/mobile/app/serializers/mobile/v0/user_serializer.rb (+3/-1) modules/mobile/spec/request/gender_identity_request_spec.rb (+91/-8) modules/mobile/spec/request/preferred_name_request_spec.rb (+70/-8) modules/mobile/spec/request/user_request_spec.rb (+87/-1) modules/mobile/spec/request/v1/user_request_spec.rb (+57/-3) 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

[Tonksthebear]

Just a note, I had to duplicate a spec so that it's included in both our module and the global directory. If I only included it in the global directory, it would have required moving all of the other cassettes in user_request_spec which are used in multiple other tests. I thought it would have required changes well beyond the scope of this pr, especially because we have an engineering initiative ticket to do just that

[kpethtel]

I think we should combine these two pundit policies into one, since they do the same thing. I think the hard part is finding a decent name. idme_or_logingov_policy isn't graceful, but it's accurate. Any better ideas?

[Tonksthebear]

Agreed, though my hesitation was if at any point the rules for the policies end up changing. If combining them is the right choice, Demographics could work. It's all handled in the VAProfile::Demographics::Service

[Tonksthebear]

@kpethtel I merged the policies into one and named it Demographics to be more consistent. @jperk51 I added an access_update? but the way the app checks for authorization to services still requires an access? function so I have both now

[kpethtel]

Excellent job on the policy name. Couple questions:

1. what's the deal with the generate static docs script being in this PR? I'm guessing maybe it was a git mistake.
2. i know you had issues with the spec users and i see that you're still having stub the logingov id. What was the issue?

[kpethtel]

This is a much better name.

[kpethtel]

In some of the tests below, you also set the idme_id to nil for these logingov users. Is there a reason that we're not doing the stubs on the instance anyhow, like:

let(:user) { FactoryBot.build(:iam_user, :logingov) }
allow(user).to receive(:logingov_uuid).and_return('b2fab2b5-6af0-45e1-a9e2-394347af91ef')

[Tonksthebear]

Ya that's a fair point, I'll swap it to stub the instance

[Tonksthebear]

Actually I don't think that's possible. The instance created in the controller is different from the instance created in the spec

[kpethtel]

That makes sense. It's unfortunate that the factory logingov and idme users don't just work for this out of the box. Not worth messing around with though.

[Tonksthebear]

Excellent job on the policy name. Couple questions:

1. what's the deal with the generate static docs script being in this PR? I'm guessing maybe it was a git mistake.
2. i know you had issues with the spec users and i see that you're still having stub the logingov id. What was the issue?

1. Mistake for sure. Let me figure out git to remove it. It's not immediately easy because it doesn't detect a difference from the master branch
2. The issue was the service that was calling the extra url had its own custom error handling that was converting the vcr error and thus hiding what the true issue was. So I just had to find all the instance of an extra url being called and sort that out

[kpethtel]

LGTM

[kpethtel]

That makes sense. It's unfortunate that the factory logingov and idme users don't just work for this out of the box. Not worth messing around with though.

[jperk51]

lgtm

[ryan-mcneil]

Thanks for all of the extra reviews! Makes our job easier! 🚀