Skip to content

Commit

Permalink
fix: remove session id from jwt on disabled server side sessions
Browse files Browse the repository at this point in the history
  • Loading branch information
lfleischmann committed Dec 6, 2024
1 parent 21fd1d4 commit 60dcc43
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 11 deletions.
27 changes: 16 additions & 11 deletions backend/session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ type Manager interface {

// Manager is used to create and verify session JWTs
type manager struct {
jwtGenerator hankoJwt.Generator
sessionLength time.Duration
cookieConfig cookieConfig
issuer string
audience []string
jwtGenerator hankoJwt.Generator
sessionLength time.Duration
cookieConfig cookieConfig
issuer string
audience []string
serverSideSessionsEnabled bool
}

type cookieConfig struct {
Expand Down Expand Up @@ -85,16 +86,13 @@ func NewManager(jwkManager hankoJwk.Manager, config config.Config) (Manager, err
SameSite: sameSite,
Secure: config.Session.Cookie.Secure,
},
audience: audience,
audience: audience,
serverSideSessionsEnabled: config.Session.ServerSide.Enabled,
}, nil
}

// GenerateJWT creates a new session JWT for the given user
func (m *manager) GenerateJWT(userId uuid.UUID, email *dto.EmailJwt, opts ...JWTOptions) (string, jwt.Token, error) {
sessionID, err := uuid.NewV4()
if err != nil {
return "", nil, err
}
issuedAt := time.Now()
expiration := issuedAt.Add(m.sessionLength)

Expand All @@ -103,7 +101,14 @@ func (m *manager) GenerateJWT(userId uuid.UUID, email *dto.EmailJwt, opts ...JWT
_ = token.Set(jwt.IssuedAtKey, issuedAt)
_ = token.Set(jwt.ExpirationKey, expiration)
_ = token.Set(jwt.AudienceKey, m.audience)
_ = token.Set("session_id", sessionID.String())

if m.serverSideSessionsEnabled {
sessionID, err := uuid.NewV4()
if err != nil {
return "", nil, err
}
_ = token.Set("session_id", sessionID.String())
}

if email != nil {
_ = token.Set("email", &email)
Expand Down
58 changes: 58 additions & 0 deletions backend/session/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,64 @@ func TestManager_GenerateJWT_AdditionalAudiences(t *testing.T) {
assert.Equal(t, "hanko", token.Issuer())
}

func Test_GenerateJWT_SessionID(t *testing.T) {
tests := []struct {
name string
config config.Config
tokenShouldHaveSessionID bool
}{
{
name: "token has no session id claim if server side sessions are disabled",
config: config.Config{
Session: config.Session{
Lifespan: "5m",
ServerSide: config.ServerSide{
Enabled: false,
},
},
},
tokenShouldHaveSessionID: false,
},
{
name: "token has a session id claim if server side sessions are disabled",
config: config.Config{
Session: config.Session{
Lifespan: "5m",
ServerSide: config.ServerSide{
Enabled: true,
},
},
},
tokenShouldHaveSessionID: true,
},
}
for _, testData := range tests {
t.Run(testData.name, func(t *testing.T) {
jwkManager := test.JwkManager{}
sessionGenerator, err := NewManager(&jwkManager, testData.config)
assert.NoError(t, err)
require.NotEmpty(t, sessionGenerator)

userId, _ := uuid.NewV4()
tokenString, _, err := sessionGenerator.GenerateJWT(userId, nil)
assert.NoError(t, err)

token, err := jwt.ParseString(tokenString, jwt.WithVerify(false))
rawSessionID, tokenHasSessionID := token.Get("session_id")
assert.NoError(t, err)

assert.Equal(t, testData.tokenShouldHaveSessionID, tokenHasSessionID)

if testData.tokenShouldHaveSessionID {
sessionIDstring, ok := rawSessionID.(string)
assert.True(t, ok)
sessionID := uuid.FromStringOrNil(sessionIDstring)
assert.NotEqual(t, sessionID, uuid.Nil)
}
})
}
}

func TestGenerator_Verify_Error(t *testing.T) {
manager := test.JwkManager{}
cfg := config.Config{}
Expand Down

0 comments on commit 60dcc43

Please sign in to comment.