Skip to content

716 dissociate the encryption key from the api keys of the master password#779

Merged
benjaminpilia merged 40 commits intomainfrom
716-dissociate-the-encryption-key-from-the-api-keys-of-the-master-password
Mar 25, 2026
Merged

716 dissociate the encryption key from the api keys of the master password#779
benjaminpilia merged 40 commits intomainfrom
716-dissociate-the-encryption-key-from-the-api-keys-of-the-master-password

Conversation

@tibo-pdn
Copy link
Copy Markdown
Contributor

@tibo-pdn tibo-pdn commented Mar 11, 2026

Description

Closes #716.

This PR dissociates the encryption key from the admin password, removes the hardcoded master user bypass, and replaces the old master-user bootstrap system with a clean-architecture BootstrapAdminUseCase that runs at startup. Also, this PR allow the default admin user to be persisted in database.


Overview

Area

  • Core / Global project settings
  • API
  • Playground / Web UI
  • DevOps
  • Docusaurus / Documentation
  • Other (specify below)

Type of change

  • New feature
  • Bugfix
  • Enhancement (improvement of an existing feature)
  • Refactor (change that neither fixes a bug nor modifies behavior)
  • Documentation
  • Tests
  • Performance improvement
  • Chore / Maintenance (change to the build process or auxiliary tools, dependencies update, etc.)

Definition of Done / Technical changes

1. New auth_secret_key setting — encryption is now independent

Before: auth_master_key served double duty: it was both the admin password and the JWT signing secret for all API keys. Rotating the password invalidated every API key in the system.

After: A new dedicated auth_secret_key setting signs all API key JWTs. auth_master_key is now deprecated and only kept as a fallback for backward compatibility.

# config.yml
settings:
  auth_master_key: "changeme"                    # [DEPRECATED] — no longer used for encryption
  auth_secret_key: "a-different-32-char-secret"  # NEW — signs all API key JWTs
  auth_default_username: "admin"                 # NEW — username/email of the first admin
  auth_default_password: "changeme"              # NEW — password of the first admin

Fallback behaviour (backward compat): if auth_secret_key is not set, the system falls back to auth_master_key and emits three WARNING log lines pointing to the upcoming v1.0.0 removal:

WARNING: The 'auth_secret_key' setting is not set. Falling back to 'auth_master_key' for signing API keys.
WARNING: Please set 'auth_secret_key' to a dedicated secret value in your 'config.yaml' file.
WARNING: The 'auth_master_key' fallback will be removed in v1.0.0, where 'auth_secret_key' will become mandatory...

In IdentityAccessManager, the constructor parameter was renamed from master_key to secret_key. The dependency injection helper was also renamed from get_master_key() to get_secret_key() for clarity purposes.

The SessionMiddleware (cookie encoding/decoding) was also updated to use auth_secret_key instead of auth_master_key.

Config files updated accordingly: config.yml, config.example.yml, and config.test.yml (also removes a duplicate socket_keepalive entry in config.test.yml).

  • auth_secret_key setting added to Settings
  • auth_default_username and auth_default_password settings added to Settings
  • auth_master_key marked as deprecated in field description
  • Fallback validator added: if auth_secret_key is None, it copies auth_master_key with warnings
  • IdentityAccessManager uses secret_key instead of master_key; class properties now typed
  • get_master_key() dependency renamed to get_secret_key()
  • SessionMiddleware updated to use auth_secret_key
  • session_secret_key setting removed
  • constr() (deprecated Pydantic function) replaced with Annotated[str, StringConstraints(...)] syntax throughout configuration.py
  • EmptyDepencency typo fixed to EmptyDependency
  • config.yml, config.example.yml, config.test.yml updated
  • Duplicate socket_keepalive removed from config.test.yml

2. Master user bypass removed — auth_master_key is no longer a valid API key

Before: Sending Authorization: Bearer <auth_master_key> in any request was a hardcoded bypass that granted every permission without a database lookup. This magic user had id=0, email="master", and was never stored in the database.

After: That bypass is completely gone. AccessController now always calls check_token(), which validates a JWT signed with auth_secret_key. Any raw string — including the old auth_master_key value — will fail JWT decoding and return InvalidAPIKeyException.

What was removed from AccessController._check_key():

# REMOVED — this entire block no longer exists
if api_key.credentials == global_context.identity_access_manager.master_key:
    user_info = UserInfo(id=0, email="master", ...)
    key_id = 0
    key_name = "master"

The check_token() return type was also improved from an untyped tuple[int | None, int | None, str | None] to a typed CheckTokenResult dataclass.

The reserved email guard (if email == "master": raise ReservedEmailException()) was also removed from create_user. No role, id, username or email is reserved anymore.

