Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 39 additions & 19 deletions auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,17 @@ type Opts struct {
JWTQuery string // default "token"
SendJWTHeader bool // if enabled, also send JWT as a response header (in addition to the cookie)
SameSiteCookie http.SameSite // limit cross-origin requests with SameSite cookie attribute
// PartitionedCookies marks auth cookies Partitioned (CHIPS) so they survive third-party cookie
// blocking when the application is framed by another site.
//
// Requires SecureCookies, since browsers reject Partitioned without Secure, and wants
// SameSiteCookie set to http.SameSiteNoneMode so the cookie is sent from a third-party frame.
//
// Only helps where the cookie is set from inside the frame: direct, telegram and email-code.
// OAuth sessions are keyed to the auth site, because a popup is its own top-level context, so
// they will not reach the frame and enabling this makes an otherwise working OAuth session
// invisible to it. See token.Opts.PartitionedCookies for the detail.
PartitionedCookies bool

Issuer string // optional value for iss claim, usually the application name, default "go-pkgz/auth"

Expand Down Expand Up @@ -122,25 +133,26 @@ func NewService(opts Opts) (res *Service) {
}

jwtService := token.NewService(token.Opts{
SecretReader: opts.SecretReader,
ClaimsUpd: opts.ClaimsUpd,
SecureCookies: opts.SecureCookies,
TokenDuration: opts.TokenDuration,
CookieDuration: opts.CookieDuration,
DisableXSRF: opts.DisableXSRF,
DisableIAT: opts.DisableIAT,
JWTCookieName: opts.JWTCookieName,
JWTCookieDomain: opts.JWTCookieDomain,
JWTHeaderKey: opts.JWTHeaderKey,
XSRFCookieName: opts.XSRFCookieName,
XSRFHeaderKey: opts.XSRFHeaderKey,
XSRFIgnoreMethods: opts.XSRFIgnoreMethods,
SendJWTHeader: opts.SendJWTHeader,
JWTQuery: opts.JWTQuery,
Issuer: res.issuer,
AudienceReader: opts.AudienceReader,
AudSecrets: opts.AudSecrets,
SameSite: opts.SameSiteCookie,
SecretReader: opts.SecretReader,
ClaimsUpd: opts.ClaimsUpd,
SecureCookies: opts.SecureCookies,
TokenDuration: opts.TokenDuration,
CookieDuration: opts.CookieDuration,
DisableXSRF: opts.DisableXSRF,
DisableIAT: opts.DisableIAT,
JWTCookieName: opts.JWTCookieName,
JWTCookieDomain: opts.JWTCookieDomain,
JWTHeaderKey: opts.JWTHeaderKey,
XSRFCookieName: opts.XSRFCookieName,
XSRFHeaderKey: opts.XSRFHeaderKey,
XSRFIgnoreMethods: opts.XSRFIgnoreMethods,
SendJWTHeader: opts.SendJWTHeader,
JWTQuery: opts.JWTQuery,
Issuer: res.issuer,
AudienceReader: opts.AudienceReader,
AudSecrets: opts.AudSecrets,
SameSite: opts.SameSiteCookie,
PartitionedCookies: opts.PartitionedCookies,
})

if opts.SecretReader == nil {
Expand All @@ -150,6 +162,14 @@ func NewService(opts Opts) (res *Service) {
res.logger.Logf("[WARN] no secret reader defined")
}

if opts.PartitionedCookies && !opts.SecureCookies {
// http.Cookie.Valid rejects this pairing, but SetCookie writes through String and never
// calls Valid, so the header goes out, browsers drop both cookies, Set returns no error
// and the login silently produces no session. Warn rather than force: overriding an
// explicit SecureCookies: false on a published field would be worse
res.logger.Logf("[WARN] PartitionedCookies requires SecureCookies, browsers will reject the cookie")
}

res.jwtService = jwtService
res.authMiddleware.JWTService = jwtService
res.authMiddleware.L = res.logger
Expand Down
44 changes: 44 additions & 0 deletions auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"fmt"
"image"
_ "image/jpeg"
_ "image/png"
Expand Down Expand Up @@ -835,3 +836,46 @@ func uniformImage(img image.Image) bool {
}
return true
}

func TestNewService_PassesCookieOptionsToTokenService(t *testing.T) {
// the token.Opts literal in NewService has silently dropped a field twice, bd39e5e3 for
// SameSite and 59656e46 for XSRFIgnoreMethods, both in diffs shaped like this one: a
// re-indent plus one added key. Nothing failed either time, because nothing asserted the
// plumbing rather than the behavior
svc := NewService(Opts{
PartitionedCookies: true,
SecureCookies: true,
SameSiteCookie: http.SameSiteNoneMode,
})

ts := svc.TokenService()
assert.True(t, ts.PartitionedCookies, "PartitionedCookies did not reach the token service")
assert.True(t, ts.SecureCookies, "SecureCookies did not reach the token service")
assert.Equal(t, http.SameSiteNoneMode, ts.SameSite, "SameSite did not reach the token service")
}

func TestNewService_WarnsOnPartitionedWithoutSecure(t *testing.T) {
// http.SetCookie writes through String and never calls Valid, so this pairing produces a
// header the browser silently drops with no error anywhere. The warning is the only signal
var logged []string
capture := logger.Func(func(format string, args ...any) {
logged = append(logged, fmt.Sprintf(format, args...))
})

NewService(Opts{PartitionedCookies: true, SecureCookies: false, Logger: capture})
assert.Condition(t, func() bool {
for _, l := range logged {
if strings.Contains(l, "PartitionedCookies requires SecureCookies") {
return true
}
}
return false
}, "no warning for Partitioned without Secure, logged: %v", logged)

logged = nil
NewService(Opts{PartitionedCookies: true, SecureCookies: true, Logger: capture})
for _, l := range logged {
assert.NotContains(t, l, "PartitionedCookies requires SecureCookies",
"warned about a correctly configured pairing")
}
}
40 changes: 40 additions & 0 deletions avatar/avatar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,46 @@ func TestAvatar_resize(t *testing.T) {

// uniformImage reports whether every pixel matches the first one, which is what a resize that
// produced an empty canvas rather than a scaled picture looks like
func TestGenerateAvatar_DiffersPerUser(t *testing.T) {
// the geometry assertions elsewhere accept a generator that ignores its user argument: a single
// fixed image decodes, is png, is 300x300 and is not uniform for every caller. The byte counts
// this replaced rejected that by accident, since different users gave different lengths. Assert
// the pixels differ, not the encoded size, which is the thing that moved with the Go release
one, err := GenerateAvatar("user-one")
require.NoError(t, err)
two, err := GenerateAvatar("user-two")
require.NoError(t, err)

imgOne, _, err := image.Decode(bytes.NewReader(one))
require.NoError(t, err)
imgTwo, _, err := image.Decode(bytes.NewReader(two))
require.NoError(t, err)
require.Equal(t, imgOne.Bounds(), imgTwo.Bounds(), "same geometry is the precondition for comparing pixels")

differs := false
b := imgOne.Bounds()
for y := b.Min.Y; y < b.Max.Y && !differs; y++ {
for x := b.Min.X; x < b.Max.X; x++ {
if imgOne.At(x, y) != imgTwo.At(x, y) {
differs = true
break
}
}
}
assert.True(t, differs, "two users produced pixel-identical identicons, so the user argument is ignored")

// and the same user twice has to be stable, or avatars churn on every login
again, err := GenerateAvatar("user-one")
require.NoError(t, err)
imgAgain, _, err := image.Decode(bytes.NewReader(again))
require.NoError(t, err)
for y := b.Min.Y; y < b.Max.Y; y++ {
for x := b.Min.X; x < b.Max.X; x++ {
require.Equal(t, imgOne.At(x, y), imgAgain.At(x, y), "identicon is not stable for the same user")
}
}
}

func uniformImage(img image.Image) bool {
b := img.Bounds()
first := img.At(b.Min.X, b.Min.Y)
Expand Down
41 changes: 37 additions & 4 deletions token/jwt.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,23 @@ type Opts struct {
AudSecrets bool // uses different secret for differed auds. important: adds pre-parsing of unverified token
SendJWTHeader bool // if enabled, also send JWT as a response header (in addition to the cookie)
SameSite http.SameSite // define a cookie attribute making it impossible for the browser to send this cookie cross-site
// PartitionedCookies marks the auth cookies Partitioned (CHIPS), keying them to the embedding
// top-level site as well as their own, so they survive the phase-out of unpartitioned
// third-party cookies where the application is framed by another site.
//
// Requires SecureCookies: http.Cookie.Valid rejects Partitioned without Secure, and browsers
// drop such a cookie. SameSite=None is separate and is what makes the cookie get sent from a
// third-party frame at all, rather than what makes it accepted.
//
// Only helps where Set runs inside the frame, which is the direct, telegram and email-code
// flows. It does NOT help OAuth: the partition key is the top-level site at the moment the
// cookie is set, and an OAuth popup is its own top-level context, so a callback cookie is
// keyed to the auth site and the framed application never sees it. Enabling this converts a
// working OAuth session into an invisible one wherever unpartitioned third-party cookies are
// still allowed. Making OAuth work in a frame needs document.requestStorageAccess, or the
// popup handing a one-shot code to the frame to exchange so Set runs under the embedder's
// partition; both are application-side work.
PartitionedCookies bool
}

// NewService makes JWT service
Expand Down Expand Up @@ -261,11 +278,11 @@ func (j *Service) Set(w http.ResponseWriter, claims Claims) (Claims, error) {
}

jwtCookie := http.Cookie{Name: j.JWTCookieName, Value: tokenString, HttpOnly: true, Path: "/", Domain: j.JWTCookieDomain, //nolint:gosec // Secure and SameSite come from service config
MaxAge: cookieExpiration, Secure: j.SecureCookies, SameSite: j.SameSite}
MaxAge: cookieExpiration, Secure: j.SecureCookies, SameSite: j.SameSite, Partitioned: j.PartitionedCookies}
http.SetCookie(w, &jwtCookie)

xsrfCookie := http.Cookie{Name: j.XSRFCookieName, Value: claims.Id, HttpOnly: false, Path: "/", Domain: j.JWTCookieDomain, //nolint:gosec // HttpOnly false by design, JS reads it for the X-XSRF-Token header
MaxAge: cookieExpiration, Secure: j.SecureCookies, SameSite: j.SameSite}
MaxAge: cookieExpiration, Secure: j.SecureCookies, SameSite: j.SameSite, Partitioned: j.PartitionedCookies}
http.SetCookie(w, &xsrfCookie)

return claims, nil
Expand Down Expand Up @@ -336,13 +353,29 @@ func (j *Service) IsExpired(claims Claims) bool {
// Reset token's cookies
func (j *Service) Reset(w http.ResponseWriter) {
jwtCookie := http.Cookie{Name: j.JWTCookieName, Value: "", HttpOnly: false, Path: "/", Domain: j.JWTCookieDomain, //nolint:gosec // expired removal cookie, carries no value
MaxAge: -1, Expires: time.Unix(0, 0), Secure: j.SecureCookies, SameSite: j.SameSite}
MaxAge: -1, Expires: time.Unix(0, 0), Secure: j.SecureCookies, SameSite: j.SameSite, Partitioned: j.PartitionedCookies}
http.SetCookie(w, &jwtCookie)

xsrfCookie := http.Cookie{Name: j.XSRFCookieName, Value: "", HttpOnly: false, Path: "/", Domain: j.JWTCookieDomain, //nolint:gosec // expired removal cookie, carries no value
MaxAge: -1, Expires: time.Unix(0, 0), Secure: j.SecureCookies, SameSite: j.SameSite}
MaxAge: -1, Expires: time.Unix(0, 0), Secure: j.SecureCookies, SameSite: j.SameSite, Partitioned: j.PartitionedCookies}
http.SetCookie(w, &xsrfCookie)

if j.PartitionedCookies {
// a partitioned expiry does not match an unpartitioned cookie, so any pair written before
// the option was turned on survives this. The browser then sends both, sorted oldest
// first, and Request.Cookie returns the first occurrence: Get reads the legacy one, which
// is expired but still validly signed, and the refresh in middleware turns it back into a
// live session. Clearing both forms is what actually signs the reader out. In a blocked
// third-party context the unpartitioned header is dropped, so it costs nothing there
legacyJWT := http.Cookie{Name: j.JWTCookieName, Value: "", HttpOnly: false, Path: "/", Domain: j.JWTCookieDomain, //nolint:gosec // expired removal cookie, carries no value
MaxAge: -1, Expires: time.Unix(0, 0), Secure: j.SecureCookies, SameSite: j.SameSite}
http.SetCookie(w, &legacyJWT)

legacyXSRF := http.Cookie{Name: j.XSRFCookieName, Value: "", HttpOnly: false, Path: "/", Domain: j.JWTCookieDomain, //nolint:gosec // expired removal cookie, carries no value
MaxAge: -1, Expires: time.Unix(0, 0), Secure: j.SecureCookies, SameSite: j.SameSite}
http.SetCookie(w, &legacyXSRF)
}

w.Header().Set("Content-Type", "text/plain; charset=utf-8")
}

