Skip to content
This repository has been archived by the owner on Jan 17, 2024. It is now read-only.

[1229] Support can impersonate users #1410

Merged
merged 5 commits into from
Mar 12, 2021
Merged

Conversation

asmega
Copy link
Contributor

@asmega asmega commented Mar 10, 2021

Context

Changes proposed in this pull request

  • Third line support staff can impersonate other users excluding other support users, computacenter users and themselves
  • This feature is found on the viewing a user typically from user search results
  • Whilst impersonating actions should be readonly to prevent quirky audits
  • Actions whilst impersonating will therefore return forbidden and prevent the action from happening
  • Users can only accept their own privacy policy, this prevents impersonation from accepting someone elses privacy policy on their behalf
  • Whilst impersonating, adds a notification banner near the top of the page to show they are currently impersonating someone and who that someone is
  • The banner has a button to end the impersonation

Screenshots

Support viewing a user to kickoff impersonation session
image

Impersonating a user
image

Guidance to review

  • Login as third line support
  • Find different users to impersonate - school user, rb user, single school user vs multi school user, user that has not accepted privacy policy
  • Ensure view a of the viewing user
  • Should see notification banner of current impersonation
  • Should not be able to perform any actions when impersonating ie make modifications eg. invite user, change who orders etc
  • Stop impersonation
  • Everything should return to normal
  • Should not be able to impersonate yourself just incase there are any quirks
  • Should not be able to impersonate another support user so there is no privilege escalation or affecting of audits
  • Should not be able to impersonate a computacenter user for the same above reason

@muqi1 muqi1 temporarily deployed to dfe-ghwt-pr-1410 March 10, 2021 14:46 Inactive
@asmega asmega force-pushed the 1229-support-impersonation branch from 3ce7409 to 9c8e155 Compare March 10, 2021 14:49
@asmega asmega temporarily deployed to dfe-ghwt-pr-1410 March 10, 2021 14:49 Inactive
Comment on lines 74 to 78
:heading,
text: 'You are currently impersonating',
link_text: impersonated_user.email_address,
link_target: support_user_path(impersonated_user)
)
Copy link
Contributor

@fofr fofr Mar 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to do:

You are impersonating [Full Name](/support/users/USER_ID) (email@email.com)
[Stop impersonating]

?

Although thinking about it more – the name of the organisation might be useful somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yea sorry, totally forgot, will do now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated, please see screenshot

@asmega asmega force-pushed the 1229-support-impersonation branch from 9c8e155 to 04aa663 Compare March 10, 2021 16:51
@asmega asmega temporarily deployed to dfe-ghwt-pr-1410 March 10, 2021 16:51 Inactive
@asmega asmega temporarily deployed to dfe-ghwt-pr-1410 March 11, 2021 10:49 Inactive
@asmega asmega temporarily deployed to dfe-ghwt-pr-1410 March 11, 2021 12:58 Inactive
@asmega asmega force-pushed the 1229-support-impersonation branch from f46d766 to c394606 Compare March 11, 2021 13:20
@asmega asmega temporarily deployed to dfe-ghwt-pr-1410 March 11, 2021 13:21 Inactive
@asmega asmega marked this pull request as ready for review March 11, 2021 14:48
Copy link
Contributor

@tonyheadford tonyheadford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, this looks a complicated problem.
Some observations, may be just down to my test data:

  • If the user has seen the privacy notice but hasn't completed the welcome wizard you can't continue
  • If you get a Forbidden error you're a bit stuck unless you use browser back button
  • Top menu is still for support user, not sure what options the user should have (if any) but if you hit a forbidden action it's browser back button or support home and then you can't get to the user's landing page unless you stop impersonating and re-impersonate
  • When I impersonated a user with 2 schools, it also displayed my support users responsible body on the 'Your organisations' landing page (not sure thats really a problem)
  • I suspect we're a bit stuck when it comes to any multi-step processes that preserve state (e.g. welcome wizard, Daily Mail)

app/controllers/support/impersonates_controller.rb Outdated Show resolved Hide resolved
@asmega asmega force-pushed the 1229-support-impersonation branch from c394606 to 4095936 Compare March 12, 2021 11:01
@asmega asmega temporarily deployed to dfe-ghwt-pr-1410 March 12, 2021 11:01 Inactive
@asmega asmega temporarily deployed to dfe-ghwt-pr-1410 March 12, 2021 11:24 Inactive
@asmega
Copy link
Contributor Author

asmega commented Mar 12, 2021

Good work, this looks a complicated problem.
Some observations, may be just down to my test data:

  • If the user has seen the privacy notice but hasn't completed the welcome wizard you can't continue
  • I did this by design. As we want the support user to experience what the user is experiencing. If the two do not align the support user can't give accurate feedback.
  • If you get a Forbidden error you're a bit stuck unless you use browser back button
  • I guess so. I don't think there is a mechanism to set a back link on a forbidden page
  • Top menu is still for support user, not sure what options the user should have (if any) but if you hit a forbidden action it's browser back button or support home and then you can't get to the user's landing page unless you stop impersonating and re-impersonate
  • yeah i found this too and just used the browser back button. not sure how else to improve it
  • When I impersonated a user with 2 schools, it also displayed my support users responsible body on the 'Your organisations' landing page (not sure thats really a problem)
  • it think in your case this is merely coincidental. if you found another use with more that one school and make sure they differ from your support user you should see the impersonated user's schools. this should be covered by the changes in app/controllers/schools_controller.rb
  • I suspect we're a bit stuck when it comes to any multi-step processes that preserve state (e.g. welcome wizard, Daily Mail)
  • so all the authorize calls i added should prevent the support user affecting the impersonated users state. they shouldn't be allowed to affect the users state and should be readonly. if they want to change state this should be done in the support namespace.
  • therefore the multi-step processes step states should be preserved unless these are kept in the session and not loaded from the database

@asmega asmega merged commit 09f77c1 into main Mar 12, 2021
@asmega asmega deleted the 1229-support-impersonation branch March 12, 2021 12:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants