fix project creation with robot account #14754

phin1x · 2021-04-26T19:14:06Z

we tried to create projects using a robot account, but the api call always fails with the message "user robot$ not found". it turns out that harbor is trying to get the id of the current authorized user when that user is not a "solution user". since a robot account is not a regular user, the api call always aborts with the error "user not found".

this pr fixes the issue by checking if the current authorized user starts with the robot prefix. if thats the case the owner id is set to one.

Related issues: #14145

codecov · 2021-04-26T19:18:13Z

Codecov Report

Merging #14754 (6e04ea8) into master (b73480e) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #14754      +/-   ##
==========================================
- Coverage   66.80%   66.80%   -0.01%     
==========================================
  Files         928      928              
  Lines       76089    76095       +6     
  Branches     2253     2253              
==========================================
- Hits        50833    50832       -1     
- Misses      21316    21323       +7     
  Partials     3940     3940

Flag	Coverage Δ
unittests	`66.80% <0.00%> (-0.01%)`	⬇️

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

Impacted Files	Coverage Δ
src/common/security/robot/context.go	`61.36% <0.00%> (-1.43%)`	⬇️
src/server/v2.0/handler/project.go	`6.12% <0.00%> (-0.08%)`	⬇️
...c/app/shared/components/filter/filter.component.ts	`73.68% <0.00%> (-5.27%)`	⬇️

heww · 2021-05-10T07:48:17Z

I don't think this is the correct way to assign the creation permission of the project to the robot account.

Please use (/system/project, create) to check the permission when the security context is not a solution user.

phin1x · 2021-05-10T11:51:18Z

@heww this is not a problem with the actual permission. the rbac check already has passed. at this point the owner of the project is searched. if the current user it is not a solution user, we search for the current authorized user in the user table! but a robot account is not a regular user so the api always repsonds with a "user not found" error. also a robot account does not has a uid, so we have to assign a fix uid (i assigned the admin uid).

heww · 2021-05-10T16:27:50Z

@heww this is not a problem with the actual permission. the rbac check already has passed. at this point the owner of the project is searched. if the current user it is not a solution user, we search for the current authorized user in the user table! but a robot account is not a regular user so the api always repsonds with a "user not found" error. also a robot account does not has a uid, so we have to assign a fix uid (i assigned the admin uid).

Thanks, I got your point.

heww · 2021-05-10T16:31:12Z

src/server/v2.0/handler/project.go