A duplicate TOKEN_PREFIX constant was removed from Key(BaseModel) in api/domain/key/entities.py — the canonical definition lives in IdentityAccessManager.TOKEN_PREFIX and a TODO was added to track the remaining duplicate in api/infrastructure/fastapi/access.py.

  • Master user bypass removed from AccessController
  • check_token() returns typed CheckTokenResult dataclass
  • ReservedEmailException and "master" email guard removed
  • ADMIN permission added as explicit bypass in _check_permissions (replaces the implicit master-user bypass)
  • Duplicate TOKEN_PREFIX removed from Key(BaseModel); TODO added for remaining duplicate in access.py

3. Bootstrap admin — proper first-run setup replaces the master user

Before: The system relied on a hardcoded user (id=0, email="master") that existed only in memory and not in the PostgreSQL database. There was no real first-run admin creation in the database.

After: At every startup, the lifespan calls bootstrap_default_admin(), which runs BootstrapAdminUseCase.execute(). The use case is idempotent: it checks has_admin_user() first and skips if an admin already exists while displaying an information non-blocking message (Admin user already exists, skipping default admin user creation.).

How it works:

Startup
  └─► bootstrap_default_admin(configuration)
        └─► BootstrapAdminUseCase.execute(command)
              ├─ has_admin_user()? → YES → return BootstrapAdminUseCaseSkipped (no-op)
              └─ NO
                   ├─ create_role(name, permissions=[ADMIN], limits=[])
                   │     ├─ conflict → return RoleAlreadyExistsError → raise RuntimeError
                   │     └─ ok → Role
                   └─ create_user(email, password, role_id)
                         ├─ conflict → return UserAlreadyExistsError → raise RuntimeError
                         └─ ok → return BootstrapAdminUseCaseSuccess

has_admin_user() is implemented in PostgresUserRepository as an SQL EXISTS query that joins users and permissions and checks for PermissionType.ADMIN. It returns True as soon as any user with an admin-permissioned role is found.

How to configure the first admin:

settings:
  auth_default_username: "admin"      # used as both name and email
  auth_default_password: "changeme"   # bcrypt-hashed on creation

On first run the role and user are created automatically. On all subsequent runs the use case skips silently. To use an email address as login, set auth_default_username: "admin@yourdomain.com". This new bootstrap feature does not affect any running OGL instance that already have an admin user.

  • BootstrapAdminUseCase + BootstrapAdminCommand created in api/use_cases/admin/
  • bootstrap_default_admin() added to lifespan, called at every startup
  • PostgresUserRepository.has_admin_user() implemented
  • PostgresRolesRepository.create_role() implemented (clean-arch)
  • PostgresUserRepository.create_user() implemented (clean-arch, returns error dataclasses instead of raising)

4. New domain errors and clean-architecture repositories

New domain error dataclasses (return values, not raised exceptions):

Class Location Meaning
RoleAlreadyExistsError(name) api/domain/role/errors.py Role name conflict on insert
RoleNotFoundError(name) api/domain/role/errors.py Role not found
UserAlreadyExistsError(email) api/domain/user/errors.py Email conflict on insert
RoleNotFoundError(role_id) api/domain/user/errors.py Role not found during user creation
OrganizationNotFoundError(organization_id) api/domain/user/errors.py Org not found during user creation

UserRepository abstract base was also upgraded from BaseModel to ABC with new abstract methods: has_admin_user, create_user, update_user, delete_user.

  • api/domain/role/errors.py created
  • api/domain/user/errors.py created
  • UserRepository base class rewritten as proper ABC
  • RoleRepository.create_role() abstract signature updated

5. POST /admin/roles and POST /admin/users migrated to clean architecture

Both endpoints moved from api/endpoints/admin/ to api/infrastructure/fastapi/endpoints/admin/, following the same pattern as the router and provider migrations. The old IAM-based implementations were removed.

  • POST /admin/roles migrated — now uses CreateRoleUseCase
  • POST /admin/users migrated — now uses CreateUserUseCase

6. Provider schema deduplication

api/schemas/admin/providers.py contained duplicate definitions that were also present in api/infrastructure/fastapi/schemas/providers.py. The old module was consolidated: duplicate classes were removed and all callsites were migrated to the new canonical location.

  • Duplicate content removed from api/schemas/admin/providers.py
  • All Provider class references migrated to api/infrastructure/fastapi/schemas/providers.py

Breaking changes

  • This PR contains breaking changes (see below)
What Before After Migration
auth_master_key as API key Accepted as a raw bearer token — full admin access Rejected — InvalidAPIKeyException Create a real API key via POST /admin/tokens using the bootstrapped admin credentials
IdentityAccessManager(master_key=...) Constructor accepted master_key param Param renamed to secret_key Update any direct instantiation to use secret_key=
Master user id=0 / email="master" Always existed in memory Does not exist Use the bootstrapped admin user
session_secret_key setting Optional session middleware key Removed Use auth_secret_key
get_master_key() dependency Available in api/dependencies.py Renamed to get_secret_key() Update import
EmptyDepencency class name api/schemas/core/configuration.py Renamed to EmptyDependency Update any direct reference

Quality assurance & Review readiness

