From f3f7f09b442291de94cfcd6998522be3f3394f24 Mon Sep 17 00:00:00 2001 From: Joseph Kleinhenz Date: Tue, 2 Apr 2024 07:29:45 -0700 Subject: [PATCH] on startup only add admin role if not already present (#705) * on startup only insert admin role if not already present * flyby: specify fallback version so install doesn't fail if .git isn't present this can happen when using git worktrees or tarballs * flyby: don't hide error if correlation ID is not present in response This can happen if for example the k8s ingress rejects the requests with 413 content too large * Test multiple server starts with tiled_admins. * CI: Ensure docker-compose is installed * Adjust for backward-incompatible change in Docker 7 --------- Co-authored-by: Joseph Kleinhenz Co-authored-by: Dan Allan --- continuous_integration/scripts/start_LDAP.sh | 2 +- pyproject.toml | 1 + tiled/_tests/test_authentication.py | 7 +++++++ tiled/authn_database/core.py | 6 ++++++ tiled/client/utils.py | 11 ++++++++--- 5 files changed, 23 insertions(+), 4 deletions(-) diff --git a/continuous_integration/scripts/start_LDAP.sh b/continuous_integration/scripts/start_LDAP.sh index 109327777..bf27477ca 100755 --- a/continuous_integration/scripts/start_LDAP.sh +++ b/continuous_integration/scripts/start_LDAP.sh @@ -3,5 +3,5 @@ set -e # Start LDAP server in docker container docker pull bitnami/openldap:latest -docker-compose -f continuous_integration/docker-configs/ldap-docker-compose.yml up -d +docker compose -f continuous_integration/docker-configs/ldap-docker-compose.yml up -d docker ps diff --git a/pyproject.toml b/pyproject.toml index efe78dd90..6430aa4d7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -300,6 +300,7 @@ Documentation = "https://blueskyproject.io/tiled" [tool.hatch] version.source = "vcs" +version.fallback-version = "0.0.0" build.hooks.vcs.version-file = "tiled/_version.py" # Defining this (empty) section invokes hatch_build.py diff --git a/tiled/_tests/test_authentication.py b/tiled/_tests/test_authentication.py index 934edcf89..8d7a61d1f 100644 --- a/tiled/_tests/test_authentication.py +++ b/tiled/_tests/test_authentication.py @@ -312,6 +312,13 @@ def test_admin(enter_password, config): with fail_with_status_code(HTTP_401_UNAUTHORIZED): context.admin.show_principal(some_principal_uuid) + # Start the server a second time. Now alice is already an admin. + with Context.from_app(build_app_from_config(config)) as context: + with enter_password("secret1"): + context.authenticate(username="alice") + admin_roles = context.whoami()["roles"] + assert "admin" in [role["name"] for role in admin_roles] + def test_api_key_activity(enter_password, config): """ diff --git a/tiled/authn_database/core.py b/tiled/authn_database/core.py index 9cfecab16..c03d87b58 100644 --- a/tiled/authn_database/core.py +++ b/tiled/authn_database/core.py @@ -210,6 +210,12 @@ async def make_admin_by_identity(db, identity_provider, id): principal = await create_user(db, identity_provider, id) else: principal = identity.principal + + # check if principal already has admin role + for role in principal.roles: + if role.name == "admin": + return principal + admin_role = (await db.execute(select(Role).filter(Role.name == "admin"))).scalar() assert admin_role is not None, "Admin role is missing from Roles table" principal.roles.append(admin_role) diff --git a/tiled/client/utils.py b/tiled/client/utils.py index 6b72ddf2f..2557ac6cf 100644 --- a/tiled/client/utils.py +++ b/tiled/client/utils.py @@ -31,18 +31,21 @@ def raise_for_status(response) -> None: if response.is_success: return response + # correlation ID may be missing if request didn't make it to the server + correlation_id = response.headers.get("x-tiled-request-id", None) + if response.has_redirect_location: message = ( "{error_type} '{0.status_code} {0.reason_phrase}' for url '{0.url}'\n" "Redirect location: '{0.headers[location]}'\n" "For more information, server admin can search server logs for " - "correlation ID {0.headers[x-tiled-request-id]}." + "correlation ID {correlation_id}." ) else: message = ( "{error_type} '{0.status_code} {0.reason_phrase}' for url '{0.url}'\n" "For more information, server admin can search server logs for " - "correlation ID {0.headers[x-tiled-request-id]}." + "correlation ID {correlation_id}." ) status_class = response.status_code // 100 @@ -53,7 +56,9 @@ def raise_for_status(response) -> None: 5: "Server error", } error_type = error_types.get(status_class, "Invalid status code") - message = message.format(response, error_type=error_type) + message = message.format( + response, error_type=error_type, correlation_id=correlation_id + ) raise httpx.HTTPStatusError(message, request=request, response=response)