Allow Deploy keys to push to protected branches once more
Problem to solve
Prior to Gitlab v12.0, Deploy Keys with write access could push commits to protected branches. This is no longer working, and makes Deploy Keys mostly unusable for our use case as they currently work.
Today, we don't treat a deploy key as a machine user, but we evaluate the permission through the owner of the key. This is not an ideal architecture as users are usually registering SSH key as a machine user, which is detached from any user accounts.
Sascha (Software Developer), Devon (DevOps Engineer)
We used a Deploy Key to push updated
package.json version to the
master branch whenever a new version of our NPM package was released. Now, we either have to unprotect the
master branch (allowing all developpers to push to
master) or create a bogus/bot user that has the correct permissions. Unprotecting
master is obvious why it is not a good idea, one simple reason that comes to mind is that it's easy to push commits on the wrong branch by mistake. Creating a CI/bot user will cost additional money, as we are on the silver plan, as well as adding user management overhead.
- A Deploy Key currently has a "Write access allowed" setting. An additional setting could be added below: "Write access to protected branches allowed".
- When setting protected branches, there could be an area next to "Roles" for deploy keys, that can be toggled on/off. It could simply say "Allow Deploy Key access".
In this proposal, we're moving towards having an isolated permission model for deploy keys. Therefore, users can see the keys in the protected branches' interface.
- Protected branches will rely on the access of the deploy key owner
- Deploy keys are able to push to protected branch if the owner does not have permission to do, but does have access to the project.
- When deploy keys are to be able to push the user is not made implicit with the same permission as this will cause unintended access.
- When a user is deleted/unassigned from a project the deploy keys will become unavailable.
- The workaround of the need for a bot/machine user account to not tie it to a real user account is known and deemed acceptable for now.
- **At the project level: - protected branches ** The drop-down for
Allowed to deploywill get an additional section dedicated to deploy keys:
- Deploy keys section will feature an information hierarchy and architecture as depicted in the mockup.
- The only exception is that we show the deploy key title/name instead of the fingerprint including the first few characters of the MD5 signature as seen at #30769 (comment 381468877)
- Ellipsing should happen if the content exceeds the width of the dropdown menu
- After this change, all of the users have to manually add a deploy key to the protected branch interface for giving it write permission. This means this regression requires user action after it's shipped (the gitlab-com/www-gitlab-com!44839 (merged) will help communicate this).
- There will be an additional help text paragraph included in between the deploy keys tabs and the configuration UI with the copy:
Note: Deleting a deploy key might affect existing configurations.as can be seen at #30769 (comment 379238690)
- Help text paragraphs
protected branchessection changes to:
Keep stable branches secure and force developers to use merge requests.
By default, protected branches
<link>are designed to:
- prevent their creation from everybody except Maintainers
<link to permissions>
- prevent pushes from everybody except Maintainers
- prevent anyone from force pushing to the branch
- prevent anyone from deleting the branch
You can change the defaults to allow specific users or deploy keys
<link>to access protected branches.
|Mockup (Figma document)|
Permissions and Security
What does success look like, and how can we measure that?
Links / references
- #223750 We eliminate the difference between CE/EE, allowing to restrict by users, in addition to by roles
- #223748 Update "protected environments" section in a similar way
Merge request scoping
- database change and api update (for autocompletion) !34875 (merged) - Done
- the frontend MR to refactor/start consolidating EE and CE: !37499 (merged) - Done
- the main frontend MR: !35638 (merged) - Done
- first support backend MR: !47088 (merged) - Done
- second support backend MR: !47305 (merged) - Done
- third support backend MR: !47181 (merged) - Done
- fourth support backend MR: !48226 (merged) - Done
- the main backend MR: !36345 (merged) - Done
- documentation: !42234 (merged) - Done
- backend long-term fix after regression: !49723 (merged) - Done
- FF rollout issue: #247866 (closed)