fix(api/swagger): send Authorization header from Swagger UI#29676
fix(api/swagger): send Authorization header from Swagger UI#29676abhay-codes07 wants to merge 2 commits into
Conversation
Endpoints document their auth with `@ApiHeader({ name: "Authorization" })`,
but the OpenAPI spec mandates that a header parameter named "Authorization"
be ignored, so Swagger UI shows the field yet never sends the header. Every
"try it out" request then fails with 401/403 even though the user filled it
in.
Register a proper bearer security scheme on the document and, for each
operation that declared the ignored Authorization header, replace that
parameter with the bearer security requirement. The "Authorize" button now
attaches `Authorization: Bearer <token>` to requests. Genuinely public
endpoints (no Authorization header) are left untouched.
The replacement runs centrally in generate-swagger.ts, so no per-controller
changes are needed. Adds unit tests for the applyAuthorizationSecurity
post-processing.
Fixes calcom#29564
|
Welcome to Cal.diy, @abhay-codes07! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/api/v2/src/swagger/generate-swagger.ts`:
- Around line 84-101: The swagger generation logic in generate-swagger.ts only
inspects operation.parameters, so shared pathItem.parameters with an
Authorization header are missed. Update the method loop that processes each path
item to also consider path-level parameters when checking for the Authorization
header, and make sure the bearer security requirement is added whenever either
the operation or the path item declares that header. Keep the existing
isAuthorizationHeader helper and the operation.parameters filtering, but extend
the logic around HttpMethods.forEach and pathItem so the shared parameter is
handled too.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a62da29-9c21-45a2-890a-e060782e40cf
📒 Files selected for processing (2)
apps/api/v2/src/swagger/generate-swagger.spec.tsapps/api/v2/src/swagger/generate-swagger.ts
OpenAPI allows header parameters to be declared at the path level, where they are inherited by every operation in the path. Extend applyAuthorizationSecurity to detect an inherited path-level Authorization header, apply the bearer security requirement to each operation in the path, and drop the path-level parameter. Adds a unit test for the path-level case.
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
What does this PR do?
Fixes Swagger UI silently dropping the
Authorizationheader so "Try it out" requests no longer fail with 401/403.API v2 endpoints document authentication with
@ApiHeader({ name: "Authorization" }). The OpenAPI specification, however, mandates that a header parameter namedAuthorizationbe ignored, so Swagger UI renders the input field and accepts a value but never attaches the header to the outgoing request. Users fill inBearer cal_xxx, hit Execute, and every authenticated call fails even though the UI suggested the token would be sent.The fix, all contained in
apps/api/v2/src/swagger/generate-swagger.ts:addBearerAuth), which gives Swagger UI a working Authorize button.applyAuthorizationSecurity(), a post-processing step that, for every operation which declared the (ignored)Authorizationheader, removes that parameter and attaches the bearer security requirement instead — so Swagger UI actually sendsAuthorization: Bearer <token>.Operations that never declared an
Authorizationheader (genuinely public endpoints) are left untouched, and any pre-existing per-operationsecurityis preserved (the requirement is appended, not overwritten). Because the transform runs centrally, no per-controller changes are needed.Authorizationheader defined with@ApiHeader#29564Visual Demo (For contributors especially)
N/A — this changes the generated OpenAPI document / Swagger UI behaviour. The resulting document now contains a
components.securitySchemes.Authorizationbearer scheme and asecurityrequirement on the affected operations, which is exactly what makes Swagger UI attach the header. This is asserted by the unit tests below.Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
No environment variables or DB are required for the unit tests —
applyAuthorizationSecurityis a pure function over an OpenAPI document.cd apps/api/v2 yarn jest src/swagger/generate-swagger.spec.tsExpected: 5 tests pass, covering:
Authorizationheader → the header parameter is removed and a{ Authorization: [] }security requirement is added.Authorizationheader) → left untouched.security→ the requirement is appended, not overwritten.$refparameters → handled without throwing.paths→ returned unchanged.Manual verification in Swagger UI: open
/docs, click Authorize, enter an API key / access token, then Execute any authenticated endpoint and confirm the generated curl now includes-H 'Authorization: Bearer <token>'.Checklist