@@ -156,10 +156,10 @@ func (a *projectAPI) CreateProject(ctx context.Context, params operation.CreateP
 	// set the owner as the system admin when the API being called by replication
 	// it's a solution to workaround the restriction of project creation API:
 	// only normal users can create projects
-	if secCtx.IsSolutionUser() {
+	ownerName := secCtx.GetUsername()
+	if secCtx.IsSolutionUser() || strings.HasPrefix(ownerName, config.RobotPrefix(ctx)) {


Please try to use the security context name to check that the request is from a robot account.

if secCtx.IsSolutionUser() || secCtx.Name() == "robot" { ownerID = 1 }

phin1x · 2021-05-10T17:42:30Z

@heww i change the code using the security context name for checking if the current user is a robot acocunt

heww · 2021-05-11T11:52:51Z

Hi, @phin1x thanks for your update. We discussed this PR today and there may need more update for it.

The project-level robot account can create a project when every authorized user (not only the system admin) can create the project, in this scenario, the owner_id of the project will be 1 when this PR accepted. So could you update this PR and disallow the project-level robot to create the project? Thanks!

phin1x · 2021-05-11T13:13:36Z

@heww if think is is already done via the rbac system. project level robot accounts have subjects with a "/project/" prefix, system level robot accounts have subjects with a "/system/" prefix. at the moment, you cannot mix project and system level permissions in a robot account, as we discused in other issues (#14512 (comment)). Since the rbac subject for creating projects is "/system/project", a project level robot account will never be able to create projects.

heww · 2021-05-11T13:39:02Z

@heww if think is is already done via the rbac system. project level robot accounts have subjects with a "/project/" prefix, system level robot accounts have subjects with a "/system/" prefix. at the moment, you cannot mix project and system level permissions in a robot account, as we discused in other issues (#14512 (comment)). Since the rbac subject for creating projects is "/system/project", a project level robot account will never be able to create projects.

There is an option to allow everyone to create the project, everyone is the default value for this option in the harbor. When this option is everyone, a project-level robot can create a project.

phin1x · 2021-05-13T19:30:26Z

i looked in the code and, yes, you are right. now harbor always checks the permissions when creating a project from a robot account.

wy65701436

lgtm

wy65701436 · 2021-05-19T06:34:17Z

src/server/v2.0/handler/project.go

 		log.Errorf("Only sys admin can create project")
 		return a.SendError(ctx, errors.ForbiddenError(nil).WithMessage("Only system admin can create project"))
 	}

+	// always check if robots have permissions to create projects


if !onlyAdmin { if it's a project level robot { return errors.ForbiddenError } }

is there a method to check, if the robot acocunt scope is system or project level? i didn'd saw something like a getLevel method.

@heww is it better to use Can to define whether it's a system level robot?

Or here we can leverage the robot name to query robot, there is an attribute to indicate robot scope.

the robot context have a isSystemLevel property, but it was not exported. i exported the property with a getter and used that getter.

Another trick way, as the robot name has a pattern, the project level name should be {projectname}+robotname, which is difference with system level robot. And + is illegal character when to issue a new robot, so here we can use whether contains + to judge.

imho using a character to identify if a robot is system or project level is not a good idea. if you set the robot prefix for example to "robot+" the check will fail. the robot security context already has a property which identifies if a robot is system or project level so why not use it?

phin1x · 2021-05-26T20:28:54Z

@wy65701436 i updated the check if the robot account is system level

src/server/v2.0/handler/project.go

wy65701436

please reslove all the review comments before merging.

phin1x · 2021-07-28T21:20:56Z

@wy65701436 any updates?

wy65701436 · 2021-08-18T05:14:34Z

@phin1x can you rebase the PR to resolve the conflict?

Signed-off-by: Fabian Weber <[email protected]>

phin1x · 2021-08-23T10:49:44Z

@wy65701436 i rebased the pr

wy65701436 · 2021-08-24T07:11:19Z

thanks @phin1x , the robot to create project was covered by #15461. Now, you can try the latest master, robot can support replication.

phin1x changed the title ~~fix robot account project creation~~ fix project creation with robot account Apr 26, 2021

ywk253100 assigned wy65701436 and heww May 10, 2021

heww reviewed May 10, 2021

View reviewed changes

heww added the target/2.3.0 label May 10, 2021

heww approved these changes May 11, 2021

View reviewed changes

wy65701436 approved these changes May 19, 2021

View reviewed changes

wy65701436 self-requested a review May 19, 2021 06:31

wy65701436 reviewed May 19, 2021

View reviewed changes

wy65701436 self-requested a review May 21, 2021 02:43

wy65701436 reviewed Jun 1, 2021

View reviewed changes

src/server/v2.0/handler/project.go Show resolved Hide resolved

wy65701436 self-requested a review June 1, 2021 06:35

wy65701436 removed the target/2.3.0 label Jun 1, 2021

wy65701436 requested changes Jun 1, 2021

View reviewed changes

phin1x force-pushed the fix-robot-project-creation branch from fd76830 to f67197f Compare June 27, 2021 19:12

phin1x requested a review from wy65701436 June 27, 2021 20:09

phin1x added 2 commits August 23, 2021 11:23

fix robot account project creation

ff3d2b5

Signed-off-by: Fabian Weber <[email protected]>

fixed missing ctx parameter

0757993

Signed-off-by: Fabian Weber <[email protected]>

phin1x and others added 6 commits August 23, 2021 11:23

use the context name to check if the authorized user is a robot account

42b709a

Signed-off-by: Fabian Weber <[email protected]>

always check for robot accounts if they are allowed to create projects

9db0e08

Signed-off-by: Fabian Weber <[email protected]>

fix spelling

2d9b249

Signed-off-by: Fabian Weber <[email protected]>

formatted code

ee0f291

Signed-off-by: Fabian Weber <[email protected]>

added better check if a robot account is system level

04c5002

Signed-off-by: Fabian Weber <[email protected]>

set project owner id 0 for robot accounts

6e04ea8

Signed-off-by: Fabian Weber <[email protected]>

phin1x force-pushed the fix-robot-project-creation branch from f67197f to 6e04ea8 Compare August 23, 2021 09:35

wy65701436 closed this Aug 24, 2021

phin1x deleted the fix-robot-project-creation branch August 24, 2021 09:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix project creation with robot account #14754

fix project creation with robot account #14754

phin1x commented Apr 26, 2021 •

edited

Loading

codecov bot commented Apr 26, 2021 •

edited

Loading

heww commented May 10, 2021

phin1x commented May 10, 2021 •

edited

Loading

heww commented May 10, 2021

heww May 10, 2021 •

edited

Loading

phin1x commented May 10, 2021

heww commented May 11, 2021 •

edited

Loading

phin1x commented May 11, 2021

heww commented May 11, 2021

phin1x commented May 13, 2021

wy65701436 left a comment

wy65701436 May 19, 2021 •

edited

Loading

phin1x May 21, 2021

wy65701436 Jun 1, 2021

wy65701436 Jun 1, 2021

phin1x Jun 1, 2021

wy65701436 Jun 1, 2021

phin1x Jun 6, 2021 •

edited

Loading

phin1x commented May 26, 2021

wy65701436 left a comment

phin1x commented Jul 28, 2021

wy65701436 commented Aug 18, 2021

phin1x commented Aug 23, 2021

wy65701436 commented Aug 24, 2021

fix project creation with robot account #14754

fix project creation with robot account #14754

Conversation

phin1x commented Apr 26, 2021 • edited Loading

codecov bot commented Apr 26, 2021 • edited Loading

Codecov Report

heww commented May 10, 2021

phin1x commented May 10, 2021 • edited Loading

heww commented May 10, 2021

heww May 10, 2021 • edited Loading

Choose a reason for hiding this comment

phin1x commented May 10, 2021

heww commented May 11, 2021 • edited Loading

phin1x commented May 11, 2021

heww commented May 11, 2021

phin1x commented May 13, 2021

wy65701436 left a comment

Choose a reason for hiding this comment

wy65701436 May 19, 2021 • edited Loading

Choose a reason for hiding this comment

phin1x May 21, 2021

Choose a reason for hiding this comment

wy65701436 Jun 1, 2021

Choose a reason for hiding this comment

wy65701436 Jun 1, 2021

Choose a reason for hiding this comment

phin1x Jun 1, 2021

Choose a reason for hiding this comment

wy65701436 Jun 1, 2021

Choose a reason for hiding this comment

phin1x Jun 6, 2021 • edited Loading

Choose a reason for hiding this comment

phin1x commented May 26, 2021

wy65701436 left a comment

Choose a reason for hiding this comment

phin1x commented Jul 28, 2021

wy65701436 commented Aug 18, 2021

phin1x commented Aug 23, 2021

wy65701436 commented Aug 24, 2021

phin1x commented Apr 26, 2021 •

edited

Loading

codecov bot commented Apr 26, 2021 •

edited

Loading

phin1x commented May 10, 2021 •

edited

Loading

heww May 10, 2021 •

edited

Loading

heww commented May 11, 2021 •

edited

Loading

wy65701436 May 19, 2021 •

edited

Loading

phin1x Jun 6, 2021 •

edited

Loading