Expand Down
84 changes: 84 additions & 0 deletions token/jwt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -753,3 +753,87 @@ var testClaims = Claims{
ID: "myid-123456",
},
}

func TestJWT_PartitionedCookies(t *testing.T) {
// CHIPS keys a cookie to the embedding top-level site as well as its own, which is the only
// form of third-party cookie browsers still accept. Both Set and Reset have to carry it: a
// partitioned cookie and an unpartitioned expiry are different cookies to the browser, so
// clearing without the attribute leaves the original in place and the user signed in.
j := NewService(Opts{SecretReader: SecretFunc(mockKeyStore), SecureCookies: true,
SameSite: http.SameSiteNoneMode, PartitionedCookies: true,
TokenDuration: time.Hour, CookieDuration: days31,
})

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/set" {
_, err := j.Set(w, testClaims)
require.NoError(t, err)
}
if r.URL.Path == "/reset" {
j.Reset(w)
}
w.WriteHeader(200)
}))
defer ts.Close()

t.Run("set writes both cookies partitioned", func(t *testing.T) {
resp, err := http.Get(ts.URL + "/set")
require.NoError(t, err)
defer resp.Body.Close()

cookies := resp.Header.Values("Set-Cookie")
require.Len(t, cookies, 2, "both the JWT and the XSRF cookie are written")
for _, c := range cookies {
assert.Contains(t, c, "Partitioned", "%s has to be partitioned", c)
assert.Contains(t, c, "Secure", "browsers reject Partitioned without Secure: %s", c)
assert.Contains(t, c, "SameSite=None", "a partitioned cookie is a cross-site one: %s", c)
}
})

