Login as EPerson #653

Atmire-Kristof · 2020-04-16T14:06:07Z

This PR is dependent on this REST PR

This PR adds the ability for admins to impersonate another user, similar to DSpace 6.

Added 3 buttons to the Edit EPerson page, below the “can login” checkbox:

"Reset Password" and "Delete EPerson" are currently placeholders hardcoded to be disabled.

Clicking “Impersonate EPerson” will start impersonating the EPerson. The user will be redirected to the homepage and a cookie with the user UUID is added.
Whenever this cookie is present, every rest request will include an x-on-behalf-of header.

A new button in the navbar menu should now be available:

Clicking this button will remove the cookie, navigate the user to the homepage and perform a hard refresh. The authenticated user should now be the original admin again.

Logging out while impersonating a user will also ensure the cookie is removed and will logout the admin as well.

Note that you need to enable preboot for this PR to work, due to #289

artlowel · 2020-04-16T15:36:26Z

As discussed in today's meeting I've created #654 to decide what to do about the caching issue

…o the browser caching the html

jtimal · 2020-04-30T04:07:03Z

Hi @Atmire-Kristof
I know it depends on another PR # 2740
I found some observations in browsers like firefox, safari and opera cache works fine but in chrome it doesn't refresh, I see that it is already reported.

when you are as an eperson without privileges lets you do administrative things, this was not in previous versions, it restricted access according to the user's profile.

artlowel · 2020-04-30T08:25:26Z

Thanks for reviewing @jtimal

I'm unsure what your first screenshot is meant to show. It looks like you're logged out. In that case the "Stop Impersonating EPerson" button will never show. You need to be logged in, and have clicked "Impersonate EPerson" for it to show

when you are as an eperson without privileges lets you do administrative things, this was not in previous versions, it restricted access according to the user's profile.

At present, the UI isn't aware of user roles yet, though this is planned to be added in the near future. So this means that when any user logs in, they'll see all the options an admin has available. The rest api does know about user roles though. So if you try to do something you shouldn't be allowed to do while impersonating a user, it should fail.

jtimal · 2020-04-30T14:13:19Z

Thanks for reviewing @jtimal

I'm unsure what your first screenshot is meant to show. It looks like you're logged out. In that case the "Stop Impersonating EPerson" button will never show. You need to be logged in, and have clicked "Impersonate EPerson" for it to show

when you are as an eperson without privileges lets you do administrative things, this was not in previous versions, it restricted access according to the user's profile.

At present, the UI isn't aware of user roles yet, though this is planned to be added in the near future. So this means that when any user logs in, they'll see all the options an admin has available. The rest api does know about user roles though. So if you try to do something you shouldn't be allowed to do while impersonating a user, it should fail.

Hi @artlowel yes, If the first image is what I mention when logging into a chome browser (updated) and then using the button function mentioned in this PR, the functionality does not appear and disconnects you from the session

artlowel · 2020-04-30T14:20:03Z

@jtimal Are you sure preboot is enabled in the config for the mode you're running (environment.dev.js when running yarn run watch, or environment.prod.js when running yarn run start)? If it isn't you'll be susceptible to this issue: #289

jtimal · 2020-04-30T14:53:06Z

@jtimal Are you sure preboot is enabled in the config for the mode you're running (environment.dev.js when running yarn run watch, or environment.prod.js when running yarn run start)? If it isn't you'll be susceptible to this issue: #289

Yer this configuration is enabled in environment.dev.js

artlowel · 2020-04-30T15:06:19Z

In that case I can't reproduce your issue. I just tested it again. It works for me on chrome.

jtimal · 2020-05-07T14:32:44Z

Hi @artlowel
We recorded some videos of what we found the error comes and what the console sends
https://drive.google.com/drive/folders/1952e0-pVwFdJChJJttEFMY5WunBeIwDQ?usp=sharing

tdonohue · 2020-05-08T21:30:21Z

@Atmire-Kristof and @artlowel : I ran some tests today (via yarn start, AoT compilation) and hit some issues with this PR:

