andwati/zitadel
1
0
Fork 0
mirror of https://github.com/andwati/zitadel.git synced 2026-10-18 05:52:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: make recovery code active on add (#11734)

Browse Source 
# Which Problems Are Solved

- Recovery codes added via `GenerateRecoveryCodes` were never listed as
active authentication methods by `ListAuthenticationMethodTypes`. The
`user_auth_method` projection reducer (`reduceInitAuthMethod`) set the
state to `MFAStateNotReady`, expecting a subsequent verification event
to transition it to `MFAStateReady`. Unlike TOTP, U2F, and Passwordless
— which all have distinct verification events — recovery codes have no
such event, so they were permanently stuck in `NotReady`.

# How the Problems Are Solved

- Introduced a dedicated `reduceAddRecoveryCodes` reducer for
`HumanRecoveryCodesAddedEvent` that sets `MFAStateReady` immediately,
since recovery codes are usable as soon as they are generated.
- Uses `NewUpsertStatement` (instead of `NewCreateStatement`) so that
regenerating recovery codes for the same user updates the existing row
rather than failing on a duplicate key conflict.
- Removed the `HumanRecoveryCodesAddedEvent` case from
`reduceInitAuthMethod`.

# Additional Changes

- Added `UserAuthMethodTypeRecoveryCode` mapping in `authMethodTypeToPb`
unit test.
- Added integration tests for `ListAuthenticationMethodTypes` covering
both the "recovery codes present" and "recovery codes removed" cases.

# Additional Context

* https://github.com/zitadel/zitadel/issues/11317

---------

