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

Fix problem assigning admin permissions to bitbucket repositories #700

Closed jafarre-bi closed 2 years ago

jafarre-bi commented 2 years ago

Fix admin permission assignment for new coponents

jafarre-bi commented 2 years ago

The problem is that, when a new repository is created, we are assigning admin permissions to a generic user group. This assignment has to be removed, but we need to validate that the resulting set of permissions is appropriate. @metmajer @clemensutschig this is not a technical decision and we would need you to validate the assigned permissions. Can you please review it? The list of assigned permissions follows.

Currently assigned permissions when the special permission set is specified on project creation:

Type Who? Level Permission
Group ${global.keyuser.role.name} project ADMIN
Group project.projectAdminGroup project ADMIN
Group project.projectUserGroup project WRITE
Group project.projectReadonlyGroup project READ
User project.cdUser (default: ${bitbucket.technical.user}) project WRITE
Group ${bitbucket.default.user.group} repo ADMIN
User ${bitbucket.technical.user} repo WRITE

Currently assigned permissions when the special permission set is not specified on project creation:

Type Who? Level Permission
Group ${bitbucket.default.user.group} project WRITE
Group ${idmanager.group.opendevstack-users} project READ
User project.cdUser (default: ${bitbucket.technical.user}) project WRITE
Group ${bitbucket.default.user.group} repo ADMIN
User ${bitbucket.technical.user} repo WRITE

Additionally, whenever a specific CD User is specified on project creation, this user gets read permissions in all repositories in scmGlobalProperties.readableRepos (such as opendevstack/ods-jenkins-shared-library and opendevstack/ods-quickstarters).

The highlighted row is the one detected as giving admin permissions to a generic user group and has to be removed. If we do that, in case the special permission set is not specified, then the respository would not get any specific admin group or user. The only administrators would be the global bitbucket administrators. Is this fine?

jafarre-bi commented 2 years ago

Removed the admin permissions at repo level. These are the resuting permissions, that need to be validated:

Assigned permissions after the fix, when the special permission set is specified on project creation:

Type Who? Level Permission
Group ${global.keyuser.role.name} project ADMIN
Group project.projectAdminGroup project ADMIN
Group project.projectUserGroup project WRITE
Group project.projectReadonlyGroup project READ
User project.cdUser (default: ${bitbucket.technical.user}) project WRITE
User ${bitbucket.technical.user} repo WRITE

Assigned permissions after the fix, when the special permission set is not specified on project creation:

Type Who? Level Permission
Group ${bitbucket.default.user.group} project WRITE
Group ${idmanager.group.opendevstack-users} project READ
User project.cdUser (default: ${bitbucket.technical.user}) project WRITE
User ${bitbucket.technical.user} repo WRITE

Additionally, whenever a specific CD User is specified on project creation, this user gets read permissions in all repositories in scmGlobalProperties.readableRepos (such as opendevstack/ods-jenkins-shared-library and opendevstack/ods-quickstarters).

jafarre-bi commented 2 years ago

After discussion, the decissions taken are:

The final permission tables follow.

Assigned permissions after the fix, when the special permission set is specified on project creation:

Type Who? Level Permission
Group ${global.keyuser.role.name} project ADMIN
Group project.projectAdminGroup project ADMIN
Group project.projectUserGroup project WRITE
Group project.projectReadonlyGroup project READ
User project.cdUser (default: ${bitbucket.technical.user}) project WRITE
User ${bitbucket.technical.user} repo WRITE

Assigned permissions after the fix, when the special permission set is not specified on project creation:

Type Who? Level Permission
Group ${bitbucket.default.user.group} project WRITE
Group ${idmanager.group.opendevstack-users} project READ
User project.cdUser (default: ${bitbucket.technical.user}) project WRITE
Group ${bitbucket.default.admin.group} (default: ${bitbucket.default.user.group}) repo ADMIN
User ${bitbucket.technical.user} repo WRITE
stitakis commented 2 years ago

@jafarre-viewnext happy to review your changes once you have implemented the rules specified in your last comment

jafarre-bi commented 2 years ago

@jafarre-viewnext happy to review your changes once you have implemented the rules specified in your last comment

Thank you, @stitakis ! I will set the PR as ready for review as soon as I have properly tested the changes.

jafarre-bi commented 2 years ago

Some notes about the implementation:

The new property has been added to the application.properties as follows: bitbucket.default.admin.group=${bitbucket.default.user.group} The code just honors this property value. This is enough to maintain backwards compatibility when the new property is not present in the config map. However, this also allows users to disable the admin group by specifying the following in the config map: bitbucket.default.admin.group=

There was a problem when assigning permissions on project creation. If the same group was specified as having, for instance, write and admin permissions, The admin permissions were overwritten by the write permissions. This was actually happenning with the default user group, but the problem was hidden, because this group was getting admin permissions at repo level. It has been fixed by reversing the order in which permissions are assigned.

jafarre-bi commented 2 years ago

@stitakis ready for review. Thanks in advance!

jafarre-bi commented 2 years ago

@jafarre-viewnext I think is time to document the implemented rules. Proper documentation for this is missing and causes recurring queries on how the provision app behaves. Can you please extends https://github.com/opendevstack/ods-provisioning-app/blob/master/docs/modules/provisioning-app/pages/index.adoc to capture the new implemented rules?

Done