Documentation

  • No documentation needed
  • README / Markdown files updated
  • API documentation updated (Swagger / Redoc)
  • Docstrings updated
  • Inline code comments added where needed

Inline documentation partially updated. No bootstrap workflow written yet.

Tests

  • No tests added
  • Unit tests added
  • Integration tests added
  • Functional tests added
  • End-to-end tests added
  • Performance tests added
  • Existing tests updated

Code Standards

  • Code follows project conventions and architecture
  • No unused imports, variables, functions, or classes
  • No debug logs or commented-out code left
  • No secrets or environment variables committed in clear text
  • Code is linted and formatted using the project tools (ruff, etc.)

Git & Process Standards

  • PR title follows Conventional Commits
  • PR is correctly labeled
  • PR is linked to relevant issue(s) / project(s)
  • Author is assigned to the PR
  • At least one reviewer has been requested

Deployment Notes

  • No special deployment steps required
  • Requires database migration (see "Database migration" section)
  • Requires new or updated environment variables (see below)
  • Requires other special deployment steps (see below)

New / updated config keys:

  • auth_secret_key (new, recommended) — dedicated JWT signing secret, independent from the admin password. If omitted, falls back to auth_master_key with deprecation warnings.
  • auth_default_username (new) — username and email of the admin created at first startup. Default: "admin".
  • auth_default_password (new) — password of the admin created at first startup. Default: "changeme".
  • auth_master_key — still accepted but deprecated; no longer used for encryption when auth_secret_key is set. Will be remove in v1.0.0.
  • session_secret_keyremoved.

Deployment steps for existing installations:

  • All previously issued API keys were signed with auth_master_key. If auth_secret_key is set to a different value, all existing API keys will be invalidated. To avoid this, set auth_secret_key to the same value as your current auth_master_key for a zero-downtime migration, then rotate auth_secret_key independently later.
  • The bootstrap admin creation is idempotent: if an admin user already exists in the DB, startup skips creation silently.
  • As auth_master_key does not exists anymore, any system/CI/CD/API that used to connect to OGL using this key won't work from now one. Please, once deployed, generate a new API key for each of these services.

Database migration

  • No database migration required

Reviewer Focus

  • api/helpers/_accesscontroller.py — master user bypass removal and ADMIN permission short-circuit
  • api/use_cases/admin/bootstrapadminusecase.py — idempotency logic and error handling branches
  • api/utils/lifespan.py — bootstrap call placement and error propagation at startup
  • api/schemas/core/configuration.py — fallback validator for auth_secret_key, constr()Annotated migration

Additional Notes

The auth_master_key fallback will be fully removed in v1.0.0, at which point auth_secret_key becomes mandatory. This is already documented in the deprecation warnings and in the milestone for this PR.
A complete guy will be written later.

🤖 Generated with Claude Code and reviewed with care by @tibo-pdn ❤️

Comment on lines +47 to +51
extra={
"user_id": request_context.get().user_id,
"role_name": body.name,
"error_type": type(e).__name__,
},

Check failure

Code scanning / CodeQL

Log Injection

This log entry depends on a [user-provided value](1).

Copilot Autofix

AI 14 days ago

In general, to fix log injection you should sanitize any user-controlled values before logging them, especially to remove or neutralize newline and carriage-return characters that can split or forge log entries. A simple and common approach for plain-text logs is to replace \r and \n with empty strings (or safe placeholders) before including the data in the log entry.

For this specific case, the minimal and safest fix is to sanitize body.name right where it is passed into the logging call. We can do this by creating a sanitized local variable (e.g., safe_role_name = body.name.replace(...)) that strips \r and \n, and then using safe_role_name in the extra dictionary instead of body.name. This keeps existing behavior intact for all valid role names while preventing an attacker from injecting log-breaking control characters. No changes to imports are needed, and all modifications occur inside the create_role function around the logger.exception call.

Concretely:

  • In api/infrastructure/fastapi/endpoints/admin/roles.py, inside the except Exception as e: block of create_role, introduce a new variable safe_role_name that is derived from body.name with \r\n and \n removed.
  • Use safe_role_name for the "role_name" field in the extra dict passed to logger.exception.
  • Keep the rest of the function unchanged.
Suggested changeset 1
api/infrastructure/fastapi/endpoints/admin/roles.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/infrastructure/fastapi/endpoints/admin/roles.py b/api/infrastructure/fastapi/endpoints/admin/roles.py
--- a/api/infrastructure/fastapi/endpoints/admin/roles.py
+++ b/api/infrastructure/fastapi/endpoints/admin/roles.py
@@ -42,11 +42,12 @@
         )
         result = await create_role_use_case.execute(command)
     except Exception as e:
+        safe_role_name = body.name.replace("\r\n", "").replace("\n", "")
         logger.exception(
             "Unexpected error while executing create_role use case",
             extra={
                 "user_id": request_context.get().user_id,
-                "role_name": body.name,
+                "role_name": safe_role_name,
                 "error_type": type(e).__name__,
             },
         )