Co-authored-by: Marco A. <[email protected]>
Co-authored-by: Cursor <[email protected]>
This commit is contained in:
Zach Hirschtritt
2026-03-27 09:47:16 +00:00
committed by GitHub
co-authored by Marco A. Cursor
parent f99142bc4a
commit ec7a6d5526
4 changed files with 130 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/api/grpc/user/v2/integration_test/user_test.go
+49
View File
@@ -1972,6 +1972,24 @@ func TestServer_ListAuthenticationMethodTypes(t *testing.T) {
})
require.NoError(t, err)
userIDWithRecoveryCodes := Instance.CreateHumanUser(OrgCTX).GetUserId()
_, err = Client.GenerateRecoveryCodes(OrgCTX, &user.GenerateRecoveryCodesRequest{
UserId: userIDWithRecoveryCodes,
Count: 5,
})
require.NoError(t, err)
userIDWithRemovedRecoveryCodes := Instance.CreateHumanUser(OrgCTX).GetUserId()
_, err = Client.GenerateRecoveryCodes(OrgCTX, &user.GenerateRecoveryCodesRequest{
UserId: userIDWithRemovedRecoveryCodes,
Count: 5,
})
require.NoError(t, err)
_, err = Client.RemoveRecoveryCodes(OrgCTX, &user.RemoveRecoveryCodesRequest{
UserId: userIDWithRemovedRecoveryCodes,
})
require.NoError(t, err)
_, userLegacyID := ctxFromNewUserWithVerifiedPasswordlessLegacy(t)
require.NoError(t, err)
@@ -2149,6 +2167,37 @@ func TestServer_ListAuthenticationMethodTypes(t *testing.T) {
},
},
},
{
name: "with auth (recovery codes)",
args: args{
OrgCTX,
&user.ListAuthenticationMethodTypesRequest{
UserId: userIDWithRecoveryCodes,
},
},
want: &user.ListAuthenticationMethodTypesResponse{
Details: &object.ListDetails{
TotalResult: 1,
},
AuthMethodTypes: []user.AuthenticationMethodType{
user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_RECOVERY_CODE,
},
},
},
{
name: "with removed recovery codes",
args: args{
OrgCTX,
&user.ListAuthenticationMethodTypesRequest{
UserId: userIDWithRemovedRecoveryCodes,
},
},
want: &user.ListAuthenticationMethodTypesResponse{
Details: &object.ListDetails{
TotalResult: 0,
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
internal/api/grpc/user/v2/user_test.go
+50 -45
View File
@@ -49,8 +49,8 @@ func Test_idpIntentToIDPIntentPb(t *testing.T) {
res res
}{
{
"decryption invalid key id error",
args{
name: "decryption invalid key id error",
args: args{
intent: &command.IDPIntentWriteModel{
WriteModel: eventstore.WriteModel{
AggregateID: "intentID",
@@ -76,13 +76,13 @@ func Test_idpIntentToIDPIntentPb(t *testing.T) {
},
alg: decryption(zerrors.ThrowInternal(nil, "id", "invalid key id")),
},
res{
res: res{
resp: nil,
err: zerrors.ThrowInternal(nil, "id", "invalid key id"),
},
}, {
"successful oauth",
args{
name: "successful oauth",
args: args{
intent: &command.IDPIntentWriteModel{
WriteModel: eventstore.WriteModel{
AggregateID: "intentID",
@@ -107,7 +107,7 @@ func Test_idpIntentToIDPIntentPb(t *testing.T) {
},
alg: decryption(nil),
},
res{
res: res{
resp: &user.RetrieveIdentityProviderIntentResponse{
Details: &object_pb.Details{
Sequence: 123,
@@ -138,8 +138,8 @@ func Test_idpIntentToIDPIntentPb(t *testing.T) {
},
},
{
"successful oauth with linked user",
args{
name: "successful oauth with linked user",
args: args{
intent: &command.IDPIntentWriteModel{
WriteModel: eventstore.WriteModel{
AggregateID: "intentID",
@@ -164,7 +164,7 @@ func Test_idpIntentToIDPIntentPb(t *testing.T) {
},
alg: decryption(nil),
},
res{
res: res{
resp: &user.RetrieveIdentityProviderIntentResponse{
Details: &object_pb.Details{
Sequence: 123,
@@ -195,8 +195,8 @@ func Test_idpIntentToIDPIntentPb(t *testing.T) {
err: nil,
},
}, {
"successful ldap",
args{
name: "successful ldap",
args: args{
intent: &command.IDPIntentWriteModel{
WriteModel: eventstore.WriteModel{
AggregateID: "intentID",
@@ -218,7 +218,7 @@ func Test_idpIntentToIDPIntentPb(t *testing.T) {
State: domain.IDPIntentStateSucceeded,
},
},
res{
res: res{
resp: &user.RetrieveIdentityProviderIntentResponse{
Details: &object_pb.Details{
Sequence: 123,
@@ -255,8 +255,8 @@ func Test_idpIntentToIDPIntentPb(t *testing.T) {
err: nil,
},
}, {
"successful ldap with linked user",
args{
name: "successful ldap with linked user",
args: args{
intent: &command.IDPIntentWriteModel{
WriteModel: eventstore.WriteModel{
AggregateID: "intentID",
@@ -278,7 +278,7 @@ func Test_idpIntentToIDPIntentPb(t *testing.T) {
State: domain.IDPIntentStateSucceeded,
},
},
res{
res: res{
resp: &user.RetrieveIdentityProviderIntentResponse{
Details: &object_pb.Details{
Sequence: 123,
@@ -333,16 +333,16 @@ func Test_authMethodTypesToPb(t *testing.T) {
want []user.AuthenticationMethodType
}{
{
"empty list",
nil,
[]user.AuthenticationMethodType{},
name: "empty list",
methodTypes: nil,
want: []user.AuthenticationMethodType{},
},
{
"list",
[]domain.UserAuthMethodType{
name: "list",
methodTypes: []domain.UserAuthMethodType{
domain.UserAuthMethodTypePasswordless,
},
[]user.AuthenticationMethodType{
want: []user.AuthenticationMethodType{
user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_PASSKEY,
},
},
@@ -361,44 +361,49 @@ func Test_authMethodTypeToPb(t *testing.T) {
want user.AuthenticationMethodType
}{
{
"uspecified",
domain.UserAuthMethodTypeUnspecified,
user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_UNSPECIFIED,
name: "uspecified",
methodType: domain.UserAuthMethodTypeUnspecified,
want: user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_UNSPECIFIED,
},
{
"totp",
domain.UserAuthMethodTypeTOTP,
user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_TOTP,
name: "totp",
methodType: domain.UserAuthMethodTypeTOTP,
want: user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_TOTP,
},
{
"u2f",
domain.UserAuthMethodTypeU2F,
user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_U2F,
name: "u2f",
methodType: domain.UserAuthMethodTypeU2F,
want: user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_U2F,
},
{
"passkey",
domain.UserAuthMethodTypePasswordless,
user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_PASSKEY,
name: "passkey",
methodType: domain.UserAuthMethodTypePasswordless,
want: user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_PASSKEY,
},
{
"password",
domain.UserAuthMethodTypePassword,
user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_PASSWORD,
name: "password",
methodType: domain.UserAuthMethodTypePassword,
want: user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_PASSWORD,
},
{
"idp",
domain.UserAuthMethodTypeIDP,
user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_IDP,
name: "idp",
methodType: domain.UserAuthMethodTypeIDP,
want: user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_IDP,
},
{
"otp sms",
domain.UserAuthMethodTypeOTPSMS,
user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_OTP_SMS,
name: "otp sms",
methodType: domain.UserAuthMethodTypeOTPSMS,
want: user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_OTP_SMS,
},
{
"otp email",
domain.UserAuthMethodTypeOTPEmail,
user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_OTP_EMAIL,
name: "otp email",
methodType: domain.UserAuthMethodTypeOTPEmail,
want: user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_OTP_EMAIL,
},
{
name: "recovery code",
methodType: domain.UserAuthMethodTypeRecoveryCode,
want: user.AuthenticationMethodType_AUTHENTICATION_METHOD_TYPE_RECOVERY_CODE,
},
}
for _, tt := range tests {
internal/query/projection/user_auth_method.go
+29 -4
View File
@@ -131,7 +131,7 @@ func (p *userAuthMethodProjection) Reducers() []handler.AggregateReducer {
},
{
Event: user.HumanRecoveryCodesAddedType,
Reduce: p.reduceInitAuthMethod,
Reduce: p.reduceAddRecoveryCodes,
},
{
Event: user.HumanRecoveryCodesRemovedType,
@@ -175,10 +175,8 @@ func (p *userAuthMethodProjection) reduceInitAuthMethod(event eventstore.Event)
rpID = &e.RPID
case *user.HumanOTPAddedEvent:
methodType = domain.UserAuthMethodTypeTOTP
case *user.HumanRecoveryCodesAddedEvent:
methodType = domain.UserAuthMethodTypeRecoveryCode
default:
return nil, zerrors.ThrowInvalidArgumentf(nil, "PROJE-f92f", "reduce.wrong.event.type %v", []eventstore.EventType{user.HumanPasswordlessTokenAddedType, user.HumanU2FTokenAddedType, user.HumanMFAOTPAddedType, user.HumanRecoveryCodesAddedType})
return nil, zerrors.ThrowInvalidArgumentf(nil, "PROJE-f92f", "reduce.wrong.event.type %v", []eventstore.EventType{user.HumanPasswordlessTokenAddedType, user.HumanU2FTokenAddedType, user.HumanMFAOTPAddedType})
}
cols := []handler.Column{
handler.NewCol(UserAuthMethodTokenIDCol, tokenID),
@@ -273,6 +271,33 @@ func (p *userAuthMethodProjection) reduceAddAuthMethod(event eventstore.Event) (
), nil
}
func (p *userAuthMethodProjection) reduceAddRecoveryCodes(event eventstore.Event) (*handler.Statement, error) {
if _, ok := event.(*user.HumanRecoveryCodesAddedEvent); !ok {
return nil, zerrors.ThrowInvalidArgumentf(nil, "PROJE-x93hq", "reduce.wrong.event.type %v", []eventstore.EventType{user.HumanRecoveryCodesAddedType})
}
return handler.NewUpsertStatement(
event,
[]handler.Column{
handler.NewCol(UserAuthMethodInstanceIDCol, nil),
handler.NewCol(UserAuthMethodUserIDCol, nil),
handler.NewCol(UserAuthMethodTypeCol, nil),
handler.NewCol(UserAuthMethodTokenIDCol, nil),
},
[]handler.Column{
handler.NewCol(UserAuthMethodTokenIDCol, ""),
handler.NewCol(UserAuthMethodCreationDateCol, handler.OnlySetValueOnInsert(UserAuthMethodTable, event.CreatedAt())),
handler.NewCol(UserAuthMethodChangeDateCol, event.CreatedAt()),
handler.NewCol(UserAuthMethodResourceOwnerCol, event.Aggregate().ResourceOwner),
handler.NewCol(UserAuthMethodInstanceIDCol, event.Aggregate().InstanceID),
handler.NewCol(UserAuthMethodUserIDCol, event.Aggregate().ID),
handler.NewCol(UserAuthMethodSequenceCol, event.Sequence()),
handler.NewCol(UserAuthMethodStateCol, domain.MFAStateReady),
handler.NewCol(UserAuthMethodTypeCol, domain.UserAuthMethodTypeRecoveryCode),
handler.NewCol(UserAuthMethodNameCol, ""),
},
), nil
}
func (p *userAuthMethodProjection) reduceRemoveAuthMethod(event eventstore.Event) (*handler.Statement, error) {
var tokenID string
var methodType domain.UserAuthMethodType
internal/query/projection/user_auth_method_test.go
+2 -2
View File
@@ -540,7 +540,7 @@ func TestUserAuthMethodProjection_reduces(t *testing.T) {
}`),
), eventstore.GenericEventMapper[user.HumanRecoveryCodesAddedEvent]),
},
reduce: (&userAuthMethodProjection{}).reduceInitAuthMethod,
reduce: (&userAuthMethodProjection{}).reduceAddRecoveryCodes,
want: wantReduce{
aggregateType: user.AggregateType,
sequence: 15,
@@ -556,7 +556,7 @@ func TestUserAuthMethodProjection_reduces(t *testing.T) {
"instance-id",
"agg-id",
uint64(15),
domain.MFAStateNotReady,
domain.MFAStateReady,
domain.UserAuthMethodTypeRecoveryCode,
"",
},