Skip to content

fix rbac project subject for robot accounts#14753

Closed
phin1x wants to merge 4 commits intogoharbor:masterfrom
phin1x:fix-rbac-project-subject-for-robot-accounts
Closed

fix rbac project subject for robot accounts#14753
phin1x wants to merge 4 commits intogoharbor:masterfrom
phin1x:fix-rbac-project-subject-for-robot-accounts

Conversation

@phin1x
Copy link
Contributor

@phin1x phin1x commented Apr 26, 2021

as we began to use system level robot acocunt we could not list, delete or get projects from the api. we discoverd that the rbac subject for projects are wrong for robot accounts.

the the naming schema for subjects is:

/<system | project>/<resource id>/<subresource>

if a robot account has premissions to list all projects, the allowed subject looks like this:

/project/*/project

but harbor expects the following subject:

/project/*

These two subjects never match, so a robot account may never manage projects. For users, the subjects are created correctly. We checked this by changing the expected subject for projects. Now robot accounts were allowed to interact with projects, but users were not.

this pr fixes this issue by omitting the subresouce for project rbac subjects.

Related issues: #14145 #14512

Signed-off-by: Fabian Weber <fabian.weber14@outlook.de>
@codecov
Copy link

codecov bot commented Apr 26, 2021

Codecov Report

Merging #14753 (aa5b754) into master (af12f9a) will increase coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14753      +/-   ##
==========================================
+ Coverage   66.19%   66.21%   +0.01%     
==========================================
  Files         937      937              
  Lines       74601    74606       +5     
  Branches     2183     2183              
==========================================
+ Hits        49383    49397      +14     
+ Misses      21292    21283       -9     
  Partials     3926     3926              
Flag Coverage Δ
unittests 66.21% <0.00%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
src/server/middleware/security/robot2.go 23.80% <0.00%> (-3.22%) ⬇️
...list-repository-ro/list-repository-ro.component.ts 45.83% <0.00%> (-4.17%) ⬇️
...tegration/tag-retention/tag-retention.component.ts 29.95% <0.00%> (+1.32%) ⬆️
src/common/utils/passports.go 89.74% <0.00%> (+5.12%) ⬆️
...es/vulnerability/vulnerability-config.component.ts 52.99% <0.00%> (+5.98%) ⬆️
src/lib/cache/util.go 89.47% <0.00%> (+15.78%) ⬆️

@phin1x phin1x force-pushed the fix-rbac-project-subject-for-robot-accounts branch from c92380a to e239f02 Compare April 26, 2021 18:59
phin1x added 3 commits April 26, 2021 21:22
Signed-off-by: Fabian Weber <fabian.weber14@outlook.de>
Signed-off-by: Fabian Weber <fabian.weber14@outlook.de>
@steven-zou steven-zou added the status/pending-on-voting The issues that need community decision. label May 31, 2021
@phin1x
Copy link
Contributor Author

phin1x commented Jun 26, 2021

@steven-zou what means "pending on voting"?

@phin1x
Copy link
Contributor Author

phin1x commented Jul 28, 2021

@heww @wy65701436 any updates on this pr?

@sidewinder12s
Copy link

sidewinder12s commented Aug 5, 2021

Any updates?

This issue mostly breaks Helm OCI integration as you can't list manifests on a helm chart (If your using a floating target instead of a specific chart version) without both project level permissions and docker catalog permissions. So, it'd probably be good to fix this before removing chart museum

@wy65701436
Copy link
Contributor

wy65701436 commented Aug 18, 2021

The PR is for robot to list projects, @phin1x can you check whether that meets your requirement? If so, let's close this PR.

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

Labels

status/pending-on-voting The issues that need community decision.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants