#1239  Enhance secret security by restricting by submitter
Released
Marcos de Oliveira opened 1 year ago

Somewhat related to issue #758. What about having an authorization rule for secrets based on actor (submitter) or group/role? But for this to work, submitter on Pull Requests must be set to PR's author (today it is set to OneDev user). When re-running a build, it is correctly set to the user which triggered the run. I tested this by creating a Groovy script that returns "build.submitter". So, when creating a pull request, if the user is authorized to use a secret, job will run successfully, otherwise, job gona fail with access denied, but a user with privilege can re-run the job.

Robin Shen commented 1 year ago

Secrets are currently authorized based on branch. Adding another authorization mechanism based on user/group complicates the situation.

Also user/group based authorization seems flawed to me. For instance, if the owner fast forward main branch to a commit whose committer/author not authorized to use a secret, the CI job on main branch will be rejected without explicitly granting main branch to use that secret.

You mentioned that other products have mechanism to manually approve pull request build of particular job for this use case, can you point me to relevant documentation so that I can check how they solve the problem?

Marcos de Oliveira commented 1 year ago

Imo it could (or should) be OR'ed with branch authorization, not AND. There is already a role definition to manage builds, so maybe, add a flag like "Allow build managers" that will override the branch authorization.

My main concern is about pull requests, so, not about who committed, but about who created the pull request.

About how other solutions handle similar problem:

Jenkins can trigger manual builds by comment

GitHub, by default, do not expose secrets to PR, but can trigger builds by comment. By default, also, GitHub require approval for first time contributors and has a setting for requiring approval for every contribution.

Woodpecker/Drone has a protected setting for repositories, which will allow using secrets in PR (blocked by default) and require approval.

Gitlab has similar setting, but is pay walled.

Marcos de Oliveira commented 1 year ago

So, posting this in another comment to emphasize what I think would be a better idea:

What about what I said in the previous comment:

A flag/checkbox: "Allow build managers to override branch protection"

This way, if the build was triggered by a build manager (which already exists), the secret can be used in any branch... But again, for this to work, builds triggered by PR should have its "submitter" set to the PR creator, and not to "OneDev".

You might be concerned if a user has the build manager role and want to discover a secret, he might set the flag, so, maybe, allow setting it only on creation, or make it in a way that setting this flag would clear (require re-setting) the secret. Just in case you are concerned, because I'll not give build manager role to untrusted people.

Robin Shen commented 1 year ago

Thanks for the info. Will investigate.

BTW: Are you sending pull requests from repository fork or from branch in same repository?

Marcos de Oliveira commented 1 year ago

Thanks for the info. Will investigate.

BTW: Are you sending pull requests from repository fork or from branch in same repository?

From fork

OneDev changed state to 'Closed' 1 year ago
Previous Value Current Value
Open
Closed
OneDev commented 1 year ago

State changed as code fixing the issue is committed (4f731a9b)

OneDev changed state to 'Released' 1 year ago
Previous Value Current Value
Closed
Released
OneDev commented 1 year ago

State changed as build #3476 is successful

issue 1 of 1
Type
Improvement
Priority
Normal
Assignee
Issue Votes (0)
Watchers (4)
Reference
onedev/server#1239
Please wait...
Page is in error, reload to recover