Skip to content

Conversation

@ericelliott
Copy link
Collaborator

Expanded security guidelines for JWT usage, including session state, storage, transport, algorithm, verification failure handling, token purpose, key handling, claims validation, authorization, cookie hardening, and lifetime.

Expanded security guidelines for JWT usage, including session state, storage, transport, algorithm, verification failure handling, token purpose, key handling, claims validation, authorization, cookie hardening, and lifetime.
Copilot AI review requested due to automatic review settings December 27, 2025 01:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR significantly expands JWT security guidelines from a basic pattern-matching structure to a comprehensive security framework. The changes transform the guideline from 9 simple rules to 50+ lines covering 10 major security categories, providing detailed recommendations for secure JWT implementation.

Key Changes:

  • Restructured from flat list to organized sections covering session state, storage & transport, algorithm & signature, verification handling, token purpose, key handling, claims validation, authorization, cookie hardening, and token lifetime
  • Added critical patterns for modern JWT security concerns including JWKS endpoint security, token type validation, issuer/audience validation, and proper failure handling
  • Expanded token lifetime guidance with more granular warnings (>15 min vs >= 1 day)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings December 27, 2025 02:03
@ericelliott ericelliott merged commit fd44e39 into main Dec 27, 2025
7 checks passed
@ericelliott ericelliott deleted the ericelliott-patch-2 branch December 27, 2025 02:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

(verification failure allows partial or degraded access) => Critical: Fail closed. Invalid token = no access.

## Token Purpose
(ID token used as access token) => Critical: Wrong token type. Use access tokens (typ: at+jwt) for API auth.
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

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

The recommendation states "Use access tokens (typ: at+jwt)" which refers to the OAuth 2.0 access token type value. However, the correct format according to RFC 9068 is "application/at+jwt" as the full media type value, though implementations often use the shortened form "at+jwt". Consider clarifying whether this guideline expects the full media type or the shortened form, as different JWT libraries handle the typ claim differently.

Suggested change
(ID token used as access token) => Critical: Wrong token type. Use access tokens (typ: at+jwt) for API auth.
(ID token used as access token) => Critical: Wrong token type. Use access tokens with typ "application/at+jwt" (or "at+jwt" where libraries use the shortened form) for API auth, and validate typ accordingly.

Copilot uses AI. Check for mistakes.
(access token lifetime >= 1 day) => Warn: Use short-lived tokens + refresh rotation.
(JWT detected) => Warn: Consider opaque tokens + server sessions.
}
(alg from token used to select verification) => Critical: Alg confusion. Strict allowlist + key type must match algorithm.
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

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

The word "Alg" should be lowercase "alg" to match the standard JWT header parameter name. The "alg" parameter is a registered JOSE header parameter defined in RFC 7515, and maintaining lowercase consistency improves clarity when referring to the actual header field.

Suggested change
(alg from token used to select verification) => Critical: Alg confusion. Strict allowlist + key type must match algorithm.
(alg from token used to select verification) => Critical: alg confusion. Strict allowlist + key type must match algorithm.

Copilot uses AI. Check for mistakes.
(JWT detected) => Warn: Consider opaque tokens + server sessions.
}
(alg from token used to select verification) => Critical: Alg confusion. Strict allowlist + key type must match algorithm.
(HS256 or symmetric algorithm) => Critical: Use asymmetric algorithms (RS256/ES256).
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

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

The severity level for "HS256 or symmetric algorithm" is marked as Critical with a recommendation to use asymmetric algorithms. However, this may be too strict for all use cases. HS256 is acceptable when used properly with strong secret management in single-service architectures where the issuer and verifier are the same entity. The critical issue is when HS256 is "shared across services" (as mentioned in the removed line). Consider changing this to "Warn" severity or clarifying that the critical risk applies specifically to multi-service scenarios where secrets can be compromised or misused.

Suggested change
(HS256 or symmetric algorithm) => Critical: Use asymmetric algorithms (RS256/ES256).
(HS256 or symmetric algorithm) => Warn: Prefer asymmetric algorithms (RS256/ES256). HS256 can be acceptable when the issuer and verifier are the same service and the secret is strongly protected; it becomes critical when shared across services or with third parties.

Copilot uses AI. Check for mistakes.
(exp not validated) => Critical: Always check exp claim.
(nbf present and not validated) => Critical: If present, must validate. Reject on failure.
(iat present and not validated) => Critical: If present, must validate. Reject on failure.
(nbf and iat not checked when absent) => Warn: Consider requiring nbf/iat. Validate with ≤60s clock skew.
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

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

The guidance states "Consider requiring nbf/iat" but doesn't explain when these claims should be required versus optional. The iat (issued at) claim is particularly important for detecting token replay attacks and validating token age, while nbf (not before) is less commonly needed. Consider providing clearer guidance on when these claims should be mandatory (e.g., for access tokens in high-security contexts) versus when they can be optional.

Suggested change
(nbf and iat not checked when absent) => Warn: Consider requiring nbf/iat. Validate with ≤60s clock skew.
(nbf and iat not checked when absent) => Warn: For access tokens in high-security contexts, require iat and enforce a maximum token age (with ≤60s clock skew) to detect replay; treat nbf as optional and use it when tokens must not be accepted before a specific time (e.g., time-gated or long-lived tokens).

Copilot uses AI. Check for mistakes.
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.

2 participants