Skip to content

Expose authorization header in Swagger API docs#18974

Merged
Jackie-Jiang merged 1 commit into
apache:masterfrom
Vamsi-klu:fix-swagger-auth-security-definition
Jul 16, 2026
Merged

Expose authorization header in Swagger API docs#18974
Jackie-Jiang merged 1 commit into
apache:masterfrom
Vamsi-klu:fix-swagger-auth-security-definition

Conversation

@Vamsi-klu

@Vamsi-klu Vamsi-klu commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #7805

What changed

This PR adds the configured authorization header to Pinot's generated Swagger API documentation.

SwaggerSetupUtils now builds a Swagger security definition for the existing Authorization header and applies that security requirement to the generated API config. The runtime authentication behavior is unchanged; this only makes the OpenAPI/Swagger description accurately expose the header that secured Pinot APIs already expect.

The PR also extracts the Swagger config construction into a testable helper so the generated API metadata can be validated without starting a controller.

Why it matters

When Swagger does not describe the authorization header, users can see secured endpoints in the API docs but have no standard way to supply credentials from the Swagger UI or generated clients. That makes secured controller APIs harder to try, document, and integrate with.

Pinot already has a shared Authorization header constant for Swagger auth. Surfacing it in the Swagger config aligns the documentation with the actual authentication contract and avoids forcing users to infer or manually patch the header into requests.

Impact

Swagger/OpenAPI consumers now see an API-key-style Authorization header security definition.

Swagger UI users can provide the authorization header through the documented security mechanism instead of manually editing each request.

Existing runtime auth behavior, filters, and controller authorization checks are not changed.

Testing

  • ./mvnw -pl pinot-common -Dtest=SwaggerSetupUtilsTest test
  • ./mvnw -pl pinot-common spotless:apply
  • ./mvnw -pl pinot-common checkstyle:check
  • ./mvnw -pl pinot-common license:check
  • git diff --check

The test verifies that the generated Swagger config contains the Authorization header security definition, uses the expected API key auth scheme, and attaches the security requirement to the API config.


Drafted-by: Codex (GPT-5); human-reviewed by Vamsi-klu before marking ready for review

@Vamsi-klu
Vamsi-klu marked this pull request as ready for review July 11, 2026 23:38
@Vamsi-klu

Copy link
Copy Markdown
Contributor Author

Review request: @apucher @Jackie-Jiang

GitHub did not allow this account/integration to add requested reviewers through the reviewer section, so I am tagging the relevant reviewers here for visibility.

This PR fixes #7805 by adding the configured Authorization header security definition to Pinot's generated Swagger API documentation and covering the generated config with SwaggerSetupUtilsTest. Review from the Swagger/controller-auth side would be especially useful to confirm the security-definition shape and whether this is the right central place to expose the header across generated API docs.


Drafted-by: Codex (GPT-5); human-reviewed by Vamsi-klu before posting

@xiangfu0

Copy link
Copy Markdown
Contributor

Can you add the UI screenshot for the testing

@codecov-commenter

codecov-commenter commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.05%. Comparing base (e0210aa) to head (2906acc).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18974      +/-   ##
============================================
- Coverage     65.06%   65.05%   -0.01%     
  Complexity     1403     1403              
============================================
  Files          3399     3399              
  Lines        212797   212805       +8     
  Branches      33567    33567              
============================================
- Hits         138457   138449       -8     
- Misses        63204    63206       +2     
- Partials      11136    11150      +14     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 65.05% <100.00%> (-0.01%) ⬇️
temurin 65.05% <100.00%> (-0.01%) ⬇️
unittests 65.05% <100.00%> (-0.01%) ⬇️
unittests1 56.99% <37.50%> (-0.02%) ⬇️
unittests2 37.39% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Vamsi-klu

Copy link
Copy Markdown
Contributor Author

@xiangfu0 thanks for asking for the UI screenshot. I captured this from the latest PR branch/commit.

Screenshot proof

  1. Swagger UI uses the generated Pinot Controller spec

    • Shows: live Pinot Controller Swagger UI loaded from /swagger.json, with the Authorize control visible.
    • Addresses: request for UI screenshot of the testing.
    • Screenshot: Pinot Swagger UI with Authorize
  2. Authorize dialog exposes the Authorization header

    • Shows: oauth (apiKey) with Name: Authorization and In: header; no real token is entered.
    • Addresses: verifies the generated security definition is visible through the UI.
    • Screenshot: Pinot Swagger authorize dialog

Validation

  • Branch/commit: fix-swagger-auth-security-definition / 2906acca573707ddc5735e3c2ca75753e171fbb1
  • Browser/viewport: Chromium, 1440x1100
  • Manual flow: pinot-tools quick-start-batch -> http://localhost:9000/api?url=/swagger.json -> Authorize
  • Local check: ./mvnw -pl pinot-common -Dtest=SwaggerSetupUtilsTest test passed (1 test)
  • Package/runtime check: ./mvnw -pl pinot-tools -am package -DskipTests passed, and the packaged quickstart served the UI
  • API payload check: curl -fsS http://localhost:9000/swagger.json | jq '.securityDefinitions.oauth' showed type: apiKey, name: Authorization, in: header; endpoint security includes oauth (242 operations, including /users)
  • CI: current PR checks are passing (11 listed checks)

@Jackie-Jiang Jackie-Jiang added ui UI related issue rest-api Related to REST API endpoints labels Jul 16, 2026
@Jackie-Jiang
Jackie-Jiang merged commit 00d1c88 into apache:master Jul 16, 2026
11 checks passed
@xiangfu0

Copy link
Copy Markdown
Contributor

Opened docs follow-up PR: pinot-contrib/pinot-docs#920

xiangfu0 added a commit to pinot-contrib/pinot-docs that referenced this pull request Jul 16, 2026
xiangfu0 added a commit to pinot-contrib/pinot-docs that referenced this pull request Jul 16, 2026
## What changed
- documented how authenticated controller Swagger sessions now use the
standard `Authorize` dialog to set the `Authorization` header
- clarified that users can enter either `Basic <token>` or `Bearer
<token>` there, depending on deployment auth

## Why
apache/pinot#18974 exposed Pinot's existing `Authorization` header in
the generated Swagger/OpenAPI security definition.

## Reader impact
Users of the controller admin UI no longer need to infer or hand-edit
the auth header for every Swagger request.

## Source cross-check
Verified against local `apache/pinot` merge commit
`00d1c88763d4015b9ce966ad7c13a52abf30f9b2`.

## Validation
- `git diff --check`
- lightweight local link validation for the edited files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rest-api Related to REST API endpoints ui UI related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security definition is missing, if auth enabled swagger API's won't work

4 participants