[codex] add user token management contract - #36
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe OpenAPI spec gains nine new endpoints under ChangesPortal Management Token Endpoints
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/test_user_token_contract.py (1)
61-70: ⚡ Quick winAssert the error contract and array wrapper too.
This test validates tags/operationIds, but it does not enforce the PR’s 403 response contract, and the list response check can pass without proving the top-level schema is an array.
Suggested test hardening
for path, methods in expected_operations.items(): for method, operation_id in methods: operation = spec["paths"][path][method] self.assertEqual(operation_id, operation["operationId"]) self.assertEqual(["Portal Management"], operation["tags"]) + self.assertEqual( + "`#/components/schemas/ExceptionResponse`", + operation["responses"]["403"]["content"]["application/json"]["schema"]["$ref"], + ) list_tokens = spec["paths"]["/openapi/v1/user-tokens"]["get"] + list_tokens_schema = list_tokens["responses"]["200"]["content"]["application/json"]["schema"] + self.assertEqual("array", list_tokens_schema["type"]) self.assertEqual( {"type": "object"}, - list_tokens["responses"]["200"]["content"]["application/json"]["schema"]["items"], + list_tokens_schema["items"], )🤖 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/test_user_token_contract.py` around lines 61 - 70, The test validates operationIds and tags but is missing two important assertions. First, add assertions to verify the 403 response contract for the error handling introduced in the PR. Second, strengthen the list_tokens response validation by asserting not only the items schema but also that the top-level schema itself is an array type wrapper, so the test cannot pass without proving the complete response structure matches the contract. Modify the assertions starting from where list_tokens is defined to include both the 403 error response validation and the top-level array schema validation for the 200 response.apollo-openapi.yaml (1)
4907-4910: 🏗️ Heavy liftUse named schemas instead of anonymous
objectpayloads.The PR goal is a generated
PortalManagementApicontract, but these bodies will generate rawObject/map-like signatures and do not document required token fields. Add reusable component schemas for token summaries, create/rotate responses, create requests, and capabilities, then updatetests/test_user_token_contract.pyLines 67-80 to assert those$refs.Example shape for the contract refactor
type: array items: - type: object + $ref: '`#/components/schemas/OpenUserTokenSummary`' @@ schema: - type: object + $ref: '`#/components/schemas/OpenCreateUserTokenRequest`' @@ schema: - type: object + $ref: '`#/components/schemas/OpenCreateUserTokenResponse`'Also applies to: 4927-4928, 4935-4936, 5011-5012, 5032-5033, 5065-5068
🤖 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 `@apollo-openapi.yaml` around lines 4907 - 4910, Replace the anonymous `type: object` schema declarations in apollo-openapi.yaml with named reusable component schemas to properly document the PortalManagementApi contract. Create new component schemas for token summaries, create/rotate responses, create requests, and capabilities in the components/schemas section, then replace all instances of anonymous object payloads (at the locations mentioned in the comment) with $ref references pointing to these named schemas. Finally, update tests/test_user_token_contract.py Lines 67-80 to assert that these $ref references exist in the contract, ensuring the API documentation properly specifies required token fields rather than generating raw Object/map-like signatures.
🤖 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 `@apollo-openapi.yaml`:
- Around line 4894-5122: The nine Portal Management operations (listUserTokens,
createUserToken, deleteUserToken, revokeUserToken, rotateUserToken,
getUserTokenCapabilities, adminListUserTokens, adminDeleteUserToken,
adminRevokeUserToken) are currently inheriting the global ApiKeyAuth security
scheme but their descriptions indicate they require Portal login-state access.
Define a new Portal session security scheme in the components/securitySchemes
section, then add an explicit security property to each of these nine operations
to override the inherited global security and apply the Portal session scheme
instead.
---
Nitpick comments:
In `@apollo-openapi.yaml`:
- Around line 4907-4910: Replace the anonymous `type: object` schema
declarations in apollo-openapi.yaml with named reusable component schemas to
properly document the PortalManagementApi contract. Create new component schemas
for token summaries, create/rotate responses, create requests, and capabilities
in the components/schemas section, then replace all instances of anonymous
object payloads (at the locations mentioned in the comment) with $ref references
pointing to these named schemas. Finally, update
tests/test_user_token_contract.py Lines 67-80 to assert that these $ref
references exist in the contract, ensuring the API documentation properly
specifies required token fields rather than generating raw Object/map-like
signatures.
In `@tests/test_user_token_contract.py`:
- Around line 61-70: The test validates operationIds and tags but is missing two
important assertions. First, add assertions to verify the 403 response contract
for the error handling introduced in the PR. Second, strengthen the list_tokens
response validation by asserting not only the items schema but also that the
top-level schema itself is an array type wrapper, so the test cannot pass
without proving the complete response structure matches the contract. Modify the
assertions starting from where list_tokens is defined to include both the 403
error response validation and the top-level array schema validation for the 200
response.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d1aaa551-9a77-43b4-8015-c3fa5edf4b31
📒 Files selected for processing (2)
apollo-openapi.yamltests/test_user_token_contract.py
There was a problem hiding this comment.
Pull request overview
This PR extends the shared OpenAPI spec to cover Apollo Portal “user access token” management so Apollo Portal can implement the endpoints via the generated PortalManagementApi interface.
Changes:
- Added Portal-session-authenticated user-token management endpoints under
/openapi/v1/user-tokens(including admin variants) taggedPortal Management. - Introduced a cookie-based
PortalSessionAuthsecurity scheme for Portal Management operations. - Added new request/response schemas for creating/rotating tokens and describing token capability/summary, plus contract tests to validate the new paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
apollo-openapi.yaml |
Adds the new Portal Management user-token endpoints, security scheme, and related schemas. |
tests/test_user_token_contract.py |
Adds contract assertions for operationIds/tags/security and key schema shapes for the new endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What changed
This PR adds the Portal user token management endpoints to the OpenAPI contract:
GET /openapi/v1/user-tokensPOST /openapi/v1/user-tokensDELETE /openapi/v1/user-tokens/{tokenId}POST /openapi/v1/user-tokens/{tokenId}/revokePOST /openapi/v1/user-tokens/{tokenId}/rotateGET /openapi/v1/user-tokens/capabilitiesGET /openapi/v1/user-tokens/adminDELETE /openapi/v1/user-tokens/admin/{tokenId}POST /openapi/v1/user-tokens/admin/{tokenId}/revokeThe endpoints are tagged as
Portal Managementso Apollo Portal can implement them through the generatedPortalManagementApiinterface instead of a separate hand-written web API contract.Why
Apollo PR apolloconfig/apollo#5632 adds Portal user access tokens. The implementation needs these management endpoints to be represented in the shared OpenAPI contract before Apollo Portal can pin a released spec tag and add generated-interface overrides.
Validation
PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=/tmp/codex-apollo-openapi-pyyaml python3 -m unittest discover tests./generate.sh --verifygit diff --checkSummary by CodeRabbit