The feature does not seem to check whether the "loginOnBehalfOf" feature is enabled (which requires setting webui.user.assumelogin=true in your dspace.cfg or local.cfg on the backend). If I leave it disabled (default setting), I still see the "Impersonate EPerson" button. Clicking that button causes a 400 error in the console that says "The login as feature is not allowed due to the current configuration." However, the page still refreshes and essentially "locks up". No other actions can be performed until I kill the UI and restart it. The expected behavior here is obviously just to not show the button if this feature is disabled on the backend, or alternatively show it grayed out with a hover over note that it's disabled. (UPDATE: This is because of Support for checking user permissions via /api/authz/features REST endpoint #664 , see discussion below)
If I enable it by setting webui.user.assumelogin=true, the basic functionality works. However, oddly, if I'm impersonating a non-Admin user (with no groups or special rights), I still see the full Admin sidebar and all admin functions are still available (including create/edit communities, collections, managing metadata schemas, etc.) While technically I have these access rights, I expected the sidebar to disappear if I'm impersonating a non-Admin, as that person does not have those access rights.
- If I attempt to use these features, it does seems like I'm not able to. For instance trying to create a new Community brings up the form, but returns a 403. Also, trying to submit a new item tells me I do not have permissions (even though it lets me select a collection to submit to).
- Strangely, I am able to edit any Collection or Community or Item, which I should not be allowed to do. This seems to be a security issue . (UPDATE: This is because of Editing a Community, Collection or Item doesn't handle errors properly #665 , see discussion below)

Besides these two issues, the feature mostly works. I "appear" to be the user I'm logged in as, and I see that persons profile & MyDSpace (if I go to those pages). It just seems to be that some of the UI still knows I'm an Admin while other parts know I'm impersonating a non-admin.

artlowel · 2020-05-11T11:09:30Z

@tdonohue There is no support for features on the angular side yet. Adding it is out of scope for this PR.

This PR aims to give you the exact same experience when using the impersonate button (barring the "stop impersonating" button) you would get if you were logged in as that person in the normal way. At the moment that means that if you log in as a non admin, you can still access and see admin only pages. However the rest should prevent you from performing admin only changes.

Once the angular side of "features"-feature is implemented to enforce what a non admin user can and cannot do when logged in the normal way, those same restrictions will also apply when you're impersonating that user as an admin, provided of course that the features endpoint on the rest side supports the X-On-Behalf-Of header.

tdonohue · 2020-05-11T15:53:53Z

@artlowel : Thanks for the explanation.

Regarding your note about the lack of support for "features" in the UI, I've created #664.

I've retested the edit Community/Collection/Item screens while impersonating a non-Admin user, and it appears that this PR is working properly (it submits a PATCH with the proper X-On-Behalf-Of header).

However, it appears that the Edit Community/Collection/Item screens are not doing proper error handling (just logged this as #665). The response is a proper 403 (Forbidden), but the Edit Community/Collection/Item screens all show a green success message, and the changes are saved into the Angular cache. So, I was previously convinced that the changes were saving, when in fact the edit screens were not doing proper error handling. In any case, this is logged separately in #665.

This PR therefore seems to be working as expected, but it needs a rebase on the latest master

Conflicts: src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts src/app/core/auth/auth.service.spec.ts src/app/profile-page/profile-page.component.spec.ts

atarix83

@Atmire-Kristof thanks for the PR
I had a first review and generally it works.
In addition to the inline question, a few comments though:

I think could be useful to have an additional Login as menu entry in the nav user menu that point ot the Edit EPerson page
the impersonate Eperson button should be added along the edit and delete button of the epersons table

I think you don't have to edit a person to be able to impersonate it.

atarix83 · 2020-05-13T10:53:21Z

src/app/core/auth/auth.reducer.ts

-  // the authenticated user
-  user?: EPerson;
+  // the authenticated user's id
+  userId?: string;


I'm nor really sure to understand this change. Storing the whole eperson object should allow to provide information about the logged user including the id. Why save only the id?

The idea is that every object is stored exactly once inside the store. If you don't, and the object changes, you need to keep all places where it is stored in sync.

In this case, the when the active user changed, the dropdown didn't change with it because the auth section of the store had a separate copy of the user object. That's why I asked @Atmire-Kristof to use only the id instead.

Correct me if i'm wrong but the userId in the state is updated with current user id, even if it's an impersonated one. After impersonating an eperson the page is reload so the store is up to date with the current user. the dropdown menu show who is the current user, but unlike before it make the request to retrieve the eperson data instead to use the object in the store. So why don't retrieve the user data once at login as before if the store seems to contain always the current user ?

It didn't work that way before Kristof changed it. The user in the auth state, and in the dropdown would always be the admin, not the impersonated user, he made this change specifically to address that.

As for the reason an extra EPerson request is sent, that seems to be due to the projections changes on the rest side. The EPerson isn't embedded in the status request unless you add an ?embed=eperson param. Embedding it could be a slight performance increase, but it is unrelated to this PR.

As I can see in the code it should be enough to change here

dspace-angular/src/app/core/auth/auth.effects.ts

Line 109 in 3894b36

map((user: EPerson) => new RetrieveAuthenticatedEpersonSuccessAction(user.id)),

artlowel · 2020-05-13T12:31:36Z

Thanks for the review @atarix83

I think could be useful to have an additional Login as menu entry in the nav user menu that point ot the Edit EPerson page

I don't think this provides much added value. You can already use the admin sidebar to go to that exact page in two clicks. Impersonating a user isn't something that is needed often enough to warrant a shortcut.

The impersonate Eperson button should be added along the edit and delete button of the epersons table

I agree, that would be useful.

However since we're already slightly over the estimate (14 hours, estimate was 12), I'll leave it up to @tdonohue to decide whether it's worth it. We could also add it to #647 and do it later.

tdonohue

👍 This looks good to me (just needs a rebase on master as it's got another merge conflict). I've retested and it all works as described.

I agree with @atarix83 and @artlowel that we should add a button to impersonate an eperson on the manage EPerson page directly. However, that can be added to #647 as one of the "cleanup" tasks with regards to the Manage Groups/People pages (I'll add it).

Conflicts: src/app/shared/shared.module.ts

Atmire-Kristof · 2020-05-19T08:04:57Z

@tdonohue , thanks for the review. I've merged the latest master into this PR and resolved the conflicts.

atarix83

As we've discussed in the last working group meeting we can merge it

Atmire-Kristof added 7 commits April 10, 2020 17:41

70373: Login as EPerson intermediate commit

638793c

70373: Store authenticated EPerson ID in store instead of object

e43aa15

70373: Stop impersonating button in navbar + clear cookie on logout

c48bb2c

70373: Fix existing texts

4cd11bc

70373: EPerson impersonate tests

7df5025

70373: AuthService impersonate tests

5e94cf3

70373: Remove unnecessary route navigate

a6d7b64

artlowel mentioned this pull request Apr 16, 2020

Force the browser not to use a cached html page after the user changes #654

Closed

artlowel added 2 commits April 24, 2020 13:16

fix issue where a hard refresh wouldn't work to clear the state due t…

c2789df

…o the browser caching the html

remove clearStateMetaReducer

df2cc22

This was referenced May 11, 2020

Support for checking user permissions via /api/authz/features REST endpoint #664

Closed

Editing a Community, Collection or Item doesn't handle errors properly #665

Closed

Atmire-Kristof added 2 commits May 12, 2020 16:54

Merge branch 'master' into Login-as-EPerson

f548d5b

Conflicts: src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts src/app/core/auth/auth.service.spec.ts src/app/profile-page/profile-page.component.spec.ts

70373: Test import fix

3894b36

atarix83 reviewed May 13, 2020

View reviewed changes

tdonohue approved these changes May 15, 2020

View reviewed changes

tdonohue mentioned this pull request May 15, 2020

UI Improvements for Manage Groups/People #647

Open

Merge branch 'master' into Login-as-EPerson

ad26ece

Conflicts: src/app/shared/shared.module.ts

atarix83 approved these changes May 20, 2020

View reviewed changes

tdonohue merged commit 5c7cd4a into DSpace:master May 21, 2020

artlowel deleted the Login-as-EPerson branch June 2, 2020 07:39

tdonohue added this to the 7.0beta3 milestone Jun 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Login as EPerson #653

Login as EPerson #653

Atmire-Kristof commented Apr 16, 2020 •

edited by artlowel

Loading

artlowel commented Apr 16, 2020

jtimal commented Apr 30, 2020

artlowel commented Apr 30, 2020

jtimal commented Apr 30, 2020

artlowel commented Apr 30, 2020 •

edited

Loading

jtimal commented Apr 30, 2020

artlowel commented Apr 30, 2020

jtimal commented May 7, 2020

tdonohue commented May 8, 2020 •

edited

Loading

artlowel commented May 11, 2020 •

edited

Loading

tdonohue commented May 11, 2020

atarix83 left a comment

atarix83 May 13, 2020

artlowel May 13, 2020

atarix83 May 13, 2020

artlowel May 13, 2020

atarix83 May 13, 2020

artlowel commented May 13, 2020

tdonohue left a comment

Atmire-Kristof commented May 19, 2020

atarix83 left a comment

Login as EPerson #653

Login as EPerson #653

Conversation

Atmire-Kristof commented Apr 16, 2020 • edited by artlowel Loading

artlowel commented Apr 16, 2020

jtimal commented Apr 30, 2020

artlowel commented Apr 30, 2020

jtimal commented Apr 30, 2020

artlowel commented Apr 30, 2020 • edited Loading

jtimal commented Apr 30, 2020

artlowel commented Apr 30, 2020

jtimal commented May 7, 2020

tdonohue commented May 8, 2020 • edited Loading

artlowel commented May 11, 2020 • edited Loading

tdonohue commented May 11, 2020

atarix83 left a comment

Choose a reason for hiding this comment

atarix83 May 13, 2020

Choose a reason for hiding this comment

artlowel May 13, 2020

Choose a reason for hiding this comment

atarix83 May 13, 2020

Choose a reason for hiding this comment

artlowel May 13, 2020

Choose a reason for hiding this comment

atarix83 May 13, 2020

Choose a reason for hiding this comment

artlowel commented May 13, 2020

tdonohue left a comment

Choose a reason for hiding this comment

Atmire-Kristof commented May 19, 2020

atarix83 left a comment

Choose a reason for hiding this comment

Atmire-Kristof commented Apr 16, 2020 •

edited by artlowel

Loading

artlowel commented Apr 30, 2020 •

edited

Loading

tdonohue commented May 8, 2020 •

edited

Loading

artlowel commented May 11, 2020 •

edited

Loading