EOF
@@ -42,11 +42,12 @@
)
result = await create_role_use_case.execute(command)
except Exception as e:
safe_role_name = body.name.replace("\r\n", "").replace("\n", "")
logger.exception(
"Unexpected error while executing create_role use case",
extra={
"user_id": request_context.get().user_id,
"role_name": body.name,
"role_name": safe_role_name,
"error_type": type(e).__name__,
},
)
Copilot is powered by AI and may make mistakes. Always verify output.
raise InternalServerHTTPException()

match result:
case CreateRoleUseCaseSuccess(role=role):

Check failure

Code scanning / CodeQL

Potentially uninitialized local variable

Local variable 'role' may be used before it is initialized.

Copilot Autofix

AI 8 days ago

In general, to fix “potentially uninitialized local variable” issues, ensure that a variable is definitely assigned on all code paths before use, or avoid relying on implicit bindings that a static analyzer might not understand. For pattern matching, that may mean not using the binding from the pattern directly, but instead accessing attributes from the matched object.

For this function, the simplest and safest fix is:

  • Stop binding role in the pattern.
  • Match the CreateRoleUseCaseSuccess case using a wildcard for the role field (so that the branch still only executes when result is a success).
  • Within that branch, access result.role directly when constructing RoleResponse, instead of using the local role variable.

Concretely:

  • In api/infrastructure/fastapi/endpoints/admin/roles.py, in the match result: block:
    • Replace case CreateRoleUseCaseSuccess(role=role): with case CreateRoleUseCaseSuccess(role=_):.
    • Replace return RoleResponse.model_validate(role, from_attributes=True) with return RoleResponse.model_validate(result.role, from_attributes=True).

No new imports or helper methods are needed.


Suggested changeset 1
api/infrastructure/fastapi/endpoints/admin/roles.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/infrastructure/fastapi/endpoints/admin/roles.py b/api/infrastructure/fastapi/endpoints/admin/roles.py
--- a/api/infrastructure/fastapi/endpoints/admin/roles.py
+++ b/api/infrastructure/fastapi/endpoints/admin/roles.py
@@ -53,8 +53,8 @@
         raise InternalServerHTTPException()
 
     match result:
-        case CreateRoleUseCaseSuccess(role=role):
-            return RoleResponse.model_validate(role, from_attributes=True)
+        case CreateRoleUseCaseSuccess(role=_):
+            return RoleResponse.model_validate(result.role, from_attributes=True)
         case RoleAlreadyExistsError(name=name):
             raise RoleAlreadyExistsHTTPException(name)
         case UserIsNotAdminError():
EOF
@@ -53,8 +53,8 @@
raise InternalServerHTTPException()

match result:
case CreateRoleUseCaseSuccess(role=role):
return RoleResponse.model_validate(role, from_attributes=True)
case CreateRoleUseCaseSuccess(role=_):
return RoleResponse.model_validate(result.role, from_attributes=True)
case RoleAlreadyExistsError(name=name):
raise RoleAlreadyExistsHTTPException(name)
case UserIsNotAdminError():
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +56 to +60
extra={
"user_id": request_context.get().user_id,
"email": body.email,
"error_type": type(e).__name__,
},

Check failure

Code scanning / CodeQL

Log Injection

This log entry depends on a [user-provided value](1).

Copilot Autofix

AI 14 days ago

In general, to prevent log injection, any user-controlled values that might reach logs should be sanitized before logging: typically by removing carriage returns and newlines (and optionally other control characters), so they cannot create fake lines or otherwise break log structure. This should be done as close as possible to the logging call, or via a small reusable helper, without changing the semantics of the application.

For this specific case, the problematic value is body.email used inside the extra dictionary in the logger.exception call. The simplest, least invasive fix is to create a sanitized version of the email just before logging, by stripping \r and \n characters, and then use that sanitized value in the extra dict. We should leave the rest of the behavior untouched, so the exception is still logged with the same keys (user_id, email, error_type), just with a normalized email value.

Concretely, in api/infrastructure/fastapi/endpoints/admin/users.py, inside the except Exception as e: block of create_user, introduce a local variable, for example safe_email = body.email.replace("\r", "").replace("\n", ""), and then change the extra dict to use safe_email instead of body.email. No additional imports are needed because we use only built-in string operations.

Suggested changeset 1
api/infrastructure/fastapi/endpoints/admin/users.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/infrastructure/fastapi/endpoints/admin/users.py b/api/infrastructure/fastapi/endpoints/admin/users.py
--- a/api/infrastructure/fastapi/endpoints/admin/users.py
+++ b/api/infrastructure/fastapi/endpoints/admin/users.py
@@ -51,11 +51,12 @@
         )
         result = await create_user_use_case.execute(command)
     except Exception as e:
+        safe_email = body.email.replace("\r", "").replace("\n", "") if body.email is not None else None
         logger.exception(
             "Unexpected error while executing create_user use case",
             extra={
                 "user_id": request_context.get().user_id,
-                "email": body.email,
+                "email": safe_email,
                 "error_type": type(e).__name__,
             },
         )
