Skip to content

fix(portal): Project Tab Access Permissions Match the User Permissions#22708

Open
vg006 wants to merge 7 commits intogoharbor:mainfrom
vg006:fix/tabs
Open

fix(portal): Project Tab Access Permissions Match the User Permissions#22708
vg006 wants to merge 7 commits intogoharbor:mainfrom
vg006:fix/tabs

Conversation

@vg006
Copy link

@vg006 vg006 commented Jan 5, 2026

Comprehensive Summary of your change

Issue being fixed

Fixes #22639

As mentioned in the issue, This PR updates the component and module, such that only the Project Admin can view and edit the project configuration. Other than the Project Admin, the tabs is set to be inaccessible.

Also updated the project.module.ts file, so that the page is redirected to the Projects, if an unauthorized member attempts to visit the restricted tabs accordingly.

As Limited Guest

Before After
image image

As Guest

Before After
image image

Please indicate you've done the following:

  • Well Written Title and Summary of the PR
  • Label the PR as needed. release-note/update, release-note/enhancement
  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Made sure tests are passing and test coverage is added if needed.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

@vg006 vg006 requested a review from a team as a code owner January 5, 2026 08:56
@vg006
Copy link
Author

vg006 commented Jan 5, 2026

To the maintainers @bupd, @Vad1mo, @OrlinVasilev, @stonezdj, @MinerYang, @chlins.

There are few tabs such as Policy and P2P Preheat with READ permissions and most of the other tabs with LIST permissions in the project.module.ts file.

As suggested in the issue, do other tabs need to be updated, such that only member with CREATE, UPDATE or DELETE access can READ/VIEW those tabs?

Thus the PR can be updated with the overall recommended changes in the Project tabs' components and modules.

@stonezdj stonezdj added the help wanted The issues that is valid but needs help from community label Jan 5, 2026
@bupd
Copy link
Contributor

bupd commented Jan 5, 2026

As suggested in the issue, do other tabs need to be updated, such that only member with CREATE, UPDATE or DELETE access can READ/VIEW those tabs?

Hello @vg006, follow what the backend follows, that is if backend doesnt allow user to READ - dont show the tab, if it doesnt allow EDIT / CREATE - disable the buttons.

Hope this helps.

@vg006
Copy link
Author

vg006 commented Jan 6, 2026

As per the requirements, I have update only the frontend components to display only tabs that are meant to be shown to the member.

Thus this PR is ready to be merged 👍

Previews

System Admin
image
Project Admin
image
Maintainer
image
Developer
image
Guest
image
Limited Guest
image

@codecov
Copy link

codecov bot commented Jan 6, 2026

Codecov Report

❌ Patch coverage is 44.44444% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.02%. Comparing base (b173fc3) to head (2e3149b).

Files with missing lines Patch % Lines
.../src/app/base/project/scanner/scanner.component.ts 22.22% 6 Missing and 1 partial ⚠️
...project/project-config/project-config.component.ts 40.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #22708   +/-   ##
=======================================
  Coverage   66.01%   66.02%           
=======================================
  Files        1073     1073           
  Lines      116495   116511   +16     
  Branches     2939     2944    +5     
=======================================
+ Hits        76903    76924   +21     
+ Misses      35338    35337    -1     
+ Partials     4254     4250    -4     
Flag Coverage Δ
unittests 66.02% <44.44%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...project/project-detail/project-detail.component.ts 64.46% <100.00%> (+0.59%) ⬆️
...project/project-config/project-config.component.ts 73.68% <40.00%> (-12.04%) ⬇️
.../src/app/base/project/scanner/scanner.component.ts 61.53% <22.22%> (-5.13%) ⬇️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vg006
Copy link
Author

vg006 commented Jan 8, 2026

@bupd
I have made the changes only in the frontend components and didn't update the backend, as you can see in the commits.
So I'm unsure why APITEST_LDAP job is failing, particularly at this test, which is an API test.

I assure that my changes are tested and safer to merge, but can you review the changes once and share your comments?

@vg006
Copy link
Author

vg006 commented Jan 8, 2026

This branch is rebased on top of upstream's main branch and is now ready for merge.

Copy link
Contributor

@bupd bupd left a comment

Choose a reason for hiding this comment

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

@vg006 I believe we should not change the permissions here. READ permission should be enough to view the tabs.

