Skip to content

feat: adds jwt auth token middleware#121

Merged
semmet95 merged 13 commits into
mainfrom
feat/auth-token-middleware
May 29, 2026
Merged

feat: adds jwt auth token middleware#121
semmet95 merged 13 commits into
mainfrom
feat/auth-token-middleware

Conversation

@semmet95
Copy link
Copy Markdown
Contributor

@semmet95 semmet95 commented May 29, 2026

Fixes: #118

Summary by CodeRabbit

  • New Features

    • Added a new /auth/token endpoint to obtain JWT bearer tokens.
  • Changes

    • Migrated authentication from API keys to JWT bearer tokens; requests must include Client-ID and Authorization: Bearer .
    • Server now protects endpoints with JWT middleware; token endpoint is exempt.
  • Tests

    • Added acceptance tests validating auth flows (invalid/expired/mismatched tokens).
  • Chores

    • Introduced required JWT_SECRET configuration.

Review Change Stack

semmet95 added 2 commits May 28, 2026 15:21
Signed-off-by: Amit Singh <singhamitch@outlook.com>
Signed-off-by: Amit Singh <singhamitch@outlook.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

Warning

Review limit reached

@semmet95, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 24 minutes and 44 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: af5de6b7-1125-47e2-8ebf-8eed2a731054

📥 Commits

Reviewing files that changed from the base of the PR and between dd32ee4 and 19698ec.

📒 Files selected for processing (2)
  • acceptance/auth_test.go
  • pkg/middleware/auth.go
📝 Walkthrough

Walkthrough

Authentication migrated from API key to JWT tokens. New /auth/token endpoint issues HS256-signed JWTs with client-ID audience and 4-hour expiration. Client-ID header now required on all operations. Domain layer introduces ClaimFilter type to decouple filtering from API parameters. Router and generated middleware wiring updated for typed parameter binding and header extraction.

Changes

JWT Token Authentication Migration

