TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2024-12-13 11:34:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: token for post authentication action and change phone and email (#5933)

Browse Source 
* fix: token for post authentication action and change phone and email

* fix checks and add tests

* improve change checks and add tests

* add more tests

* remove unintended test

---------

Co-authored-by: Livio Spring <livio.a@gmail.com>
This commit is contained in:
Stefan Benz 2023-05-26 09:24:52 +02:00 committed by Livio Spring
parent 4982af898a
commit 57e189e590
No known key found for this signature in database
GPG Key ID: 26BB1C2FA5952CF0
6 changed files with 514 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

144
internal/api/ui/login/external_provider_handler.go
View File

@ -558,54 +558,106 @@ func (l *Login) updateExternalUser(ctx context.Context, authReq *domain.AuthRequ
if user.Human == nil {
return errors.ThrowPreconditionFailed(nil, "LOGIN-WLTce", "Errors.User.NotHuman")
}
if externalUser.Email != "" && (externalUser.Email != user.Human.Email || externalUser.IsEmailVerified != user.Human.IsEmailVerified) {
emailCodeGenerator, err := l.query.InitEncryptionGenerator(ctx, domain.SecretGeneratorTypeVerifyEmailCode, l.userCodeAlg)
logging.WithFields("authReq", authReq.ID, "user", authReq.UserID).OnError(err).Error("unable to update email")
if err == nil {
_, err = l.command.ChangeHumanEmail(setContext(ctx, authReq.UserOrgID),
&domain.Email{
ObjectRoot: models.ObjectRoot{AggregateID: authReq.UserID},
EmailAddress: externalUser.Email,
IsEmailVerified: externalUser.IsEmailVerified,
},
emailCodeGenerator)
logging.WithFields("authReq", authReq.ID, "user", authReq.UserID).OnError(err).Error("unable to update email")
}
}
if externalUser.Phone != "" && (externalUser.Phone != user.Human.Phone || externalUser.IsPhoneVerified != user.Human.IsPhoneVerified) {
phoneCodeGenerator, err := l.query.InitEncryptionGenerator(ctx, domain.SecretGeneratorTypeVerifyPhoneCode, l.userCodeAlg)
logging.WithFields("authReq", authReq.ID, "user", authReq.UserID).OnError(err).Error("unable to update phone")
if err == nil {
_, err = l.command.ChangeHumanPhone(setContext(ctx, authReq.UserOrgID),
&domain.Phone{
ObjectRoot: models.ObjectRoot{AggregateID: authReq.UserID},
PhoneNumber: externalUser.Phone,
IsPhoneVerified: externalUser.IsPhoneVerified,
},
authReq.UserOrgID,
phoneCodeGenerator)
logging.WithFields("authReq", authReq.ID, "user", authReq.UserID).OnError(err).Error("unable to update phone")
}
}
if externalUser.FirstName != user.Human.FirstName ||
externalUser.LastName != user.Human.LastName ||
externalUser.NickName != user.Human.NickName ||
externalUser.DisplayName != user.Human.DisplayName ||
externalUser.PreferredLanguage != user.Human.PreferredLanguage {
_, err = l.command.ChangeHumanProfile(setContext(ctx, authReq.UserOrgID), &domain.Profile{
ObjectRoot: models.ObjectRoot{AggregateID: authReq.UserID},
FirstName: externalUser.FirstName,
LastName: externalUser.LastName,
NickName: externalUser.NickName,
DisplayName: externalUser.DisplayName,
PreferredLanguage: externalUser.PreferredLanguage,
Gender: user.Human.Gender,
})
logging.WithFields("authReq", authReq.ID, "user", authReq.UserID).OnError(err).Error("unable to update profile")
}
err = l.updateExternalUserEmail(ctx, user, externalUser)
logging.WithFields("authReq", authReq.ID, "user", authReq.UserID).OnError(err).Error("unable to update email")
err = l.updateExternalUserPhone(ctx, user, externalUser)
logging.WithFields("authReq", authReq.ID, "user", authReq.UserID).OnError(err).Error("unable to update phone")
err = l.updateExternalUserProfile(ctx, user, externalUser)
logging.WithFields("authReq", authReq.ID, "user", authReq.UserID).OnError(err).Error("unable to update profile")
return nil
}
func (l *Login) updateExternalUserEmail(ctx context.Context, user *query.User, externalUser *domain.ExternalUser) error {
changed := hasEmailChanged(user, externalUser)
if !changed {
return nil
}
// if the email has changed and / or was not verified, we change it
emailCodeGenerator, err := l.query.InitEncryptionGenerator(ctx, domain.SecretGeneratorTypeVerifyEmailCode, l.userCodeAlg)
if err != nil {
return err
}
_, err = l.command.ChangeHumanEmail(setContext(ctx, user.ResourceOwner),
&domain.Email{
ObjectRoot: models.ObjectRoot{AggregateID: user.ID},
EmailAddress: externalUser.Email,
IsEmailVerified: externalUser.IsEmailVerified,
},
emailCodeGenerator)
return err
}
func (l *Login) updateExternalUserPhone(ctx context.Context, user *query.User, externalUser *domain.ExternalUser) error {
changed, err := hasPhoneChanged(user, externalUser)
if !changed || err != nil {
return err
}
// if the phone has changed and / or was not verified, we change it
phoneCodeGenerator, err := l.query.InitEncryptionGenerator(ctx, domain.SecretGeneratorTypeVerifyPhoneCode, l.userCodeAlg)
if err != nil {
return err
}
_, err = l.command.ChangeHumanPhone(setContext(ctx, user.ResourceOwner),
&domain.Phone{
ObjectRoot: models.ObjectRoot{AggregateID: user.ID},
PhoneNumber: externalUser.Phone,
IsPhoneVerified: externalUser.IsPhoneVerified,
},
user.ResourceOwner,
phoneCodeGenerator)
return err
}
func (l *Login) updateExternalUserProfile(ctx context.Context, user *query.User, externalUser *domain.ExternalUser) error {
if externalUser.FirstName == user.Human.FirstName &&
externalUser.LastName == user.Human.LastName &&
externalUser.NickName == user.Human.NickName &&
externalUser.DisplayName == user.Human.DisplayName &&
externalUser.PreferredLanguage == user.Human.PreferredLanguage {
return nil
}
_, err := l.command.ChangeHumanProfile(setContext(ctx, user.ResourceOwner), &domain.Profile{
ObjectRoot: models.ObjectRoot{AggregateID: user.ID},
FirstName: externalUser.FirstName,
LastName: externalUser.LastName,
NickName: externalUser.NickName,
DisplayName: externalUser.DisplayName,
PreferredLanguage: externalUser.PreferredLanguage,
Gender: user.Human.Gender,
})
return err
}
func hasEmailChanged(user *query.User, externalUser *domain.ExternalUser) bool {
externalUser.Email = externalUser.Email.Normalize()
if externalUser.Email == "" {
return false
}
// ignore if the same email is not set to verified anymore
if externalUser.Email == user.Human.Email && user.Human.IsEmailVerified {
return false
}
return externalUser.Email != user.Human.Email || externalUser.IsEmailVerified != user.Human.IsEmailVerified
}
func hasPhoneChanged(user *query.User, externalUser *domain.ExternalUser) (_ bool, err error) {
if externalUser.Phone == "" {
return false, nil
}
externalUser.Phone, err = externalUser.Phone.Normalize()
if err != nil {
return false, err
}
// ignore if the same phone is not set to verified anymore
if externalUser.Phone == user.Human.Phone && user.Human.IsPhoneVerified {
return false, nil
}
return externalUser.Phone != user.Human.Phone || externalUser.IsPhoneVerified != user.Human.IsPhoneVerified, nil
}
func (l *Login) ldapProvider(ctx context.Context, identityProvider *query.IDPTemplate) (*ldap.Provider, error) {
password, err := crypto.DecryptString(identityProvider.LDAPIDPTemplate.BindPassword, l.idpConfigAlg)
if err != nil {
@ -851,6 +903,8 @@ func tokens(session idp.Session) *oidc.Tokens[*oidc.IDTokenClaims] {
return s.Tokens
case *jwt.Session:
return s.Tokens
case *oauth.Session:
return s.Tokens
}
return nil
}

286
internal/api/ui/login/external_provider_handler_test.go Normal file
View File

@ -0,0 +1,286 @@
package login
import (
"testing"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/query"
)
func Test_hasEmailChanged(t *testing.T) {
type args struct {
user *query.User
externalUser *domain.ExternalUser
}
tests := []struct {
name string
args args
want bool
}{
{
"no external mail",
args{
user: &query.User{},
externalUser: &domain.ExternalUser{},
},
false,
},
{
"same email unverified",
args{
user: &query.User{
Human: &query.Human{
Email: domain.EmailAddress("email@test.com"),
},
},
externalUser: &domain.ExternalUser{
Email: domain.EmailAddress("email@test.com"),
},
},
false,
},
{
"same email verified",
args{
user: &query.User{
Human: &query.Human{
Email: domain.EmailAddress("email@test.com"),
IsEmailVerified: true,
},
},
externalUser: &domain.ExternalUser{
Email: domain.EmailAddress("email@test.com"),
IsEmailVerified: true,
},
},
false,
},
{
"email already verified",
args{
user: &query.User{
Human: &query.Human{
Email: domain.EmailAddress("email@test.com"),
IsEmailVerified: true,
},
},
externalUser: &domain.ExternalUser{
Email: domain.EmailAddress("email@test.com"),
},
},
false,
},
{
"email changed to verified",
args{
user: &query.User{
Human: &query.Human{
Email: domain.EmailAddress("email@test.com"),
},
},
externalUser: &domain.ExternalUser{
Email: domain.EmailAddress("email@test.com"),
IsEmailVerified: true,
},
},
true,
},
{
"email changed",
args{
user: &query.User{
Human: &query.Human{
Email: domain.EmailAddress("email@test.com"),
},
},
externalUser: &domain.ExternalUser{
Email: domain.EmailAddress("new-email@test.com"),
},
},
true,
},
{
"email changed and verified",
args{
user: &query.User{
Human: &query.Human{
Email: domain.EmailAddress("email@test.com"),
IsEmailVerified: false,
},
},
externalUser: &domain.ExternalUser{
Email: domain.EmailAddress("new-email@test.com"),
IsEmailVerified: true,
},
},
true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := hasEmailChanged(tt.args.user, tt.args.externalUser); got != tt.want {
t.Errorf("hasEmailChanged() = %v, want %v", got, tt.want)
}
})
}
}
func Test_hasPhoneChanged(t *testing.T) {
type args struct {
user *query.User
externalUser *domain.ExternalUser
}
tests := []struct {
name string
args args
want bool
wantErr bool
}{
{
"no external phone",
args{
user: &query.User{},
externalUser: &domain.ExternalUser{},
},
false,
false,
},
{
"invalid phone",
args{
user: &query.User{
Human: &query.Human{
Phone: domain.PhoneNumber("+41791234567"),
},
},
externalUser: &domain.ExternalUser{
Phone: domain.PhoneNumber("invalid"),
},
},
false,
true,
},
{
"same phone unverified",
args{
user: &query.User{
Human: &query.Human{
Phone: domain.PhoneNumber("+41791234567"),
},
},
externalUser: &domain.ExternalUser{
Phone: domain.PhoneNumber("+41791234567"),
},
},
false,
false,
},
{
"same phone verified",
args{
user: &query.User{
Human: &query.Human{
Phone: domain.PhoneNumber("+41791234567"),
IsPhoneVerified: true,
},
},
externalUser: &domain.ExternalUser{
Phone: domain.PhoneNumber("+41791234567"),
IsPhoneVerified: true,
},
},
false,
false,
},
{
"phone already verified",
args{
user: &query.User{
Human: &query.Human{
Phone: domain.PhoneNumber("+41791234567"),
IsPhoneVerified: true,
},
},
externalUser: &domain.ExternalUser{
Phone: domain.PhoneNumber("+41791234567"),
},
},
false,
false,
},
{
"phone changed to verified",
args{
user: &query.User{
Human: &query.Human{
Phone: domain.PhoneNumber("+41791234567"),
},
},
externalUser: &domain.ExternalUser{
Phone: domain.PhoneNumber("+41791234567"),
IsPhoneVerified: true,
},
},
true,
false,
},
{
"phone changed",
args{
user: &query.User{
Human: &query.Human{
Phone: domain.PhoneNumber("+41791234567"),
},
},
externalUser: &domain.ExternalUser{
Phone: domain.PhoneNumber("+4179654321"),
},
},
true,
false,
},
{
"phone changed",
args{
user: &query.User{
Human: &query.Human{
Phone: domain.PhoneNumber("+41791234567"),
},
},
externalUser: &domain.ExternalUser{
Phone: domain.PhoneNumber("+4179654321"),
IsPhoneVerified: true,
},
},
true,
false,
},
{
"normalized phone unchanged",
args{
user: &query.User{
Human: &query.Human{
Phone: domain.PhoneNumber("+41791234567"),
},
},
externalUser: &domain.ExternalUser{
Phone: domain.PhoneNumber("0791234567"),
},
},
false,
false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := hasPhoneChanged(tt.args.user, tt.args.externalUser)
if (err != nil) != tt.wantErr {
t.Errorf("hasPhoneChanged() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("hasPhoneChanged() got = %v, want %v", got, tt.want)
}
})
}
}

10
internal/command/user_human_email.go
View File

@ -33,12 +33,16 @@ func (c *Commands) ChangeHumanEmail(ctx context.Context, email *domain.Email, em
}
userAgg := UserAggregateFromWriteModel(&existingEmail.WriteModel)
changedEvent, hasChanged := existingEmail.NewChangedEvent(ctx, userAgg, email.EmailAddress)
if !hasChanged {
// only continue if there were changes or there were no changes and the email should be set to verified
if !hasChanged && !(email.IsEmailVerified && existingEmail.IsEmailVerified != email.IsEmailVerified) {
return nil, caos_errs.ThrowPreconditionFailed(nil, "COMMAND-2b7fM", "Errors.User.Email.NotChanged")
}
events := []eventstore.Command{changedEvent}
events := make([]eventstore.Command, 0)
if hasChanged {
events = append(events, changedEvent)
}
if email.IsEmailVerified {
events = append(events, user.NewHumanEmailVerifiedEvent(ctx, userAgg))
} else {

54
internal/command/user_human_email_test.go
View File

@ -219,6 +219,60 @@ func TestCommandSide_ChangeHumanEmail(t *testing.T) {
},
},
},
{
name: "email verified, ok",
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,
),
),
),
expectPush(
[]*repository.Event{
eventFromEventPusher(
user.NewHumanEmailVerifiedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
),
),
},
),
),
},
args: args{
ctx: context.Background(),
email: &domain.Email{
ObjectRoot: models.ObjectRoot{
AggregateID: "user1",
},
EmailAddress: "email@test.ch",
IsEmailVerified: true,
},
resourceOwner: "org1",
},
res: res{
want: &domain.Email{
ObjectRoot: models.ObjectRoot{
AggregateID: "user1",
ResourceOwner: "org1",
},
EmailAddress: "email@test.ch",
IsEmailVerified: true,
},
},
},
{
name: "email changed with code, ok",
fields: fields{

10
internal/command/user_human_phone.go
View File

@ -6,6 +6,7 @@ import (
"github.com/zitadel/zitadel/internal/eventstore"
"github.com/zitadel/logging"
"github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain"
caos_errs "github.com/zitadel/zitadel/internal/errors"
@ -27,11 +28,16 @@ func (c *Commands) ChangeHumanPhone(ctx context.Context, phone *domain.Phone, re
userAgg := UserAggregateFromWriteModel(&existingPhone.WriteModel)
changedEvent, hasChanged := existingPhone.NewChangedEvent(ctx, userAgg, phone.PhoneNumber)
if !hasChanged {
// only continue if there were changes or there were no changes and the phone should be set to verified
if !hasChanged && !(phone.IsPhoneVerified && existingPhone.IsPhoneVerified != phone.IsPhoneVerified) {
return nil, caos_errs.ThrowPreconditionFailed(nil, "COMMAND-wF94r", "Errors.User.Phone.NotChanged")
}
events := []eventstore.Command{changedEvent}
events := make([]eventstore.Command, 0)
if hasChanged {
events = append(events, changedEvent)
}
if phone.IsPhoneVerified {
events = append(events, user.NewHumanPhoneVerifiedEvent(ctx, userAgg))
} else {

60
internal/command/user_human_phone_test.go
View File

@ -188,6 +188,66 @@ func TestCommandSide_ChangeHumanPhone(t *testing.T) {
},
},
},
{
name: "phone changed to verified, ok",
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,
),
),
eventFromEventPusher(
user.NewHumanPhoneChangedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"+41711234567",
),
),
),
expectPush(
[]*repository.Event{
eventFromEventPusher(
user.NewHumanPhoneVerifiedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
),
),
},
),
),
},
args: args{
ctx: context.Background(),
email: &domain.Phone{
ObjectRoot: models.ObjectRoot{
AggregateID: "user1",
},
PhoneNumber: "+41711234567",
IsPhoneVerified: true,
},
resourceOwner: "org1",
},
res: res{
want: &domain.Phone{
ObjectRoot: models.ObjectRoot{
AggregateID: "user1",
ResourceOwner: "org1",
},
PhoneNumber: "+41711234567",
IsPhoneVerified: true,
},
},
},
{
name: "phone changed with code, ok",
fields: fields{