feat(auth): add optional static API key authentication - #970
Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughStatic API-key JWT issuance, validation, gateway integration, CLI commands, shared signing configuration, deployment wiring, authentication documentation, tests, and customizer OpenAPI updates were added. ChangesStatic API-key authentication
Customizer OpenAPI
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant AuthService
participant AuthMiddleware
participant PDP
Client->>Gateway: Create API-key request
Gateway->>AuthService: POST /apis/auth/v2/api-keys
AuthService-->>Gateway: Bearer JWT and metadata
Client->>Gateway: Bearer API-key request
Gateway->>AuthMiddleware: Validate JWT and map principal
AuthMiddleware->>PDP: Authorize principal and scopes
PDP-->>AuthMiddleware: Authorization result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/nmp_common/tests/auth/test_api_keys.py (1)
171-183: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
now=1785280061is still inside the 30s leeway.PyJWT 2.13.0doesn’t acceptcurrent_time, so this can trip the broadexceptinstead of testing expiry. Use a real time override or removenow=here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nmp_common/tests/auth/test_api_keys.py` around lines 171 - 183, The expired-key test still validates within the JWT expiration leeway. Update test_validate_static_api_key_token_rejects_expired_key to use a time override beyond the 30-second leeway, or remove the now override so real time is used, while preserving the assertion that validation returns None.
🧹 Nitpick comments (3)
packages/nmp_common/src/nmp/common/auth/api_keys.py (1)
172-213: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBroad
except Exception: return Nonehides real failures.Network errors fetching the JWKS, bugs, and actually-invalid tokens are all indistinguishable — all become a silent "invalid token" with no log line. Makes diagnosing a JWKS outage or a regression indistinguishable from routine auth rejections.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nmp_common/src/nmp/common/auth/api_keys.py` around lines 172 - 213, Replace the broad exception handling around JWT validation with targeted handling for expected invalid-token and JWKS lookup failures, while allowing unexpected programming or configuration errors to propagate and be observable. Preserve the existing None return for routine authentication rejection paths in the token-decoding flow.openapi/ga/openapi.yaml (1)
201-231: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRevoke's
200response is untyped (schema: {}).Sibling delete endpoints in this spec (role-bindings L385, projects L1226, workspaces L681) return
DeleteResponse. An untyped{}here will generate an untyped return value in the auto-generated CLI/SDK client forrevoke_api_key.As per path instructions,
packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/**/*.py"are auto-generated from templates" — this spec is their source of truth, so the untyped schema propagates into generated CLI code.♻️ Suggested fix
responses: '200': description: Successful Response content: application/json: - schema: {} + schema: + $ref: '`#/components/schemas/DeleteResponse`'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openapi/ga/openapi.yaml` around lines 201 - 231, Update the 200 response schema for the revoke_api_key operation at /apis/auth/v2/api-keys/{jti} to reference the existing DeleteResponse component, matching the typed responses used by sibling delete endpoints. Leave the other response definitions unchanged.Source: Path instructions
services/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.py (1)
49-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse conventional status codes: 201 for create, 204 for delete.
create_api_keyreturns 200 andrevoke_api_keyreturns 200 with a null body. Every other create/delete route in this API (create_workspace, delete_workspace, create_model, delete_model, etc.) uses 201/204. This new resource breaks that pattern for no apparent reason.♻️ Proposed fix
-@router.post("/v2/api-keys", response_model=schemas.APIKeyCreateResponse) +@router.post("/v2/api-keys", response_model=schemas.APIKeyCreateResponse, status_code=status.HTTP_201_CREATED) async def create_api_key(-@router.delete("/v2/api-keys/{jti}", responses={501: {"model": schemas.APIKeyNotImplementedErrorResponse}}) +@router.delete( + "/v2/api-keys/{jti}", + status_code=status.HTTP_204_NO_CONTENT, + responses={501: {"model": schemas.APIKeyNotImplementedErrorResponse}}, +) async def revoke_api_key(jti: str, issuer: APIKeyIssuer = Depends(get_api_key_issuer)) -> None:Also applies to: 135-142
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.py` around lines 49 - 59, Update the create_api_key route to declare HTTP 201 Created, and update the corresponding revoke_api_key route to return HTTP 204 No Content with no response body. Follow the existing status-code conventions used by the other create/delete endpoints while preserving their current success and error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contrib/auth/authentik/compose/implementation-details.md`:
- Around line 132-135: The documentation incorrectly claims Envoy calls the
`/apis/auth/v2/api-keys/authenticate` endpoint. Update the rationale around the
API-key JWKS and authentication endpoints to describe the endpoint’s actual
caller and purpose, or remove its public bypass and related documentation if no
configured component uses it; keep the JWKS endpoint public for Envoy key
fetching.
In `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 120-231: Update the API-key endpoint response definitions for
create_api_key, list_api_keys, revoke_api_key, and both authenticate operations
to document every error status returned by their implementations: 404 for
disabled or unavailable features, 400 for RuntimeError from create_api_key, 501
for list_api_keys where applicable, and 401 for invalid bearer authentication.
Reuse the appropriate existing error response schemas and preserve the current
success responses.
In `@openapi/ga/openapi.yaml`:
- Around line 163-187: Update both GET and POST operations for
/apis/auth/v2/api-keys/authenticate, identified by operationIds
get_authenticate_api_key_for_gateway and post_authenticate_api_key_for_gateway,
to document 401 responses for missing or invalid bearer tokens and 404 responses
when API keys are disabled, while preserving the existing 200 response schemas.
In
`@packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/__init__.py`:
- Around line 18-25: Align the api-keys visibility across both sources: update
hidden in
packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/__init__.py:18-25
and set the matching intended visibility in
tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/cli_generator/cli_config.yaml:18-21,
then regenerate the API CLI output so TopLevelEntry remains consistent with the
generator configuration.
In
`@packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/types.py`:
- Around line 21-25: Update the expires_in_seconds field in the API-key type to
enforce a finite maximum lifetime in addition to the existing minimum of 1
second, using the project’s established lifetime limit if available. Preserve
None as the value for non-time-delimited keys.
In `@packages/nmp_common/src/nmp/common/auth/api_keys.py`:
- Around line 47-58: Cache the parsed RSA private key instead of rereading and
reparsing it for every API-key creation. Update the key-loading flow around
_private_key_pem and _private_key, and reuse the cached key from
APIKeyIssuerService.create() while preserving existing configuration and
validation errors.
- Around line 160-183: Update validate_static_api_key_token to reuse a cached
jwt.PyJWKClient instead of constructing one per validation, preserving the
existing cache_keys and lifespan settings; store or retrieve the client through
an appropriate shared/config-scoped cache. Move the synchronous
get_signing_key_from_jwt call off the async event loop, and ensure JWKS fetch
failures are not silently converted to invalid-token results by the broad
exception handling.
In `@packages/nmp_common/src/nmp/common/config/base.py`:
- Around line 310-317: Update the auth settings model_config to use a double
underscore as env_nested_delimiter and remove env_nested_max_split. Preserve the
existing auth prefix and other configuration options so nested variables such as
AUTH_TOKEN_SIGNING_KEY_ID and AUTH_API_KEYS_ENABLED map to their intended
fields.
In `@plugins/nemo-customizer/openapi/openapi.yaml`:
- Around line 395-400: The OpenAPI specification must retain the existing
rl/jobs routes as deprecated aliases alongside the unsloth/jobs routes. Restore
the rl/jobs path definitions and mark them deprecated while preserving their
operation contracts, so existing clients remain supported during migration.
In `@services/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.py`:
- Around line 62-67: Update api_key_jwks to cache the parsed JWK or key object
produced by public_jwk_from_private_key_pem, reusing it across requests instead
of rereading config.token_signing.private_key_file each time. Preserve the
existing disabled-api-keys 404 behavior and response shape, while ensuring cache
initialization is safe for concurrent async requests.
---
Outside diff comments:
In `@packages/nmp_common/tests/auth/test_api_keys.py`:
- Around line 171-183: The expired-key test still validates within the JWT
expiration leeway. Update test_validate_static_api_key_token_rejects_expired_key
to use a time override beyond the 30-second leeway, or remove the now override
so real time is used, while preserving the assertion that validation returns
None.
---
Nitpick comments:
In `@openapi/ga/openapi.yaml`:
- Around line 201-231: Update the 200 response schema for the revoke_api_key
operation at /apis/auth/v2/api-keys/{jti} to reference the existing
DeleteResponse component, matching the typed responses used by sibling delete
endpoints. Leave the other response definitions unchanged.
In `@packages/nmp_common/src/nmp/common/auth/api_keys.py`:
- Around line 172-213: Replace the broad exception handling around JWT
validation with targeted handling for expected invalid-token and JWKS lookup
failures, while allowing unexpected programming or configuration errors to
propagate and be observable. Preserve the existing None return for routine
authentication rejection paths in the token-decoding flow.
In `@services/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.py`:
- Around line 49-59: Update the create_api_key route to declare HTTP 201
Created, and update the corresponding revoke_api_key route to return HTTP 204 No
Content with no response body. Follow the existing status-code conventions used
by the other create/delete endpoints while preserving their current success and
error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4e43a1b1-ccb6-41e4-968b-c771c6b42efd
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/api/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/api/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/api/api_keys/jwks.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/api_keys.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/jwks.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_authenticate_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_create_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_create_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_jwks_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_list_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_metadata_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/api_keys/test_jwks.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_api_keys.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (50)
contrib/auth/authentik/compose/docker-compose.ymlcontrib/auth/authentik/compose/implementation-details.mdcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/helm/templates/_envoy-config.tplcontrib/auth/authentik/helm/values.yamlcontrib/auth/authentik/kubernetes/implementation-details.mdcontrib/auth/authentik/manifest.yamldocs/auth/authentication/using-authentication.mdxdocs/auth/deployment/configuration.mdxdocs/cli/reference.mdxdocs/fern/snippets/_snippets/cli-summary.mdxdocs/set-up/config-reference.mdxopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/__init__.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/api_keys/__init__.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/api_keys/jwks.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/__init__.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/endpoints.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/issuer.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/types.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/method.pypackages/nemo_platform_plugin/tests/auth/api_keys/test_client.pypackages/nemo_platform_plugin/tests/auth/api_keys/test_endpoints.pypackages/nmp_common/src/nmp/common/auth/__init__.pypackages/nmp_common/src/nmp/common/auth/api_keys.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/config/base.pypackages/nmp_common/tests/auth/test_api_keys.pypackages/nmp_common/tests/auth/test_middleware.pyplugins/nemo-customizer/openapi/openapi.yamlservices/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/api_keys/schemas.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/integration/test_static_api_keys.pyservices/core/auth/tests/test_api_keys.pyservices/core/auth/tests/test_workload_token_exchange.pytests/auth_idp/authentik_live.pytests/auth_idp/contracts/test_api_keys.pytests/auth_idp/runtime_kubernetes.pytests/auth_idp/static/test_authentik_kubernetes_demo.pytests/auth_idp/static/test_provider_layout.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/cli_generator/cli_config.yaml
11beb1b to
3526987
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/auth_idp/contracts/test_api_keys.py (1)
82-92: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winImmediate 200 assertion after granting the role can flake. Authz propagation is asynchronous; poll with a short retry/timeout instead of a single request.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/auth_idp/contracts/test_api_keys.py` around lines 82 - 92, Replace the immediate status assertion in the API-key authorization flow with polling of the httpx request after grant_workspace_role completes. Retry briefly until the response status is 200 or a short timeout is reached, then assert success while preserving the response text for failure diagnostics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/cli/reference.mdx`:
- Around line 263-304: Update the source that generates the `nemo auth api-keys`
reference so `list` and `revoke` are not advertised as supported for stateless
V1 keys, or explicitly document their “not implemented” limitation. Regenerate
`docs/cli/reference.mdx` using the designated documentation generation command.
In `@packages/nmp_common/src/nmp/common/auth/middleware.py`:
- Around line 376-383: Update validate_static_api_key_token to reuse a shared
PyJWKClient instance across API-key requests instead of constructing one per
call, preserving its JWKS cache; also move the synchronous JWKS lookup off the
event loop, using the existing async/threading approach where available.
In `@services/core/auth/tests/integration/test_static_api_keys.py`:
- Around line 104-111: Update the patch target in the integration test to
replace the validate_static_api_key_token binding imported by
nmp.core.auth.api.v2.api_keys.endpoints, rather than patching its defining
module. Keep validate_with_local_jwks and the request assertions unchanged.
---
Nitpick comments:
In `@tests/auth_idp/contracts/test_api_keys.py`:
- Around line 82-92: Replace the immediate status assertion in the API-key
authorization flow with polling of the httpx request after grant_workspace_role
completes. Retry briefly until the response status is 200 or a short timeout is
reached, then assert success while preserving the response text for failure
diagnostics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 92a01a9d-8e65-48e2-8c16-956a97e94823
⛔ Files ignored due to path filters (25)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/manifest_registry.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/manifest.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/api_keys.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/jwks.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_authenticate_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_create_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_create_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_jwks_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_list_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_metadata_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/api_keys/test_jwks.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_api_keys.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_agent.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/test_app.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (51)
contrib/auth/authentik/compose/docker-compose.ymlcontrib/auth/authentik/compose/implementation-details.mdcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/helm/templates/_envoy-config.tplcontrib/auth/authentik/helm/values.yamlcontrib/auth/authentik/kubernetes/implementation-details.mdcontrib/auth/authentik/manifest.yamldocs/auth/authentication/using-authentication.mdxdocs/auth/deployment/configuration.mdxdocs/cli/reference.mdxdocs/fern/snippets/_snippets/cli-summary.mdxdocs/set-up/config-reference.mdxopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/manifest_registry.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/manifest.pypackages/nemo_platform_ext/tests/cli/commands/test_agent.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/cli/test_app.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/__init__.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/endpoints.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/issuer.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/types.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/method.pypackages/nemo_platform_plugin/tests/auth/api_keys/test_client.pypackages/nemo_platform_plugin/tests/auth/api_keys/test_endpoints.pypackages/nmp_common/src/nmp/common/auth/__init__.pypackages/nmp_common/src/nmp/common/auth/api_keys.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/config/base.pypackages/nmp_common/tests/auth/test_api_keys.pypackages/nmp_common/tests/auth/test_middleware.pyplugins/nemo-customizer/openapi/openapi.yamlservices/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/api_keys/schemas.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/integration/test_static_api_keys.pyservices/core/auth/tests/test_api_keys.pyservices/core/auth/tests/test_workload_token_exchange.pytests/auth_idp/authentik_live.pytests/auth_idp/contracts/test_api_keys.pytests/auth_idp/runtime_kubernetes.pytests/auth_idp/static/test_authentik_kubernetes_demo.pytests/auth_idp/static/test_provider_layout.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/cli_generator/cli_config.yaml
💤 Files with no reviewable changes (1)
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/manifest_registry.py
🚧 Files skipped from review as they are similar to previous changes (33)
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/cli_generator/cli_config.yaml
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/issuer.py
- services/core/auth/src/nmp/core/auth/api/v2/api_keys/schemas.py
- tests/auth_idp/authentik_live.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/init.py
- contrib/auth/authentik/compose/implementation-details.md
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/method.py
- contrib/auth/authentik/compose/docker-compose.yml
- packages/nemo_platform_plugin/tests/auth/api_keys/test_client.py
- packages/nemo_platform_plugin/tests/auth/api_keys/test_endpoints.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/endpoints.py
- docs/set-up/config-reference.mdx
- services/core/auth/src/nmp/core/auth/service.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/client.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- contrib/auth/authentik/kubernetes/implementation-details.md
- tests/auth_idp/runtime_kubernetes.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/types.py
- packages/nmp_common/tests/auth/test_api_keys.py
- services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py
- docs/auth/deployment/configuration.mdx
- openapi/ga/openapi.yaml
- packages/nmp_common/tests/auth/test_middleware.py
- services/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.py
- services/core/auth/tests/test_workload_token_exchange.py
- contrib/auth/authentik/helm/templates/_envoy-config.tpl
- openapi/openapi.yaml
- packages/nmp_common/src/nmp/common/auth/api_keys.py
- packages/nmp_common/src/nmp/common/config/base.py
- contrib/auth/authentik/helm/values.yaml
- openapi/ga/individual/platform.openapi.yaml
- contrib/auth/authentik/gateway/envoy.yaml
3526987 to
68ea5bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/nmp_common/tests/auth/test_api_keys.py (1)
326-338: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a timestamp beyond the 30s leeway.
now=1785280061is still within the JWT grace window forexp=1785280060; move it pastexp + 30(for example,1785280091) to test rejection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nmp_common/tests/auth/test_api_keys.py` around lines 326 - 338, Update test_validate_static_api_key_token_rejects_expired_key to pass a validation timestamp beyond the JWT 30-second leeway, such as 1785280091, while keeping the issued expiration at 1785280060.
♻️ Duplicate comments (1)
services/core/auth/tests/integration/test_static_api_keys.py (1)
104-111: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPatch target still wrong — won't intercept the real validation call.
nmp.core.auth.api.v2.authenticateimportsvalidate_static_api_key_tokendirectly into its own namespace (confirmed by the correct pattern used intest_authenticate.py:patch.object(authenticate, "validate_static_api_key_token", ...)). Patchingnmp.common.auth.api_keys.validate_static_api_key_tokenhere does not rebind that name in theauthenticatemodule, so the real (network-calling) implementation runs during the request.🐛 Proposed fix
- with patch("nmp.common.auth.api_keys.validate_static_api_key_token", validate_with_local_jwks): + with patch( + "nmp.core.auth.api.v2.authenticate.validate_static_api_key_token", validate_with_local_jwks + ):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/core/auth/tests/integration/test_static_api_keys.py` around lines 104 - 111, Update the patch in the static API key integration test to target the `validate_static_api_key_token` symbol imported into the `nmp.core.auth.api.v2.authenticate` module, using the same `patch.object(authenticate, "validate_static_api_key_token", ...)` pattern as `test_authenticate.py`; keep `validate_with_local_jwks` as the replacement implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/auth/deployment/configuration.mdx`:
- Around line 130-131: Update the API-key accepted-formats configuration example
to advertise only currently supported formats, removing opaque or explicitly
marking it as future-only; keep the surrounding list-valued environment-variable
guidance unchanged.
In `@openapi/ga/openapi.yaml`:
- Around line 218-230: Update the FastAPI api_key_jwks route’s responses mapping
to document its 404 response when API keys are disabled, then regenerate the
OpenAPI specification so the /apis/auth/v2/api-keys/jwks entry includes the
matching 404 response.
In `@services/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.py`:
- Around line 63-72: Update create_api_key to catch
APIKeyOperationNotImplementedError from issuer.create(request) and translate it
with _not_implemented(exc), matching the handling already used by list_api_keys
and revoke_api_key while preserving the existing exception mappings.
In `@services/core/auth/src/nmp/core/auth/api/v2/authenticate.py`:
- Around line 100-131: Update _validate_workload_access_token to distinguish
workload signing-key/configuration failures from expected JWT validation
failures: catch and surface signing-key retrieval or key-construction errors
with server-side error logging, while retaining the existing None result for
genuinely invalid tokens. Do not let misconfiguration silently appear as
ordinary authentication failure.
In `@tests/auth_idp/static/test_provider_layout.py`:
- Around line 204-207: Align the Authentik compose configuration with the
advertised platform_api_keys capability: update auth.api_keys.enabled in
platform-compose-authentik.yaml to match the compose contract, or remove
platform_api_keys if the endpoint should remain disabled. Update the
corresponding assertion in test_provider_layout.py to reflect the chosen
configuration.
---
Outside diff comments:
In `@packages/nmp_common/tests/auth/test_api_keys.py`:
- Around line 326-338: Update
test_validate_static_api_key_token_rejects_expired_key to pass a validation
timestamp beyond the JWT 30-second leeway, such as 1785280091, while keeping the
issued expiration at 1785280060.
---
Duplicate comments:
In `@services/core/auth/tests/integration/test_static_api_keys.py`:
- Around line 104-111: Update the patch in the static API key integration test
to target the `validate_static_api_key_token` symbol imported into the
`nmp.core.auth.api.v2.authenticate` module, using the same
`patch.object(authenticate, "validate_static_api_key_token", ...)` pattern as
`test_authenticate.py`; keep `validate_with_local_jwks` as the replacement
implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9592dd5f-b8e3-407f-a0c9-cbdfe3d6437c
⛔ Files ignored due to path filters (31)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/manifest_registry.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/manifest.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/api_keys.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/jwks.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/auth/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/auth/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/auth/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_create_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_create_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_jwks_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_list_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_metadata_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/auth/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/auth/authenticate_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/api_keys/test_jwks.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/auth/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_api_keys.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_agent.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/test_app.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (58)
contrib/auth/authentik/compose/docker-compose.ymlcontrib/auth/authentik/compose/implementation-details.mdcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/helm/templates/_envoy-config.tplcontrib/auth/authentik/helm/values.yamlcontrib/auth/authentik/kubernetes/implementation-details.mdcontrib/auth/authentik/manifest.yamldocs/auth/authentication/using-authentication.mdxdocs/auth/deployment/configuration.mdxdocs/cli/reference.mdxdocs/fern/snippets/_snippets/cli-summary.mdxdocs/set-up/config-reference.mdxe2e/authz_oidc/conftest.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/manifest_registry.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/manifest.pypackages/nemo_platform_ext/tests/cli/commands/test_agent.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/cli/test_app.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/__init__.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/endpoints.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/issuer.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/types.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/method.pypackages/nemo_platform_plugin/tests/auth/api_keys/test_client.pypackages/nemo_platform_plugin/tests/auth/api_keys/test_endpoints.pypackages/nmp_common/src/nmp/common/auth/__init__.pypackages/nmp_common/src/nmp/common/auth/api_keys.pypackages/nmp_common/src/nmp/common/auth/bearer.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/auth/signing_keys.pypackages/nmp_common/src/nmp/common/config/base.pypackages/nmp_common/tests/auth/test_api_keys.pypackages/nmp_common/tests/auth/test_bearer.pypackages/nmp_common/tests/auth/test_middleware.pypackages/nmp_common/tests/auth/test_signing_keys.pyplugins/nemo-customizer/openapi/openapi.yamlservices/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/api_keys/schemas.pyservices/core/auth/src/nmp/core/auth/api/v2/authenticate.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/integration/test_static_api_keys.pyservices/core/auth/tests/test_api_keys.pyservices/core/auth/tests/test_authenticate.pyservices/core/auth/tests/test_workload_token_exchange.pytests/auth_idp/authentik_live.pytests/auth_idp/contracts/test_api_keys.pytests/auth_idp/runtime_kubernetes.pytests/auth_idp/static/test_authentik_kubernetes_demo.pytests/auth_idp/static/test_provider_layout.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/cli_generator/cli_config.yaml
💤 Files with no reviewable changes (1)
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/manifest_registry.py
🚧 Files skipped from review as they are similar to previous changes (33)
- contrib/auth/authentik/compose/docker-compose.yml
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/issuer.py
- packages/nemo_platform_ext/tests/cli/commands/test_agent.py
- contrib/auth/authentik/manifest.yaml
- docs/fern/snippets/_snippets/cli-summary.mdx
- packages/nmp_common/src/nmp/common/auth/init.py
- services/core/auth/src/nmp/core/auth/api/v2/api_keys/schemas.py
- contrib/auth/authentik/compose/implementation-details.md
- services/core/auth/src/nmp/core/auth/service.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/method.py
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/cli_generator/cli_config.yaml
- docs/set-up/config-reference.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/manifest.py
- contrib/auth/authentik/helm/values.yaml
- packages/nemo_platform_plugin/tests/auth/api_keys/test_endpoints.py
- packages/nemo_platform_ext/tests/cli/test_app.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/types.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- packages/nemo_platform_plugin/tests/auth/api_keys/test_client.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/client.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/init.py
- packages/nmp_common/src/nmp/common/auth/api_keys.py
- docs/cli/reference.mdx
- contrib/auth/authentik/kubernetes/implementation-details.md
- packages/nmp_common/tests/auth/test_middleware.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- tests/auth_idp/static/test_authentik_kubernetes_demo.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/endpoints.py
- services/core/auth/tests/test_workload_token_exchange.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- docs/auth/authentication/using-authentication.mdx
- packages/nmp_common/src/nmp/common/config/base.py
- packages/nmp_common/src/nmp/common/auth/middleware.py
68ea5bb to
d711d00
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
services/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.py (1)
60-69: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
create_api_keystill doesn't handleAPIKeyOperationNotImplementedError.
issuer.create()can raiseAPIKeyOperationNotImplementedErrorfor unsupported issue formats, but onlyAPIKeyFeatureDisabledError/RuntimeErrorare caught here — it will bubble as a 500 instead of the501used bylist_api_keys/revoke_api_key. This matches an unaddressed prior review comment.🐛 Proposed fix
try: return issuer.create(request) except APIKeyFeatureDisabledError as exc: raise _disabled(exc) from exc + except APIKeyOperationNotImplementedError as exc: + raise _not_implemented(exc) from exc except RuntimeError as exc: raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc)) from exc🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.py` around lines 60 - 69, Update create_api_key to catch APIKeyOperationNotImplementedError from issuer.create and convert it using the same 501 response handling as list_api_keys and revoke_api_key, while preserving the existing APIKeyFeatureDisabledError and RuntimeError handling.
🧹 Nitpick comments (5)
packages/nmp_common/tests/auth/test_api_keys.py (2)
326-331: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThese two tests are
asyncbut never await, and the names describe the issuer, not the validator.
test_validate_static_api_key_token_rejects_service_principal_subjectexercisesAPIKeyIssuerService.create. Make it sync and rename totest_api_key_issuer_service_rejects_service_principal_subject.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nmp_common/tests/auth/test_api_keys.py` around lines 326 - 331, Update the test exercising APIKeyIssuerService.create by removing async from test_validate_static_api_key_token_rejects_service_principal_subject and renaming it to test_api_key_issuer_service_rejects_service_principal_subject. Preserve the existing setup and assertion behavior.
168-185: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClear the signing-key cache in an autouse fixture instead of ad hoc calls.
Only these two tests clear the cache, and neither clears on exit; the cached key survives into later tests. Path-keyed caching plus unique
tmp_pathhides it today, ordering changes won't.♻️ Suggested fixture
`@pytest.fixture`(autouse=True) def _clear_signing_key_cache(): clear_static_api_key_signing_key_cache() yield clear_static_api_key_signing_key_cache()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nmp_common/tests/auth/test_api_keys.py` around lines 168 - 185, Replace the ad hoc clear_static_api_key_signing_key_cache calls in the affected tests with an autouse fixture that clears the cache before each test and again after it yields. Keep the existing cache assertions unchanged, and ensure the fixture is applied to all tests in the module so cached signing keys cannot leak between tests.services/core/auth/tests/test_authenticate.py (1)
154-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis regression test can't fail.
The app only mounts
authenticate.router, so/v2/api-keys/authenticateis 404 whether or not the route was reintroduced in the api-keys router. Assert against the real app's route table (or mount the api-keys router) instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/core/auth/tests/test_authenticate.py` around lines 154 - 160, Update test_api_key_specific_authenticate_route_is_removed to validate the application route table rather than posting to an unmounted path. Inspect the real app’s registered routes and assert that the api-keys-specific authenticate route is absent, or mount the api-keys router so the request can distinguish route removal from router absence.packages/nmp_common/src/nmp/common/auth/middleware.py (1)
414-420: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a concrete type for
claimsinstead ofAny.Both producers (
TokenClaimsfromapi_keys.py, and the OIDC validator's claims) duck-type.subject/.groups/.scopes. AProtocolwould give static-typing coverage without coupling to either concrete class.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nmp_common/src/nmp/common/auth/middleware.py` around lines 414 - 420, Replace the Any annotation on claims in _handle_validated_bearer_claims with a shared structural Protocol exposing subject, email, groups, and scopes, and define or reuse that Protocol where the middleware types are shared. Keep the Protocol independent of TokenClaims and the OIDC claims class while preserving the existing attribute access and behavior.services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py (1)
216-242: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winWorkload signing-key path still blocks the event loop synchronously; API-key path was fixed, workload wasn't.
auth_jwks_responseawaits the async API-key JWK helper but callsworkload_token_exchange_service.public_jwk(config)synchronously (line 344);token_exchangelikewise callsworkload_signing_key(config)synchronously (line 557). Both go throughRSASigningKeyCache, which does a blockingpath.stat()on every call and a blocking PEM read/parse on cache miss — the same class of issue previously flagged for the JWKS hot path, now only fixed for the API-key branch. Consider adding async wrappers onWorkloadTokenExchangeService(mirroringget_from_file_async/public_jwk_from_file_async) and awaiting them fromauth_jwks_response/token_exchange.Also applies to: 338-347, 557-563
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py` around lines 216 - 242, Add async workload-key helpers to WorkloadTokenExchangeService that delegate to the cache’s asynchronous equivalents, preserving the existing key IDs, file paths, and error messages. Update auth_jwks_response to await the async public-JWK helper and token_exchange to await the async signing-key helper, removing synchronous RSASigningKeyCache access from both request paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 7-43: Add a 404 response to both GET and POST operations under the
/apis/auth/authenticate path, reusing the existing AuthenticateErrorResponse
schema and matching the sibling api-keys endpoint response structure. Keep the
current 200 and 401 responses unchanged.
In `@packages/nmp_common/src/nmp/common/auth/middleware.py`:
- Around line 412-420: Extract the duplicated PDP invocation and exception
handling from _handle_validated_bearer_claims and _handle_auth_check into a
shared helper such as _call_pdp, passing the auth client, request, scopes, and a
flag for the denial-status behavior. Preserve each caller’s existing 401/403
semantics, including the always-403 behavior for validated bearer claims, while
centralizing handling for ConnectError, TimeoutException, HTTPStatusError,
InvalidScopeFormatError, and generic exceptions.
In `@packages/nmp_common/src/nmp/common/auth/signing_keys.py`:
- Around line 55-86: Update the signing-key loading flow around _file_cache_key,
path.read_bytes, and serialization.load_pem_private_key to catch
missing/unreadable file errors and PEM parsing or decryption failures, mapping
configuration/path failures to missing_private_key_message and malformed or
unusable key content to invalid_private_key_message. Preserve the existing RSA
type validation, cache behavior, and RSASigningKey construction for valid keys.
In `@packages/nmp_common/tests/auth/test_api_keys.py`:
- Around line 40-44: The JWKS URI assertions in
test_api_key_jwks_uri_uses_canonical_auth_jwks_path conflict with the Envoy
platform_api_key provider contract. Reconcile api_key_jwks_uri and its test with
the gateway’s expected /apis/auth/v2/api-keys/jwks path, updating the stale
implementation or assertion while preserving consistent API-key validation
behavior.
In `@tests/auth_idp/contracts/test_api_keys.py`:
- Around line 87-93: Update the post-grant allow check using allowed_response to
retry the workspace request with a short backoff before asserting status 200.
Preserve the existing authorization headers, timeout, verify settings, and
response-text assertion while allowing eventual consistency after the role
grant.
In `@tests/auth_idp/static/test_provider_layout.py`:
- Around line 154-160: Update the platform_api_key remote JWKS URI assertion to
use the canonical path returned by api_key_jwks_uri(), removing the /api-keys/
segment. Keep the issuer, audience, host, and other remote_jwks settings
unchanged.
---
Duplicate comments:
In `@services/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.py`:
- Around line 60-69: Update create_api_key to catch
APIKeyOperationNotImplementedError from issuer.create and convert it using the
same 501 response handling as list_api_keys and revoke_api_key, while preserving
the existing APIKeyFeatureDisabledError and RuntimeError handling.
---
Nitpick comments:
In `@packages/nmp_common/src/nmp/common/auth/middleware.py`:
- Around line 414-420: Replace the Any annotation on claims in
_handle_validated_bearer_claims with a shared structural Protocol exposing
subject, email, groups, and scopes, and define or reuse that Protocol where the
middleware types are shared. Keep the Protocol independent of TokenClaims and
the OIDC claims class while preserving the existing attribute access and
behavior.
In `@packages/nmp_common/tests/auth/test_api_keys.py`:
- Around line 326-331: Update the test exercising APIKeyIssuerService.create by
removing async from
test_validate_static_api_key_token_rejects_service_principal_subject and
renaming it to test_api_key_issuer_service_rejects_service_principal_subject.
Preserve the existing setup and assertion behavior.
- Around line 168-185: Replace the ad hoc clear_static_api_key_signing_key_cache
calls in the affected tests with an autouse fixture that clears the cache before
each test and again after it yields. Keep the existing cache assertions
unchanged, and ensure the fixture is applied to all tests in the module so
cached signing keys cannot leak between tests.
In `@services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py`:
- Around line 216-242: Add async workload-key helpers to
WorkloadTokenExchangeService that delegate to the cache’s asynchronous
equivalents, preserving the existing key IDs, file paths, and error messages.
Update auth_jwks_response to await the async public-JWK helper and
token_exchange to await the async signing-key helper, removing synchronous
RSASigningKeyCache access from both request paths.
In `@services/core/auth/tests/test_authenticate.py`:
- Around line 154-160: Update
test_api_key_specific_authenticate_route_is_removed to validate the application
route table rather than posting to an unmounted path. Inspect the real app’s
registered routes and assert that the api-keys-specific authenticate route is
absent, or mount the api-keys router so the request can distinguish route
removal from router absence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c0c21053-8aab-46f7-abb3-3cbccf4be822
⛔ Files ignored due to path filters (31)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/manifest_registry.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/manifest.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/api_keys.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/jwks.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/auth/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/auth/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/auth/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_create_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_create_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_jwks_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_list_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_metadata_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/auth/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/auth/authenticate_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/api_keys/test_jwks.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/auth/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_api_keys.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_agent.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/test_app.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (58)
contrib/auth/authentik/compose/docker-compose.ymlcontrib/auth/authentik/compose/implementation-details.mdcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/helm/templates/_envoy-config.tplcontrib/auth/authentik/helm/values.yamlcontrib/auth/authentik/kubernetes/implementation-details.mdcontrib/auth/authentik/manifest.yamldocs/auth/authentication/using-authentication.mdxdocs/auth/deployment/configuration.mdxdocs/cli/reference.mdxdocs/fern/snippets/_snippets/cli-summary.mdxdocs/set-up/config-reference.mdxe2e/authz_oidc/conftest.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/manifest_registry.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/manifest.pypackages/nemo_platform_ext/tests/cli/commands/test_agent.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/cli/test_app.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/__init__.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/endpoints.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/issuer.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/types.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/method.pypackages/nemo_platform_plugin/tests/auth/api_keys/test_client.pypackages/nemo_platform_plugin/tests/auth/api_keys/test_endpoints.pypackages/nmp_common/src/nmp/common/auth/__init__.pypackages/nmp_common/src/nmp/common/auth/api_keys.pypackages/nmp_common/src/nmp/common/auth/bearer.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/auth/signing_keys.pypackages/nmp_common/src/nmp/common/config/base.pypackages/nmp_common/tests/auth/test_api_keys.pypackages/nmp_common/tests/auth/test_bearer.pypackages/nmp_common/tests/auth/test_middleware.pypackages/nmp_common/tests/auth/test_signing_keys.pyplugins/nemo-customizer/openapi/openapi.yamlservices/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/api_keys/schemas.pyservices/core/auth/src/nmp/core/auth/api/v2/authenticate.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/integration/test_static_api_keys.pyservices/core/auth/tests/test_api_keys.pyservices/core/auth/tests/test_authenticate.pyservices/core/auth/tests/test_workload_token_exchange.pytests/auth_idp/authentik_live.pytests/auth_idp/contracts/test_api_keys.pytests/auth_idp/runtime_kubernetes.pytests/auth_idp/static/test_authentik_kubernetes_demo.pytests/auth_idp/static/test_provider_layout.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/cli_generator/cli_config.yaml
💤 Files with no reviewable changes (1)
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/manifest_registry.py
🚧 Files skipped from review as they are similar to previous changes (37)
- packages/nemo_platform_ext/tests/cli/commands/test_agent.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/issuer.py
- packages/nmp_common/src/nmp/common/auth/init.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/manifest.py
- contrib/auth/authentik/manifest.yaml
- docs/set-up/config-reference.mdx
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/method.py
- packages/nmp_common/tests/auth/test_bearer.py
- docs/fern/snippets/_snippets/cli-summary.mdx
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/cli_generator/cli_config.yaml
- contrib/auth/authentik/compose/implementation-details.md
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/endpoints.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/client.py
- contrib/auth/authentik/kubernetes/implementation-details.md
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- packages/nemo_platform_ext/tests/cli/test_app.py
- contrib/auth/authentik/gateway/envoy.yaml
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/init.py
- services/core/auth/src/nmp/core/auth/service.py
- tests/auth_idp/authentik_live.py
- e2e/authz_oidc/conftest.py
- packages/nemo_platform_plugin/tests/auth/api_keys/test_client.py
- packages/nemo_platform_plugin/tests/auth/api_keys/test_endpoints.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/types.py
- tests/auth_idp/runtime_kubernetes.py
- docs/auth/deployment/configuration.mdx
- packages/nmp_common/tests/auth/test_signing_keys.py
- tests/auth_idp/static/test_authentik_kubernetes_demo.py
- services/core/auth/src/nmp/core/auth/api/v2/api_keys/schemas.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- docs/cli/reference.mdx
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- contrib/auth/authentik/helm/templates/_envoy-config.tpl
- services/core/auth/tests/test_workload_token_exchange.py
- services/core/auth/src/nmp/core/auth/api/v2/authenticate.py
- packages/nmp_common/src/nmp/common/auth/api_keys.py
- packages/nmp_common/src/nmp/common/config/base.py
d711d00 to
9b67383
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py (1)
338-347: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUse the async signing-key path for the workload JWK too.
public_jwkreads the PEM synchronously on cache miss, blocking the event loop, while the API-key branch already usespublic_jwk_from_private_key_pem_async.RSASigningKeyCache.public_jwk_from_file_asyncexists — route the workload branch through it for symmetry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py` around lines 338 - 347, Update auth_jwks_response to obtain the workload token exchange JWK through the asynchronous signing-key path, using RSASigningKeyCache.public_jwk_from_file_async instead of workload_token_exchange_service.public_jwk. Preserve the existing configuration guard and JWK deduplication/response construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openapi/ga/openapi.yaml`:
- Around line 8906-8921: Update the source model for AuthenticateResponse so its
email and jti fields accept null in addition to strings, then regenerate the
OpenAPI specification. Ensure the generated schema marks both email and jti as
nullable while preserving their existing string definitions.
In `@packages/nmp_common/src/nmp/common/auth/jwks.py`:
- Around line 46-54: Update get_signing_key_from_jwt so a cached lookup
refreshes JWKS only when signing_jwk_from_jwks indicates an unknown key ID;
immediately re-raise malformed or missing-kid InvalidTokenError cases. Add
shared concurrency coalescing for the forced refresh and rate-limit repeated
unknown-kid refresh attempts, while preserving normal cache-hit and cache-miss
behavior.
In `@tests/auth_idp/static/test_provider_layout.py`:
- Around line 154-168: Update the platform_api_key JWKS URI and related
bypass-rule assertion in tests/auth_idp/static/test_provider_layout.py:154-168
to use the canonical /apis/auth/jwks path, and update the corresponding Envoy
values in contrib/auth/authentik/gateway/envoy.yaml and _envoy-config.tpl. Leave
packages/nmp_common/tests/auth/test_api_keys.py:43-47 unchanged because the
canonical service path is correct.
---
Nitpick comments:
In `@services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py`:
- Around line 338-347: Update auth_jwks_response to obtain the workload token
exchange JWK through the asynchronous signing-key path, using
RSASigningKeyCache.public_jwk_from_file_async instead of
workload_token_exchange_service.public_jwk. Preserve the existing configuration
guard and JWK deduplication/response construction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9c0dd27d-ea5f-4ae9-a04d-be926a37bf32
⛔ Files ignored due to path filters (29)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/manifest_registry.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/manifest.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/api_keys/api_keys.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/auth/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/auth/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/auth/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/inference/virtual_models.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_create_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_create_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_list_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/api_keys/api_key_metadata_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/auth/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/auth/authenticate_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/api_keys/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/auth/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_api_keys.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_agent.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/test_app.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (61)
contrib/auth/authentik/compose/docker-compose.ymlcontrib/auth/authentik/compose/implementation-details.mdcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/helm/templates/_envoy-config.tplcontrib/auth/authentik/helm/values.yamlcontrib/auth/authentik/kubernetes/implementation-details.mdcontrib/auth/authentik/manifest.yamldocs/auth/authentication/using-authentication.mdxdocs/auth/deployment/configuration.mdxdocs/cli/reference.mdxdocs/fern/snippets/_snippets/cli-summary.mdxdocs/set-up/config-reference.mdxe2e/authz_oidc/conftest.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/manifest_registry.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/manifest.pypackages/nemo_platform_ext/tests/cli/commands/test_agent.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/cli/test_app.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/__init__.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/endpoints.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/issuer.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/types.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/method.pypackages/nemo_platform_plugin/tests/auth/api_keys/test_client.pypackages/nemo_platform_plugin/tests/auth/api_keys/test_endpoints.pypackages/nmp_common/src/nmp/common/auth/__init__.pypackages/nmp_common/src/nmp/common/auth/api_keys.pypackages/nmp_common/src/nmp/common/auth/bearer.pypackages/nmp_common/src/nmp/common/auth/jwks.pypackages/nmp_common/src/nmp/common/auth/jwt.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/auth/signing_keys.pypackages/nmp_common/src/nmp/common/config/base.pypackages/nmp_common/tests/auth/test_api_keys.pypackages/nmp_common/tests/auth/test_bearer.pypackages/nmp_common/tests/auth/test_jwt.pypackages/nmp_common/tests/auth/test_middleware.pypackages/nmp_common/tests/auth/test_signing_keys.pyplugins/nemo-customizer/openapi/openapi.yamlservices/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/api_keys/schemas.pyservices/core/auth/src/nmp/core/auth/api/v2/authenticate.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/integration/test_static_api_keys.pyservices/core/auth/tests/test_api_keys.pyservices/core/auth/tests/test_authenticate.pyservices/core/auth/tests/test_workload_token_exchange.pytests/auth_idp/authentik_live.pytests/auth_idp/contracts/test_api_keys.pytests/auth_idp/runtime_kubernetes.pytests/auth_idp/static/test_authentik_kubernetes_demo.pytests/auth_idp/static/test_provider_layout.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/cli_generator/cli_config.yaml
💤 Files with no reviewable changes (1)
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/manifest_registry.py
🚧 Files skipped from review as they are similar to previous changes (44)
- contrib/auth/authentik/manifest.yaml
- contrib/auth/authentik/compose/docker-compose.yml
- packages/nemo_platform_ext/tests/cli/commands/test_agent.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/issuer.py
- tests/auth_idp/runtime_kubernetes.py
- packages/nmp_common/src/nmp/common/auth/bearer.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- docs/fern/snippets/_snippets/cli-summary.mdx
- services/core/auth/src/nmp/core/auth/api/v2/api_keys/schemas.py
- packages/nmp_common/src/nmp/common/auth/init.py
- packages/nemo_platform_plugin/tests/auth/api_keys/test_endpoints.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/method.py
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/cli_generator/cli_config.yaml
- contrib/auth/authentik/gateway/envoy.yaml
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/manifest.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/endpoints.py
- docs/cli/reference.mdx
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/init.py
- contrib/auth/authentik/helm/values.yaml
- docs/auth/deployment/configuration.mdx
- services/core/auth/src/nmp/core/auth/service.py
- packages/nmp_common/tests/auth/test_bearer.py
- packages/nmp_common/src/nmp/common/config/base.py
- contrib/auth/authentik/compose/implementation-details.md
- packages/nemo_platform_plugin/tests/auth/api_keys/test_client.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/types.py
- contrib/auth/authentik/helm/templates/_envoy-config.tpl
- services/core/auth/tests/test_workload_token_exchange.py
- contrib/auth/authentik/kubernetes/implementation-details.md
- packages/nmp_common/src/nmp/common/auth/middleware.py
- docs/auth/authentication/using-authentication.mdx
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- packages/nmp_common/tests/auth/test_signing_keys.py
- services/core/auth/src/nmp/core/auth/api/v2/authenticate.py
- tests/auth_idp/static/test_authentik_kubernetes_demo.py
- services/core/auth/src/nmp/core/auth/api/v2/api_keys/endpoints.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/auth/api_keys/client.py
- packages/nmp_common/src/nmp/common/auth/signing_keys.py
- packages/nemo_platform_ext/tests/cli/test_app.py
- packages/nmp_common/src/nmp/common/auth/api_keys.py
- packages/nmp_common/tests/auth/test_middleware.py
- openapi/ga/individual/platform.openapi.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
👮 Files not reviewed due to content moderation or server errors (1)
- openapi/openapi.yaml
8852596 to
df762a3
Compare
Signed-off-by: Ryan S <267728323+ironcommit@users.noreply.github.com>
df762a3 to
1c8758b
Compare
mckornfield
left a comment
There was a problem hiding this comment.
oh boy
the word static is getting me. other types of keys that aren't "static", like oauth etc. aren't considered API keys though, right? they're tokens/ bearer tokens?
I guess I just don't see the point in the word static lol
| https://nmp.company.com/apis/entities/v2/workspaces | ||
| ``` | ||
|
|
||
| V1 static API keys are signed JWTs. They can have an optional expiration, but |
There was a problem hiding this comment.
v1? not sure that matches any versioning we currently have does it?
| List-valued env vars for API-key accepted formats are written as comma-separated | ||
| values, for example `NMP_AUTH_API_KEYS__ACCEPTED_FORMATS=jwt`. | ||
|
|
||
| Finite API keys are capped at 30 days by default. Set |
There was a problem hiding this comment.
finite?
Also is 30 days the default time before we have to reset? not 60/90?
| # Shared token signing configuration for NeMo Platform-minted JWTs. | ||
| token_signing: | ||
| # Shared issuer for NeMo Platform-minted JWTs. Defaults to <platform.base_url>/apis/auth. | ||
| issuer: |
There was a problem hiding this comment.
do we prefer this over null ?
| ) -> None: | ||
| """Revoke a static API key (stateless V1 not implemented). | ||
|
|
||
| V1 static API keys are stateless and not persisted, so this command currently reports that revocation is not implemented. |
|
|
||
|
|
||
| class APIKeyMetadataResponse(BaseModel): | ||
| """Metadata for a static API key.""" |
There was a problem hiding this comment.
Are these comments required? some of them are just the names of the class lol
| status_code=status_code, | ||
| content={"detail": "Unauthorized" if status_code == 401 else "Forbidden"}, | ||
| ) | ||
| if error_response := await self._call_pdp( |
| description="Expected audience for NeMo Platform static API key JWTs.", | ||
| ) | ||
| max_expires_in_seconds: int | None = Field( | ||
| default=30 * 24 * 60 * 60, |
There was a problem hiding this comment.
was it in the RFC or somewhere that 30 days was the default?
| return jwt.encode({"sub": "user"}, private_key, algorithm="RS256", headers=headers) | ||
|
|
||
|
|
||
| class FakeResponse: |
There was a problem hiding this comment.
Saw in the other test file a FakeResponse, but declared inline. Wonder if these should be the standard or go in a pytest conf (might be different response/client objects, but still)
There was a problem hiding this comment.
hmm is this diff expected?
| retrieve: get /apis/intake/v2/workspaces/{workspace}/experiments/{name} | ||
| update: put /apis/intake/v2/workspaces/{workspace}/experiments/{name} | ||
| delete: delete /apis/intake/v2/workspaces/{workspace}/experiments/{name} | ||
| auth: |
There was a problem hiding this comment.
wtb deleting stainless
Summary by CodeRabbit
/apis/auth/authenticate(Bearer callout) and/apis/auth/v2/api-keysmanagement (create/list/revoke).nemo auth api-keys {create,list,revoke}in the CLI.