Fix identity extraction in cedar_guard middleware & correct revoke_bot_token OpenAPI docs#2028
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce5d151b77
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR aims to preserve correct Bot/Human identity in the cedar_guard authorization middleware by running Axum extractors inside the middleware, and to align the OpenAPI documentation for revoke_bot_token with its idempotent implementation.
Changes:
- Update
cedar_guardto extractBotIdentity/LoginUserviareq.extract::<...>().await. - Fix OpenAPI 404 response description for
revoke_bot_tokento “Bot not found”. - Update router module declarations (currently introduces a duplicate module entry).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| mono/src/api/router/mod.rs | Router module exports (currently has a duplicate bot_router module line). |
| mono/src/api/router/bot_router.rs | OpenAPI response description for revoke_bot_token 404 updated. |
| mono/src/api/guard/cedar_guard.rs | Middleware now actively runs identity extractors to determine Cedar principal. |
| jupiter/src/storage/bots_storage.rs | Adds bot token helper functions, but currently duplicates existing helpers causing compile errors. |
Signed-off-by: Hongze Gao <15101764808@163.com>
- BotsStorage: add get_bot_by_id() for bot existence checks - BotsStorage: use update_many() in revoke_bot_tokens_by_bot to avoid N+1 - BotsStorage: validate HMAC secret (non-empty, min 32 chars) in load_bot_token_hmac_key - BotsStorage: use DateTimeWithTimeZone for expiry comparison in find_bot_by_token - AuditStorage: set created_at with DateTimeWithTimeZone (Utc::now().into()) - bot_router: ensure bot exists before create/list/revoke; return 404 when missing - bot_router: validate expires_in range (1..=10y) to avoid overflow/panic - tests: add audit_storage to AppService in test_storage() Signed-off-by: Hongze Gao <15101764808@163.com>
…dleware and revoke_bot_token 404 OpenAPI docs - Use req.extract() to resolve LoginUser/BotIdentity instead of direct extensions access - Fix missing Bot/Human context and incorrect "reader" fallback - Update 404 description to "Bot not found" - Align docs with idempotent implementation Signed-off-by: Hongze Gao <15101764808@163.com>
WiedersehenM
force-pushed
the
feature/bot-token
branch
from
ce5d151 to
974ed6f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 974ed6f158
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ule export - remove duplicated in - deduplicate bot token HMAC helpers in to fix E0428 - short-circuit auth extraction in : try first, then only if needed Signed-off-by: Hongze Gao <15101764808@163.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccb6e638e1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…d middleware - replace state-less request extraction in with for both and - fix incorrect import by removing and importing instead - keep short-circuit principal resolution: try bot identity first, then attempt login user only when bot extraction fails Signed-off-by: Hongze Gao <15101764808@163.com>
…y extraction in cedar_guard - replace invalid usage with and extract identities from - run first, then fall back to - preserve middleware flow by reconstructing the request with before calling Signed-off-by: Hongze Gao <15101764808@163.com>
…ts directly - replace + with direct matching in - keep principal resolution order unchanged: first, then , then fallback Signed-off-by: Hongze Gao <15101764808@163.com>
Summary
This PR fixes two key issues identified in the code review:
revoke_bot_tokenRelated issue
#1999
Changes Made
1. Fix
cedar_guardmiddleware identity extractionreq.extensions(), butLoginUser/BotIdentitywere not populated before the middleware ran, causing all requests to fall back to the default "reader" user and losing Bot/Human context.req.extract::<LoginUser>().awaitandreq.extract::<BotIdentity>().awaitto actively run extractors inside the middleware.2. Correct
revoke_bot_tokenOpenAPI 404 description404 Bot or token not found, but the implementation is idempotent — token absence does not return 404.Checklist