feat(v2): implement user register OTP (#6030)

* feat(v2): implement user register OTP

* feat(v2): implement user verify OTP

* session: retry on permission denied
This commit is contained in:
Tim Möhlmann
2023-06-20 12:36:21 +02:00
committed by GitHub
parent 4eaf3fb21e
commit 09aafb35eb
10 changed files with 1113 additions and 12 deletions

View File

@@ -125,6 +125,18 @@ func expectPushFailed(err error, events []*repository.Event, uniqueConstraints .
}
}
func expectRandomPush(events []*repository.Event, uniqueConstraints ...*repository.UniqueConstraint) expect {
return func(m *mock.MockRepository) {
m.ExpectRandomPush(events, uniqueConstraints...)
}
}
func expectRandomPushFailed(err error, events []*repository.Event, uniqueConstraints ...*repository.UniqueConstraint) expect {
return func(m *mock.MockRepository) {
m.ExpectRandomPushFailed(err, events, uniqueConstraints...)
}
}
func expectFilter(events ...*repository.Event) expect {
return func(m *mock.MockRepository) {
m.ExpectFilterEvents(events...)

View File

@@ -3,12 +3,14 @@ package command
import (
"context"
"github.com/pquerna/otp"
"github.com/zitadel/logging"
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain"
caos_errs "github.com/zitadel/zitadel/internal/errors"
"github.com/zitadel/zitadel/internal/eventstore"
"github.com/zitadel/zitadel/internal/eventstore/v1/models"
"github.com/zitadel/zitadel/internal/repository/user"
"github.com/zitadel/zitadel/internal/telemetry/tracing"
@@ -43,7 +45,32 @@ func (c *Commands) AddHumanOTP(ctx context.Context, userID, resourceowner string
if userID == "" {
return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-5M0sd", "Errors.User.UserIDMissing")
}
human, err := c.getHuman(ctx, userID, resourceowner)
prep, err := c.createHumanOTP(ctx, userID, resourceowner)
if err != nil {
return nil, err
}
_, err = c.eventstore.Push(ctx, prep.cmds...)
if err != nil {
return nil, err
}
return &domain.OTP{
ObjectRoot: models.ObjectRoot{
AggregateID: prep.userAgg.ID,
},
SecretString: prep.key.Secret(),
Url: prep.key.URL(),
}, nil
}
type preparedOTP struct {
wm *HumanOTPWriteModel
userAgg *eventstore.Aggregate
key *otp.Key
cmds []eventstore.Command
}
func (c *Commands) createHumanOTP(ctx context.Context, userID, resourceOwner string) (*preparedOTP, error) {
human, err := c.getHuman(ctx, userID, resourceOwner)
if err != nil {
logging.Log("COMMAND-DAqe1").WithError(err).WithField("traceID", tracing.TraceIDFromCtx(ctx)).Debug("unable to get human for loginname")
return nil, caos_errs.ThrowPreconditionFailed(err, "COMMAND-MM9fs", "Errors.User.NotFound")
@@ -59,7 +86,7 @@ func (c *Commands) AddHumanOTP(ctx context.Context, userID, resourceowner string
return nil, caos_errs.ThrowPreconditionFailed(err, "COMMAND-8ugTs", "Errors.Org.DomainPolicy.NotFound")
}
otpWriteModel, err := c.otpWriteModelByID(ctx, userID, resourceowner)
otpWriteModel, err := c.otpWriteModelByID(ctx, userID, resourceOwner)
if err != nil {
return nil, err
}
@@ -80,16 +107,13 @@ func (c *Commands) AddHumanOTP(ctx context.Context, userID, resourceowner string
if err != nil {
return nil, err
}
_, err = c.eventstore.Push(ctx, user.NewHumanOTPAddedEvent(ctx, userAgg, secret))
if err != nil {
return nil, err
}
return &domain.OTP{
ObjectRoot: models.ObjectRoot{
AggregateID: human.AggregateID,
return &preparedOTP{
wm: otpWriteModel,
userAgg: userAgg,
key: key,
cmds: []eventstore.Command{
user.NewHumanOTPAddedEvent(ctx, userAgg, secret),
},
SecretString: key.Secret(),
Url: key.URL(),
}, nil
}

View File

@@ -2,11 +2,17 @@ package command
import (
"context"
"io"
"testing"
"time"
"github.com/golang/mock/gomock"
"github.com/pquerna/otp/totp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/text/language"
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain"
caos_errs "github.com/zitadel/zitadel/internal/errors"
@@ -231,6 +237,476 @@ func TestCommandSide_AddHumanOTP(t *testing.T) {
}
}
func TestCommands_createHumanOTP(t *testing.T) {
type fields struct {
eventstore *eventstore.Eventstore
}
type args struct {
ctx context.Context
userID string
resourceOwner string
}
tests := []struct {
name string
fields fields
args args
want bool
wantErr error
}{
{
name: "user not existing, not found error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(),
),
},
args: args{
ctx: context.Background(),
resourceOwner: "org1",
userID: "user1",
},
wantErr: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-MM9fs", "Errors.User.NotFound"),
},
{
name: "org not existing, not found error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.German,
domain.GenderUnspecified,
"email@test.ch",
true,
),
),
),
expectFilter(),
),
},
args: args{
ctx: context.Background(),
resourceOwner: "org1",
userID: "user1",
},
wantErr: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-55M9f", "Errors.Org.NotFound"),
},
{
name: "org iam policy not existing, not found error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.German,
domain.GenderUnspecified,
"email@test.ch",
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewOrgAddedEvent(context.Background(),
&user.NewAggregate("org1", "org1").Aggregate,
"org",
),
),
),
expectFilter(),
expectFilter(),
),
},
args: args{
ctx: context.Background(),
resourceOwner: "org1",
userID: "user1",
},
wantErr: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-8ugTs", "Errors.Org.DomainPolicy.NotFound"),
},
{
name: "otp already exists, already exists error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.German,
domain.GenderUnspecified,
"email@test.ch",
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewOrgAddedEvent(context.Background(),
&user.NewAggregate("org1", "org1").Aggregate,
"org",
),
),
),
expectFilter(
eventFromEventPusher(
org.NewDomainPolicyAddedEvent(context.Background(),
&org.NewAggregate("org1").Aggregate,
true,
true,
true,
),
),
),
expectFilter(
eventFromEventPusher(
user.NewHumanOTPAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
&crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("a"),
}),
),
eventFromEventPusher(
user.NewHumanOTPVerifiedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"agent1")),
),
),
},
args: args{
ctx: context.Background(),
resourceOwner: "org1",
userID: "user1",
},
wantErr: caos_errs.ThrowAlreadyExists(nil, "COMMAND-do9se", "Errors.User.MFA.OTP.AlreadyReady"),
},
{
name: "issuer not in context",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.German,
domain.GenderUnspecified,
"email@test.ch",
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewOrgAddedEvent(context.Background(),
&user.NewAggregate("org1", "org1").Aggregate,
"org",
),
),
),
expectFilter(
eventFromEventPusher(
org.NewDomainPolicyAddedEvent(context.Background(),
&org.NewAggregate("org1").Aggregate,
true,
true,
true,
),
),
),
expectFilter(),
),
},
args: args{
ctx: context.Background(),
resourceOwner: "org1",
userID: "user1",
},
wantErr: caos_errs.ThrowInternal(nil, "TOTP-ieY3o", "Errors.Internal"),
},
{
name: "success",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.German,
domain.GenderUnspecified,
"email@test.ch",
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewOrgAddedEvent(context.Background(),
&user.NewAggregate("org1", "org1").Aggregate,
"org",
),
),
),
expectFilter(
eventFromEventPusher(
org.NewDomainPolicyAddedEvent(context.Background(),
&org.NewAggregate("org1").Aggregate,
true,
true,
true,
),
),
),
expectFilter(),
),
},
args: args{
ctx: authz.WithRequestedDomain(context.Background(), "zitadel.com"),
resourceOwner: "org1",
userID: "user1",
},
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := &Commands{
eventstore: tt.fields.eventstore,
multifactors: domain.MultifactorConfigs{
OTP: domain.OTPConfig{
CryptoMFA: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
},
}
got, err := c.createHumanOTP(tt.args.ctx, tt.args.userID, tt.args.resourceOwner)
require.ErrorIs(t, err, tt.wantErr)
if tt.want {
require.NotNil(t, got)
assert.NotNil(t, got.wm)
assert.NotNil(t, got.userAgg)
require.NotNil(t, got.key)
assert.NotEmpty(t, got.key.URL())
assert.NotEmpty(t, got.key.Secret())
assert.Len(t, got.cmds, 1)
}
})
}
}
func TestCommands_HumanCheckMFAOTPSetup(t *testing.T) {
ctx := authz.NewMockContext("inst1", "org1", "user1")
cryptoAlg := crypto.CreateMockEncryptionAlg(gomock.NewController(t))
key, secret, err := domain.NewOTPKey("example.com", "user1", cryptoAlg)
require.NoError(t, err)
userAgg := &user.NewAggregate("user1", "org1").Aggregate
code, err := totp.GenerateCode(key.Secret(), time.Now())
require.NoError(t, err)
type fields struct {
eventstore *eventstore.Eventstore
}
type args struct {
userID string
code string
resourceOwner string
}
tests := []struct {
name string
fields fields
args args
want bool
wantErr error
}{
{
name: "missing user id",
args: args{},
wantErr: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-8N9ds", "Errors.User.UserIDMissing"),
},
{
name: "filter error",
fields: fields{
eventstore: eventstoreExpect(t,
expectFilterError(io.ErrClosedPipe),
),
},
args: args{
userID: "user1",
resourceOwner: "org1",
},
wantErr: io.ErrClosedPipe,
},
{
name: "otp not existing error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanOTPAddedEvent(ctx, userAgg, secret),
),
eventFromEventPusher(
user.NewHumanOTPRemovedEvent(ctx, userAgg),
),
),
),
},
args: args{
resourceOwner: "org1",
userID: "user1",
},
wantErr: caos_errs.ThrowNotFound(nil, "COMMAND-3Mif9s", "Errors.User.MFA.OTP.NotExisting"),
},
{
name: "otp already ready error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanOTPAddedEvent(ctx, userAgg, secret),
),
eventFromEventPusher(
user.NewHumanOTPVerifiedEvent(context.Background(),
userAgg,
"agent1",
),
),
),
),
},
args: args{
resourceOwner: "org1",
userID: "user1",
},
wantErr: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-qx4ls", "Errors.Users.MFA.OTP.AlreadyReady"),
},
{
name: "wrong code",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanOTPAddedEvent(ctx, userAgg, secret),
),
),
),
},
args: args{
resourceOwner: "org1",
code: "wrong",
userID: "user1",
},
wantErr: caos_errs.ThrowInvalidArgument(nil, "EVENT-8isk2", "Errors.User.MFA.OTP.InvalidCode"),
},
{
name: "push error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanOTPAddedEvent(ctx, userAgg, secret),
),
),
expectPushFailed(io.ErrClosedPipe,
[]*repository.Event{eventFromEventPusher(
user.NewHumanOTPVerifiedEvent(ctx,
userAgg,
"agent1",
),
)},
),
),
},
args: args{
resourceOwner: "org1",
code: code,
userID: "user1",
},
wantErr: io.ErrClosedPipe,
},
{
name: "success",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanOTPAddedEvent(ctx, userAgg, secret),
),
),
expectPush([]*repository.Event{eventFromEventPusher(
user.NewHumanOTPVerifiedEvent(ctx,
userAgg,
"agent1",
),
)}),
),
},
args: args{
resourceOwner: "org1",
code: code,
userID: "user1",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := &Commands{
eventstore: tt.fields.eventstore,
multifactors: domain.MultifactorConfigs{
OTP: domain.OTPConfig{
CryptoMFA: cryptoAlg,
},
},
}
got, err := c.HumanCheckMFAOTPSetup(ctx, tt.args.userID, tt.args.code, "agent1", tt.args.resourceOwner)
require.ErrorIs(t, err, tt.wantErr)
if tt.want {
require.NotNil(t, got)
assert.Equal(t, "org1", got.ResourceOwner)
}
})
}
}
func TestCommandSide_RemoveHumanOTP(t *testing.T) {
type fields struct {
eventstore *eventstore.Eventstore

View File

@@ -0,0 +1,33 @@
package command
import (
"context"
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/domain"
)
func (c *Commands) AddUserOTP(ctx context.Context, userID, resourceowner string) (*domain.OTPv2, error) {
if err := authz.UserIDInCTX(ctx, userID); err != nil {
return nil, err
}
prep, err := c.createHumanOTP(ctx, userID, resourceowner)
if err != nil {
return nil, err
}
if err = c.pushAppendAndReduce(ctx, prep.wm, prep.cmds...); err != nil {
return nil, err
}
return &domain.OTPv2{
ObjectDetails: writeModelToObjectDetails(&prep.wm.WriteModel),
Secret: prep.key.Secret(),
URI: prep.key.URL(),
}, nil
}
func (c *Commands) CheckUserOTP(ctx context.Context, userID, code, resourceOwner string) (*domain.ObjectDetails, error) {
if err := authz.UserIDInCTX(ctx, userID); err != nil {
return nil, err
}
return c.HumanCheckMFAOTPSetup(ctx, userID, code, "", resourceOwner)
}

View File

@@ -0,0 +1,263 @@
package command
import (
"io"
"testing"
"time"
"github.com/golang/mock/gomock"
"github.com/pquerna/otp/totp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/text/language"
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain"
caos_errs "github.com/zitadel/zitadel/internal/errors"
"github.com/zitadel/zitadel/internal/eventstore"
"github.com/zitadel/zitadel/internal/eventstore/repository"
"github.com/zitadel/zitadel/internal/repository/org"
"github.com/zitadel/zitadel/internal/repository/user"
)
func TestCommands_AddUserOTP(t *testing.T) {
ctx := authz.NewMockContext("inst1", "org1", "user1")
userAgg := &user.NewAggregate("user1", "org1").Aggregate
type fields struct {
eventstore *eventstore.Eventstore
}
type args struct {
userID string
resourceowner string
}
tests := []struct {
name string
fields fields
args args
want bool
wantErr error
}{
{
name: "wrong user",
args: args{
userID: "foo",
resourceowner: "org1",
},
wantErr: caos_errs.ThrowUnauthenticated(nil, "AUTH-Bohd2", "Errors.User.UserIDWrong"),
},
{
name: "create otp error",
args: args{
userID: "user1",
resourceowner: "org1",
},
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(),
),
},
wantErr: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-MM9fs", "Errors.User.NotFound"),
},
{
name: "push error",
args: args{
userID: "user1",
resourceowner: "org1",
},
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(ctx,
userAgg,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.German,
domain.GenderUnspecified,
"email@test.ch",
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewOrgAddedEvent(ctx,
userAgg,
"org",
),
),
),
expectFilter(
eventFromEventPusher(
org.NewDomainPolicyAddedEvent(ctx,
userAgg,
true,
true,
true,
),
),
),
expectFilter(),
expectRandomPushFailed(io.ErrClosedPipe, []*repository.Event{eventFromEventPusher(
user.NewHumanOTPAddedEvent(ctx, userAgg, nil),
)}),
),
},
wantErr: io.ErrClosedPipe,
},
{
name: "success",
args: args{
userID: "user1",
resourceowner: "org1",
},
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(ctx,
userAgg,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.German,
domain.GenderUnspecified,
"email@test.ch",
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewOrgAddedEvent(ctx,
userAgg,
"org",
),
),
),
expectFilter(
eventFromEventPusher(
org.NewDomainPolicyAddedEvent(ctx,
userAgg,
true,
true,
true,
),
),
),
expectFilter(),
expectRandomPush([]*repository.Event{eventFromEventPusher(
user.NewHumanOTPAddedEvent(ctx, userAgg, nil),
)}),
),
},
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := &Commands{
eventstore: tt.fields.eventstore,
multifactors: domain.MultifactorConfigs{
OTP: domain.OTPConfig{
Issuer: "zitadel.com",
CryptoMFA: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
},
}
got, err := c.AddUserOTP(ctx, tt.args.userID, tt.args.resourceowner)
require.ErrorIs(t, err, tt.wantErr)
if tt.want {
require.NotNil(t, got)
assert.Equal(t, "org1", got.ResourceOwner)
assert.NotEmpty(t, got.Secret)
assert.NotEmpty(t, got.URI)
}
})
}
}
func TestCommands_CheckUserOTP(t *testing.T) {
ctx := authz.NewMockContext("inst1", "org1", "user1")
cryptoAlg := crypto.CreateMockEncryptionAlg(gomock.NewController(t))
key, secret, err := domain.NewOTPKey("example.com", "user1", cryptoAlg)
require.NoError(t, err)
userAgg := &user.NewAggregate("user1", "org1").Aggregate
code, err := totp.GenerateCode(key.Secret(), time.Now())
require.NoError(t, err)
type fields struct {
eventstore *eventstore.Eventstore
}
type args struct {
userID string
code string
resourceOwner string
}
tests := []struct {
name string
fields fields
args args
want bool
wantErr error
}{
{
name: "wrong user id",
args: args{
userID: "foo",
},
wantErr: caos_errs.ThrowUnauthenticated(nil, "AUTH-Bohd2", "Errors.User.UserIDWrong"),
},
{
name: "success",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanOTPAddedEvent(ctx, userAgg, secret),
),
),
expectPush([]*repository.Event{eventFromEventPusher(
user.NewHumanOTPVerifiedEvent(ctx, userAgg, ""),
)}),
),
},
args: args{
resourceOwner: "org1",
code: code,
userID: "user1",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := &Commands{
eventstore: tt.fields.eventstore,
multifactors: domain.MultifactorConfigs{
OTP: domain.OTPConfig{
CryptoMFA: cryptoAlg,
},
},
}
got, err := c.CheckUserOTP(ctx, tt.args.userID, tt.args.code, tt.args.resourceOwner)
require.ErrorIs(t, err, tt.wantErr)
if tt.want {
require.NotNil(t, got)
assert.Equal(t, "org1", got.ResourceOwner)
}
})
}
}