feat(auth): validate encrypted (JWE) session tokens - #805
Conversation
Add decrypt-then-verify support for encrypted session tokens. When a project enables JWT encryption, session tokens are minted as a 5-part JWE; the SDK now decrypts them with the recipient private key before verifying the inner signed JWS. - Config.PrivateKey / Config.PrivateKeyProvider / DESCOPE_PRIVATE_KEY supply the decryption key (JWK, JWK Set, or PEM). Keys are indexed by their RFC-7638 SHA-256 thumbprint, matching the kid the server stamps on the JWE header. - ValidateJWT detects a JWE (isJWE), decrypts to the inner JWS, and continues the existing signature + claims validation unchanged. Plain JWS tokens are unaffected. - ErrJWEDecrypt (G030005) is returned when a JWE arrives with no decryption key configured. - examples/jwe: end-to-end sample that enables JWE on a project, signs a user in, and validates the resulting encrypted token through the SDK. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🐕 Review complete — View session on Shuni Portal 🐾 |
![github-license-compliance[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
🐕 Suggested ReviewersThe review assignment strategically covers different areas impacted by the change—core auth logic, client configuration, token handling, and error robustness—by selecting contributors with relevant and distinct expertise.
Suggested by Shuni based on git history and PR context. Names are not @-mentioned to avoid notifying anyone — request a review from whoever fits best. |
|
@shuni review |
|
🐕 Review complete — View session on Shuni Portal 🐾 |
![shuni-bot[bot]](https://avatars.githubusercontent.com/in/2536750?s=60&v=4){kind=small}
There was a problem hiding this comment.
🐕 Shuni's Review
Adds decrypt-then-verify so the SDK can validate encrypted (JWE) session tokens — solid, well-tested core logic with good coverage of PEM/JWK/JWK-Set keys and provider fallback.
Sniffed out 2 issues:
- 1 🟠 HIGH:
token.JWTbecomes the decrypted inner JWS, silently downgrading JWE session cookies to plaintext - 1 🟡 MEDIUM: attacker-controlled JWE
algheader is honored without pinning to the configured key
See inline comments. Good bones, but needs a quick bath! Woof!
| return nil, derr | ||
| } | ||
| // Continue with the decrypted inner signed JWS; everything downstream is unchanged. | ||
| JWT = inner |
There was a problem hiding this comment.
🟠 HIGH: After decryption, token.JWT carries the decrypted inner JWS, not the original JWE. NewToken(JWT, …) at line 836 stores the plaintext, and createCookie (auth.go:999) writes token.JWT into the Set-Cookie value.
So for a JWE-enabled project with SessionJWTViaCookie, the login/refresh flow (collectJwts → validateJWT) rewrites the browser cookie from a 5-part JWE to an unencrypted 3-part JWS — defeating the confidentiality JWT encryption is meant to provide. Callers that round-trip token.JWT to a downstream service also get the wrong wire format.
Fix: keep the original compact for the returned token — capture originalJWT := JWT before the isJWE block and pass it to NewToken, feeding only the decrypted form to jwt.Parse.
There was a problem hiding this comment.
Good catch — fixed in 9378cdb. ValidateJWT now verifies a separate signed copy of the decrypted inner JWS and passes the original JWT to NewToken, so token.JWT keeps the encrypted JWE wire format — a SessionJWTViaCookie project no longer has its cookie rewritten to plaintext, and round-trips preserve the wire format. Added TestValidateJWT_JWE_PreservesEncryptedJWT.
| return "", descope.ErrJWEDecrypt.WithMessage("Invalid JWE header") | ||
| } | ||
| var keyAlg jwa.KeyEncryptionAlgorithm | ||
| if err := keyAlg.Accept(hdr.Alg); err != nil { |
There was a problem hiding this comment.
🟡 MEDIUM: The key-wrap algorithm is read from the attacker-controlled JWE header and passed straight to jwe.Decrypt(… WithKey(keyAlg, key)). keyAlg.Accept admits any registered JWA alg (e.g. RSA1_5), so a caller can force a PKCS#1 v1.5 unwrap against your RSA recipient key even though the project only issues RSA-OAEP-256 — an algorithm-policy bypass and a Bleichenbacher padding-oracle surface.
Fix: pin the accepted algorithm(s) to the configured key (e.g. validate hdr.Alg against key.Algorithm(), or drop the explicit alg and let the library enforce header-vs-key consistency) rather than trusting the header.
There was a problem hiding this comment.
Fixed in 9378cdb. After keyAlg.Accept, the algorithm is now pinned to the two key-wrap algs Descope issues (RSA-OAEP-256, ECDH-ES+A256KW); any other header alg (e.g. RSA1_5) returns ErrJWEDecrypt before any unwrap is attempted, closing the downgrade / Bleichenbacher surface. Added TestValidateJWT_JWE_DisallowedAlgRejected.
There was a problem hiding this comment.
🐕 Shuni's Review
Adds decrypt-then-verify so the SDK can validate encrypted (JWE) session tokens, decrypting with a configured private key before the existing JWS validation.
Sniffed out 3 issues:
- 2 🟠 HIGH: decrypted plaintext token stored back into cookie/auth-info, attacker-controlled JWE
algaccepted without an allow-list - 1 🟢 LOW: no-regression test doesn't configure the private key it claims to
Solid bones overall — but the round-trip leaks the cleartext token. Needs a little grooming! Woof!
| return nil, derr | ||
| } | ||
| // Continue with the decrypted inner signed JWS; everything downstream is unchanged. | ||
| JWT = inner |
There was a problem hiding this comment.
🟠 HIGH: Reassigning JWT = inner makes Token.JWT the decrypted inner JWS, not the original JWE. When the server issues encrypted session tokens, extractTokens→collectJwts runs them through validateJWT, and createCookie then writes token.JWT (now cleartext) into the Set-Cookie value (line 999); the same plaintext also flows into the returned AuthenticationInfo. This defeats the encryption on the round-trip — the browser cookie / returned session token end up as the unencrypted JWS.
Preserve the original token for output and only use the decrypted form for validation, e.g. keep originalJWT := JWT before overwriting and pass originalJWT to NewToken at line 836.
There was a problem hiding this comment.
Already addressed in 9378cdb — this comment is anchored to the earlier commit 1dc58d8. ValidateJWT no longer reassigns JWT: it verifies a separate signed copy (auth.go:803, 812) and passes the original JWT to NewToken (auth.go:839), so token.JWT keeps the encrypted JWE. Covered by TestValidateJWT_JWE_PreservesEncryptedJWT.
| return "", err | ||
| } | ||
|
|
||
| plaintext, err := jwe.Decrypt([]byte(compact), jwe.WithKey(keyAlg, key)) |
There was a problem hiding this comment.
🟠 HIGH: The key-encryption algorithm is taken verbatim from the attacker-controlled JWE header (hdr.Alg) and passed to jwe.WithKey(keyAlg, key). keyAlg.Accept allows any known JWA, so a client presenting a JWE to an RSA-key-configured SDK can force alg:"RSA1_5" instead of the intended RSA-OAEP-256, exercising the PKCS#1 v1.5 path (classic Bleichenbacher padding-oracle target). Signature verification still runs, so no auth bypass — but it's an algorithm downgrade against the recipient key.
Pin the accepted alg to what the backend actually uses (RSA-OAEP-256 for RSA keys, ECDH-ES+A256KW for EC) and reject anything else before decrypting.
There was a problem hiding this comment.
The alg is pinned just above this line: jwt.go:248-249 (added in this same commit) rejects any keyAlg other than RSA-OAEP-256 / ECDH-ES+A256KW with ErrJWEDecrypt before jwe.Decrypt at 257, so an RSA1_5 header never reaches the unwrap. Covered by TestValidateJWT_JWE_DisallowedAlgRejected.
| // A plain 3-part JWS must validate exactly as before, even with a decryption key configured. | ||
| _, encPub, _ := genEncKey(t, jwa.RSA_OAEP_256) | ||
| _ = encPub | ||
| a, err := newTestAuthConf(&AuthParams{ProjectID: "a", PublicKey: publicKey}, nil, DoOk(nil)) |
There was a problem hiding this comment.
🟢 LOW: The comment says a plain JWS must validate "even with a decryption key configured", but this service is built with only PublicKey — no PrivateKey/provider — so encPub is dead (_ = encPub) and the with-key regression path is never exercised. Pass a private key so the test matches its stated intent:
| a, err := newTestAuthConf(&AuthParams{ProjectID: "a", PublicKey: publicKey}, nil, DoOk(nil)) | |
| a, err := newTestAuthConf(&AuthParams{ProjectID: "a", PublicKey: publicKey, PrivateKey: encPriv}, nil, DoOk(nil)) |
(capture encPriv from genEncKey above instead of discarding it).
There was a problem hiding this comment.
Good catch — fixed in 42ccad6: the test now builds the service with PrivateKey: encPriv, so the plain-JWS path is genuinely exercised with a decryption key configured.
Address review findings on encrypted-session-token validation: - Return the original encrypted JWT in the Token, not the decrypted inner JWS. ValidateJWT now verifies a separate `signed` copy; token.JWT was being overwritten with the plaintext, which for a SessionJWTViaCookie project would rewrite the browser cookie from a JWE to an unencrypted JWS and defeat the confidentiality JWT encryption is meant to provide. - Reject key-wrap algorithms other than the ones Descope issues (RSA-OAEP-256, ECDH-ES+A256KW). The alg is read from the attacker-controllable JWE header, so accepting arbitrary registered algorithms (e.g. RSA1_5) allowed an algorithm downgrade and a Bleichenbacher padding-oracle surface. Adds regression tests for both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@shuni review |
|
🐕 Review complete — View session on Shuni Portal 🐾 |
There was a problem hiding this comment.
🐕 Shuni's Review
Re-sniffed at 9378cdb — both HIGH issues from the last pass are now buried and covered. Good boy!
Verified fixed:
- 🟠→✅ Wire format preserved:
ValidateJWTverifies a separatesignedcopy and returns the original encrypted JWE viaNewToken(auth.go:839), soSessionJWTViaCookiecookies no longer get rewritten to plaintext. Covered byTestValidateJWT_JWE_PreservesEncryptedJWT. - 🟠→✅ Alg pinned to
RSA-OAEP-256/ECDH-ES+A256KW;RSA1_5and friends are rejected before any unwrap, closing the downgrade / Bleichenbacher surface. Covered byTestValidateJWT_JWE_DisallowedAlgRejected.
Only leftover is the 🟢 LOW test nit already in thread 7 (jwe_test.go dead encPub) — non-blocking. Clean bones now. Woof!
The no-regression test claimed to validate a plain JWS "even with a decryption key configured" but built the service with only PublicKey, so the with-key path was never exercised. Pass PrivateKey so the test matches its stated intent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@shuni review |
1 participant
What
Adds decrypt-then-verify so the SDK can validate encrypted (JWE) session tokens, not just plain JWS. When a project enables JWT encryption, session tokens are minted as a 5-part JWE; the SDK now decrypts them with the recipient private key before verifying the inner signed JWS.
How
Config.PrivateKey/Config.PrivateKeyProvider/DESCOPE_PRIVATE_KEYsupply the decryption key (JWK, JWK Set, or PEM). Keys are indexed by their RFC-7638 SHA-256 thumbprint, which equals thekidthe server stamps on the JWE header.ValidateJWTdetects a JWE (isJWE, 5 parts), decrypts to the inner JWS, then runs the existing signature + claims validation unchanged. Plain JWS tokens are unaffected.ErrJWEDecrypt(G030005) when a JWE arrives with no decryption key configured.examples/jweruns the full flow end-to-end against a live project.Backend compatibility
Verified against
descope/backendmain (PRs #1711 mgmt API, #1810 console):kid=Thumbprint(crypto.SHA256)+base64.RawURLEncodingon both sides — identical.Test plan
go test ./descope/...— newjwe_test.gocovers decrypt-then-verify, PEM/JWK/JWK-Set keys, provider fallback, and no-key rejection.examples/jwevalidated end-to-end against a JWE-enabled project.🤖 Generated with Claude Code