opendevstack / ods-provisioning-app

Provisioning app, which triggers project and component provisions (including Jira / Confluence / BitBucket and OCP resource creation)
Apache License 2.0
15 stars 20 forks source link

Project specific CD user not permissioned on provision of new component repository #670

Closed fkressmann closed 3 years ago

fkressmann commented 3 years ago

Finding together with @clemensutschig : When a new component is provisioned, the bitbucket technical user is still set with write permissions to that repository here: https://github.com/opendevstack/ods-provisioning-app/blob/27d561e2f6b82cfae75c34398b3d08f257296e74/src/main/java/org/opendevstack/provision/services/BitbucketAdapter.java#L660

This allows access to the repos with the global cd_user even if a project specific user is set and thus is a security issue.

Maybe we chan do it somehow like below to use the project specific cdUser instead of the global one: https://github.com/opendevstack/ods-provisioning-app/blob/27d561e2f6b82cfae75c34398b3d08f257296e74/src/main/java/org/opendevstack/provision/services/BitbucketAdapter.java#L254-L274

segator commented 3 years ago

My suggestion is to make this configurable. for our use case we already have the specific tecnhical account in the group we already setting permissions. so is not even needed to set the user only. but for other uses cases could be the case no groups are used or technical account is not member of the groups.

So a parameter to define if we want to set the technical account and then if in the project creation a specific account is provided then we set this one instead the default one that comes from the configuration.

stitakis commented 3 years ago

@clemensutschig @michaelsauter I'm looking at this

clemensutschig commented 3 years ago

@maksymmaksym fyi

stitakis commented 3 years ago

@clemensutschig @michaelsauter @maksymmaksym @fkressmann The cd_user (whether global or project specific) only needs read permission to the component repository. This is needed for the ODS CI pipelines to be able to check out the code. It looks like the permission was mistakenly reduced from admin to write instead to read last time this was touched. I'll prepare a fix this and come back to you.

michaelsauter commented 3 years ago

@stitakis I wish it were so, but the pipeline also commits in the orchestration pipeline, and when provisioning quickstarters, it also pushes code into the created repo. We need write permissions, but the technical user of the prov app (the "global cd_user") should not be added to the provisioned repo.

stitakis commented 3 years ago

@michaelsauter ok, we keep write permission. The bug is in this line: https://github.com/opendevstack/ods-provisioning-app/blob/27d561e2f6b82cfae75c34398b3d08f257296e74/src/main/java/org/opendevstack/provision/services/BitbucketAdapter.java#L660

is using the technicalUser which in this content holds as value the default cd_user (global). Instead it should has a value the project specific cd_user. I'll create a fix for this.

fkressmann commented 3 years ago

@stitakis Thats what I intended initially 👍 But one further question: Does the project specific user even need write permissions? Maybe for initially committing to it but afterwards, this permission should not be necessary anymore, right?

stitakis commented 3 years ago

@fkressmann for initial commit the technical account setup in provision app is used. Please see michaels comment above regarding why write permission is needed.

segator commented 3 years ago

but the user we use for project/QS creation is no the same. as far as I know the user that logins to prov-app is the user used for creations(that btw is something should be changed) because meaning we keep the password in plain in memory.

stitakis commented 3 years ago

@fkressmann @segator sorry, my bad... to correct: the initial commit is done by the cd_user (global or specific). The prov app creates the repository in advance.

stitakis commented 3 years ago

@fkressmann almost done with the fix... I'll change the title of this issue to reflect better the bug

stitakis commented 3 years ago

@fkressmann I couldn't add you to the reviewers list... but anyway follow the link to the PR #671 and you will see the changes

stitakis commented 3 years ago

@fkressmann @segator please note that for already existing projects the stored value of cdUser might be null. For this projects you will need to add manually the project cd user.