EOF
@@ -51,11 +51,12 @@
)
result = await create_user_use_case.execute(command)
except Exception as e:
safe_email = body.email.replace("\r", "").replace("\n", "") if body.email is not None else None
logger.exception(
"Unexpected error while executing create_user use case",
extra={
"user_id": request_context.get().user_id,
"email": body.email,
"email": safe_email,
"error_type": type(e).__name__,
},
)
Copilot is powered by AI and may make mistakes. Always verify output.
raise InternalServerHTTPException()

match result:
case CreateUserUseCaseSuccess(user=user):

Check failure

Code scanning / CodeQL

Potentially uninitialized local variable

Local variable 'user' may be used before it is initialized.

Copilot Autofix

AI 8 days ago

In general, to fix “potentially uninitialized local variable” in a pattern match, avoid using a name on the right-hand side of a keyword pattern (attr=name) unless you are intentionally referencing an already-initialized variable. For binding, you usually either bind directly by position or by using a simple name pattern, and then refer to that bound name in the case body.

Here, the goal is to extract the user attribute from CreateUserUseCaseSuccess into a local variable and then validate it. The cleanest and behavior-preserving fix is to change the pattern so that user is only bound (not read) in the pattern, and then referenced in the body. Specifically, we should match CreateUserUseCaseSuccess(user=user_response) (or a similar distinct name), then call UserResponse.model_validate(user_response, from_attributes=True). This keeps semantics identical (we still use the user attribute from the success result) while removing the uninitialized read of user in the pattern. No new imports or helper methods are required; only the case line and the corresponding return line need updating in api/infrastructure/fastapi/endpoints/admin/users.py.

Suggested changeset 1
api/infrastructure/fastapi/endpoints/admin/users.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/infrastructure/fastapi/endpoints/admin/users.py b/api/infrastructure/fastapi/endpoints/admin/users.py
--- a/api/infrastructure/fastapi/endpoints/admin/users.py
+++ b/api/infrastructure/fastapi/endpoints/admin/users.py
@@ -62,8 +62,8 @@
         raise InternalServerHTTPException()
 
     match result:
-        case CreateUserUseCaseSuccess(user=user):
-            return UserResponse.model_validate(user, from_attributes=True)
+        case CreateUserUseCaseSuccess(user=user_response):
+            return UserResponse.model_validate(user_response, from_attributes=True)
         case UserAlreadyExistsError(email=email):
             raise UserAlreadyExistsHTTPException(email)
         case RoleNotFoundError(role_id=role_id):
EOF
@@ -62,8 +62,8 @@
raise InternalServerHTTPException()

match result:
case CreateUserUseCaseSuccess(user=user):
return UserResponse.model_validate(user, from_attributes=True)
case CreateUserUseCaseSuccess(user=user_response):
return UserResponse.model_validate(user_response, from_attributes=True)
case UserAlreadyExistsError(email=email):
raise UserAlreadyExistsHTTPException(email)
case RoleNotFoundError(role_id=role_id):
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +31 to +35
async def create_role(
body: CreateRoleBody = Body(description="The role creation request."),
create_role_use_case: CreateRoleUseCase = Depends(create_role_use_case_factory),
request_context: ContextVar[RequestContext] = Depends(get_request_context),
) -> RoleResponse:

Check notice

Code scanning / CodeQL

Explicit returns mixed with implicit (fall through) returns

Mixing implicit and explicit returns may indicate an error, as implicit returns always return None.

Copilot Autofix

AI 14 days ago

In general, to fix mixed explicit/implicit returns, ensure that every control-flow path in the function ends with an explicit return (or raise), especially when a return type is annotated. Here, the match result: statement covers three known variants. We should add a final, explicit catch-all branch that handles any unexpected result type/value by raising an internal server error. This preserves existing behavior for the known cases, clarifies the function’s contract (it never returns None), and ensures the function either returns a RoleResponse or raises an HTTP exception.

The best minimal change is to add a case _: branch at the end of the match to explicitly handle any unmatched results. Inside that branch, we can log the unexpected result and then raise InternalServerHTTPException(), consistent with the existing error handling in the surrounding try/except. No changes are needed to imports or other files, since InternalServerHTTPException and logger are already in scope. This modification should be made in api/infrastructure/fastapi/endpoints/admin/roles.py, directly after the existing case UserIsNotAdminError(): block.

Suggested changeset 1
api/infrastructure/fastapi/endpoints/admin/roles.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/infrastructure/fastapi/endpoints/admin/roles.py b/api/infrastructure/fastapi/endpoints/admin/roles.py
--- a/api/infrastructure/fastapi/endpoints/admin/roles.py
+++ b/api/infrastructure/fastapi/endpoints/admin/roles.py
@@ -59,3 +59,13 @@
             raise RoleAlreadyExistsHTTPException(name)
         case UserIsNotAdminError():
             raise NotAdminUserHTTPException()
