mirror of
https://github.com/pocket-id/pocket-id.git
synced 2026-08-19 03:16:28 +00:00
fix: fail early if webauthn session invalid
This commit is contained in:
@@ -124,6 +124,13 @@ type MissingSessionIdError struct{}
|
||||
func (e MissingSessionIdError) Error() string { return "Missing session id" }
|
||||
func (e MissingSessionIdError) HttpStatusCode() int { return http.StatusBadRequest }
|
||||
|
||||
type InvalidWebauthnSessionError struct{}
|
||||
|
||||
func (e InvalidWebauthnSessionError) Error() string {
|
||||
return "WebAuthn session is invalid or has expired"
|
||||
}
|
||||
func (e InvalidWebauthnSessionError) HttpStatusCode() int { return http.StatusBadRequest }
|
||||
|
||||
type ReservedClaimError struct {
|
||||
Key string
|
||||
}
|
||||
|
||||
@@ -130,13 +130,15 @@ func (s *Service) VerifyRegistration(ctx context.Context, sessionID string, user
|
||||
|
||||
// Load & delete the session row
|
||||
var storedSession WebauthnSession
|
||||
err := tx.
|
||||
result := tx.
|
||||
WithContext(ctx).
|
||||
Clauses(clause.Returning{}).
|
||||
Delete(&storedSession, "id = ?", sessionID).
|
||||
Error
|
||||
if err != nil {
|
||||
return model.WebauthnCredential{}, fmt.Errorf("failed to load WebAuthn session: %w", err)
|
||||
Delete(&storedSession, "id = ?", sessionID)
|
||||
if result.Error != nil {
|
||||
return model.WebauthnCredential{}, fmt.Errorf("failed to load WebAuthn session: %w", result.Error)
|
||||
}
|
||||
if result.RowsAffected == 0 {
|
||||
return model.WebauthnCredential{}, &common.InvalidWebauthnSessionError{}
|
||||
}
|
||||
|
||||
session := gowebauthn.SessionData{
|
||||
@@ -148,7 +150,7 @@ func (s *Service) VerifyRegistration(ctx context.Context, sessionID string, user
|
||||
}
|
||||
|
||||
var user model.User
|
||||
err = tx.
|
||||
err := tx.
|
||||
WithContext(ctx).
|
||||
Find(&user, "id = ?", userID).
|
||||
Error
|
||||
@@ -238,13 +240,15 @@ func (s *Service) VerifyLogin(ctx context.Context, dbConfig *appconfig.AppConfig
|
||||
|
||||
// Load & delete the session row
|
||||
var storedSession WebauthnSession
|
||||
err := tx.
|
||||
result := tx.
|
||||
WithContext(ctx).
|
||||
Clauses(clause.Returning{}).
|
||||
Delete(&storedSession, "id = ?", sessionID).
|
||||
Error
|
||||
if err != nil {
|
||||
return model.User{}, "", fmt.Errorf("failed to load WebAuthn session: %w", err)
|
||||
Delete(&storedSession, "id = ?", sessionID)
|
||||
if result.Error != nil {
|
||||
return model.User{}, "", fmt.Errorf("failed to load WebAuthn session: %w", result.Error)
|
||||
}
|
||||
if result.RowsAffected == 0 {
|
||||
return model.User{}, "", &common.InvalidWebauthnSessionError{}
|
||||
}
|
||||
|
||||
session := gowebauthn.SessionData{
|
||||
@@ -255,7 +259,7 @@ func (s *Service) VerifyLogin(ctx context.Context, dbConfig *appconfig.AppConfig
|
||||
}
|
||||
|
||||
var user *model.User
|
||||
_, err = s.webAuthn.ValidateDiscoverableLogin(func(_, userHandle []byte) (gowebauthn.User, error) {
|
||||
_, err := s.webAuthn.ValidateDiscoverableLogin(func(_, userHandle []byte) (gowebauthn.User, error) {
|
||||
innerErr := tx.
|
||||
WithContext(ctx).
|
||||
Preload("Credentials").
|
||||
@@ -442,13 +446,15 @@ func (s *Service) CreateReauthenticationTokenWithWebauthn(ctx context.Context, s
|
||||
|
||||
// Retrieve and delete the session
|
||||
var storedSession WebauthnSession
|
||||
err := tx.
|
||||
result := tx.
|
||||
WithContext(ctx).
|
||||
Clauses(clause.Returning{}).
|
||||
Delete(&storedSession, "id = ? AND expires_at > ?", sessionID, datatype.DateTime(time.Now())).
|
||||
Error
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("failed to load WebAuthn session: %w", err)
|
||||
Delete(&storedSession, "id = ? AND expires_at > ?", sessionID, datatype.DateTime(time.Now()))
|
||||
if result.Error != nil {
|
||||
return "", fmt.Errorf("failed to load WebAuthn session: %w", result.Error)
|
||||
}
|
||||
if result.RowsAffected == 0 {
|
||||
return "", &common.InvalidWebauthnSessionError{}
|
||||
}
|
||||
|
||||
session := gowebauthn.SessionData{
|
||||
@@ -460,7 +466,7 @@ func (s *Service) CreateReauthenticationTokenWithWebauthn(ctx context.Context, s
|
||||
|
||||
// Validate the credential assertion
|
||||
var user *model.User
|
||||
_, err = s.webAuthn.ValidateDiscoverableLogin(func(_, userHandle []byte) (gowebauthn.User, error) {
|
||||
_, err := s.webAuthn.ValidateDiscoverableLogin(func(_, userHandle []byte) (gowebauthn.User, error) {
|
||||
innerErr := tx.
|
||||
WithContext(ctx).
|
||||
Preload("Credentials").
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/go-webauthn/webauthn/protocol"
|
||||
"github.com/lestrrat-go/jwx/v3/jwt"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
@@ -132,6 +133,74 @@ func TestWebAuthnDisplayNameUsesRequestConfig(t *testing.T) {
|
||||
require.Equal(t, "Custom App", service.webAuthn.Config.RPDisplayName)
|
||||
}
|
||||
|
||||
// A ceremony that references a session which does not exist must be rejected outright
|
||||
// The delete-and-return leaves the struct zero-valued when nothing matched, and a zero session has an
|
||||
// empty user verification requirement, a zero expiry and an empty challenge, so letting it reach the
|
||||
// library would validate the assertion with user verification and expiry enforcement silently disabled
|
||||
func TestCeremoniesRejectSessionThatDoesNotExist(t *testing.T) {
|
||||
const userID = "ceremony-user"
|
||||
|
||||
setupService := func(t *testing.T) *Service {
|
||||
t.Helper()
|
||||
|
||||
db := testutils.NewDatabaseForTest(t)
|
||||
require.NoError(t, db.Create(&model.User{
|
||||
Base: model.Base{ID: userID},
|
||||
Username: userID,
|
||||
}).Error)
|
||||
|
||||
return &Service{db: db}
|
||||
}
|
||||
|
||||
t.Run("registration rejects an unknown session", func(t *testing.T) {
|
||||
service := setupService(t)
|
||||
|
||||
_, err := service.VerifyRegistration(t.Context(), "does-not-exist", userID, nil, "127.0.0.1")
|
||||
|
||||
require.Error(t, err)
|
||||
assert.ErrorAs(t, err, new(*common.InvalidWebauthnSessionError))
|
||||
})
|
||||
|
||||
t.Run("login rejects an unknown session", func(t *testing.T) {
|
||||
service := setupService(t)
|
||||
|
||||
_, token, err := service.VerifyLogin(t.Context(), &appconfig.AppConfigModel{}, "does-not-exist", nil, "127.0.0.1", "test-agent")
|
||||
|
||||
assert.Empty(t, token)
|
||||
require.Error(t, err)
|
||||
assert.ErrorAs(t, err, new(*common.InvalidWebauthnSessionError))
|
||||
})
|
||||
|
||||
t.Run("reauthentication rejects an unknown session", func(t *testing.T) {
|
||||
service := setupService(t)
|
||||
|
||||
token, err := service.CreateReauthenticationTokenWithWebauthn(t.Context(), "does-not-exist", nil)
|
||||
|
||||
assert.Empty(t, token)
|
||||
require.Error(t, err)
|
||||
assert.ErrorAs(t, err, new(*common.InvalidWebauthnSessionError))
|
||||
})
|
||||
|
||||
// The reauthentication query filters expired rows out in SQL, so an expired session matches no row
|
||||
// and previously produced the same zero-valued session as an unknown one
|
||||
t.Run("reauthentication rejects an expired session", func(t *testing.T) {
|
||||
service := setupService(t)
|
||||
|
||||
expiredSession := WebauthnSession{
|
||||
Challenge: "expired-challenge",
|
||||
ExpiresAt: datatype.DateTime(time.Now().Add(-time.Minute)),
|
||||
UserVerification: string(protocol.VerificationRequired),
|
||||
}
|
||||
require.NoError(t, service.db.Create(&expiredSession).Error)
|
||||
|
||||
token, err := service.CreateReauthenticationTokenWithWebauthn(t.Context(), expiredSession.ID, nil)
|
||||
|
||||
assert.Empty(t, token)
|
||||
require.Error(t, err)
|
||||
assert.ErrorAs(t, err, new(*common.InvalidWebauthnSessionError))
|
||||
})
|
||||
}
|
||||
|
||||
func TestConsumeReauthenticationTokenReturnsTokenCreationTime(t *testing.T) {
|
||||
db := testutils.NewDatabaseForTest(t)
|
||||
service := &Service{db: db}
|
||||
|
||||
Reference in New Issue
Block a user