Conversation
…) endpoints Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
|
@copilot Introspect is supposed to have some gating around it, to prevent anyone from inspecting any token. we may not want to get overly prescriptive here, but best practices would be to limit this to a user inspecting their own token, or an elevated role (admin type) being able to inspect their own token and others in their domain, or another approach is simply to wrap the endpoint with HTTP Basic Auth. If we don't have support for Basic Auth already, we should add it as it's own step, and then example documentation should show how to use basic auth + introspection. The introspection step itself should have a flag that allows a token to inspect just self or self + others. if it supports inspecting others, we should optionally support a condition match for a claim or scope in the token as a prerequisite to inspect others. |
There was a problem hiding this comment.
Pull request overview
Adds RFC-compliant OAuth2 token revocation and introspection support to the auth.m2m module, completing common OAuth2 management flows for issued JWT access tokens.
Changes:
- Added
POST /oauth/revoke(RFC 7009) andPOST /oauth/introspect(RFC 7662) routes toauth.m2m. - Introduced per-token
jtigeneration and an in-memory JTI blacklist checked during authentication and introspection. - Refactored signature validation into a shared
parseTokenClaims()helper and added comprehensive tests for the new endpoints and JTI behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| module/auth_m2m.go | Adds revoke/introspect handlers, JTI generation + blacklist, and refactors token parsing/validation. |
| module/auth_m2m_test.go | Adds tests covering revocation behavior, introspection responses, and JTI presence/uniqueness. |
Comments suppressed due to low confidence (1)
module/auth_m2m.go:658
parseTokenClaimsusesjwt.Parsewithout options likejwt.WithExpirationRequired()and only checks the signing method type (HMAC/ECDSA), not the specific algorithm (HS256/ES256). This means a token withoutexpcould be treated as valid, and HS384/HS512 (or ES384/ES512) could be accepted when the module is configured for HS256/ES256. Consider enforcing the exact algorithm (e.g.,t.Method.Alg() == "HS256"/"ES256" or usingjwt.WithValidMethods`) and requiring expiration.
// parseTokenClaims parses and cryptographically validates a token string,
// returning the claims and whether the token is valid.
// It does NOT check the JTI blacklist; callers must do that separately.
func (m *M2MAuthModule) parseTokenClaims(tokenStr string) (jwt.MapClaims, bool) {
var (
token *jwt.Token
err error
)
switch m.algorithm {
case SigningAlgES256:
if m.publicKey == nil {
return nil, false
}
token, err = jwt.Parse(tokenStr, func(t *jwt.Token) (any, error) {
if _, ok := t.Method.(*jwt.SigningMethodECDSA); !ok {
return nil, fmt.Errorf("unexpected signing method: %v", t.Header["alg"])
}
return m.publicKey, nil
})
default: // HS256
token, err = jwt.Parse(tokenStr, func(t *jwt.Token) (any, error) {
if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok {
return nil, fmt.Errorf("unexpected signing method: %v", t.Header["alg"])
}
return m.hmacSecret, nil
})
| if claims, ok := m.parseTokenClaims(tokenStr); ok { | ||
| if jti, _ := claims["jti"].(string); jti != "" { | ||
| m.mu.Lock() | ||
| m.jtiBlacklist[jti] = struct{}{} | ||
| m.mu.Unlock() | ||
| } |
There was a problem hiding this comment.
The JTI blacklist is an unbounded in-memory map; revoked JTIs are never expired or garbage-collected. In long-running processes (or under misuse), this can grow without bound and become a memory/DoS risk. Consider storing expiry alongside the JTI (using the token's exp) and periodically purging expired entries, or enforcing a max size / LRU.
| now := time.Now() | ||
| jti, err := generateJTI() | ||
| if err != nil { | ||
| return "", fmt.Errorf("generate JTI: %w", err) |
There was a problem hiding this comment.
issueToken wraps errors from generateJTI with fmt.Errorf("generate JTI: %w", err), but generateJTI already wraps its own errors with the same prefix. This results in duplicated context like "generate JTI: generate JTI: ...". Consider removing the extra wrapping at one of these layers (typically let generateJTI return the raw rand.Read error context and wrap once at the call site).
| return "", fmt.Errorf("generate JTI: %w", err) | |
| return "", err |
| func (m *M2MAuthModule) handleRevoke(w http.ResponseWriter, r *http.Request) { | ||
| if err := r.ParseForm(); err != nil { | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| _ = json.NewEncoder(w).Encode(oauthError("invalid_request", "failed to parse form")) | ||
| return | ||
| } | ||
| tokenStr := r.FormValue("token") | ||
| if tokenStr == "" { | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| _ = json.NewEncoder(w).Encode(oauthError("invalid_request", "token is required")) | ||
| return | ||
| } |
There was a problem hiding this comment.
/oauth/revoke currently accepts just a token and performs revocation without any client authentication. RFC 7009 expects the revocation endpoint to be protected (typically via the same client authentication as the token endpoint). As implemented, anyone who obtains a token can revoke it, and anonymous callers can also use this endpoint as a CPU oracle for signature checks. Consider requiring client_id/client_secret (Basic auth or form, reusing extractClientCredentials + authenticateClient) and returning 401 invalid_client when missing/invalid.
| // handleIntrospect implements token introspection per RFC 7662. | ||
| // It validates the token and returns its active status along with claims. | ||
| func (m *M2MAuthModule) handleIntrospect(w http.ResponseWriter, r *http.Request) { | ||
| if err := r.ParseForm(); err != nil { | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| _ = json.NewEncoder(w).Encode(oauthError("invalid_request", "failed to parse form")) | ||
| return | ||
| } | ||
| tokenStr := r.FormValue("token") | ||
| if tokenStr == "" { | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| _ = json.NewEncoder(w).Encode(oauthError("invalid_request", "token is required")) | ||
| return | ||
| } |
There was a problem hiding this comment.
/oauth/introspect is exposed without any authentication/authorization checks. RFC 7662 requires the introspection endpoint to be protected because it leaks token metadata and provides a token-validity oracle to anonymous callers. Consider requiring client authentication (Basic auth or form, reusing extractClientCredentials/authenticateClient) and failing with 401 invalid_client when the caller is not authorized.
…l policy Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Implemented in commit 5f900e7. What changed: The
Access-control modes (via
YAML config example: - name: m2m-auth
type: auth.m2m
config:
algorithm: ES256
introspect:
allowOthers: true
requiredScope: "introspect:admin" # ****** must have this scope
# requiredClaim: "role" # or a claim-based check
# requiredClaimVal: "admin"Unauthenticated requests → 401. Invalid credentials → 401. Self-inspection is always allowed regardless of policy. |
|
@copilot apply changes based on the comments in this thread Additionally, craft a test scenario that backs the JWT token storage with a database, so we can model an example of how such a revocation would also revoke an entry in the database. If the step doesn't provide the necessary functionality/info to support this, adapt the step accordingly so it's flexible enough for this use case. |
…list, TokenRevocationStore interface, SQLite tests Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Addressed in commit 186650f. Here's what was done: Review thread changes applied:
DB-backed revocation store: type TokenRevocationStore interface {
RevokeToken(ctx context.Context, jti string, expiry time.Time) error
IsRevoked(ctx context.Context, jti string) (bool, error)
}
New tests (SQLite-backed): |
auth.m2mwas missing token revocation and introspection, leaving the OAuth2 implementation incomplete for clients that depend on these flows.Changes
New endpoints
POST /oauth/revoke(RFC 7009) — acceptstokenform param, requires client authentication (HTTP Basic Auth or formclient_id/client_secretper RFC 7009 §2.1), adds the token's JTI to a bounded in-memory blacklist. Returns 200 per spec for valid/unknown tokens; returns401 invalid_clientwhen credentials are missing or invalid.POST /oauth/introspect(RFC 7662) — acceptstokenform param, returns active status + claims (client_id,scope,exp,iat,iss), or{"active": false}for invalid/revoked tokens. The endpoint requires caller authentication and enforces a configurable access-control policy.JTI support
issueTokennow generates a cryptographically randomjti(16-byte base64url) for every issued token.jtiis added to the protected-claims set soextraClaimscannot override it.Authenticatechecks the JTI blacklist after signature validation — revoked tokens are rejected.exptime andpurgeExpiredJTIsLocked()removes stale entries on every write, preventing unbounded growth in long-running processes.Introspection access control
The
/oauth/introspectendpoint requires the caller to authenticate before any token data is revealed. Two authentication methods are supported:client_id+client_secret): treated as admin-levelTwo access-control modes are available via
SetIntrospectPolicy(or theintrospectYAML config block):Self-only (default,
allowOthers: false): a caller may only introspect its own token (the inspected token'ssubmust match the caller's identity). Returns 403 for cross-token inspection.Allow-others (
allowOthers: true): any authenticated caller may inspect any token. Optional prerequisites for ****** callers:requiredScope: the caller's token must contain this scope (e.g.introspect:admin)requiredClaim/requiredClaimVal: the caller's token must have this claim, optionally with a specific valueHTTP Basic Auth callers always bypass scope/claim prerequisites (admin-level).
Pluggable DB-backed revocation store
Added a
TokenRevocationStoreinterface so revocations can be persisted across process restarts:Wire in a store via
SetRevocationStore(store). When set,POST /oauth/revokealso callsstore.RevokeToken, andAuthenticate/introspect fall back tostore.IsRevokedfor tokens absent from the in-memory blacklist — enabling revocations to survive process restarts.Refactor
parseTokenClaims()to consolidate signature-validation logic, reused byAuthenticate,handleRevoke, andhandleIntrospect.authenticateIntrospectCaller,callerMeetsIntrospectPolicy, andisJTIRevokedhelpers.generateJTIreturns raw errors;issueTokenwraps once with context.Usage
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.