Content Admins can alter API data that should be locked.

[swirtSJW]

Describe the defect

Some time within the past 4 months, the behavior of external content has changed. It used to be locked for all users except admins, but now is unlocked for both admins and content_admins.
Used to look like this 4 months ago

Now looks like this

This makes it too easy for one of our ~20 users with All content section access AND Content Admin role to alter content that should not be altered. However, this is mitigated by the fact that the nightly migration can and will overwrite the changed data the next time the lighthouse record is changed. Also we can force the data across all of them with an "update migration" (forced migration)

To Reproduce

Steps to reproduce the behavior:

1. Login as a content admin
2. Go to https://prod.cms.va.gov/node/3612/edit
3. Scroll to Locations and contact information
4. See that you can edit fields

Screenshots

If applicable, add screenshots to help explain your problem.

Additional context

Add any other context about the problem here. Reach out to the Product Managers to determine if it should be escalated as critical (prevents users from accomplishing their work with no known workaround and needs to be addressed within 2 business days).

AC / Expected behavior

• Content admins should have no access to external content fields same as all other roles (except admin)
• Admins should have access to the editable fields
• Tests are created that would have caught this failure
• Logic here is updated use Userperms->hasAdmin

[jilladams]

Possible red herring, but: PW made changes on VA Benefits taxonomy last sprint to unlock the API ID & Name of VA Benefit taxonomy terms to Content Admins: https://github.com/department-of-veterans-affairs/va.gov-cms/pull/15273/files#diff-fc201850e713ee5608b348314dfe66fb5806403bc69671a8b52d07514f25a950

[jilladams]

From planning: Not high priority for this sprint. Possible 5 pts to root cause this change in order to fix.

[omahane]

Our hasAdminRole in va_gov_user/src/Service/UserPermsService.php checks is true for admin and content_admin. So maybe we're checking this in more places than we were before.

[swirtSJW]

Confirmed as working correctly

• 8/7/23
• 8/14/23
• 9/5/23
• 9/12/23 0bb3ec5

Shows broken on environment from 9/22.

So somewhere in that 10 day span it broke. e7e4e61

[swirtSJW]

Our hasAdminRole in va_gov_user/src/Service/UserPermsService.php checks is true for admin and content_admin. So maybe we're checking this in more places than we were before.

Yah it has a bool to pass if you want it for TRUE Admin.

[swirtSJW]

The problem lies in here someplace https://github.com/department-of-veterans-affairs/va.gov-cms/compare/0bb3ec5..e7e4e61

[swirtSJW]

This is the PR that broke it. #15069 When I revert this PR it works as expected.

edit: flawed test, that revert was on an older database so it looked like it fixed it but it was never broken.

[jilladams]

WEIRD. @chri5tia (and @dsasser ) FYI for awareness: On Facilities Vet Center content type, 2 API fields are now editable by Content Admins that were previously locked down. Steve git bisected and determined this was weirdly caused somehow by #14793 / #15069, the PR to hide Link Description Field in Footers. (NOT by our content admin / unlocking fields change on VA Benefits taxonomy.)

Facilities did not prioritize this fix into current sprint, so it'll be a minute before we have a fix to review.

[dsasser]

@jilladams thanks for flagging us; very weird indeed. I can pick this up as a stretch item if Facilities is good with that.

[swirtSJW]

False alarm. THis was not code related at all.

Michelle was granted superpowers which changed her view of the world

[swirtSJW]

I was tracking code changes through the various tugboat environments and was not looking for user changes during that same time frame. :(

[swirtSJW]

I am closing this ticket, but created another one for the backlog just to cover

• Tests are created that would have caught this failure
• Logic here is updated use Userperms->hasAdmin

[chri5tia]

For sanity and to confirm, I double-checked on prod and on my local in an updated state and this defect is not present.
Tests are a good idea.

@omahane About hasAdminRole():
userPermsService->hasAdminRole(TRUE) is for administrator_only
userPermsService->hasAdminRole() is for admins and content admins

The function is defined here:

https://github.com/department-of-veterans-affairs/va.gov-cms/blob/main/docroot/modules/custom/va_gov_user/src/Service/UserPermsService.php#L182-L189