TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2025-08-11 20:47:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(session api): respect lockout policy (#8027)

Browse Source 
# Which Problems Are Solved

The session API was designed to be flexible enough for multiple use
cases / login scenarios, where the login could respect the login policy
or not. The session API itself does not have a corresponding policy and
would not check for a required MFA or alike. It therefore also did not
yet respect the lockout policy and would leave it to the login UI to
handle that.
Since the lockout policy is related to the user and not the login
itself, we decided to handle the lockout also on calls of the session
API.

# How the Problems Are Solved

If a lockout policy is set for either password or (T)OTP checks, the
corresponding check on the session API be run against the lockout check.
This means that any failed check, regardless if occurred in the session
API or the current hosted login will be counted against the maximum
allowed checks of that authentication mechanism. TOTP, OTP SMS and OTP
Email are each treated as a separate mechanism.

For implementation:
- The existing lockout check functions were refactored to be usable for
session API calls.
- `SessionCommand` type now returns not only an error, but also
`[]eventstore.Command`
  - these will be executed in case of an error

# Additional Changes

None.

# Additional Context

Closes #7967

---------

Co-authored-by: Elio Bischof <elio@zitadel.com>
This commit is contained in:
Livio Spring
2024-05-31 00:08:48 +02:00
committed by GitHub
parent 7ede3ec189
commit aabefb9382
13 changed files with 732 additions and 250 deletions
Download Patch File Download Diff File Expand all files Collapse all files

304
internal/command/session_otp_test.go
View File

@@ -11,6 +11,7 @@ import (
"github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/eventstore"
"github.com/zitadel/zitadel/internal/repository/org"
"github.com/zitadel/zitadel/internal/repository/session"
"github.com/zitadel/zitadel/internal/repository/user"
"github.com/zitadel/zitadel/internal/zerrors"
@@ -110,8 +111,9 @@ func TestCommands_CreateOTPSMSChallengeReturnCode(t *testing.T) {
now: time.Now,
}
err := cmd(context.Background(), cmds)
gotCmds, err := cmd(context.Background(), cmds)
assert.ErrorIs(t, err, tt.res.err)
assert.Empty(t, gotCmds)
assert.Equal(t, tt.res.returnCode, dst)
assert.Equal(t, tt.res.commands, cmds.eventCommands)
})
@@ -210,8 +212,9 @@ func TestCommands_CreateOTPSMSChallenge(t *testing.T) {
now: time.Now,
}
err := cmd(context.Background(), cmds)
gotCmds, err := cmd(context.Background(), cmds)
assert.ErrorIs(t, err, tt.res.err)
assert.Empty(t, gotCmds)
assert.Equal(t, tt.res.commands, cmds.eventCommands)
})
}
@@ -410,8 +413,9 @@ func TestCommands_CreateOTPEmailChallengeURLTemplate(t *testing.T) {
now: time.Now,
}
err = cmd(context.Background(), cmds)
gotCmds, err := cmd(context.Background(), cmds)
assert.ErrorIs(t, err, tt.res.err)
assert.Empty(t, gotCmds)
assert.Equal(t, tt.res.commands, cmds.eventCommands)
})
}
@@ -511,8 +515,9 @@ func TestCommands_CreateOTPEmailChallengeReturnCode(t *testing.T) {
now: time.Now,
}
err := cmd(context.Background(), cmds)
gotCmds, err := cmd(context.Background(), cmds)
assert.ErrorIs(t, err, tt.res.err)
assert.Empty(t, gotCmds)
assert.Equal(t, tt.res.returnCode, dst)
assert.Equal(t, tt.res.commands, cmds.eventCommands)
})
@@ -611,8 +616,9 @@ func TestCommands_CreateOTPEmailChallenge(t *testing.T) {
now: time.Now,
}
err := cmd(context.Background(), cmds)
gotCmds, err := cmd(context.Background(), cmds)
assert.ErrorIs(t, err, tt.res.err)
assert.Empty(t, gotCmds)
assert.Equal(t, tt.res.commands, cmds.eventCommands)
})
}
@@ -701,8 +707,9 @@ func TestCheckOTPSMS(t *testing.T) {
code string
}
type res struct {
err error
commands []eventstore.Command
err error
commands []eventstore.Command
errorCommands []eventstore.Command
}
tests := []struct {
name string
@@ -720,13 +727,43 @@ func TestCheckOTPSMS(t *testing.T) {
code: "code",
},
res: res{
err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-VDrh3", "Errors.User.UserIDMissing"),
err: zerrors.ThrowInvalidArgument(nil, "COMMAND-S453v", "Errors.User.UserIDMissing"),
},
},
{
name: "missing code",
fields: fields{
eventstore: expectEventstore(),
userID: "userID",
},
args: args{},
res: res{
err: zerrors.ThrowInvalidArgument(nil, "COMMAND-SJl2g", "Errors.User.Code.Empty"),
},
},
{
name: "not set up",
fields: fields{
eventstore: expectEventstore(
expectFilter(),
),
userID: "userID",
},
args: args{
code: "code",
},
res: res{
err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-d2r52", "Errors.User.MFA.OTP.NotReady"),
},
},
{
name: "missing challenge",
fields: fields{
eventstore: expectEventstore(),
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(user.NewHumanOTPSMSAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)),
),
),
userID: "userID",
otpCodeChallenge: nil,
},
@@ -734,14 +771,26 @@ func TestCheckOTPSMS(t *testing.T) {
code: "code",
},
res: res{
err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-SF3tv", "Errors.User.Code.NotFound"),
err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-S34gh", "Errors.User.Code.NotFound"),
},
},
{
name: "invalid code",
fields: fields{
eventstore: expectEventstore(),
userID: "userID",
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(user.NewHumanOTPSMSAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)),
),
expectFilter(), // recheck
expectFilter(
eventFromEventPusher(
org.NewLockoutPolicyAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate,
0, 0, false,
),
),
),
),
userID: "userID",
otpCodeChallenge: &OTPCode{
Code: &crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
@@ -759,13 +808,61 @@ func TestCheckOTPSMS(t *testing.T) {
},
res: res{
err: zerrors.ThrowPreconditionFailed(nil, "CODE-QvUQ4P", "Errors.User.Code.Expired"),
errorCommands: []eventstore.Command{
user.NewHumanOTPSMSCheckFailedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil),
},
},
},
{
name: "invalid code, locked",
fields: fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(user.NewHumanOTPSMSAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)),
),
expectFilter(), // recheck
expectFilter(
eventFromEventPusher(
org.NewLockoutPolicyAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate,
0, 1, false,
),
),
),
),
userID: "userID",
otpCodeChallenge: &OTPCode{
Code: &crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("code"),
},
Expiry: 5 * time.Minute,
CreationDate: testNow.Add(-10 * time.Minute),
},
otpAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args: args{
code: "code",
},
res: res{
err: zerrors.ThrowPreconditionFailed(nil, "CODE-QvUQ4P", "Errors.User.Code.Expired"),
errorCommands: []eventstore.Command{
user.NewHumanOTPSMSCheckFailedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil),
user.NewUserLockedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate),
},
},
},
{
name: "check ok",
fields: fields{
eventstore: expectEventstore(),
userID: "userID",
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(user.NewHumanOTPSMSAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)),
),
expectFilter(), // recheck
),
userID: "userID",
otpCodeChallenge: &OTPCode{
Code: &crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
@@ -783,12 +880,44 @@ func TestCheckOTPSMS(t *testing.T) {
},
res: res{
commands: []eventstore.Command{
user.NewHumanOTPSMSCheckSucceededEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil),
session.NewOTPSMSCheckedEvent(context.Background(), &session.NewAggregate("sessionID", "instanceID").Aggregate,
testNow,
),
},
},
},
{
name: "check ok, locked in the meantime",
fields: fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(user.NewHumanOTPSMSAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)),
),
expectFilter(
user.NewUserLockedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate),
),
),
userID: "userID",
otpCodeChallenge: &OTPCode{
Code: &crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("code"),
},
Expiry: 5 * time.Minute,
CreationDate: testNow,
},
otpAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args: args{
code: "code",
},
res: res{
err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-S6h4R", "Errors.User.Locked"),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@@ -811,8 +940,9 @@ func TestCheckOTPSMS(t *testing.T) {
},
}
err := cmd(context.Background(), cmds)
gotCmds, err := cmd(context.Background(), cmds)
assert.ErrorIs(t, err, tt.res.err)
assert.Equal(t, tt.res.errorCommands, gotCmds)
assert.Equal(t, tt.res.commands, cmds.eventCommands)
})
}
@@ -829,8 +959,9 @@ func TestCheckOTPEmail(t *testing.T) {
code string
}
type res struct {
err error
commands []eventstore.Command
err error
commands []eventstore.Command
errorCommands []eventstore.Command
}
tests := []struct {
name string
@@ -848,13 +979,43 @@ func TestCheckOTPEmail(t *testing.T) {
code: "code",
},
res: res{
err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-ejo2w", "Errors.User.UserIDMissing"),
err: zerrors.ThrowInvalidArgument(nil, "COMMAND-S453v", "Errors.User.UserIDMissing"),
},
},
{
name: "missing code",
fields: fields{
eventstore: expectEventstore(),
userID: "userID",
},
args: args{},
res: res{
err: zerrors.ThrowInvalidArgument(nil, "COMMAND-SJl2g", "Errors.User.Code.Empty"),
},
},
{
name: "not set up",
fields: fields{
eventstore: expectEventstore(
expectFilter(),
),
userID: "userID",
},
args: args{
code: "code",
},
res: res{
err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-d2r52", "Errors.User.MFA.OTP.NotReady"),
},
},
{
name: "missing challenge",
fields: fields{
eventstore: expectEventstore(),
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(user.NewHumanOTPEmailAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)),
),
),
userID: "userID",
otpCodeChallenge: nil,
},
@@ -862,14 +1023,26 @@ func TestCheckOTPEmail(t *testing.T) {
code: "code",
},
res: res{
err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-zF3g3", "Errors.User.Code.NotFound"),
err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-S34gh", "Errors.User.Code.NotFound"),
},
},
{
name: "invalid code",
fields: fields{
eventstore: expectEventstore(),
userID: "userID",
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(user.NewHumanOTPEmailAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)),
),
expectFilter(), // recheck
expectFilter(
eventFromEventPusher(
org.NewLockoutPolicyAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate,
0, 0, false,
),
),
),
),
userID: "userID",
otpCodeChallenge: &OTPCode{
Code: &crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
@@ -887,13 +1060,61 @@ func TestCheckOTPEmail(t *testing.T) {
},
res: res{
err: zerrors.ThrowPreconditionFailed(nil, "CODE-QvUQ4P", "Errors.User.Code.Expired"),
errorCommands: []eventstore.Command{
user.NewHumanOTPEmailCheckFailedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil),
},
},
},
{
name: "invalid code, locked",
fields: fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(user.NewHumanOTPEmailAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)),
),
expectFilter(), // recheck
expectFilter(
eventFromEventPusher(
org.NewLockoutPolicyAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate,
0, 1, false,
),
),
),
),
userID: "userID",
otpCodeChallenge: &OTPCode{
Code: &crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("code"),
},
Expiry: 5 * time.Minute,
CreationDate: testNow.Add(-10 * time.Minute),
},
otpAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args: args{
code: "code",
},
res: res{
err: zerrors.ThrowPreconditionFailed(nil, "CODE-QvUQ4P", "Errors.User.Code.Expired"),
errorCommands: []eventstore.Command{
user.NewHumanOTPEmailCheckFailedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil),
user.NewUserLockedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate),
},
},
},
{
name: "check ok",
fields: fields{
eventstore: expectEventstore(),
userID: "userID",
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(user.NewHumanOTPEmailAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)),
),
expectFilter(), // recheck
),
userID: "userID",
otpCodeChallenge: &OTPCode{
Code: &crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
@@ -911,12 +1132,44 @@ func TestCheckOTPEmail(t *testing.T) {
},
res: res{
commands: []eventstore.Command{
user.NewHumanOTPEmailCheckSucceededEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil),
session.NewOTPEmailCheckedEvent(context.Background(), &session.NewAggregate("sessionID", "instanceID").Aggregate,
testNow,
),
},
},
},
{
name: "check ok, locked in the meantime",
fields: fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(user.NewHumanOTPEmailAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)),
),
expectFilter(
user.NewUserLockedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate),
),
),
userID: "userID",
otpCodeChallenge: &OTPCode{
Code: &crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("code"),
},
Expiry: 5 * time.Minute,
CreationDate: testNow,
},
otpAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args: args{
code: "code",
},
res: res{
err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-S6h4R", "Errors.User.Locked"),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@@ -939,8 +1192,9 @@ func TestCheckOTPEmail(t *testing.T) {
},
}
err := cmd(context.Background(), cmds)
gotCmds, err := cmd(context.Background(), cmds)
assert.ErrorIs(t, err, tt.res.err)
assert.Equal(t, tt.res.errorCommands, gotCmds)
assert.Equal(t, tt.res.commands, cmds.eventCommands)
})
}