Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: rework the OTP code submit count mechanism #4251

Merged
merged 5 commits into from
Dec 23, 2024
Merged

feat: rework the OTP code submit count mechanism #4251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ba177c3
chore: bump dependencies
alnr Dec 19, 2024
e1ecc70
feat: rework the OTP code submit count mechanism
alnr Dec 19, 2024
5600b49
feat: don't use transaction for MySQL
alnr Dec 20, 2024
e70ecf3
fix: test
alnr Dec 23, 2024
b094bdc
fix: add stack
alnr Dec 23, 2024
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
9 changes: 9 additions & 0 deletions .github/workflows/ci.yaml
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@

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 @@
formatString = "email"
formatter, ok := jsonschema.Formats[formatString]
if !ok {
supportedKeys := maps.Keys(jsonschema.Formats)
supportedKeys := slices.Collect(maps.Keys(jsonschema.Formats))

Check warning on line 63 in identity/extension_verification.go

View check run for this annotation

Codecov / codecov/patch

identity/extension_verification.go#L63 

Added line #L63 was not covered by tests
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
"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 @@
*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 @@
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 @@

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)

Check warning on line 78 in persistence/sql/persister_code.go

View check run for this annotation

Codecov / codecov/patch

persistence/sql/persister_code.go#L78 

Added line #L78 was not covered by tests
}

return err
}

Expand All @@ -107,7 +95,7 @@
}

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 @@

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)

Check warning on line 147 in persistence/sql/persister_code.go

View check run for this annotation

Codecov / codecov/patch

persistence/sql/persister_code.go#L147 

Added line #L147 was not covered by tests
}
//#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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

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

Expand Down Expand Up @@ -113,13 +116,27 @@
_, 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

Check warning on line 127 in selfservice/strategy/code/test/persistence.go

View check run for this annotation

Codecov / codecov/patch

selfservice/strategy/code/test/persistence.go#L127 

Added line #L127 was not covered by tests
}
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
Loading