Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the editor role #738

Merged
merged 7 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions conda-store-server/conda_store_server/api.py
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
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,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 @@ -108,6 +110,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
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
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
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
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo fix: "Lets" -> "Let's"

the default configuration of conda-store.

> Suppose we have an unauthenticated user trying to view the
Expand Down
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
Loading