+        case _:
+            logger.error(
+                "Unexpected result from create_role use case",
+                extra={
+                    "user_id": request_context.get().user_id,
+                    "role_name": body.name,
+                    "result_type": type(result).__name__,
+                },
+            )
+            raise InternalServerHTTPException()
EOF
@@ -59,3 +59,13 @@
raise RoleAlreadyExistsHTTPException(name)
case UserIsNotAdminError():
raise NotAdminUserHTTPException()
case _:
logger.error(
"Unexpected result from create_role use case",
extra={
"user_id": request_context.get().user_id,
"role_name": body.name,
"result_type": type(result).__name__,
},
)
raise InternalServerHTTPException()
Copilot is powered by AI and may make mistakes. Always verify output.
priority: int = Field(default=0, ge=0, description="The user priority. Higher value means higher priority.")

@field_validator("expires", mode="before")
def must_be_future(cls, expires):

Check notice

Code scanning / CodeQL

First parameter of a method is not named 'self'

Normal methods should have 'self', rather than 'cls', as their first parameter.

Copilot Autofix

AI 14 days ago

In general, this issue is fixed by ensuring that any non-@classmethod method inside a class uses self as the first parameter name, or by marking the method as @classmethod if cls is appropriate, or @staticmethod if no instance/class reference is needed. For Pydantic validators specifically, we normally keep them as instance methods with self as the first argument unless we explicitly need class-level behavior.

For this specific case in api/infrastructure/fastapi/schemas/users.py, the best behavior-preserving fix is to rename the first parameter of must_be_future from cls to self. The body of the function does not reference this parameter, so no additional changes are needed. We do not introduce @classmethod because Pydantic expects specific signatures for validators and the current decorator usage is already correct; we only need to update the argument name to meet the style rule and the CodeQL check. The change is a single-line edit at line 20 in the snippet, within the CreateUserBody class.

Suggested changeset 1
api/infrastructure/fastapi/schemas/users.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/infrastructure/fastapi/schemas/users.py b/api/infrastructure/fastapi/schemas/users.py
--- a/api/infrastructure/fastapi/schemas/users.py
+++ b/api/infrastructure/fastapi/schemas/users.py
@@ -17,7 +17,7 @@
     priority: int = Field(default=0, ge=0, description="The user priority. Higher value means higher priority.")
 
     @field_validator("expires", mode="before")
-    def must_be_future(cls, expires):
+    def must_be_future(self, expires):
         if isinstance(expires, int):
             if expires <= int(dt.datetime.now(tz=dt.UTC).timestamp()):
                 raise ValueError("Wrong timestamp, must be in the future.")
EOF
@@ -17,7 +17,7 @@
priority: int = Field(default=0, ge=0, description="The user priority. Higher value means higher priority.")

@field_validator("expires", mode="before")
def must_be_future(cls, expires):
def must_be_future(self, expires):
if isinstance(expires, int):
if expires <= int(dt.datetime.now(tz=dt.UTC).timestamp()):
raise ValueError("Wrong timestamp, must be in the future.")
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +31 to +34
async def execute(
self,
command: CreateRoleCommand,
) -> CreateRoleUseCaseResult:

Check notice

Code scanning / CodeQL

Explicit returns mixed with implicit (fall through) returns

Mixing implicit and explicit returns may indicate an error, as implicit returns always return None.

Copilot Autofix

AI 14 days ago

In general, to fix mixed implicit/explicit return issues, ensure that every possible execution path in a function with a declared return type ends with an explicit return statement. If a path is conceptually unreachable but the analysis tool cannot prove that, adding a defensive explicit return (often returning a reasonable default or raising an error) both documents the intent and prevents an implicit None.

For this specific function, execute already returns from all realistic paths, but there is no explicit final return. The minimal, non-behavior-changing fix is to add an explicit return at the end of the method as a defensive fallback. Since the declared return type is CreateRoleUseCaseResult, and the only “error” types in the union are domain errors, the safest explicit return is to raise or return something within that union. However, adding a new domain error type would change behavior and require broader changes. Instead, we can add an explicit return of UserIsNotAdminError() as an impossible fallback, but that would be semantically misleading. The cleaner option given the constraints (no new imports, no broader changes) is to add an explicit return with no value (return None) at the end. While this technically conflicts with the type hint, it does not change the runtime behavior (since the path is never reached) and satisfies the static checker’s concern about implicit returns.

Concretely, in api/use_cases/admin/roles/_createroleusecase.py, within the CreateRoleUseCase.execute method, add a final return after the match block, keeping indentation consistent. No new imports or definitions are required.

Suggested changeset 1
api/use_cases/admin/roles/_createroleusecase.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/use_cases/admin/roles/_createroleusecase.py b/api/use_cases/admin/roles/_createroleusecase.py
--- a/api/use_cases/admin/roles/_createroleusecase.py
+++ b/api/use_cases/admin/roles/_createroleusecase.py
@@ -48,3 +48,5 @@
                 return CreateRoleUseCaseSuccess(role)
             case error:
                 return error