Scanner and config tab may not be valuable to limited guest and guest roles though. other roles can infer what the project is.

given that all roles have read permission on the config means simply are we going to show it in ui or not. and I see no value in showing config tab to a guest role.

@vg006
Copy link
Author

vg006 commented Jan 17, 2026

@bupd

I have reverted the changes in the permission list and updated the modules and now the scanner and configuration tabs aren't visible to the Limited Guest and Guest.

Additionally if such member attempts to view those tabs by directly entering the endpoint, It will be redirected automatically.
I rebased the branch on the main, so it is ready to merge. Can you kindly review these changes?

Previews

System Admin
image
Project Admin
image
Maintainer
image
Developer
image
Guest
image
Limited Guest
image

Copy link
Contributor

@bupd bupd left a comment

Choose a reason for hiding this comment

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

/lgtm

@Vad1mo Vad1mo enabled auto-merge (squash) February 18, 2026 13:22
@vg006
Copy link
Author

vg006 commented Feb 18, 2026

@Vad1mo @bupd
Out of 12, 10 tests were successful and release note label and patch coverage are failing.

Copy link
Contributor

@bupd bupd left a comment

Choose a reason for hiding this comment

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

/lgtm

auto-merge was automatically disabled March 4, 2026 13:15

Head branch was pushed to by a user without write access

@Vad1mo Vad1mo added the release-note/update Update or Fix label Mar 4, 2026
@Vad1mo Vad1mo requested a review from Copilot March 4, 2026 13:40
@Vad1mo Vad1mo changed the title fix(portal): Update Project Tabs Access permissions fix(portal): Project Tab Access Permissions Match the User Permissions Mar 4, 2026
@Vad1mo Vad1mo self-requested a review March 4, 2026 13:44
@Vad1mo Vad1mo enabled auto-merge (squash) March 4, 2026 13:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR attempts to fix a UX issue (#22639) where Limited Guest and Guest users could see project tabs (Scanner, Configuration) they couldn't fully use, causing 403 errors. The fix hides the Scanner and Configuration tabs from Guest/LimitedGuest users via role-name checks in the tab permission lambdas, and adds component-level redirects so that direct URL access is also blocked.

Changes:

  • project-detail.component.ts: Adds an isLimitedGuestOrGuest() method and uses it to hide the Scanner and Configuration tabs for Guest/LimitedGuest users
  • scanner.component.ts: Adds a role-based redirect in ngOnInit that sends Guest/LimitedGuest users to the projects list
  • project-config.component.ts: Adds a role-based redirect in ngOnInit that sends Guest/LimitedGuest users to the projects list

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File Description
project-detail/project-detail.component.ts Adds isLimitedGuestOrGuest() helper and uses it in the Scanner and Config tab permission expressions
scanner/scanner.component.ts Imports Router and CommonRoutes; adds redirect for Guest/LimitedGuest in ngOnInit
project-config/project-config.component.ts Imports CommonRoutes; adds redirect for Guest/LimitedGuest in ngOnInit

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Vad1mo
Copy link
Member

Vad1mo commented Mar 4, 2026

@vg006, can you check on the comments created by @copilot code review[agent]

@vg006
Copy link
Author

vg006 commented Mar 4, 2026

Sure I will, working on it.

auto-merge was automatically disabled March 5, 2026 09:19

Head branch was pushed to by a user without write access

@vg006 vg006 requested a review from Vad1mo March 5, 2026 09:20
vg006 added 7 commits March 18, 2026 08:25
Signed-off-by: vg006 <devvg006@gmail.com>
Signed-off-by: vg006 <devvg006@gmail.com>
Signed-off-by: vg006 <devvg006@gmail.com>
Signed-off-by: vg006 <devvg006@gmail.com>
Signed-off-by: vg006 <devvg006@gmail.com>
Signed-off-by: vg006 <devvg006@gmail.com>
Signed-off-by: vg006 <devvg006@gmail.com>
@vg006
Copy link
Author

vg006 commented Mar 18, 2026

@Vad1mo All the tests were passing, except the code coverage of the patch.
The changes were reviewed by you and @bupd multiple times.
Hope this PR is ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted The issues that is valid but needs help from community release-note/update Update or Fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Limited Guest user sees unauthorized tabs (P2P Preheat, Webhooks, Configuration) causing 403 Bad UX

7 participants