t.Run("reset clears the partitioned pair and any legacy one", func(t *testing.T) {
resp, err := http.Get(ts.URL + "/reset")
require.NoError(t, err)
defer resp.Body.Close()

cookies := resp.Header.Values("Set-Cookie")
require.Len(t, cookies, 4, "each name is expired in both its partitioned and legacy form")

var partitioned, legacy int
for _, c := range cookies {
assert.Contains(t, c, "Max-Age=0", "%s has to be an expiry", c)
if strings.Contains(c, "Partitioned") {
partitioned++
continue
}
legacy++
}
// a partitioned expiry does not match a cookie stored before the option was turned on, and
// that stale cookie is expired-but-signed, so the refresh path would sign the reader back in
assert.Equal(t, 2, partitioned, "the partitioned pair has to be cleared")
assert.Equal(t, 2, legacy, "so does any pair written before the option was enabled")
})
}

func TestJWT_PartitionedCookiesOffByDefault(t *testing.T) {
// every existing deployment has to keep writing byte-identical cookies
j := NewService(Opts{SecretReader: SecretFunc(mockKeyStore), SecureCookies: true,
SameSite: http.SameSiteNoneMode, TokenDuration: time.Hour, CookieDuration: days31,
})

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, err := j.Set(w, testClaims)
require.NoError(t, err)
w.WriteHeader(200)
}))
defer ts.Close()

resp, err := http.Get(ts.URL + "/")
require.NoError(t, err)
defer resp.Body.Close()

cookies := resp.Header.Values("Set-Cookie")
require.Len(t, cookies, 2, "without a length check this passes when Set writes nothing")
for _, c := range cookies {
assert.NotContains(t, c, "Partitioned", "%s must stay unpartitioned unless asked", c)
}
}
Loading
Loading