protected branch, allow user, other users can push too

Summary

Setup to protect branch, pushing allowed by No one + one user

Steps to reproduce

see above The error occurs as well, if we don't specify a role but only the user

Example Project

https://gitlab.com/robstoll/test-move-multiple-issues

branch test is protected with role no-one and user robstoll

What is the current bug behavior?

Everyone can push

What is the expected correct behavior?

No one can push but the expected user

Possible fixes

(If you can, link to the line of code that might be responsible for the problem)

mentioned in issue gitlab-org/quality/triage-reports#288 (closed)

@robstoll - Are you able to recreate this on GitLab.com? I was not able to but may not be understanding the steps to recreate.

The information submitted in the Bug Template is not clear on the environment you are encountering this on. Based on this I will be closing this issue. Please fill out more of the ~bug template and reopen the issue and I will take another look

closed

added groupsource code severity3 typebug labels

changed the description

@kwiebers try it with https://gitlab.com/robstoll/test-move-multiple-issues, I have not tried to push (as I should be allowed) but I quickly checked if I can modify files in the corresponding branch with role no-one without assign me as exception and was able to do so which I shouldn't I guess as I am not allowed to push -- but maybe modifications via web are allowed intentional? Anyway... I hope this way you can try it out again.

Thanks for adding this detail to the issue description

@m_gill - would this ~bug fall under groupsource code? I didn't find any existing issues that were the same as this report but curious if you have seen this.

Thanks @kwiebers, this is groupsource code. We have looked into this before, or at least have seen some existing issues.

I'm thinking of #33632 (closed) and #207503 (closed). We actually have ~2 others that seem to spawn from confusing settings.

@robstoll would you provide a screenshot of your branch settings in that project, or add me to the project as an owner? (Or both!)

@m_gill I don't see how I could make you owner but you are a maintainer now (also @kwiebers)

reopened

added devopscreate label

mentioned in issue #233318 (closed)

mentioned in issue #235073 (closed)

added sectiondev label

mentioned in issue #237811 (closed)

mentioned in issue #240842 (closed)

mentioned in issue #243455 (closed)

marked this issue as related to #223746 (closed)

mentioned in issue #246446 (closed)

mentioned in issue #249048 (closed)

mentioned in issue #254149 (closed)

mentioned in issue #257775 (closed)

added priority3 label

changed milestone to %Backlog

added [deprecated] Accepting merge requests label

Setting label(s) Category:Source Code Management based on groupsource code.

added Category:Source Code Management label

Hi everyone , so a costumer was experiencing the same bug (on version 12.6.8) and I found this issue. I tried to reproduce this in GitLab.com but I couldn't, so I went a little bit deeper and found this issue: #35097 (closed) that seems to have fixed this bug on version 13.5. Is that accurate? cc: @m_gill @kwiebers

@marc_shaw can you speak to this? This was during a weird transition period where this issue belong to groupsource code but had a groupcode review engineer working on it because of the splitting of the 2 groups.

cc @mnohr

No worries! I will get to this today! Sorry for the slow reply @b_freitas

Thank you @m_gill and @marc_shaw for your collaboration. Just let me know when you have an update so that I can get back to the customer. Cheers

@m_gill @b_freitas

What it looks like it happening in this issue that Admins can push, regardless of whether they are on the list of allowed users or not (https://gitlab.com/gitlab-org/gitlab/-/blob/67a52000c9215598ef947de25e7e8132371bb822/ee/app/models/concerns/ee/protected_ref_access.rb#L69). However, when only No One is selected, this means No One, not even admins can (https://gitlab.com/gitlab-org/gitlab/-/blob/67a52000c9215598ef947de25e7e8132371bb822/app/models/concerns/protected_branch_access.rb#L14). This doesn't seem to be clear from the documentation (https://docs.gitlab.com/ee/user/project/protected_branches.html#protected-branches)

WDYT @m_gill ? Maybe I missed it, if not i will create an MR to update the docs.

@b_freitas depending on how the customer has their protected branches setup - it may be that this is what is happening, or it may be solved by the issue you linked above.

Thanks @marc_shaw. I think this is best left to @sean_carroll to answer now in terms of what is expected or needs documented.

Hi @marc_shaw,

I think the customer is experiencing something slightly different:

If in the configuration of protected branches we have selected a user in "Allowed to push", when we try to commit a change, we can commit even though we are not the user that we have registered.

So, when they select Role="No one" + User:"User" (see this comment) everyone can push to the protected branch, regardless of being an Admin or not. I wasn't able to reproduce it myself, but that is what they are experiencing.

They are on version 12.6.8, so my guess is that this problem has been fixed in more recent versions. I just wanted to confirm that.

Ahoy hoy @b_freitas

Yes, that is a bit strange, I can not reproduce this either, so I think you're most likely correct that it has been fixed since

Thanks a lot @marc_shaw, I'll suggest upgrading to at least version 13.5

We are on the latest version and it still happens

@robstoll Is the person trying to push an admin? See the comment here: #231333 (comment 596550705)

Sorry, I missed that comment. Yes in the last particular case it was a user with admin rights with ssh connection. Any chance that an admin can create an api token with less power? It's not an option for us to create a second account for each admin as the pricing model went up quite a bit

Another customer is affected by this bug(users with admin permissions can perform push operations): ZD(Internal)

added customer label

added vintage label

mentioned in issue #383530 (closed)

added SLOMissed label