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.

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

add proposal for robot permission expansion #249

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

wy65701436
Copy link
Contributor

No description provided.

Copy link
Member

@chlins chlins left a comment

Choose a reason for hiding this comment

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

lgtm

Add a new column of creator for table robot.

```
ALTER TABLE robot ADD COLUMN IF NOT EXISTS creator varchar(255);
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO more details are needed for the value of this column, how to differentiate a user from a robot account?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the creator is identified by the full name of either a Harbor user or a robot account, and robot account names include a unique prefix, it allows easy distinction between robot accounts and human users.


![audit_log](../images/robot-expand-permission/robot2.png)

## Data provider
Copy link
Contributor

@reasonerjt reasonerjt Aug 22, 2024

Choose a reason for hiding this comment

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

Will we need to make update to the security context of a robot to list the permissions? Otherwise how do we ensure there's no elevation when a robot account is calling API to create another robot account?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The API handler knows that the current user is a robot. With this knowledge, it can determine both the scope of the creator and the scope of the robot. The handler then decides whether to proceed based on the comparison of these scopes.

3. The creation and deletion of robot accounts will be recorded in the audit log.
4. As a system administrator, I can identify the creator of each robot account by performing an SQL query in the database.
5. A robot account can create another robot account, but the new account’s scope must be less than or equal to that of the creator.
6. A robot account created by another robot can only be updated or deleted by either the human with the relevant permissions, the creator of the robot account, or the robot account itself.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wish to suggest in addition to the items in user story, we should zoom into the scenario where robot account managing robot account, b/c this is where the most requirement come from and there are quite a few technical decisions we made that should be documented for discussion with other maintainers and for reference in future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add a separate section for the robot that created by another robot.

Signed-off-by: wang yan <[email protected]>
2. any system level robot account can be created by a system level robot account who with the robot creation permission.

Deletion/Update: A service account cannot assign permissions to another service account that exceed its own permissions.
1. any robot account that created by another robot can be updated by the creator robot or the itself.
Copy link
Contributor

Choose a reason for hiding this comment

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

By "the itself" do you mean "itself"?
Are you saying a robot account can remove itself even if it doesn't have permission to delete "robot account"?
How does it work before we allow a robot account to create a robot account?

Copy link
Contributor Author

@wy65701436 wy65701436 Sep 9, 2024

Choose a reason for hiding this comment

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

yes, I mean only when the creator has the permission of robot account deletion can delete the created robot.

As for the robot that created by another robot,

  1. any human/system level robot with the relevant permission can remove it.
  2. if the creator has the robot deletion permission, it can remove the created robots.
  3. if itself was granted the robot deletion permission when it was created, it can remove itself.

2. The creation and deletion of robot accounts will be recorded in the audit log.
3. As a system administrator, I can identify the creator of each robot account by performing an SQL query in the database.
4. A robot account can create another robot account, but the new account’s scope must be less than or equal to that of the creator.
5. A robot account created by another robot can only be updated or deleted by either the human with the relevant permissions, the creator of the robot account, or the robot account itself.
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. When a robot account updates a robot account that it created, we should also make sure there's elevation.
  2. I don't quite understand why we wanna limit the deletion. If a system-level robot account has the permission to delete a robot account, why we don't allow it to delete any other robot account? Isn't this model a little easier to understand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1, limitation of updating is the same as creation.
2, as for the system level robot, it can remove any other robots if it has the relevant permission.

2. any system level robot account can be created by a system level robot account who with the robot creation permission.

Deletion/Update: A robot account cannot assign permissions to another robot account that exceed its own permissions.
1. any robot account that created by another robot can be updated by the creator robot or the itself, provided that the update permission has been granted to the creator or the robot itself.
Copy link
Contributor

Choose a reason for hiding this comment

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

the itself should be itself

## Robot accounts that are created and managed by another robot accounts

Robot accounts are principals, it means that you can grant robot account to Harbor resources. However, from the perspective of preventing privilege escalation, generally, a robot account
cannot grant roles that are higher or more powerful than the roles it possesses. When a robot account creates a new robot account, the new robot account doesn't automatically inherit any roles
Copy link
Contributor

Choose a reason for hiding this comment

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

replace role with permission, avoid using role because the role is only used in project member,

Signed-off-by: wang yan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants