fix: consistent permission check on user v2 (#8807)

# Which Problems Are Solved

Some user v2 API calls checked for permission only on the user itself.

# How the Problems Are Solved

Consistent check for permissions on user v2 API.

# Additional Changes

None

# Additional Context

Closes #7944

---------

Co-authored-by: Livio Spring <livio.a@gmail.com>
This commit is contained in:
Stefan Benz 2024-12-03 11:14:04 +01:00 committed by GitHub
parent 26e936aec3
commit c07a5f4277
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 213 additions and 105 deletions

View File

@ -58,7 +58,7 @@ func TestServer_AddOTPSMS(t *testing.T) {
wantErr: true, wantErr: true,
}, },
{ {
name: "user mismatch", name: "no permission",
args: args{ args: args{
ctx: integration.WithAuthorizationToken(context.Background(), sessionTokenOtherUser), ctx: integration.WithAuthorizationToken(context.Background(), sessionTokenOtherUser),
req: &user.AddOTPSMSRequest{ req: &user.AddOTPSMSRequest{
@ -127,14 +127,24 @@ func TestServer_RemoveOTPSMS(t *testing.T) {
userVerified := Instance.CreateHumanUser(CTX) userVerified := Instance.CreateHumanUser(CTX)
Instance.RegisterUserPasskey(CTX, userVerified.GetUserId()) Instance.RegisterUserPasskey(CTX, userVerified.GetUserId())
_, sessionTokenVerified, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userVerified.GetUserId()) _, err := Instance.Client.UserV2.VerifyPhone(CTX, &user.VerifyPhoneRequest{
userVerifiedCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenVerified)
_, err := Instance.Client.UserV2.VerifyPhone(userVerifiedCtx, &user.VerifyPhoneRequest{
UserId: userVerified.GetUserId(), UserId: userVerified.GetUserId(),
VerificationCode: userVerified.GetPhoneCode(), VerificationCode: userVerified.GetPhoneCode(),
}) })
require.NoError(t, err) require.NoError(t, err)
_, err = Instance.Client.UserV2.AddOTPSMS(userVerifiedCtx, &user.AddOTPSMSRequest{UserId: userVerified.GetUserId()}) _, err = Instance.Client.UserV2.AddOTPSMS(CTX, &user.AddOTPSMSRequest{UserId: userVerified.GetUserId()})
require.NoError(t, err)
userSelf := Instance.CreateHumanUser(CTX)
Instance.RegisterUserPasskey(CTX, userSelf.GetUserId())
_, sessionTokenSelf, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userSelf.GetUserId())
userSelfCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenSelf)
_, err = Instance.Client.UserV2.VerifyPhone(CTX, &user.VerifyPhoneRequest{
UserId: userSelf.GetUserId(),
VerificationCode: userSelf.GetPhoneCode(),
})
require.NoError(t, err)
_, err = Instance.Client.UserV2.AddOTPSMS(CTX, &user.AddOTPSMSRequest{UserId: userSelf.GetUserId()})
require.NoError(t, err) require.NoError(t, err)
type args struct { type args struct {
@ -157,10 +167,24 @@ func TestServer_RemoveOTPSMS(t *testing.T) {
}, },
wantErr: true, wantErr: true,
}, },
{
name: "success, self",
args: args{
ctx: userSelfCtx,
req: &user.RemoveOTPSMSRequest{
UserId: userSelf.GetUserId(),
},
},
want: &user.RemoveOTPSMSResponse{
Details: &object.Details{
ResourceOwner: Instance.DefaultOrg.Details.ResourceOwner,
},
},
},
{ {
name: "success", name: "success",
args: args{ args: args{
ctx: userVerifiedCtx, ctx: CTX,
req: &user.RemoveOTPSMSRequest{ req: &user.RemoveOTPSMSRequest{
UserId: userVerified.GetUserId(), UserId: userVerified.GetUserId(),
}, },
@ -230,7 +254,7 @@ func TestServer_AddOTPEmail(t *testing.T) {
wantErr: true, wantErr: true,
}, },
{ {
name: "user mismatch", name: "no permission",
args: args{ args: args{
ctx: integration.WithAuthorizationToken(context.Background(), sessionTokenOtherUser), ctx: integration.WithAuthorizationToken(context.Background(), sessionTokenOtherUser),
req: &user.AddOTPEmailRequest{ req: &user.AddOTPEmailRequest{
@ -301,14 +325,24 @@ func TestServer_RemoveOTPEmail(t *testing.T) {
userVerified := Instance.CreateHumanUser(CTX) userVerified := Instance.CreateHumanUser(CTX)
Instance.RegisterUserPasskey(CTX, userVerified.GetUserId()) Instance.RegisterUserPasskey(CTX, userVerified.GetUserId())
_, sessionTokenVerified, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userVerified.GetUserId()) _, err := Instance.Client.UserV2.VerifyEmail(CTX, &user.VerifyEmailRequest{
userVerifiedCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenVerified)
_, err := Instance.Client.UserV2.VerifyEmail(userVerifiedCtx, &user.VerifyEmailRequest{
UserId: userVerified.GetUserId(), UserId: userVerified.GetUserId(),
VerificationCode: userVerified.GetEmailCode(), VerificationCode: userVerified.GetEmailCode(),
}) })
require.NoError(t, err) require.NoError(t, err)
_, err = Instance.Client.UserV2.AddOTPEmail(userVerifiedCtx, &user.AddOTPEmailRequest{UserId: userVerified.GetUserId()}) _, err = Instance.Client.UserV2.AddOTPEmail(CTX, &user.AddOTPEmailRequest{UserId: userVerified.GetUserId()})
require.NoError(t, err)
userSelf := Instance.CreateHumanUser(CTX)
Instance.RegisterUserPasskey(CTX, userSelf.GetUserId())
_, sessionTokenSelf, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userSelf.GetUserId())
userSelfCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenSelf)
_, err = Instance.Client.UserV2.VerifyEmail(CTX, &user.VerifyEmailRequest{
UserId: userSelf.GetUserId(),
VerificationCode: userSelf.GetEmailCode(),
})
require.NoError(t, err)
_, err = Instance.Client.UserV2.AddOTPEmail(CTX, &user.AddOTPEmailRequest{UserId: userSelf.GetUserId()})
require.NoError(t, err) require.NoError(t, err)
type args struct { type args struct {
@ -331,10 +365,25 @@ func TestServer_RemoveOTPEmail(t *testing.T) {
}, },
wantErr: true, wantErr: true,
}, },
{
name: "success, self",
args: args{
ctx: userSelfCtx,
req: &user.RemoveOTPEmailRequest{
UserId: userSelf.GetUserId(),
},
},
want: &user.RemoveOTPEmailResponse{
Details: &object.Details{
ChangeDate: timestamppb.Now(),
ResourceOwner: Instance.DefaultOrg.Details.ResourceOwner,
},
},
},
{ {
name: "success", name: "success",
args: args{ args: args{
ctx: userVerifiedCtx, ctx: CTX,
req: &user.RemoveOTPEmailRequest{ req: &user.RemoveOTPEmailRequest{
UserId: userVerified.GetUserId(), UserId: userVerified.GetUserId(),
}, },

View File

@ -93,15 +93,30 @@ func TestServer_RegisterPasskey(t *testing.T) {
wantErr: true, wantErr: true,
}, },
{ {
name: "user mismatch", name: "user no permission",
args: args{ args: args{
ctx: CTX, ctx: UserCTX,
req: &user.RegisterPasskeyRequest{ req: &user.RegisterPasskeyRequest{
UserId: userID, UserId: userID,
}, },
}, },
wantErr: true, wantErr: true,
}, },
{
name: "user permission",
args: args{
ctx: IamCTX,
req: &user.RegisterPasskeyRequest{
UserId: userID,
},
},
want: &user.RegisterPasskeyResponse{
Details: &object.Details{
ChangeDate: timestamppb.Now(),
ResourceOwner: Instance.DefaultOrg.Id,
},
},
},
{ {
name: "user setting its own passkey", name: "user setting its own passkey",
args: args{ args: args{

View File

@ -13,7 +13,6 @@ func (s *Server) AddOTPSMS(ctx context.Context, req *user.AddOTPSMSRequest) (*us
return nil, err return nil, err
} }
return &user.AddOTPSMSResponse{Details: object.DomainToDetailsPb(details)}, nil return &user.AddOTPSMSResponse{Details: object.DomainToDetailsPb(details)}, nil
} }
func (s *Server) RemoveOTPSMS(ctx context.Context, req *user.RemoveOTPSMSRequest) (*user.RemoveOTPSMSResponse, error) { func (s *Server) RemoveOTPSMS(ctx context.Context, req *user.RemoveOTPSMSRequest) (*user.RemoveOTPSMSResponse, error) {

View File

@ -58,7 +58,7 @@ func TestServer_AddOTPSMS(t *testing.T) {
wantErr: true, wantErr: true,
}, },
{ {
name: "user mismatch", name: "no permission",
args: args{ args: args{
ctx: integration.WithAuthorizationToken(context.Background(), sessionTokenOtherUser), ctx: integration.WithAuthorizationToken(context.Background(), sessionTokenOtherUser),
req: &user.AddOTPSMSRequest{ req: &user.AddOTPSMSRequest{
@ -127,14 +127,24 @@ func TestServer_RemoveOTPSMS(t *testing.T) {
userVerified := Instance.CreateHumanUser(CTX) userVerified := Instance.CreateHumanUser(CTX)
Instance.RegisterUserPasskey(CTX, userVerified.GetUserId()) Instance.RegisterUserPasskey(CTX, userVerified.GetUserId())
_, sessionTokenVerified, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userVerified.GetUserId()) _, err := Instance.Client.UserV2beta.VerifyPhone(CTX, &user.VerifyPhoneRequest{
userVerifiedCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenVerified)
_, err := Client.VerifyPhone(userVerifiedCtx, &user.VerifyPhoneRequest{
UserId: userVerified.GetUserId(), UserId: userVerified.GetUserId(),
VerificationCode: userVerified.GetPhoneCode(), VerificationCode: userVerified.GetPhoneCode(),
}) })
require.NoError(t, err) require.NoError(t, err)
_, err = Client.AddOTPSMS(userVerifiedCtx, &user.AddOTPSMSRequest{UserId: userVerified.GetUserId()}) _, err = Instance.Client.UserV2beta.AddOTPSMS(CTX, &user.AddOTPSMSRequest{UserId: userVerified.GetUserId()})
require.NoError(t, err)
userSelf := Instance.CreateHumanUser(CTX)
Instance.RegisterUserPasskey(CTX, userSelf.GetUserId())
_, sessionTokenSelf, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userSelf.GetUserId())
userSelfCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenSelf)
_, err = Instance.Client.UserV2beta.VerifyPhone(CTX, &user.VerifyPhoneRequest{
UserId: userSelf.GetUserId(),
VerificationCode: userSelf.GetPhoneCode(),
})
require.NoError(t, err)
_, err = Instance.Client.UserV2beta.AddOTPSMS(CTX, &user.AddOTPSMSRequest{UserId: userSelf.GetUserId()})
require.NoError(t, err) require.NoError(t, err)
type args struct { type args struct {
@ -157,10 +167,24 @@ func TestServer_RemoveOTPSMS(t *testing.T) {
}, },
wantErr: true, wantErr: true,
}, },
{
name: "success, self",
args: args{
ctx: userSelfCtx,
req: &user.RemoveOTPSMSRequest{
UserId: userSelf.GetUserId(),
},
},
want: &user.RemoveOTPSMSResponse{
Details: &object.Details{
ResourceOwner: Instance.DefaultOrg.Details.ResourceOwner,
},
},
},
{ {
name: "success", name: "success",
args: args{ args: args{
ctx: userVerifiedCtx, ctx: CTX,
req: &user.RemoveOTPSMSRequest{ req: &user.RemoveOTPSMSRequest{
UserId: userVerified.GetUserId(), UserId: userVerified.GetUserId(),
}, },
@ -301,14 +325,24 @@ func TestServer_RemoveOTPEmail(t *testing.T) {
userVerified := Instance.CreateHumanUser(CTX) userVerified := Instance.CreateHumanUser(CTX)
Instance.RegisterUserPasskey(CTX, userVerified.GetUserId()) Instance.RegisterUserPasskey(CTX, userVerified.GetUserId())
_, sessionTokenVerified, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userVerified.GetUserId()) _, err := Client.VerifyEmail(CTX, &user.VerifyEmailRequest{
userVerifiedCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenVerified)
_, err := Client.VerifyEmail(userVerifiedCtx, &user.VerifyEmailRequest{
UserId: userVerified.GetUserId(), UserId: userVerified.GetUserId(),
VerificationCode: userVerified.GetEmailCode(), VerificationCode: userVerified.GetEmailCode(),
}) })
require.NoError(t, err) require.NoError(t, err)
_, err = Client.AddOTPEmail(userVerifiedCtx, &user.AddOTPEmailRequest{UserId: userVerified.GetUserId()}) _, err = Client.AddOTPEmail(CTX, &user.AddOTPEmailRequest{UserId: userVerified.GetUserId()})
require.NoError(t, err)
userSelf := Instance.CreateHumanUser(CTX)
Instance.RegisterUserPasskey(CTX, userSelf.GetUserId())
_, sessionTokenSelf, _, _ := Instance.CreateVerifiedWebAuthNSession(t, IamCTX, userSelf.GetUserId())
userSelfCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenSelf)
_, err = Client.VerifyEmail(CTX, &user.VerifyEmailRequest{
UserId: userSelf.GetUserId(),
VerificationCode: userSelf.GetEmailCode(),
})
require.NoError(t, err)
_, err = Client.AddOTPEmail(CTX, &user.AddOTPEmailRequest{UserId: userSelf.GetUserId()})
require.NoError(t, err) require.NoError(t, err)
type args struct { type args struct {
@ -331,10 +365,25 @@ func TestServer_RemoveOTPEmail(t *testing.T) {
}, },
wantErr: true, wantErr: true,
}, },
{
name: "success, self",
args: args{
ctx: userSelfCtx,
req: &user.RemoveOTPEmailRequest{
UserId: userSelf.GetUserId(),
},
},
want: &user.RemoveOTPEmailResponse{
Details: &object.Details{
ChangeDate: timestamppb.Now(),
ResourceOwner: Instance.DefaultOrg.Details.ResourceOwner,
},
},
},
{ {
name: "success", name: "success",
args: args{ args: args{
ctx: userVerifiedCtx, ctx: CTX,
req: &user.RemoveOTPEmailRequest{ req: &user.RemoveOTPEmailRequest{
UserId: userVerified.GetUserId(), UserId: userVerified.GetUserId(),
}, },

View File

@ -92,15 +92,30 @@ func TestServer_RegisterPasskey(t *testing.T) {
wantErr: true, wantErr: true,
}, },
{ {
name: "user mismatch", name: "user no permission",
args: args{ args: args{
ctx: CTX, ctx: UserCTX,
req: &user.RegisterPasskeyRequest{ req: &user.RegisterPasskeyRequest{
UserId: userID, UserId: userID,
}, },
}, },
wantErr: true, wantErr: true,
}, },
{
name: "user permission",
args: args{
ctx: IamCTX,
req: &user.RegisterPasskeyRequest{
UserId: userID,
},
},
want: &user.RegisterPasskeyResponse{
Details: &object.Details{
ChangeDate: timestamppb.Now(),
ResourceOwner: Instance.DefaultOrg.Id,
},
},
},
{ {
name: "user setting its own passkey", name: "user setting its own passkey",
args: args{ args: args{

View File

@ -7,7 +7,6 @@ import (
"github.com/pquerna/otp" "github.com/pquerna/otp"
"github.com/zitadel/logging" "github.com/zitadel/logging"
"github.com/zitadel/zitadel/internal/api/authz"
http_util "github.com/zitadel/zitadel/internal/api/http" http_util "github.com/zitadel/zitadel/internal/api/http"
"github.com/zitadel/zitadel/internal/command/preparation" "github.com/zitadel/zitadel/internal/command/preparation"
"github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/crypto"
@ -79,10 +78,8 @@ func (c *Commands) createHumanTOTP(ctx context.Context, userID, resourceOwner st
logging.WithError(err).WithField("traceID", tracing.TraceIDFromCtx(ctx)).Debug("unable to get human for loginname") logging.WithError(err).WithField("traceID", tracing.TraceIDFromCtx(ctx)).Debug("unable to get human for loginname")
return nil, zerrors.ThrowPreconditionFailed(err, "COMMAND-SqyJz", "Errors.User.NotFound") return nil, zerrors.ThrowPreconditionFailed(err, "COMMAND-SqyJz", "Errors.User.NotFound")
} }
if authz.GetCtxData(ctx).UserID != userID { if err := c.checkPermissionUpdateUserCredentials(ctx, human.ResourceOwner, userID); err != nil {
if err := c.checkPermission(ctx, domain.PermissionUserCredentialWrite, human.ResourceOwner, userID); err != nil { return nil, err
return nil, err
}
} }
org, err := c.getOrg(ctx, human.ResourceOwner) org, err := c.getOrg(ctx, human.ResourceOwner)
if err != nil { if err != nil {
@ -139,10 +136,8 @@ func (c *Commands) HumanCheckMFATOTPSetup(ctx context.Context, userID, code, use
if err != nil { if err != nil {
return nil, err return nil, err
} }
if authz.GetCtxData(ctx).UserID != userID { if err := c.checkPermissionUpdateUserCredentials(ctx, existingOTP.ResourceOwner, userID); err != nil {
if err := c.checkPermission(ctx, domain.PermissionUserCredentialWrite, existingOTP.ResourceOwner, userID); err != nil { return nil, err
return nil, err
}
} }
if existingOTP.State == domain.MFAStateUnspecified || existingOTP.State == domain.MFAStateRemoved { if existingOTP.State == domain.MFAStateUnspecified || existingOTP.State == domain.MFAStateRemoved {
return nil, zerrors.ThrowNotFound(nil, "COMMAND-3Mif9s", "Errors.User.MFA.OTP.NotExisting") return nil, zerrors.ThrowNotFound(nil, "COMMAND-3Mif9s", "Errors.User.MFA.OTP.NotExisting")
@ -242,10 +237,8 @@ func (c *Commands) HumanRemoveTOTP(ctx context.Context, userID, resourceOwner st
if existingOTP.State == domain.MFAStateUnspecified || existingOTP.State == domain.MFAStateRemoved { if existingOTP.State == domain.MFAStateUnspecified || existingOTP.State == domain.MFAStateRemoved {
return nil, zerrors.ThrowNotFound(nil, "COMMAND-Hd9sd", "Errors.User.MFA.OTP.NotExisting") return nil, zerrors.ThrowNotFound(nil, "COMMAND-Hd9sd", "Errors.User.MFA.OTP.NotExisting")
} }
if userID != authz.GetCtxData(ctx).UserID { if err := c.checkPermissionUpdateUser(ctx, existingOTP.ResourceOwner, userID); err != nil {
if err := c.checkPermission(ctx, domain.PermissionUserWrite, existingOTP.ResourceOwner, userID); err != nil { return nil, err
return nil, err
}
} }
userAgg := UserAggregateFromWriteModel(&existingOTP.WriteModel) userAgg := UserAggregateFromWriteModel(&existingOTP.WriteModel)
pushedEvents, err := c.eventstore.Push(ctx, user.NewHumanOTPRemovedEvent(ctx, userAgg)) pushedEvents, err := c.eventstore.Push(ctx, user.NewHumanOTPRemovedEvent(ctx, userAgg))
@ -286,10 +279,8 @@ func (c *Commands) addHumanOTPSMS(ctx context.Context, userID, resourceOwner str
if err != nil { if err != nil {
return nil, err return nil, err
} }
if authz.GetCtxData(ctx).UserID != userID { if err := c.checkPermissionUpdateUserCredentials(ctx, otpWriteModel.ResourceOwner(), userID); err != nil {
if err := c.checkPermission(ctx, domain.PermissionUserCredentialWrite, otpWriteModel.ResourceOwner(), userID); err != nil { return nil, err
return nil, err
}
} }
if otpWriteModel.otpAdded { if otpWriteModel.otpAdded {
return nil, zerrors.ThrowAlreadyExists(nil, "COMMAND-Ad3g2", "Errors.User.MFA.OTP.AlreadyReady") return nil, zerrors.ThrowAlreadyExists(nil, "COMMAND-Ad3g2", "Errors.User.MFA.OTP.AlreadyReady")
@ -318,10 +309,8 @@ func (c *Commands) RemoveHumanOTPSMS(ctx context.Context, userID, resourceOwner
if err != nil { if err != nil {
return nil, err return nil, err
} }
if userID != authz.GetCtxData(ctx).UserID { if err := c.checkPermissionUpdateUser(ctx, existingOTP.WriteModel.ResourceOwner, userID); err != nil {
if err := c.checkPermission(ctx, domain.PermissionUserWrite, existingOTP.WriteModel.ResourceOwner, userID); err != nil { return nil, err
return nil, err
}
} }
if !existingOTP.otpAdded { if !existingOTP.otpAdded {
return nil, zerrors.ThrowNotFound(nil, "COMMAND-Sr3h3", "Errors.User.MFA.OTP.NotExisting") return nil, zerrors.ThrowNotFound(nil, "COMMAND-Sr3h3", "Errors.User.MFA.OTP.NotExisting")
@ -420,10 +409,8 @@ func (c *Commands) addHumanOTPEmail(ctx context.Context, userID, resourceOwner s
if err != nil { if err != nil {
return nil, err return nil, err
} }
if authz.GetCtxData(ctx).UserID != userID { if err := c.checkPermissionUpdateUserCredentials(ctx, otpWriteModel.ResourceOwner(), userID); err != nil {
if err := c.checkPermission(ctx, domain.PermissionUserCredentialWrite, otpWriteModel.ResourceOwner(), userID); err != nil { return nil, err
return nil, err
}
} }
if otpWriteModel.otpAdded { if otpWriteModel.otpAdded {
return nil, zerrors.ThrowAlreadyExists(nil, "COMMAND-MKL2s", "Errors.User.MFA.OTP.AlreadyReady") return nil, zerrors.ThrowAlreadyExists(nil, "COMMAND-MKL2s", "Errors.User.MFA.OTP.AlreadyReady")
@ -452,10 +439,8 @@ func (c *Commands) RemoveHumanOTPEmail(ctx context.Context, userID, resourceOwne
if err != nil { if err != nil {
return nil, err return nil, err
} }
if userID != authz.GetCtxData(ctx).UserID { if err := c.checkPermissionUpdateUser(ctx, existingOTP.WriteModel.ResourceOwner, userID); err != nil {
if err := c.checkPermission(ctx, domain.PermissionUserWrite, existingOTP.WriteModel.ResourceOwner, userID); err != nil { return nil, err
return nil, err
}
} }
if !existingOTP.otpAdded { if !existingOTP.otpAdded {
return nil, zerrors.ThrowNotFound(nil, "COMMAND-b312D", "Errors.User.MFA.OTP.NotExisting") return nil, zerrors.ThrowNotFound(nil, "COMMAND-b312D", "Errors.User.MFA.OTP.NotExisting")

View File

@ -110,7 +110,7 @@ type setPasswordVerification func(ctx context.Context) (newEncodedPassword strin
// setPasswordWithPermission returns a permission check as [setPasswordVerification] implementation // setPasswordWithPermission returns a permission check as [setPasswordVerification] implementation
func (c *Commands) setPasswordWithPermission(userID, orgID string) setPasswordVerification { func (c *Commands) setPasswordWithPermission(userID, orgID string) setPasswordVerification {
return func(ctx context.Context) (_ string, err error) { return func(ctx context.Context) (_ string, err error) {
return "", c.checkPermission(ctx, domain.PermissionUserWrite, orgID, userID) return "", c.checkPermissionUpdateUser(ctx, orgID, userID)
} }
} }

View File

@ -6,7 +6,6 @@ import (
"github.com/zitadel/logging" "github.com/zitadel/logging"
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/eventstore"
@ -146,10 +145,8 @@ func (c *Commands) addHumanWebAuthN(ctx context.Context, userID, resourceowner,
if err != nil { if err != nil {
return nil, nil, nil, err return nil, nil, nil, err
} }
if authz.GetCtxData(ctx).UserID != userID { if err := c.checkPermissionUpdateUserCredentials(ctx, user.ResourceOwner, userID); err != nil {
if err = c.checkPermission(ctx, domain.PermissionUserCredentialWrite, user.ResourceOwner, userID); err != nil { return nil, nil, nil, err
return nil, nil, nil, err
}
} }
org, err := c.getOrg(ctx, user.ResourceOwner) org, err := c.getOrg(ctx, user.ResourceOwner)
if err != nil { if err != nil {
@ -603,10 +600,9 @@ func (c *Commands) removeHumanWebAuthN(ctx context.Context, userID, webAuthNID,
if existingWebAuthN.State == domain.MFAStateUnspecified || existingWebAuthN.State == domain.MFAStateRemoved { if existingWebAuthN.State == domain.MFAStateUnspecified || existingWebAuthN.State == domain.MFAStateRemoved {
return nil, zerrors.ThrowNotFound(nil, "COMMAND-DAfb2", "Errors.User.WebAuthN.NotFound") return nil, zerrors.ThrowNotFound(nil, "COMMAND-DAfb2", "Errors.User.WebAuthN.NotFound")
} }
if userID != authz.GetCtxData(ctx).UserID {
if err := c.checkPermission(ctx, domain.PermissionUserWrite, existingWebAuthN.ResourceOwner, existingWebAuthN.AggregateID); err != nil { if err := c.checkPermissionUpdateUser(ctx, existingWebAuthN.ResourceOwner, existingWebAuthN.AggregateID); err != nil {
return nil, err return nil, err
}
} }
userAgg := UserAggregateFromWriteModel(&existingWebAuthN.WriteModel) userAgg := UserAggregateFromWriteModel(&existingWebAuthN.WriteModel)

View File

@ -127,6 +127,16 @@ func (c *Commands) checkPermissionUpdateUser(ctx context.Context, resourceOwner,
return nil return nil
} }
func (c *Commands) checkPermissionUpdateUserCredentials(ctx context.Context, resourceOwner, userID string) error {
if userID != "" && userID == authz.GetCtxData(ctx).UserID {
return nil
}
if err := c.checkPermission(ctx, domain.PermissionUserCredentialWrite, resourceOwner, userID); err != nil {
return err
}
return nil
}
func (c *Commands) checkPermissionDeleteUser(ctx context.Context, resourceOwner, userID string) error { func (c *Commands) checkPermissionDeleteUser(ctx context.Context, resourceOwner, userID string) error {
if userID != "" && userID == authz.GetCtxData(ctx).UserID { if userID != "" && userID == authz.GetCtxData(ctx).UserID {
return nil return nil

View File

@ -6,7 +6,6 @@ import (
"github.com/zitadel/logging" "github.com/zitadel/logging"
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/eventstore"
@ -118,10 +117,8 @@ func (c *Commands) changeUserEmailWithGeneratorEvents(ctx context.Context, userI
if err != nil { if err != nil {
return nil, err return nil, err
} }
if authz.GetCtxData(ctx).UserID != userID { if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil {
if err = c.checkPermission(ctx, domain.PermissionUserWrite, cmd.aggregate.ResourceOwner, userID); err != nil { return nil, err
return nil, err
}
} }
if err = cmd.Change(ctx, domain.EmailAddress(email)); err != nil { if err = cmd.Change(ctx, domain.EmailAddress(email)); err != nil {
return nil, err return nil, err
@ -137,10 +134,8 @@ func (c *Commands) resendUserEmailCodeWithGeneratorEvents(ctx context.Context, u
if err != nil { if err != nil {
return nil, err return nil, err
} }
if authz.GetCtxData(ctx).UserID != userID { if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil {
if err = c.checkPermission(ctx, domain.PermissionUserWrite, cmd.aggregate.ResourceOwner, userID); err != nil { return nil, err
return nil, err
}
} }
if cmd.model.Code == nil { if cmd.model.Code == nil {
return nil, zerrors.ThrowPreconditionFailed(err, "EMAIL-5w5ilin4yt", "Errors.User.Code.Empty") return nil, zerrors.ThrowPreconditionFailed(err, "EMAIL-5w5ilin4yt", "Errors.User.Code.Empty")

View File

@ -6,7 +6,6 @@ import (
"github.com/zitadel/logging" "github.com/zitadel/logging"
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/eventstore"
@ -74,10 +73,8 @@ func (c *Commands) ResendInviteCode(ctx context.Context, userID, resourceOwner,
if err != nil { if err != nil {
return nil, err return nil, err
} }
if authz.GetCtxData(ctx).UserID != userID { if err := c.checkPermissionUpdateUser(ctx, existingCode.ResourceOwner, userID); err != nil {
if err := c.checkPermission(ctx, domain.PermissionUserWrite, existingCode.ResourceOwner, userID); err != nil { return nil, err
return nil, err
}
} }
if !existingCode.UserState.Exists() { if !existingCode.UserState.Exists() {
return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-H3b2a", "Errors.User.NotFound") return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-H3b2a", "Errors.User.NotFound")

View File

@ -6,7 +6,6 @@ import (
"github.com/zitadel/logging" "github.com/zitadel/logging"
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/command/preparation" "github.com/zitadel/zitadel/internal/command/preparation"
"github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/domain"
@ -18,7 +17,7 @@ import (
// RegisterUserPasskey creates a passkey registration for the current authenticated user. // RegisterUserPasskey creates a passkey registration for the current authenticated user.
// UserID, usually taken from the request is compared against the user ID in the context. // UserID, usually taken from the request is compared against the user ID in the context.
func (c *Commands) RegisterUserPasskey(ctx context.Context, userID, resourceOwner, rpID string, authenticator domain.AuthenticatorAttachment) (*domain.WebAuthNRegistrationDetails, error) { func (c *Commands) RegisterUserPasskey(ctx context.Context, userID, resourceOwner, rpID string, authenticator domain.AuthenticatorAttachment) (*domain.WebAuthNRegistrationDetails, error) {
if err := authz.UserIDInCTX(ctx, userID); err != nil { if err := c.checkPermissionUpdateUserCredentials(ctx, resourceOwner, userID); err != nil {
return nil, err return nil, err
} }
return c.registerUserPasskey(ctx, userID, resourceOwner, rpID, authenticator) return c.registerUserPasskey(ctx, userID, resourceOwner, rpID, authenticator)

View File

@ -34,8 +34,9 @@ func TestCommands_RegisterUserPasskey(t *testing.T) {
} }
userAgg := &user.NewAggregate("user1", "org1").Aggregate userAgg := &user.NewAggregate("user1", "org1").Aggregate
type fields struct { type fields struct {
eventstore *eventstore.Eventstore eventstore func(t *testing.T) *eventstore.Eventstore
idGenerator id.Generator idGenerator id.Generator
checkPermission domain.PermissionCheck
} }
type args struct { type args struct {
userID string userID string
@ -51,18 +52,22 @@ func TestCommands_RegisterUserPasskey(t *testing.T) {
wantErr error wantErr error
}{ }{
{ {
name: "wrong user", name: "no permission",
fields: fields{
eventstore: expectEventstore(),
checkPermission: newMockPermissionCheckNotAllowed(),
},
args: args{ args: args{
userID: "foo", userID: "foo",
resourceOwner: "org1", resourceOwner: "org1",
authenticator: domain.AuthenticatorAttachmentCrossPlattform, authenticator: domain.AuthenticatorAttachmentCrossPlattform,
}, },
wantErr: zerrors.ThrowPermissionDenied(nil, "AUTH-Bohd2", "Errors.User.UserIDWrong"), wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"),
}, },
{ {
name: "get human passwordless error", name: "get human passwordless error",
fields: fields{ fields: fields{
eventstore: eventstoreExpect(t, eventstore: expectEventstore(
expectFilterError(io.ErrClosedPipe), expectFilterError(io.ErrClosedPipe),
), ),
}, },
@ -76,7 +81,7 @@ func TestCommands_RegisterUserPasskey(t *testing.T) {
{ {
name: "id generator error", name: "id generator error",
fields: fields{ fields: fields{
eventstore: eventstoreExpect(t, eventstore: expectEventstore(
expectFilter(), // getHumanPasswordlessTokens expectFilter(), // getHumanPasswordlessTokens
expectFilter(eventFromEventPusher( expectFilter(eventFromEventPusher(
user.NewHumanAddedEvent(ctx, user.NewHumanAddedEvent(ctx,
@ -118,9 +123,10 @@ func TestCommands_RegisterUserPasskey(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
c := &Commands{ c := &Commands{
eventstore: tt.fields.eventstore, eventstore: tt.fields.eventstore(t),
idGenerator: tt.fields.idGenerator, idGenerator: tt.fields.idGenerator,
webauthnConfig: webauthnConfig, webauthnConfig: webauthnConfig,
checkPermission: tt.fields.checkPermission,
} }
_, err := c.RegisterUserPasskey(ctx, tt.args.userID, tt.args.resourceOwner, tt.args.rpID, tt.args.authenticator) _, err := c.RegisterUserPasskey(ctx, tt.args.userID, tt.args.resourceOwner, tt.args.rpID, tt.args.authenticator)
require.ErrorIs(t, err, tt.wantErr) require.ErrorIs(t, err, tt.wantErr)

View File

@ -4,7 +4,6 @@ import (
"context" "context"
"io" "io"
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/repository/user" "github.com/zitadel/zitadel/internal/repository/user"
"github.com/zitadel/zitadel/internal/zerrors" "github.com/zitadel/zitadel/internal/zerrors"
@ -50,10 +49,8 @@ func (c *Commands) requestPasswordReset(ctx context.Context, userID string, retu
if model.UserState == domain.UserStateInitial { if model.UserState == domain.UserStateInitial {
return nil, nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-Sfe4g", "Errors.User.NotInitialised") return nil, nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-Sfe4g", "Errors.User.NotInitialised")
} }
if authz.GetCtxData(ctx).UserID != userID { if err = c.checkPermissionUpdateUser(ctx, model.ResourceOwner, userID); err != nil {
if err = c.checkPermission(ctx, domain.PermissionUserWrite, model.ResourceOwner, userID); err != nil { return nil, nil, err
return nil, nil, err
}
} }
var passwordCode *EncryptedCode var passwordCode *EncryptedCode
var generatorID string var generatorID string

View File

@ -82,10 +82,8 @@ func (c *Commands) changeUserPhoneWithGenerator(ctx context.Context, userID, pho
if err != nil { if err != nil {
return nil, err return nil, err
} }
if authz.GetCtxData(ctx).UserID != userID { if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil {
if err = c.checkPermission(ctx, domain.PermissionUserWrite, cmd.aggregate.ResourceOwner, userID); err != nil { return nil, err
return nil, err
}
} }
if err = cmd.Change(ctx, domain.PhoneNumber(phone)); err != nil { if err = cmd.Change(ctx, domain.PhoneNumber(phone)); err != nil {
return nil, err return nil, err
@ -104,10 +102,8 @@ func (c *Commands) resendUserPhoneCodeWithGenerator(ctx context.Context, userID
if err != nil { if err != nil {
return nil, err return nil, err
} }
if authz.GetCtxData(ctx).UserID != userID { if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil {
if err = c.checkPermission(ctx, domain.PermissionUserWrite, cmd.aggregate.ResourceOwner, userID); err != nil { return nil, err
return nil, err
}
} }
if cmd.model.Code == nil && cmd.model.GeneratorID == "" { if cmd.model.Code == nil && cmd.model.GeneratorID == "" {
return nil, zerrors.ThrowPreconditionFailed(err, "PHONE-5xrra88eq8", "Errors.User.Code.Empty") return nil, zerrors.ThrowPreconditionFailed(err, "PHONE-5xrra88eq8", "Errors.User.Code.Empty")