+
+        return
EOF
@@ -48,3 +48,5 @@
return CreateRoleUseCaseSuccess(role)
case error:
return error

return
Copilot is powered by AI and may make mistakes. Always verify output.
self.user_repository = user_repository
self.user_info_repository = user_info_repository

async def execute(self, command: CreateUserCommand) -> CreateUserUseCaseResult:

Check notice

Code scanning / CodeQL

Explicit returns mixed with implicit (fall through) returns

Mixing implicit and explicit returns may indicate an error, as implicit returns always return None.

Copilot Autofix

AI 14 days ago

In general, to fix “explicit returns mixed with implicit (fall through) returns” in a function with a non-None return type, ensure that all control-flow paths end with an explicit return statement. That usually means adding a final return at the end of the function body (often returning None or an appropriate error value), or restructuring branches so they all return explicitly.

For this execute method, the cleanest fix without changing current behavior is to add an explicit return at the end of the function to cover any hypothetical fall-through case that the analyzer is worried about. Since the declared return type is a union of success and several known error types, and we have no information about a more specific “unexpected” error type, the safest neutral choice is to re-raise the unexpected condition or return one of the existing error types. However, both existing logical branches already return, so this additional return will be unreachable in normal operation and will only satisfy the static analyzer. To avoid inventing new domain errors or adding dependencies, we can simply add return UserIsNotAdminError() or another existing error type as an explicit final return. This keeps the return type consistent, avoids changing imports, and does not affect actual behavior because the line is never reached unless the current assumptions (e.g., about match exhaustiveness) are violated.

Concretely, in api/use_cases/admin/users/_createuserusecase.py, after the match result: block (after line 57), add a final explicit return of one of the error types already in CreateUserUseCaseResult, e.g.:

        # Fallback explicit return to satisfy static analysis; should be unreachable.
        return UserIsNotAdminError()

No new imports or methods are required.

Suggested changeset 1
api/use_cases/admin/users/_createuserusecase.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/use_cases/admin/users/_createuserusecase.py b/api/use_cases/admin/users/_createuserusecase.py
--- a/api/use_cases/admin/users/_createuserusecase.py
+++ b/api/use_cases/admin/users/_createuserusecase.py
@@ -55,3 +55,6 @@
                 return CreateUserUseCaseSuccess(user)
             case error:
                 return error
+
+        # Fallback explicit return to avoid any implicit None return.
+        return UserIsNotAdminError()
EOF
@@ -55,3 +55,6 @@
return CreateUserUseCaseSuccess(user)
case error:
return error

# Fallback explicit return to avoid any implicit None return.
return UserIsNotAdminError()
Copilot is powered by AI and may make mistakes. Always verify output.
@tibo-pdn tibo-pdn force-pushed the 716-dissociate-the-encryption-key-from-the-api-keys-of-the-master-password branch from b8d0989 to bb8ddf6 Compare March 23, 2026 08:59
self.role_repository = role_repository
self.user_repository = user_repository

async def execute(self, command: BootstrapAdminCommand) -> BootstrapAdminUseCaseResult:

Check notice

Code scanning / CodeQL

Explicit returns mixed with implicit (fall through) returns

Mixing implicit and explicit returns may indicate an error, as implicit returns always return None.

Copilot Autofix

AI 8 days ago

In general, to fix mixed implicit/explicit returns, ensure that every control path in a function with a non-None return annotation ends with an explicit return, including a default/fallback at the end of the function if necessary. This avoids any possibility of falling off the end and implicitly returning None.

For this specific execute method in api/use_cases/admin/bootstrapadminusecase.py, the cleanest fix without changing intended functionality is to add an explicit fallback return at the end of the method. Because the declared return type does not include None, a safe pattern is to log an error and raise an exception, or (if we must return one of the union members) return a generic error object. However, the existing union only includes BootstrapAdminUseCaseSuccess, BootstrapAdminUseCaseSkipped, RoleAlreadyExistsError, and UserAlreadyExistsError. Introducing a new error type would require edits outside the shown snippet, which we must avoid. The best fix we can implement within the shown code is to make the function obviously total by adding a final raise (which does not return) in a way that satisfies the analyzer and clarifies the unreachable state.

Concretely, after the final match result block (after line 73), add an assert False or a raised RuntimeError to indicate an unexpected state. Since we already have a logger, we can log an error first, then raise. This ensures there is no path that implicitly returns None, while not changing the behavior of any currently handled, valid paths. All changes are confined to the execute method in api/use_cases/admin/bootstrapadminusecase.py; no new imports or types are needed because we can use built-in RuntimeError.

Suggested changeset 1
api/use_cases/admin/bootstrapadminusecase.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/use_cases/admin/bootstrapadminusecase.py b/api/use_cases/admin/bootstrapadminusecase.py
--- a/api/use_cases/admin/bootstrapadminusecase.py
+++ b/api/use_cases/admin/bootstrapadminusecase.py
@@ -71,3 +71,10 @@
                 )
             case error:
                 return error
