Skip to content

Commit

Permalink
ENH - Add support for the editor role (#738)
Browse files Browse the repository at this point in the history
  • Loading branch information
Nikita Karetnikov authored Feb 13, 2024
1 parent f3b161d commit d7d701d
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 90 deletions.
15 changes: 12 additions & 3 deletions conda-store-server/conda_store_server/api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,18 @@ def update_namespace_role(
raise ValueError(f"Namespace='{other}' not found")

nrm = orm.NamespaceRoleMappingV2
db.query(nrm).filter(nrm.namespace_id == namespace.id).filter(
nrm.other_namespace_id == other_namespace.id
).update({"role": role})
q = (
db.query(nrm)
.filter(nrm.namespace_id == namespace.id)
.filter(nrm.other_namespace_id == other_namespace.id)
.first()
)
# Important: this modifies a field of a table entry and calls 'db.add'
# instead of using '.update({"role": role})' on the query because the latter
# would bypass the ORM validation logic, which maps the 'editor' role to
# 'developer'
q.role = role
db.add(q)


# v2 API
Expand Down
4 changes: 4 additions & 0 deletions conda-store-server/conda_store_server/orm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ def validate_entity(self, key, entity):

@validates("role")
def validate_role(self, key, role):
if role == "editor":
role = "developer" # alias
if role not in ["admin", "viewer", "developer"]:
raise ValueError(f"invalid entity={role}")

Expand All @@ -121,6 +123,8 @@ class NamespaceRoleMappingV2(Base):

@validates("role")
def validate_role(self, key, role):
if role == "editor":
role = "developer" # alias
if role not in ["admin", "viewer", "developer"]:
raise ValueError(f"invalid role={role}")
return role
Expand Down
91 changes: 56 additions & 35 deletions conda-store-server/conda_store_server/server/auth.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,42 +138,46 @@ def _check_role_mappings_version(self, proposal):
)
return proposal.value

_viewer_permissions = {
schema.Permissions.ENVIRONMENT_READ,
schema.Permissions.NAMESPACE_READ,
schema.Permissions.NAMESPACE_ROLE_MAPPING_READ,
}
_editor_permissions = {
schema.Permissions.BUILD_CANCEL,
schema.Permissions.ENVIRONMENT_CREATE,
schema.Permissions.ENVIRONMENT_READ,
schema.Permissions.ENVIRONMENT_UPDATE,
schema.Permissions.ENVIRONMENT_SOLVE,
schema.Permissions.NAMESPACE_READ,
schema.Permissions.NAMESPACE_ROLE_MAPPING_READ,
schema.Permissions.SETTING_READ,
}
_admin_permissions = {
schema.Permissions.BUILD_DELETE,
schema.Permissions.BUILD_CANCEL,
schema.Permissions.ENVIRONMENT_CREATE,
schema.Permissions.ENVIRONMENT_DELETE,
schema.Permissions.ENVIRONMENT_READ,
schema.Permissions.ENVIRONMENT_UPDATE,
schema.Permissions.ENVIRONMENT_SOLVE,
schema.Permissions.NAMESPACE_CREATE,
schema.Permissions.NAMESPACE_DELETE,
schema.Permissions.NAMESPACE_READ,
schema.Permissions.NAMESPACE_UPDATE,
schema.Permissions.NAMESPACE_ROLE_MAPPING_CREATE,
schema.Permissions.NAMESPACE_ROLE_MAPPING_READ,
schema.Permissions.NAMESPACE_ROLE_MAPPING_UPDATE,
schema.Permissions.NAMESPACE_ROLE_MAPPING_DELETE,
schema.Permissions.SETTING_READ,
schema.Permissions.SETTING_UPDATE,
}

role_mappings = Dict(
{
"viewer": {
schema.Permissions.ENVIRONMENT_READ,
schema.Permissions.NAMESPACE_READ,
schema.Permissions.NAMESPACE_ROLE_MAPPING_READ,
},
"developer": {
schema.Permissions.BUILD_CANCEL,
schema.Permissions.ENVIRONMENT_CREATE,
schema.Permissions.ENVIRONMENT_READ,
schema.Permissions.ENVIRONMENT_UPDATE,
schema.Permissions.ENVIRONMENT_SOLVE,
schema.Permissions.NAMESPACE_READ,
schema.Permissions.NAMESPACE_ROLE_MAPPING_READ,
schema.Permissions.SETTING_READ,
},
"admin": {
schema.Permissions.BUILD_DELETE,
schema.Permissions.BUILD_CANCEL,
schema.Permissions.ENVIRONMENT_CREATE,
schema.Permissions.ENVIRONMENT_DELETE,
schema.Permissions.ENVIRONMENT_READ,
schema.Permissions.ENVIRONMENT_UPDATE,
schema.Permissions.ENVIRONMENT_SOLVE,
schema.Permissions.NAMESPACE_CREATE,
schema.Permissions.NAMESPACE_DELETE,
schema.Permissions.NAMESPACE_READ,
schema.Permissions.NAMESPACE_UPDATE,
schema.Permissions.NAMESPACE_ROLE_MAPPING_CREATE,
schema.Permissions.NAMESPACE_ROLE_MAPPING_READ,
schema.Permissions.NAMESPACE_ROLE_MAPPING_UPDATE,
schema.Permissions.NAMESPACE_ROLE_MAPPING_DELETE,
schema.Permissions.SETTING_READ,
schema.Permissions.SETTING_UPDATE,
},
"viewer": _viewer_permissions,
"editor": _editor_permissions,
"admin": _admin_permissions,
},
help="default role to permissions mapping to use",
config=True,
Expand Down Expand Up @@ -271,7 +275,24 @@ def get_entity_bindings(self, entity: schema.AuthenticationToken):
def convert_roles_to_permissions(self, roles):
permissions = set()
for role in roles:
permissions = permissions | self.role_mappings[role]
# 'editor' is the new alias of 'developer'. The new name is
# preferred in user-visible settings (like 'role_mappings') and when
# calling the role-mappings HTTP APIs, but it's ALWAYS mapped to
# 'developer' in the database for compatibility reasons.
# Additionally, this code allows for legacy 'role_mappings' that
# used to specify the role as 'developer'. Because it's a
# user-visible setting, we cannot break compatibility here
if role == "editor":
raise ValueError("role must never be 'editor' in the database")
if role == "developer":
# Checks the new user-visible name first, then tries the legacy
# one. This will raise an exception if both keys are not found
role_mappings = (
self.role_mappings.get("editor") or self.role_mappings["developer"]
)
else:
role_mappings = self.role_mappings[role]
permissions = permissions | role_mappings
return permissions

def get_entity_binding_permissions(self, entity: schema.AuthenticationToken):
Expand Down
100 changes: 60 additions & 40 deletions conda-store-server/tests/test_auth.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,53 @@ def test_authorization(conda_store, entity_bindings, arn, permissions, authorize
assert authorized == authorization.authorize(entity, arn, permissions)


_viewer_permissions = {
Permissions.ENVIRONMENT_READ,
Permissions.NAMESPACE_READ,
Permissions.NAMESPACE_ROLE_MAPPING_READ,
}
_editor_permissions = {
Permissions.BUILD_CANCEL,
Permissions.ENVIRONMENT_CREATE,
Permissions.ENVIRONMENT_READ,
Permissions.ENVIRONMENT_UPDATE,
Permissions.ENVIRONMENT_SOLVE,
Permissions.NAMESPACE_READ,
Permissions.NAMESPACE_ROLE_MAPPING_READ,
Permissions.SETTING_READ,
}
_admin_permissions = {
Permissions.BUILD_DELETE,
Permissions.BUILD_CANCEL,
Permissions.ENVIRONMENT_CREATE,
Permissions.ENVIRONMENT_DELETE,
Permissions.ENVIRONMENT_READ,
Permissions.ENVIRONMENT_UPDATE,
Permissions.ENVIRONMENT_SOLVE,
Permissions.NAMESPACE_CREATE,
Permissions.NAMESPACE_DELETE,
Permissions.NAMESPACE_READ,
Permissions.NAMESPACE_UPDATE,
Permissions.NAMESPACE_ROLE_MAPPING_CREATE,
Permissions.NAMESPACE_ROLE_MAPPING_READ,
Permissions.NAMESPACE_ROLE_MAPPING_UPDATE,
Permissions.NAMESPACE_ROLE_MAPPING_DELETE,
Permissions.SETTING_READ,
Permissions.SETTING_UPDATE,
}


@pytest.mark.parametrize(
"role, permissions",
[
("viewer", _viewer_permissions),
("developer", _editor_permissions),
("editor", _editor_permissions),
("admin", _admin_permissions),
],
)
@pytest.mark.parametrize("role_mappings_version", [1, 2])
def test_end_to_end_auth_flow_v1(conda_store_server, testclient, authenticate, role_mappings_version):
def test_end_to_end_auth_flow_v1(conda_store_server, testclient, authenticate, role_mappings_version, role, permissions):
# Configures authentication
namespace = f"this-{uuid.uuid4()}"
other_namespace = f"other-{uuid.uuid4()}"
Expand Down Expand Up @@ -173,24 +218,7 @@ def authorize():
role_bindings=token_model.role_bindings,
),
f"{other_namespace}/example-name",
{
Permissions.BUILD_DELETE,
Permissions.ENVIRONMENT_CREATE,
Permissions.ENVIRONMENT_DELETE,
Permissions.ENVIRONMENT_READ,
Permissions.ENVIRONMENT_UPDATE,
Permissions.ENVIRONMENT_SOLVE,
Permissions.NAMESPACE_CREATE,
Permissions.NAMESPACE_DELETE,
Permissions.NAMESPACE_READ,
Permissions.NAMESPACE_UPDATE,
Permissions.NAMESPACE_ROLE_MAPPING_CREATE,
Permissions.NAMESPACE_ROLE_MAPPING_READ,
Permissions.NAMESPACE_ROLE_MAPPING_UPDATE,
Permissions.NAMESPACE_ROLE_MAPPING_DELETE,
Permissions.SETTING_READ,
Permissions.SETTING_UPDATE,
},
permissions,
)
# No default roles
assert authorize() is False
Expand All @@ -213,7 +241,7 @@ def authorize():
response = testclient.put(
f"api/v1/namespace/{namespace}",
json={"role_mappings": {
f"{other_namespace}/ex*-name": ["admin"],
f"{other_namespace}/ex*-name": [role],
}}
)
response.raise_for_status()
Expand All @@ -235,8 +263,17 @@ def authorize():
assert authorize() is False


@pytest.mark.parametrize(
"role, permissions",
[
("viewer", _viewer_permissions),
("developer", _editor_permissions),
("editor", _editor_permissions),
("admin", _admin_permissions),
],
)
@pytest.mark.parametrize("role_mappings_version", [1, 2])
def test_end_to_end_auth_flow_v2(conda_store_server, testclient, authenticate, role_mappings_version):
def test_end_to_end_auth_flow_v2(conda_store_server, testclient, authenticate, role_mappings_version, role, permissions):
# Configures authentication
namespace = f"this-{uuid.uuid4()}"
other_namespace = f"other-{uuid.uuid4()}"
Expand Down Expand Up @@ -267,24 +304,7 @@ def authorize():
role_bindings=token_model.role_bindings,
),
f"{other_namespace}/example-name",
{
Permissions.BUILD_DELETE,
Permissions.ENVIRONMENT_CREATE,
Permissions.ENVIRONMENT_DELETE,
Permissions.ENVIRONMENT_READ,
Permissions.ENVIRONMENT_UPDATE,
Permissions.ENVIRONMENT_SOLVE,
Permissions.NAMESPACE_CREATE,
Permissions.NAMESPACE_DELETE,
Permissions.NAMESPACE_READ,
Permissions.NAMESPACE_UPDATE,
Permissions.NAMESPACE_ROLE_MAPPING_CREATE,
Permissions.NAMESPACE_ROLE_MAPPING_READ,
Permissions.NAMESPACE_ROLE_MAPPING_UPDATE,
Permissions.NAMESPACE_ROLE_MAPPING_DELETE,
Permissions.SETTING_READ,
Permissions.SETTING_UPDATE,
},
permissions,
)
# No default roles
assert authorize() is False
Expand All @@ -307,7 +327,7 @@ def authorize():
f"api/v1/namespace/{other_namespace}/role",
json={
"other_namespace": namespace,
"role": "admin"
"role": role
},
)
response.raise_for_status()
Expand Down
15 changes: 11 additions & 4 deletions conda-store-server/tests/test_db_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,14 @@ def test_namespace_role_mapping(db):
NamespaceRoleMapping(namespace=namespace, namespace_id=namespace.id, entity="*/*")


