department-of-veterans-affairs / caseflow

Caseflow is a web application that enables the tracking and processing of appealed claims at the Board of Veterans' Appeals.
Other
54 stars 19 forks source link

Users with "Global Admin" role should be able to impersonate other users #3865

Closed mdbenjam closed 5 years ago

mdbenjam commented 7 years ago

When we run eFolder in the staging environment we prompt users with a login screen. Developers can insert any css_id/station_id to impersonate any user. While we have system admin roles in prod that can impersonate any role, it can be useful to impersonate a specific user for the following reasons:

Let's enable users with the role "Global Admin" to log in as other users in Caseflow after providing user id and station id. The UI for this should appear on the "Switch User" page, and only be visible to Global Admins.

See a screenshot of the current "Switch User" page. We should add the new UI somewhere appropriate. image

Technical details

Possible breakdown of PRs (just a suggestion, the implementor can change this as needed)

1) Add roles in UAT/local 2) Add non-functional UI in Switch Users page and have it only appear for Global Admin users (validate this in UAT) 3) When the new UI is filled out, update Redis with the user id and station id we want to impersonate 4) Make the necessary changes in user.rb to return the overriden info

AC

1) A new role called "Global Admin" should be created and given to a small number of users using our Functions config files

Possible dependency

https://github.com/department-of-veterans-affairs/caseflow/issues/3920 (if log out doesn't work, how would we test this?)

amprokop commented 7 years ago

Question — is there a reason we only want to do this in staging mode? Wouldn't it be even better to impersonate users on the live, deployed sites?

mdbenjam commented 7 years ago

I think we can hook up staging environments to prod. Though I've never done it. That would be the best since it makes it easier to debug. Maybe that's dangerous though. You could run non-reviewed code against production systems. If that's not a valid path, then we can let system admins in prod impersonate users instead.

amprokop commented 7 years ago

@shanear @askldjd @NickHeiner — any thoughts on these two questions:

  1. Is it a good idea to streamline hooking a local staging dev env up to prod? My knee-jerk instinct is no, bad idea, one mistake here could result in endless pain. Curious if you have different thoughts.

  2. Any concerns with letting system admins in prod impersonate other users? Keep in mind that some actions you can take are stateful. It would be very disconcerting if a user appears and sees someone else has added annotations as them for example.

shanear commented 7 years ago
  1. Not only is it not a good idea, it's actually just impossible (because of the VPCs)
  2. We can already mimic permissions. What is the situation we are talking about we would want to mimic everything about a user?
nickheiner-usds commented 7 years ago
  1. Is it a good idea to streamline hooking a local staging dev env up to prod? My knee-jerk instinct is no, bad idea, one mistake here could result in endless pain.

Agreed that it's a bad idea.

  1. Any concerns with letting system admins in prod impersonate other users?

I think masquerading is a valuable debugging tool, and I'd hope that the system admins are a sufficiently well-vetted group that they are mindful of their actions.

mdbenjam commented 7 years ago

It would enable us to replicate the exact environment a user. We could just ask a user reporting a problem for the URL the issues happens on, and then investigate ourselves by impersonating them. It could prevent a lot of back and forth and make debugging easier.

amprokop commented 6 years ago

I don't have a lot of context on the Reader needs, but I can see this being quite useful for Work Queue, which will have different logic for each team and potentially even sometimes each user, so I'm happy to have it in advance of Work Queue.

@shanear, do you have any more reservations about this?

nickheiner-usds commented 6 years ago

I am happy with the proposal as it currently stands.

ghost commented 6 years ago

Doesn't System Admin function do that already? @nemodjuric has implemented it

amprokop commented 6 years ago

@kierachell — no, the System Admin role doesn't do what we're describing here. Maybe this is unclear? Wondering why you would think that it's already done.

ghost commented 6 years ago

As a System Admin I can access whatever app I need and I can toggle any feature I want via console;

I don't see the value in overriding some user's data. If we need such granularity to debug the app: then we need better logging, feature implementation, and data access:

Bottom line: can we reduce the complexity, and not add more to it?

nickheiner-usds commented 6 years ago

I think it will overall simplify our lives to be able to masquerade.

As a System Admin I can access whatever app I need and I can toggle any feature I want via console;

The point is that we don't need to manually set feature toggles and recreate any other conditions that a user may have. We can just immediately reproduce the user's state.

amprokop commented 6 years ago

e.g. the whole featuretoggle vs functions mess is already hard to parse

They are similar in some ways, but they are importantly different. The way we administer them is sort of similar in Caseflow, but conceptually they're quite distinct. Maybe a quick explainer, as we're probably not going to get rid of the distinction anytime soon:

Let me know if you still find it confusing.

amprokop commented 6 years ago

which features are enable for which user with which function on which app server?

This is the exact complexity that reproducing a user's state helps resolve. (Minus the on which app server bit— we store feature toggles/functions in Redis, so they will never be specific to app servers).

ghost commented 6 years ago

now we will have yet another role? function? feature? to impersonate features/functions/etc.

This sounds like it needs to be a separate service (to decouple eFolder from auth delegation)

nemodjuric commented 6 years ago

To verify: DEMO: Switch user to System Admin and make sure Global Admin feature is visible. Try to log in as Reader (station id 283), and make sure Global Admin feature is not visible anymore and the current user is Reader(DSUSER) UAT and preprod - Login as CF_Q_283 and make sure Global Admin feature is visible. Try to login as CF_Q_397 (station id 397) and make sure Global Admin feature is still visible and the current user is CF_Q_397(RO97). Try to log in as CF_SPOCK_283 (station 283), and make sure Global Admin feature is not visible anymore and the current user is CF_SPOCK_283(DSUSER).

astewarttistatech commented 6 years ago

PASSED Cannot be validated until

Validated: Preprod/Console

screen shot 2018-01-04 at 10 43 43 am

Steps to Validate:

  1. Access an app in Caseflow that uses featuretoggles (normally off or set to off)
  2. Go through the happy path of that app
  3. After a successful completion of the happy path, it can be concluded that the user (Global Admin) was able to bypass the featuretoggles that had been set to off

NOTE : This cannot be validated through UI of "Switch Users" because that feature no longer exists on testing environment, exc. DEMO.

@mdbenjam can you verify that the above steps taken are enough to conclude this ticket as being passed, since we do not conduct testing in DEMO.

nemodjuric commented 6 years ago

The code for this was deployed, I was able to verify it in DEMO. However... UI for this was placed on Switch User page as per AC, but switch User page is hidden in staging environment. We should create issue to have UI for this feature available in staging env as well.

amprokop commented 6 years ago

oh nooo that was a mistake. @nemodjuric, can you take a shot at writing the issue for this since you're closest to the work, and ping me with the issue?

astewarttistatech commented 6 years ago

Moving this back in progress.

lpciferri commented 5 years ago

I think this is done - because I was given Global Admin and can impersonate users! Closing.