Skip to content

Commit

Permalink
feat: rework the OTP code submit count mechanism (#4251)
Browse files Browse the repository at this point in the history
* feat: rework the OTP code submit count mechanism

Unlike what the previous comment suggested, incrementing and checking the submit count inside the
database transaction is not actually optimal peformance- or security-wise.

We now check atomically increment and check the submit count as the first part of the operation,
and abort as early as possible if we detect brute-forcing. This prevents a situation where the
check works only on certain transaction isolation levels.

* chore: bump dependencies
  • Loading branch information
alnr authored Dec 23, 2024
1 parent deb3661 commit 4ca4d79
Show file tree
Hide file tree
Showing 17 changed files with 156 additions and 92 deletions.
9 changes: 9 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ jobs:
uses: sonatype-nexus-community/[email protected]
with:
nancyVersion: v1.0.42
- run: |
sudo apt-get update
name: apt-get update
- run: npm install
name: Install node deps
- name: Run golangci-lint
Expand Down Expand Up @@ -158,6 +161,9 @@ jobs:
- uses: ory/ci/checkout@master
with:
fetch-depth: 2
- run: |
sudo apt-get update
name: apt-get update
- run: |
npm ci
cd test/e2e; npm ci
Expand Down Expand Up @@ -261,6 +267,9 @@ jobs:
- uses: ory/ci/checkout@master
with:
fetch-depth: 2
- run: |
sudo apt-get update
name: apt-get update
- run: |
npm ci
cd test/e2e; npm ci
Expand Down
11 changes: 6 additions & 5 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ require (
go.opentelemetry.io/otel/sdk v1.32.0
go.opentelemetry.io/otel/trace v1.32.0
golang.org/x/crypto v0.31.0
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
golang.org/x/net v0.31.0
golang.org/x/exp v0.0.0-20241217172543-b2144cdd0a67 // indirect
golang.org/x/net v0.33.0
golang.org/x/oauth2 v0.24.0
golang.org/x/sync v0.10.0
golang.org/x/text v0.21.0
Expand All @@ -119,6 +119,7 @@ require (
github.com/jackc/pgx/v5 v5.6.0 // indirect
github.com/rjeczalik/notify v0.9.3 // indirect
golang.org/x/term v0.27.0 // indirect
golang.org/x/time v0.8.0 // indirect
gopkg.in/alecthomas/kingpin.v2 v2.2.6 // indirect
mvdan.cc/sh/v3 v3.6.0 // indirect
)
Expand Down Expand Up @@ -312,10 +313,10 @@ require (
go.opentelemetry.io/otel/metric v1.32.0 // indirect
go.opentelemetry.io/proto/otlp v1.3.1 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/mod v0.19.0 // indirect
golang.org/x/mod v0.22.0 // indirect
golang.org/x/sys v0.28.0 // indirect
golang.org/x/tools v0.23.0 // indirect
golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028 // indirect
golang.org/x/tools v0.28.0 // indirect
golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20241104194629-dd2ea8efbc28 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20241104194629-dd2ea8efbc28 // indirect
google.golang.org/protobuf v1.35.1
Expand Down
24 changes: 12 additions & 12 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -878,8 +878,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0LeHDbnYEryqj5Q1ug8=
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY=
golang.org/x/exp v0.0.0-20241217172543-b2144cdd0a67 h1:1UoZQm6f0P/ZO0w1Ri+f+ifG/gXhegadRdwBIXEFWDo=
golang.org/x/exp v0.0.0-20241217172543-b2144cdd0a67/go.mod h1:qj5a5QZpwLU2NLQudwIN5koi3beDhSAlJwa67PuM98c=
golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
Expand All @@ -904,8 +904,8 @@ golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
golang.org/x/mod v0.10.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
golang.org/x/mod v0.19.0 h1:fEdghXQSo20giMthA7cd28ZC+jts4amQ3YMXiP5oMQ8=
golang.org/x/mod v0.19.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4=
golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
Expand Down Expand Up @@ -954,8 +954,8 @@ golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns=
golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44=
golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
golang.org/x/net v0.31.0 h1:68CPQngjLL0r2AlUKiSxtQFKvzRVbnzLwMUn5SzcLHo=
golang.org/x/net v0.31.0/go.mod h1:P4fl1q7dY2hnZFxEk4pPSkDHF+QqjitcnDjUQyMM+pM=
golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I=
golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
Expand Down Expand Up @@ -1079,8 +1079,8 @@ golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk=
golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
golang.org/x/time v0.8.0 h1:9i3RxcPv3PZnitoVGMPDKZSq1xW1gK1Xy3ArNOGZfEg=
golang.org/x/time v0.8.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY=
Expand Down Expand Up @@ -1128,14 +1128,14 @@ golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
golang.org/x/tools v0.8.0/go.mod h1:JxBZ99ISMI5ViVkT1tr6tdNmXeTrcpVSD3vZ1RsRdN4=
golang.org/x/tools v0.23.0 h1:SGsXPZ+2l4JsgaCKkx+FQ9YZ5XEtA1GZYuoDjenLjvg=
golang.org/x/tools v0.23.0/go.mod h1:pnu6ufv6vQkll6szChhK3C3L/ruaIv5eBeztNG8wtsI=
golang.org/x/tools v0.28.0 h1:WuB6qZ4RPCQo5aP3WdKZS7i595EdWqWR8vqJTlwTVK8=
golang.org/x/tools v0.28.0/go.mod h1:dcIOrVd3mfQKTgrDVQHqCPMWy6lnhfhtX3hLXYVLfRw=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028 h1:+cNy6SZtPcJQH3LJVLOSmiC7MMxXNOb3PU/VUEz+EhU=
golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028/go.mod h1:NDW/Ps6MPRej6fsCIbMTohpP40sJ/P/vI1MoTEGwX90=
golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da h1:noIWHXmPHxILtqtCOPIhSt0ABwskkZKjD3bXGnZGpNY=
golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da/go.mod h1:NDW/Ps6MPRej6fsCIbMTohpP40sJ/P/vI1MoTEGwX90=
google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE=
google.golang.org/api v0.7.0/go.mod h1:WtwebWUNSVBH/HAw79HIFXZNqEvBhG+Ra+ax0hx3E3M=
google.golang.org/api v0.8.0/go.mod h1:o4eAsZoiT+ibD93RtjEohWalFOjRDx6CVaqeizhEnKg=
Expand Down
6 changes: 3 additions & 3 deletions identity/extension_verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ package identity

import (
"fmt"
"maps"
"slices"
"strings"
"sync"
"time"

"golang.org/x/exp/maps"

"github.com/ory/jsonschema/v3"
"github.com/ory/kratos/schema"
)
Expand Down Expand Up @@ -60,7 +60,7 @@ func (r *SchemaExtensionVerification) Run(ctx jsonschema.ValidationContext, s sc
formatString = "email"
formatter, ok := jsonschema.Formats[formatString]
if !ok {
supportedKeys := maps.Keys(jsonschema.Formats)
supportedKeys := slices.Collect(maps.Keys(jsonschema.Formats))
return ctx.Error("format", "format %q is not supported. Supported formats are [%s]", formatString, strings.Join(supportedKeys, ", "))
}

Expand Down
97 changes: 67 additions & 30 deletions persistence/sql/persister_code.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/gobuffalo/pop/v6"
"github.com/gofrs/uuid"
"github.com/pkg/errors"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"

"github.com/ory/kratos/selfservice/strategy/code"
"github.com/ory/x/otelx"
Expand Down Expand Up @@ -41,7 +43,7 @@ func useOneTimeCode[P any, U interface {
*P
oneTimeCodeProvider
}](ctx context.Context, p *Persister, flowID uuid.UUID, userProvidedCode string, flowTableName string, foreignKeyName string, opts ...codeOption,
) (_ U, err error) {
) (target U, err error) {
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.useOneTimeCode")
defer otelx.End(span, &err)

Expand All @@ -50,33 +52,21 @@ func useOneTimeCode[P any, U interface {
opt(o)
}

var target U
nid := p.NetworkID(ctx)
if err := p.Transaction(ctx, func(ctx context.Context, tx *pop.Connection) error {
//#nosec G201 -- TableName is static
if err := tx.RawQuery(fmt.Sprintf("UPDATE %s SET submit_count = submit_count + 1 WHERE id = ? AND nid = ?", flowTableName), flowID, nid).Exec(); err != nil {
return err
}

var submitCount int
// Because MySQL does not support "RETURNING" clauses, but we need the updated `submit_count` later on.
//#nosec G201 -- TableName is static
if err := sqlcon.HandleError(tx.RawQuery(fmt.Sprintf("SELECT submit_count FROM %s WHERE id = ? AND nid = ?", flowTableName), flowID, nid).First(&submitCount)); err != nil {
if errors.Is(err, sqlcon.ErrNoRows) {
// Return no error, as that would roll back the transaction
return nil
}
return err
}
// Before we do anything else, increment the submit count and check if we're
// being brute-forced. This is a separate statement/transaction to the rest
// of the operations so that it is correct for all transaction isolation
// levels.
submitCount, err := incrementOTPCodeSubmitCount(ctx, p, flowID, flowTableName)
if err != nil {
return nil, err
}
if submitCount > 5 {
return nil, errors.WithStack(code.ErrCodeSubmittedTooOften)
}

// This check prevents parallel brute force attacks by checking the submit count inside this database
// transaction. If the flow has been submitted more than 5 times, the transaction is aborted (regardless of
// whether the code was correct or not) and we thus give no indication whether the supplied code was correct or
// not. For more explanation see [this comment](https://github.com/ory/kratos/pull/2645#discussion_r984732899).
if submitCount > 5 {
return errors.WithStack(code.ErrCodeSubmittedTooOften)
}
nid := p.NetworkID(ctx)

if err := p.Transaction(ctx, func(ctx context.Context, tx *pop.Connection) error {
var codes []U
codesQuery := tx.Where(fmt.Sprintf("nid = ? AND %s = ?", foreignKeyName), nid, flowID)
if o.IdentityID != nil {
Expand All @@ -85,10 +75,8 @@ func useOneTimeCode[P any, U interface {

if err := sqlcon.HandleError(codesQuery.All(&codes)); err != nil {
if errors.Is(err, sqlcon.ErrNoRows) {
// Return no error, as that would roll back the transaction and reset the submit count.
return nil
return errors.WithStack(code.ErrCodeNotFound)
}

return err
}

Expand All @@ -107,7 +95,7 @@ func useOneTimeCode[P any, U interface {
}

if target.Validate() != nil {
// Return no error, as that would roll back the transaction
// Return no error, as that would roll back the transaction. We re-validate the code after the transaction.
return nil
}

Expand All @@ -123,3 +111,52 @@ func useOneTimeCode[P any, U interface {

return target, nil
}

func incrementOTPCodeSubmitCount(ctx context.Context, p *Persister, flowID uuid.UUID, flowTableName string) (submitCount int, err error) {
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.incrementOTPCodeSubmitCount",
trace.WithAttributes(attribute.Stringer("flow_id", flowID), attribute.String("flow_table_name", flowTableName)))
defer otelx.End(span, &err)
defer func() {
span.SetAttributes(attribute.Int("submit_count", submitCount))
}()

nid := p.NetworkID(ctx)

// The branch below is a marginal performance optimization for databases
// supporting RETURNING (one query instead of two). There is no real
// security difference here, but there is an observable difference in
// behavior.
//
// Databases supporting RETURNING will perform the increment+select
// atomically. That means that always exactly 5 attempts will be allowed for
// each flow, no matter how many concurrent attempts are made.
//
// Databases without support for RETURNING (MySQL) will perform the UPDATE
// and SELECT in two queries, which are not atomic. The effect is that there
// will still never be more than 5 attempts for each flow, but there may be
// fewer before we reject. Under normal operation, this is never a problem
// because a human will never submit their code as quickly as would be
// required to trigger this race condition.
//
// In a very strict sense of the word, the MySQL implementation is even more
// secure than the RETURNING implementation. But we're ok either way :)
if p.c.Dialect.Name() == "mysql" {
//#nosec G201 -- TableName is static
qUpdate := fmt.Sprintf("UPDATE %s SET submit_count = submit_count + 1 WHERE id = ? AND nid = ?", flowTableName)
if err := p.GetConnection(ctx).RawQuery(qUpdate, flowID, nid).Exec(); err != nil {
return 0, sqlcon.HandleError(err)
}
//#nosec G201 -- TableName is static
qSelect := fmt.Sprintf("SELECT submit_count FROM %s WHERE id = ? AND nid = ?", flowTableName)
err = sqlcon.HandleError(p.GetConnection(ctx).RawQuery(qSelect, flowID, nid).First(&submitCount))
} else {
//#nosec G201 -- TableName is static
q := fmt.Sprintf("UPDATE %s SET submit_count = submit_count + 1 WHERE id = ? AND nid = ? RETURNING submit_count", flowTableName)
err = sqlcon.HandleError(p.Connection(ctx).RawQuery(q, flowID, nid).First(&submitCount))
}
if errors.Is(err, sqlcon.ErrNoRows) {
return 0, errors.WithStack(code.ErrCodeNotFound)
}

return submitCount, err
}
2 changes: 1 addition & 1 deletion persistence/sql/persister_login_code.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (p *Persister) UseLoginCode(ctx context.Context, flowID uuid.UUID, identity
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.UseLoginCode")
defer otelx.End(span, &err)

codeRow, err := useOneTimeCode[code.LoginCode, *code.LoginCode](ctx, p, flowID, userProvidedCode, new(login.Flow).TableName(ctx), "selfservice_login_flow_id", withCheckIdentityID(identityID))
codeRow, err := useOneTimeCode[code.LoginCode](ctx, p, flowID, userProvidedCode, new(login.Flow).TableName(ctx), "selfservice_login_flow_id", withCheckIdentityID(identityID))
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion persistence/sql/persister_recovery_code.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (p *Persister) UseRecoveryCode(ctx context.Context, flowID uuid.UUID, userP
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.UseRecoveryCode")
defer otelx.End(span, &err)

codeRow, err := useOneTimeCode[code.RecoveryCode, *code.RecoveryCode](ctx, p, flowID, userProvidedCode, new(recovery.Flow).TableName(ctx), "selfservice_recovery_flow_id")
codeRow, err := useOneTimeCode[code.RecoveryCode](ctx, p, flowID, userProvidedCode, new(recovery.Flow).TableName(ctx), "selfservice_recovery_flow_id")
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion persistence/sql/persister_registration_code.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (p *Persister) UseRegistrationCode(ctx context.Context, flowID uuid.UUID, u
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.UseRegistrationCode")
defer otelx.End(span, &err)

codeRow, err := useOneTimeCode[code.RegistrationCode, *code.RegistrationCode](ctx, p, flowID, userProvidedCode, new(registration.Flow).TableName(ctx), "selfservice_registration_flow_id")
codeRow, err := useOneTimeCode[code.RegistrationCode](ctx, p, flowID, userProvidedCode, new(registration.Flow).TableName(ctx), "selfservice_registration_flow_id")
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion persistence/sql/persister_verification_code.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (p *Persister) UseVerificationCode(ctx context.Context, flowID uuid.UUID, u
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.UseVerificationCode")
defer otelx.End(span, &err)

codeRow, err := useOneTimeCode[code.VerificationCode, *code.VerificationCode](ctx, p, flowID, userProvidedCode, new(verification.Flow).TableName(ctx), "selfservice_verification_flow_id")
codeRow, err := useOneTimeCode[code.VerificationCode](ctx, p, flowID, userProvidedCode, new(verification.Flow).TableName(ctx), "selfservice_verification_flow_id")
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion selfservice/hook/web_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"encoding/json"
"fmt"
"io"
"maps"
"net/http"
"net/textproto"
"time"
Expand All @@ -21,7 +22,6 @@ import (
"go.opentelemetry.io/otel/codes"
semconv "go.opentelemetry.io/otel/semconv/v1.11.0"
"go.opentelemetry.io/otel/trace"
"golang.org/x/exp/maps"
grpccodes "google.golang.org/grpc/codes"

"github.com/ory/herodot"
Expand Down
2 changes: 1 addition & 1 deletion selfservice/hook/web_hook_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"slices"
"strconv"
"sync"
"testing"
Expand All @@ -27,7 +28,6 @@ import (
"go.opentelemetry.io/otel/attribute"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
"go.opentelemetry.io/otel/sdk/trace/tracetest"
"golang.org/x/exp/slices"

"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/identity"
Expand Down
27 changes: 22 additions & 5 deletions selfservice/strategy/code/test/persistence.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ package code

import (
"context"
"errors"
"sync"
"sync/atomic"
"testing"
"time"

Expand Down Expand Up @@ -113,13 +116,27 @@ func TestPersister(ctx context.Context, p interface {
_, err := p.CreateRecoveryCode(ctx, dto)
require.NoError(t, err)

for i := 1; i <= 5; i++ {
_, err = p.UseRecoveryCode(ctx, f.ID, "i-do-not-exist")
require.Error(t, err)
var tooOften, wrongCode int32
var wg sync.WaitGroup
for range 50 {
wg.Add(1)
go func() {
defer wg.Done()
_, err := p.UseRecoveryCode(ctx, f.ID, "i-do-not-exist")
if !assert.Error(t, err) {
return
}
if errors.Is(err, code.ErrCodeSubmittedTooOften) {
atomic.AddInt32(&tooOften, 1)
} else {
atomic.AddInt32(&wrongCode, 1)
}
}()
}
wg.Wait()

_, err = p.UseRecoveryCode(ctx, f.ID, "i-do-not-exist")
require.ErrorIs(t, err, code.ErrCodeSubmittedTooOften)
require.EqualValues(t, 50, wrongCode+tooOften, "all 50 attempts made")
require.LessOrEqual(t, wrongCode, int32(5), "max. 5 attempts have gone past the duplication check")

// Submit again, just to be sure
_, err = p.UseRecoveryCode(ctx, f.ID, "i-do-not-exist")
Expand Down
Loading

0 comments on commit 4ca4d79

Please sign in to comment.