def test_namespace_role_mapping_v2(db):
@pytest.mark.parametrize(
"editor_role",
[
"editor",
"developer",
],
)
def test_namespace_role_mapping_v2(db, editor_role):
namespace_name = "pytest-namespace"
other_namespace_name1 = "pytest-other-namespace1"
other_namespace_name2 = "pytest-other-namespace2"
Expand Down Expand Up @@ -109,11 +116,11 @@ def test_namespace_role_mapping_v2(db):
r"namespace_role_mapping_v2.other_namespace_id")):
# Runs in a nested transaction since a constraint violation will cause a rollback
with db.begin_nested():
api.create_namespace_role(db, name=namespace_name, other=other_namespace_name2, role="developer")
api.create_namespace_role(db, name=namespace_name, other=other_namespace_name2, role=editor_role)
db.commit()

# Updates a role mapping
api.update_namespace_role(db, name=namespace_name, other=other_namespace_name2, role="developer")
api.update_namespace_role(db, name=namespace_name, other=other_namespace_name2, role=editor_role)
db.commit()

# Gets all role mappings
Expand All @@ -129,7 +136,7 @@ def test_namespace_role_mapping_v2(db):
assert roles[1].id == 2
assert roles[1].namespace == namespace_name
assert roles[1].other_namespace == other_namespace_name2
assert roles[1].role == 'developer'
assert roles[1].role == 'developer' # always developer in the DB