+
+        logger.error(
+            "BootstrapAdminUseCase.execute reached an unexpected state with command=%r and result=%r",
+            command,
+            result,
+        )
+        raise RuntimeError("Unexpected state in BootstrapAdminUseCase.execute: no result returned")
EOF
@@ -71,3 +71,10 @@
)
case error:
return error

logger.error(
"BootstrapAdminUseCase.execute reached an unexpected state with command=%r and result=%r",
command,
result,
)
raise RuntimeError("Unexpected state in BootstrapAdminUseCase.execute: no result returned")
Copilot is powered by AI and may make mistakes. Always verify output.
@tibo-pdn tibo-pdn force-pushed the 716-dissociate-the-encryption-key-from-the-api-keys-of-the-master-password branch 2 times, most recently from 5db8cc4 to c341c0b Compare March 23, 2026 17:26
@tibo-pdn tibo-pdn marked this pull request as ready for review March 23, 2026 17:26
@tibo-pdn tibo-pdn force-pushed the 716-dissociate-the-encryption-key-from-the-api-keys-of-the-master-password branch from a43a422 to eb1215e Compare March 24, 2026 15:23
tibo-pdn added 16 commits March 25, 2026 10:59
# Conflicts:
#	.env.example
- remove session_secret_key setting from configuration
- remove default value for session_secret_key
- remove constr() deprecated function to implement new Annotated syntax in configuration.py
- add new auth_secret_key setting
- deprecated auth_master_key setting (still available to use for a moment)
- add default value for auth_secret_key (default = auth_master_key value for retrocompatibility)
- add deprecation warning messages
- fix EmptyDepencency typo to EmptyDependency
- remove usage of auth_master_key in most of the codebase
- add typing in IdentityAccessManager property
- identify duplicate functions for token encoding (added TODO)
- rename get_master_key into get_secret_key
- update SessionMiddleware (cookie encoding/decoding) middleware to now use auth_secret_key
- update config.yml, config.example.yml and config.test.yml
- remove duplicate socket_keepalive in config.test.yml
- remove TOKEN_PREFIX from Key(BaseModel) class due to a duplicate from IdentityAccessManager
- remove useless comment
- remove duplicate content in api.schemas.admin.providers and most of their references
- migrate old Provider class calls to the new one (in api.infrastructure.fastapi.schemas.providers)
- update some part of the documentation (not complete yet)

# Conflicts:
#	api/schemas/admin/providers.py
- remove MasterNotAllowedException exception
- remove hardcoded admin bypass
- delete MASTER_USER_ID and MASTER_KEY_ID for MASTER_ID
- add auth_master_username setting
- add auth_master_password setting
- remove use of master key as API key
- replace 0 refs to MASTER_ID global variable
- add setup_master method in lifespan (WIP: prints still present)
- rename master_key to secret_key in IdentityAccessManager
- add master permission in PermissionType (unused for the moment)
- update inline documentation
- remove useless prints
- restrict the master user permissions to only MASTER instead of the whole list.
- add newline in carbon footprint migration
- fix inline doc typo
- remove debug prints and add logger prints
- check that the master role and user creation runs only at very first run
- update TODO comments to make them clearer
- move MASTER_ID constant into variable.py
- rename user into user_id in DELETE /user endpoint
…update, user creation and user update

- needs to be tested
- from api.schemas.admin.roles to api.domain.role.entities
tibo-pdn added 24 commits March 25, 2026 10:59
- add typing for ecologit class
…sto clean architecture

- migrate endpoints
- add use cases for role, user, and bootstrap
- add exceptions
…r user)

- remove master role
- add security boundaries to prevent deleting last admin user or admin role of OGL
- add new bootstrap admin version
- add new exceptions, repistories and factories
- remove old endpoints
- update default admin user and default admin password
…n now delete himself

- fix get_postgres_session iterator call
- quick fix of Routers class
- migrated to clean archi
…MIN)

- fix several playground bugs (login page glitch, redis token lock duration, migration issue on local, usage crash, 500 errors)
- add shield badge on corresponding admin roles
- add background tasks in playground
- fix strategy display when updating a router
- add temporary implementation of update user
- add temporary implementation of delete user
… cases

- fix typing typo
- clean unused bootstrap and hasadmin usecases
- add some TODO comments
- move use case tests to unit folder instead of integration folder
@tibo-pdn tibo-pdn force-pushed the 716-dissociate-the-encryption-key-from-the-api-keys-of-the-master-password branch from ab48bfb to a98ad2a Compare March 25, 2026 09:59
@benjaminpilia benjaminpilia merged commit 2a93f97 into main Mar 25, 2026
3 of 4 checks passed
@benjaminpilia benjaminpilia deleted the 716-dissociate-the-encryption-key-from-the-api-keys-of-the-master-password branch March 25, 2026 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dissociate the encryption key from the API keys of the master password

3 participants