Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix user authorization issues with admin sidebar #1999

Merged
merged 8 commits into from
Feb 2, 2023
Merged

Fix user authorization issues with admin sidebar #1999

merged 8 commits into from
Feb 2, 2023

Conversation

KoenP
Copy link

@KoenP KoenP commented Dec 9, 2022 •
edited
Loading

References

Description

  • Frontend: hides admin sidebar sections for users who lack relevant permissions.
  • Backend: adds authorization features to support the frontend changes.

Instructions for Reviewers

List of changes in this PR:

  • The menu resolver now checks the new canSubmit and canEditItem authorizations to determine whether to add the corresponding sections to the admin sidebar for a logged-in user.
  • If this results in the user having no permissions at all, the admin sidebar is omitted altogether.

To test

  1. Make some users:
    1. one with no permissions at all
    2. one with write permissions on a single item (or admin permissions on a collection containing an item, or ...)
    3. one with add permissions on a collection (or admin on a community containing a collection, or ...)
  2. Log in as those users.
  3. Then see whether you can see the admin sidebar and whether it contains the "New->Item" and/or "Edit->Item" sections.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using yarn lint
  • My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

Koen Pauwels added 7 commits November 30, 2022 15:25
97183 Workaround: reorder admin sidebar sections
627da93 
The issue described at DSpace#1643 was
no longer reproducible: The menu component ultimately retrieves menu section
information from the store, but in the `MenuComponent#ngOnInit` method, this
information is piped through `distinctUntilChanged(compareArraysUsingIds())`,
which discards an update that sets these menu elements to be visible.

The behavior of this pipe is probably incorrect, but a proper fix is out of
scope for the current task. For now, we work around the problem by adding
top-level menu sections _after_ their children while initializing the menu
section store, which side-steps this issue.
97183 Hide sidebar items w/o appropriate permissions.
68bac1f 
See DSpace#1643

Sidebar now omits "New" and/or "Edit" options in the sidebar if the user does
not have permissions to respectively create new Items or edit Items.

If this results in an empty sidebar, the sidebar is hidden in its entirety.
97183 MenuResolver tests for regular user
fa87b1d
97183 Tests for MenuService.isMenuVisibleWithVisibleSections
4f20d20
97183 Fixed linting issues
92c7922
97183 Minor fixes (code review)
742b68b
97183 Fix linter issue
69bd324
@KoenP KoenP mentioned this pull request Dec 12, 2022
Merged
8 tasks
@ybnd ybnd added bug usability high priority authorization related to authorization, permissions or groups claimed: Atmire Atmire team is working on this issue & will contribute back labels Dec 12, 2022
@ybnd ybnd added this to the 7.5 milestone Dec 12, 2022
@KoenP KoenP changed the title W2p 97183 fix user authorization issues with admin sidebar Fix user authorization issues with admin sidebar Dec 12, 2022
@KoenP KoenP marked this pull request as ready for review December 12, 2022 09:31
tdonohue
tdonohue approved these changes Dec 20, 2022
View reviewed changes
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

👍 Thanks @KoenP ! I tested this with the backend PR and found that it works for full Admin, Comm Admin, Coll Admin, Submitter and a user with no privileges. The menu options all look correct now.

@KoenP KoenP mentioned this pull request Dec 22, 2022
Open
8 tasks
atarix83
atarix83 approved these changes Feb 2, 2023
View reviewed changes
Copy link
Contributor

@atarix83 atarix83 left a comment

Choose a reason for hiding this comment

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

thanks @KoenP for this PR

LGTM

@tdonohue
Copy link
Member

tdonohue commented Feb 2, 2023

Merging as this PR & the corresponding backend PR are both now at +2. Thanks again @KoenP !

@tdonohue tdonohue merged commit 0e6a6ce into DSpace:main Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tdonohue tdonohue tdonohue approved these changes

@atarix83 atarix83 atarix83 approved these changes

Assignees

@KoenP KoenP

Labels
authorization related to authorization, permissions or groups bug claimed: Atmire Atmire team is working on this issue & will contribute back high priority usability
Projects
No open projects
DSpace 7.5 release
Status: ✅ Done
Milestone
7.5
Development

Successfully merging this pull request may close these issues.

Users with read-only access rights should not be able to access the sidebar menu
4 participants
@KoenP @tdonohue @atarix83 @ybnd