Layer / File(s) Summary
JWT Authentication Foundation
pkg/conf/conf.go, pkg/conf/conf.yaml, pkg/conf/conf_test.go, go.mod, pkg/middleware/auth.go, cmd/app/main.go
JWT secret loaded from JWT_SECRET config; new AuthTokenMiddleware validates Bearer tokens and enforces Client-ID header with JWT claim validation (audience and issuer); CORS updated to allow Authorization header; conditional JWT auth applied to all routes except /auth/token.
Token Endpoint & Handler
pkg/handlers/auth.go, pkg/http/router.go, pkg/api/server.gen.go
New AuthHandler generates HS256 JWTs with issuer (source-score), audience (client ID), issued-at, and 4-hour expiration; returns 400 if Client-ID header missing, 500 on signing failure, 200 with token on success; router and generated bindings forward the request to the handler.
OpenAPI Spec Updates & Security Schemes
api/source-score.yaml
OpenAPI spec switches global security to BearerAuth, adds ClientIDHeader reusable parameter, and annotates existing operations with required Client-ID header; adds /auth/token operation.
Generated Server Types & Interface Changes
pkg/api/server.gen.go
Generated param structs now include ClientID and ServerInterface methods accept operation-specific params; GetAuthToken added to the interface.
Request Header Binding Middleware
pkg/api/server.gen.go
Generated ServerInterfaceWrapper implementations extract Client-ID header from requests, validate presence, bind into params structs (or abort with 400), and invoke handlers with typed params while setting bearer auth scopes.
ClaimFilter Domain Type
pkg/domain/claim/claim_service.go, pkg/domain/claim/claim_repository.go, pkg/domain/claim/claimfakes/*, pkg/domain/claim/*_test.go
New exported ClaimFilter struct with optional Checked boolean; ClaimService and ClaimRepository interfaces updated to accept *ClaimFilter instead of *api.GetClaimsParams; fakes and tests updated accordingly.
Handler & Router Integration
pkg/http/router.go, pkg/handlers/claim.go, pkg/handlers/swagger.go
Router methods accept typed api.*Params; ClaimHandler.GetClaims constructs domain ClaimFilter from params before delegating to service; Swagger UI preauthorization removed; API-key middleware removed.
Acceptance Tests & Integration
acceptance/acceptance_suite_test.go, acceptance/auth_test.go, acceptance/compose.yaml
New BeforeSuite hook fetches JWT token from /auth/token using Client-ID header, stores it, and sets Authorization: Bearer <token> header for subsequent test requests; added auth-focused acceptance tests; compose file replaced API_KEY with JWT_SECRET.

Sequence Diagram(s)

sequenceDiagram
  participant TestSuite as BeforeSuite
  participant AuthEndpoint as GET /auth/token
  participant AuthHandler
  participant AppServer
  participant JWTAuth as AuthTokenMiddleware
  participant Handler

  TestSuite->>AuthEndpoint: GET /auth/token (Client-ID)
  AuthEndpoint->>AuthHandler: routed
  AuthHandler->>AuthHandler: validate Client-ID
  AuthHandler->>AuthHandler: create & sign JWT (aud=client, iss=source-score, exp=4h)
  AuthHandler->>TestSuite: 200 { "token": "<jwt>" }
  Note over TestSuite: Subsequent requests include Authorization: Bearer <jwt>

  TestSuite->>AppServer: GET /api/v1/claims (Authorization, Client-ID)
  AppServer->>JWTAuth: incoming request
  JWTAuth->>JWTAuth: parse & validate JWT (HS256, aud, iss, exp)
  JWTAuth->>Handler: c.Next() on success
  Handler->>TestSuite: 200 OK [claims]
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes


Possibly related PRs

  • SatyaLens/source-score#119: Modifies GetClaims code path across handlers, router, server.gen, and claim repository/service/fakes—directly overlapping parameter wiring and domain type changes.

Poem

🐰 A secret tucked safe in a key so small,
I hop to the token endpoint, answer the call.
Bearer of JWTs with audience true,
Client-ID in hand, the server says who.
Claims filtered and signed, we dance through the night.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive The PR references issue #118 'Secure the endpoint' but the issue description is empty, making it impossible to validate whether coding requirements are met. Verify that issue #118 contains detailed requirements and confirm the PR implementation aligns with those requirements.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: addition of JWT authentication token middleware throughout the codebase.
Out of Scope Changes check ✅ Passed All changes are within scope of JWT authentication implementation: middleware, handlers, configuration, API spec updates, and related refactoring are cohesive.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/auth-token-middleware

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: Amit Singh <singhamitch@outlook.com>
Signed-off-by: Amit Singh <singhamitch@outlook.com>
…face

Signed-off-by: Amit Singh <singhamitch@outlook.com>
Signed-off-by: Amit Singh <singhamitch@outlook.com>
Signed-off-by: Amit Singh <singhamitch@outlook.com>
Signed-off-by: Amit Singh <singhamitch@outlook.com>
Signed-off-by: Amit Singh <singhamitch@outlook.com>
Signed-off-by: Amit Singh <singhamitch@outlook.com>
@semmet95 semmet95 marked this pull request as ready for review May 29, 2026 11:01
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (1)
pkg/http/router.go (1)

82-85: VerifyClaim is a no-op and returns an empty 200.

It writes no response and doesn't invoke the handler, so callers get an empty success for individual claim verification. This appears to be the existing TODO state, just flagging it now that the signature is touched.

Want me to open an issue to track wiring (or removing) the individual claim-verification endpoint?

🤖 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 `@pkg/http/router.go` around lines 82 - 85, The VerifyClaim handler is
currently a no-op; wire it to call the claim validation logic and return a
proper response instead of an empty 200. Specifically, invoke
r.claimHandler.ValidateClaimByUriDigest(ctx, claimDigest) (or the existing claim
handler method used for bulk verification) and translate its result/error into
an HTTP response using the same pattern your other handlers use (e.g., ctx.JSON
with the validation result on success and ctx.AbortWithStatusJSON or similar on
error), or if individual verification is intentionally unsupported, remove the
endpoint or return a clear 4xx/5xx (e.g., 501 Not Implemented) with an
explanatory body. Ensure you reference VerifyClaim and
r.claimHandler.ValidateClaimByUriDigest when making the change.
🤖 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 `@api/source-score.yaml`:
- Around line 507-515: The OpenAPI spec currently defines only a ClientIDHeader
parameter (components.parameters.ClientIDHeader / name: Client-ID) but omits the
bearer Authorization security scheme; add a components.securitySchemes entry for
a bearer JWT (type: http, scheme: bearer, bearerFormat: JWT) and then apply it
to protected operations via either top-level components.security or
operation-level security arrays so that routes requiring JWT (all secured
endpoints except the explicit /auth/token path) document the required
Authorization: Bearer <token> header while leaving /auth/token unauthenticated
if intended.
- Around line 12-20: The /auth/token endpoint (operationId getAuthToken)
currently issues tokens based solely on the ClientIDHeader parameter — change
the contract and implementation so tokens are only minted after proper client
authentication: replace or augment ClientIDHeader with a strong credential
(e.g., Client-Secret) or require an OAuth2 / external IdP flow (e.g.,
client_credentials grant or authorization code) and validate the secret/IdP
assertion before creating JWTs; ensure the spec documents the required
credential/redirect and that server-side code verifies the credential/IdP token
and enforces audience/scopes before issuing any bearer token.

In `@pkg/conf/conf.go`:
- Line 21: The JwtSecret struct tag currently provides a hardcoded default which
is unsafe; update the JwtSecret field tag (symbol: JwtSecret) to remove
env-default:"default-secret-string" and mark the variable as required (e.g.,
replace the tag with `env:"JWT_SECRET" yaml:"JWT_SECRET" env-required:"true"` or
equivalent used elsewhere), ensuring no fallback default is present so the
service fails to start if JWT_SECRET is unset.

In `@pkg/handlers/auth.go`:
- Around line 22-40: GetAuthToken currently mints a JWT for any request with a
Client-ID header; add a credential verification step before signing: retrieve
the client's credential (e.g., Client-Secret header or mTLS info) and validate
it against the persisted client record using a method on AuthHandler (e.g.,
ah.ValidateClientSecret or ah.clientStore.LookupAndVerify), and if validation
fails return http.StatusUnauthorized; only on successful verification proceed to
create the token using ah.jwtSecret and the existing claims (ensure you
reference middleware.ClientIDHeader and middleware.TokenIssuer when validating
and issuing).

In `@pkg/middleware/auth.go`:
- Around line 41-44: The current check uses err.Error() which can panic if err
is nil and also leaks internal parse errors to clients; update the token
validation branch (the variables token, err and the call c.AbortWithStatusJSON)
to first separately handle err != nil and !token.Valid: if err != nil respond
with a generic unauthorized message (e.g., {"error":"invalid or expired token"})
without including err.Error(), and if err == nil but token.Valid is false
respond with the same generic message; ensure you do not call err.Error() when
err is nil and do not return raw error strings to the client.
- Around line 25-32: The Client-ID header check in the auth middleware writes a
400 with c.JSON but does not abort the Gin context, allowing downstream handlers
to run; update the block that checks clientID (using ClientIDHeader and the
gin.Context variable c in the auth middleware function) to call
c.AbortWithStatusJSON(...) with the same status and payload instead of c.JSON
and return, mirroring the Bearer-prefix branch so the request chain is halted
and no further handlers execute when the header is missing.
- Around line 34-40: The JWT parsing call accepts any signing algorithm because
the keyfunc does not check token.Method; update the jwt.ParseWithClaims
invocation (the call that currently passes jwt.WithAudience and jwt.WithIssuer)
to include jwt.WithValidMethods([]string{"HS256"}) to pin accepted algorithms to
HS256, and also add a simple sanity check inside the keyfunc used there
(jwt.ParseWithClaims's func(*jwt.Token)...) to assert token.Method.Alg() ==
"HS256" and return an error if not, ensuring tokens with other alg headers are
rejected.

---

Nitpick comments:
In `@pkg/http/router.go`:
- Around line 82-85: The VerifyClaim handler is currently a no-op; wire it to
call the claim validation logic and return a proper response instead of an empty
200. Specifically, invoke r.claimHandler.ValidateClaimByUriDigest(ctx,
claimDigest) (or the existing claim handler method used for bulk verification)
and translate its result/error into an HTTP response using the same pattern your
other handlers use (e.g., ctx.JSON with the validation result on success and
ctx.AbortWithStatusJSON or similar on error), or if individual verification is
intentionally unsupported, remove the endpoint or return a clear 4xx/5xx (e.g.,
501 Not Implemented) with an explanatory body. Ensure you reference VerifyClaim
and r.claimHandler.ValidateClaimByUriDigest when making the change.
🪄 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 Plus

Run ID: 2e82ba4b-981e-4c05-b5c6-1e793f1f5353

📥 Commits

Reviewing files that changed from the base of the PR and between 2cd1c5f and a835965.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (21)
  • acceptance/acceptance_suite_test.go
  • acceptance/compose.yaml
  • api/source-score.yaml
  • cmd/app/main.go
  • go.mod
  • pkg/api/server.gen.go
  • pkg/conf/conf.go
  • pkg/conf/conf.yaml
  • pkg/conf/conf_test.go
  • pkg/domain/claim/claim_repository.go
  • pkg/domain/claim/claim_repository_test.go
  • pkg/domain/claim/claim_service.go
  • pkg/domain/claim/claim_service_test.go
  • pkg/domain/claim/claimfakes/fake_claim_repository.go
  • pkg/domain/claim/claimfakes/fake_claim_service.go
  • pkg/handlers/auth.go
  • pkg/handlers/claim.go
  • pkg/handlers/swagger.go
  • pkg/http/router.go
  • pkg/middleware/apikey.go
  • pkg/middleware/auth.go
💤 Files with no reviewable changes (2)
  • pkg/middleware/apikey.go
  • acceptance/compose.yaml

Comment thread api/source-score.yaml
Comment thread api/source-score.yaml
Comment thread pkg/conf/conf.go Outdated
Comment thread pkg/handlers/auth.go
Comment thread pkg/middleware/auth.go
Comment thread pkg/middleware/auth.go
Comment thread pkg/middleware/auth.go
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12 issues found across 22 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="pkg/handlers/auth.go">

<violation number="1" location="pkg/handlers/auth.go:23">
P0: [CRITICAL] `/auth/token` mints JWTs from an untrusted `Client-ID` header without authenticating the caller, enabling token issuance/impersonation for arbitrary clients.</violation>
</file>

<file name="pkg/http/router.go">

<violation number="1" location="pkg/http/router.go:34">
P1: Do not initialize JWT auth with a default/shared secret; require a non-default `JWT_SECRET` before wiring `AuthHandler`, or tokens can be forged.</violation>
</file>

<file name="pkg/conf/conf.go">

<violation number="1" location="pkg/conf/conf.go:21">
P0: [CRITICAL] Do not use a hardcoded default JWT secret. Require `JWT_SECRET` to be explicitly provided so the service fails fast instead of running with a predictable signing key.</violation>
</file>

<file name="pkg/middleware/auth.go">

<violation number="1" location="pkg/middleware/auth.go:20">
P2: [SUGGESTION] Match the `Authorization` scheme case-insensitively; valid lowercase/mixed-case `bearer` headers are currently rejected.</violation>

<violation number="2" location="pkg/middleware/auth.go:39">
P2: [CRITICAL] Restrict JWT parsing to expected signing method(s) using `jwt.WithValidMethods`; currently the parser is not pinned to HS256.</violation>
</file>

<file name="cmd/app/main.go">

<violation number="1" location="cmd/app/main.go:82">
P1: `Client-ID` is required by your API but missing from CORS `AllowHeaders`, which will block browser clients during preflight.</violation>

<violation number="2" location="cmd/app/main.go:102">
P0: Fail fast when `JWT_SECRET` is unset/default; using a known default secret makes JWT authentication forgeable.</violation>
</file>

<file name="api/source-score.yaml">

<violation number="1" location="api/source-score.yaml:13">
P2: Use `POST` for `/auth/token` instead of `GET` to avoid cache/read-only semantics on token issuance endpoints.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread pkg/handlers/auth.go
Comment thread pkg/conf/conf.go Outdated
Comment thread cmd/app/main.go
Comment thread api/source-score.yaml
Comment thread pkg/http/router.go
Comment thread cmd/app/main.go Outdated
Comment thread api/source-score.yaml Outdated
Comment thread pkg/middleware/auth.go Outdated
Comment thread pkg/middleware/auth.go
Comment thread api/source-score.yaml Outdated
Signed-off-by: Amit Singh <singhamitch@outlook.com>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="pkg/handlers/auth.go">

<violation number="1" location="pkg/handlers/auth.go:23">
P0: [CRITICAL] `/auth/token` mints JWTs from an untrusted `Client-ID` header without authenticating the caller, enabling token issuance/impersonation for arbitrary clients.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread api/source-score.yaml
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 8 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread api/source-score.yaml
Comment thread acceptance/auth_test.go Outdated
Comment thread pkg/middleware/auth.go Outdated
Comment thread pkg/conf/conf.go
Signed-off-by: Amit Singh <singhamitch@outlook.com>
@semmet95 semmet95 force-pushed the feat/auth-token-middleware branch from 6505eda to dd32ee4 Compare May 29, 2026 12:11
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@acceptance/auth_test.go`:
- Line 18: The "Validation tests" block is marked pending via Ginkgo's XContext,
so those auth/JWT specs never run; open the block by replacing XContext with
Context (or Describe/When as appropriate) in acceptance/auth_test.go for the
"Validation tests" group and also scan the contained specs for any other pending
markers (XIt, XDescribe, Pending, PIt) and convert them to active equivalents so
the validation specs execute in CI.
- Around line 14-17: The protectedEndpoint and its assertion are being executed
at spec-tree construction inside Describe; move the url.JoinPath call and the
Expect(err).To(BeNil()) into a runtime hook such as BeforeEach so the
computation and assertion run during test execution. Locate the current
protectedEndpoint variable and the url.JoinPath(baseUrl, "/api/v1/claims") call
inside the Describe block and refactor them so protectedEndpoint is assigned in
a BeforeEach and the Expect(err).To(BeNil()) assertion is performed there (leave
Describe for declarations only).
🪄 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 Plus

Run ID: d54f9579-0a9d-4a07-b047-bd0fd5d02df5

📥 Commits

Reviewing files that changed from the base of the PR and between a835965 and dd32ee4.

📒 Files selected for processing (8)
  • acceptance/acceptance_suite_test.go
  • acceptance/auth_test.go
  • acceptance/compose.yaml
  • api/source-score.yaml
  • cmd/app/main.go
  • pkg/api/server.gen.go
  • pkg/conf/conf.go
  • pkg/middleware/auth.go
🚧 Files skipped from review as they are similar to previous changes (3)
  • pkg/middleware/auth.go
  • cmd/app/main.go
  • acceptance/acceptance_suite_test.go

Comment thread acceptance/auth_test.go Outdated
Comment thread acceptance/auth_test.go Outdated
Signed-off-by: Amit Singh <singhamitch@outlook.com>
@semmet95 semmet95 merged commit 29f60d8 into main May 29, 2026
5 checks passed
@semmet95 semmet95 deleted the feat/auth-token-middleware branch May 29, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Secure the endpoint

1 participant