Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
15 changes: 12 additions & 3 deletions internal/storage/v2/dialect/postgres/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,15 @@ const (
VALUES ($1, $2, $3, $4::INTERVAL, $5) RETURNING created_at, updated_at, expires_at`
updateSessionTokenIDStmt = `UPDATE zitadel_nextgen.sessions SET token_id = $3 WHERE project_id = $1 AND id = $2`
deleteSessionByIDStmt = `DELETE FROM zitadel_nextgen.sessions WHERE project_id = $1 AND id = $2`
sessionQuery = `SELECT s.project_id, s.id, s.created_at, s.updated_at, s.expires_at, s.time_to_live, s.token_id, s.user_id,
// The inner subquery filters, orders and limits the sessions; user agents
// and checks are then joined onto that page. Limiting after the
// one-to-many checks join would bound joined rows, not sessions.
sessionQuerySelect = `SELECT s.project_id, s.id, s.created_at, s.updated_at, s.expires_at, s.time_to_live, s.token_id, s.user_id,
ua.id, ua.info,
c.type, c.id, c.last_challenged_at, c.last_verified_at, c.last_failed_at, c.failure_count, c.challenge_payload, c.factor_payload
FROM zitadel_nextgen.sessions s
FROM (`
sessionQueryInner = `SELECT s.* FROM zitadel_nextgen.sessions s`
sessionQueryJoins = `) s
LEFT JOIN zitadel_nextgen.user_agents ua ON s.project_id = ua.project_id AND s.user_agent_id = ua.id
LEFT JOIN zitadel_nextgen.checks c ON c.project_id = s.project_id AND c.session_id = s.id`
loadAttemptChecksStmt = `SELECT id, type, last_verified_at FROM zitadel_nextgen.checks WHERE project_id = $1 AND auth_attempt_id = $2 AND last_verified_at IS NOT NULL`
Expand Down Expand Up @@ -131,9 +136,13 @@ func (ss sessionStatements) LoadVerifiedChecks(ctx context.Context, projectID, i

func (ss sessionStatements) querySessions(ctx context.Context, filter *database.ListOptions[domain.SessionField]) ([]*domain.Session, error) {
var compiler statementCompiler
if err := compileRead(&compiler, sessionQuery, filter, sessionSchema); err != nil {
compiler.WriteString(sessionQuerySelect)
if err := compileRead(&compiler, sessionQueryInner, filter, sessionSchema); err != nil {
return nil, err
}
compiler.WriteString(sessionQueryJoins)
// The joins do not preserve the subquery's order.
compileOrderBy(&compiler, filter.Pagination.OrderBy, sessionSchema)
rows, err := ss.client.Query(ctx, compiler.String(), compiler.args...)
if err != nil {
return nil, wrapError(err)
Expand Down
17 changes: 14 additions & 3 deletions internal/storage/v2/dialect/spanner/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,29 @@ func (s sessionExchangeStore) DeleteAuthAttempt(ctx context.Context, projectID,
return newAuthAttemptStatements(s.db).DeleteAuthAttemptByID(ctx, projectID, attemptID)
}

const sessionQuery = `SELECT s.project_id, s.id, s.created_at, s.updated_at, s.expires_at, s.time_to_live, s.token_id, s.user_id,
const (
// The inner subquery filters, orders and limits the sessions; user agents
// and checks are then joined onto that page. Limiting after the
// one-to-many checks join would bound joined rows, not sessions.
sessionQuerySelect = `SELECT s.project_id, s.id, s.created_at, s.updated_at, s.expires_at, s.time_to_live, s.token_id, s.user_id,
ua.id, ua.info,
c.type, c.id, c.last_challenged_at, c.last_verified_at, c.last_failed_at, c.failure_count, c.challenge_payload, c.factor_payload
FROM sessions s
FROM (`
sessionQueryInner = `SELECT s.* FROM sessions s`
sessionQueryJoins = `) s
LEFT JOIN user_agents ua ON s.project_id = ua.project_id AND s.user_agent_id = ua.id
LEFT JOIN checks c ON c.project_id = s.project_id AND c.session_id = s.id`
)

func (ss sessionStatements) querySessions(ctx context.Context, filter *database.ListOptions[domain.SessionField]) ([]*domain.Session, error) {
var compiler statementCompiler
if err := compileRead(&compiler, sessionQuery, filter, sessionSchema); err != nil {
compiler.WriteString(sessionQuerySelect)
if err := compileRead(&compiler, sessionQueryInner, filter, sessionSchema); err != nil {
return nil, err
}
compiler.WriteString(sessionQueryJoins)
// The joins do not preserve the subquery's order.
compileOrderBy(&compiler, filter.Pagination.OrderBy, sessionSchema)
var sessions []*domain.Session
err := ss.db.Query(ctx, compiler.statement(), func(iter *spanner.RowIterator) error {
var err error
Expand Down
17 changes: 14 additions & 3 deletions internal/storage/v2/dialect/sqlite/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,19 @@ import (
v2session "github.com/zitadel/nextgen/internal/storage/v2/session"
)

const sessionQuery = `SELECT s.project_id, s.id, s.created_at, s.updated_at, s.expires_at, s.time_to_live, s.token_id, s.user_id,
const (
// The inner subquery filters, orders and limits the sessions; user agents
// and checks are then joined onto that page. Limiting after the
// one-to-many checks join would bound joined rows, not sessions.
sessionQuerySelect = `SELECT s.project_id, s.id, s.created_at, s.updated_at, s.expires_at, s.time_to_live, s.token_id, s.user_id,
ua.id, ua.info,
c.type, c.id, c.last_challenged_at, c.last_verified_at, c.last_failed_at, c.failure_count, c.challenge_payload, c.factor_payload
FROM sessions s
FROM (`
sessionQueryInner = `SELECT s.* FROM sessions s`
sessionQueryJoins = `) s
LEFT JOIN user_agents ua ON s.project_id = ua.project_id AND s.user_agent_id = ua.id
LEFT JOIN checks c ON c.project_id = s.project_id AND c.session_id = s.id`
)

type sessionStatements struct{ statement }

Expand Down Expand Up @@ -113,9 +120,13 @@ func (s sessionExchangeStore) DeleteAuthAttempt(ctx context.Context, projectID,

func (ss sessionStatements) querySessions(ctx context.Context, filter *database.ListOptions[domain.SessionField]) ([]*domain.Session, error) {
var compiler statementCompiler
if err := compileRead(&compiler, sessionQuery, filter, sessionSchema); err != nil {
compiler.WriteString(sessionQuerySelect)
if err := compileRead(&compiler, sessionQueryInner, filter, sessionSchema); err != nil {
return nil, err
}
compiler.WriteString(sessionQueryJoins)
// The joins do not preserve the subquery's order.
compileOrderBy(&compiler, filter.Pagination.OrderBy, sessionSchema)
rows, err := ss.client.Query(ctx, compiler.String(), compiler.args...)
if err != nil {
return nil, wrapError(err)
Expand Down
109 changes: 109 additions & 0 deletions internal/storage/v2/stmttest/session_multicheck_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
//go:build postgres_integration || spanner_integration || sqlite_integration

package stmttest

import (
"context"
"slices"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/zitadel/nextgen/internal/domain"
"github.com/zitadel/nextgen/internal/service"
"github.com/zitadel/nextgen/internal/storage/v2/database"
)

// createTwoCheckSession exchanges an attempt with verified user and password
// factors, so the session carries two check rows.
func createTwoCheckSession(t *testing.T, stmts service.AllStatements, projectID, userID string) *domain.Session {
t.Helper()
plain, _ := handoffCompletedAttemptWithUser(t, stmts, projectID, userID)
session, err := stmts.ExchangeSession(t.Context(), projectID, plain, nil, time.Hour)
require.NoError(t, err)
sessionID := session.ID
t.Cleanup(func() {
_ = stmts.DeleteSessionByID(context.Background(), projectID, sessionID)
})
return session
}

// TestSessionStatements_List_LimitBoundsSessions pages three sessions that
// carry two check rows each. The limit must bound sessions, not joined rows:
// a limit on joined rows shrinks the page and withholds the cursor, making
// the remaining sessions unreachable (issue 782).
func TestSessionStatements_List_LimitBoundsSessions(t *testing.T) {
forEachDialect(t, func(t *testing.T, d dialect) {
projectID, schemaURL := ensureUserTestProject(t, d.stmts)

userID := "usr-2fa-" + uniqueSuffix(t)
require.NoError(t, d.stmts.CreateUser(t.Context(), newTestUser(t, projectID, schemaURL, userID, userID+"@example.com", "Two Check User")))
want := make([]string, 0, 3)
for range 3 {
want = append(want, createTwoCheckSession(t, d.stmts, projectID, userID).ID)
}
slices.Sort(want)

list := func(cursor []byte) *database.ListResult[*domain.Session] {
result, err := d.stmts.ListSessions(t.Context(), &database.ListOptions[domain.SessionField]{
Filter: database.Equal(database.Col(domain.SessionFieldProjectID), projectID),
Pagination: database.Page[domain.SessionField]{
Limit: 2,
Cursor: cursor,
OrderBy: database.OrderBy[domain.SessionField]{
Columns: []database.Column[domain.SessionField]{database.Col(domain.SessionFieldID)},
},
},
})
require.NoError(t, err)
return result
}

page1 := list(nil)
require.Len(t, page1.Items, 2, "a full page must hold as many sessions as the limit")
require.NotEmpty(t, page1.NextCursor, "a full page must issue a next cursor")

page2 := list(page1.NextCursor)
require.Len(t, page2.Items, 1)
assert.Empty(t, page2.NextCursor)

got := make([]string, 0, 3)
for _, session := range append(page1.Items, page2.Items...) {
assert.Len(t, session.Factors, 2, "session %s must keep its complete factor list", session.ID)
got = append(got, session.ID)
}
// Exact order, not ElementsMatch: the ORDER BY after the joins is
// otherwise unfenced.
assert.Equal(t, want, got, "pages must return every session exactly once, in ID order")
})
}

// TestSessionStatements_List_LimitKeepsFactorsComplete lists a two-check
// session with limit 1. A limit on joined rows would cut inside the session's
// check rows and truncate its factor list (issue 782).
func TestSessionStatements_List_LimitKeepsFactorsComplete(t *testing.T) {
forEachDialect(t, func(t *testing.T, d dialect) {
projectID, schemaURL := ensureUserTestProject(t, d.stmts)

userID := "usr-2fa-" + uniqueSuffix(t)
require.NoError(t, d.stmts.CreateUser(t.Context(), newTestUser(t, projectID, schemaURL, userID, userID+"@example.com", "Two Check User")))
created := createTwoCheckSession(t, d.stmts, projectID, userID)

result, err := d.stmts.ListSessions(t.Context(), &database.ListOptions[domain.SessionField]{
Filter: database.Equal(database.Col(domain.SessionFieldProjectID), projectID),
Pagination: database.Page[domain.SessionField]{Limit: 1},
})
require.NoError(t, err)
require.Len(t, result.Items, 1)
require.Equal(t, created.ID, result.Items[0].ID)

gotTypes := make([]domain.AuthCheckType, 0, len(result.Items[0].Factors))
for _, factor := range result.Items[0].Factors {
gotTypes = append(gotTypes, factor.Type())
}
assert.ElementsMatch(t, []domain.AuthCheckType{domain.AuthCheckTypeUser, domain.AuthCheckTypePassword}, gotTypes,
"the paged session must carry its complete factor list")
})
}
Loading