assert roles[2].id == 3
assert roles[2].namespace == namespace_name
Expand Down
7 changes: 5 additions & 2 deletions docusaurus-docs/conda-store/references/auth.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ c.RBACAuthorizationBackend.role_mappings = {
"viewer": {
Permissions.ENVIRONMENT_READ
},
"developer": {
"editor": {
Permissions.ENVIRONMENT_CREATE,
Permissions.ENVIRONMENT_READ,
Permissions.ENVIRONMENT_UPDATE,
Expand All @@ -101,7 +101,10 @@ c.RBACAuthorizationBackend.role_mappings = {
}
```

Lets go through a few examples to make this more concrete and assume
Additionally, the role `developer` is supported, which is a legacy alias of
`editor`. The name `editor` is preferred.

Let's go through a few examples to make this more concrete and assume
the default configuration of conda-store.

> Suppose we have an unauthenticated user trying to view the
Expand Down
3 changes: 2 additions & 1 deletion docusaurus-docs/conda-store/references/configuration-options.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,8 @@ the `schema.AuthenticaitonToken` all fields are optional.

`AuthorizationBackend.role_mappings` is a dictionary that maps `roles`
to application `permissions`. There are three default roles at the
moment `viewer`, `developer`, and `admin`.
moment `viewer`, `editor`, and `admin`. Additionally, the role `developer` is
supported, which is a legacy alias of `editor`. The name `editor` is preferred.

`AuthorizationBackend.unauthenticated_role_bindings` are the role
bindings that an unauthenticated user assumes.
Expand Down
Loading

0 comments on commit d7d701d

Please sign in to comment.