fix: add additional permission tests to user v2 query endpoints (#7382)

Add additional permission integration tests to the user v2 query endpoints including some fixes to correctly check the permissions after the data is known which you want to query.

internal/api/grpc/user/v2/query.go

@@ -14,17 +14,15 @@ import (
 )
 
 func (s *Server) GetUserByID(ctx context.Context, req *user.GetUserByIDRequest) (_ *user.GetUserByIDResponse, err error) {
-	ctxData := authz.GetCtxData(ctx)
-	if ctxData.UserID != req.GetUserId() {
-		if err := s.checkPermission(ctx, domain.PermissionUserRead, ctxData.OrgID, req.GetUserId()); err != nil {
-			return nil, err
-		}
-	}
-
 	resp, err := s.query.GetUserByID(ctx, true, req.GetUserId())
 	if err != nil {
 		return nil, err
 	}
+	if authz.GetCtxData(ctx).UserID != req.GetUserId() {
+		if err := s.checkPermission(ctx, domain.PermissionUserRead, resp.ResourceOwner, req.GetUserId()); err != nil {
+			return nil, err
+		}
+	}
 	return &user.GetUserByIDResponse{
 		Details: object.DomainToDetailsPb(&domain.ObjectDetails{
 			Sequence:      resp.Sequence,

internal/api/grpc/user/v2/query_integration_test.go

@@ -5,10 +5,11 @@ package user_test
 import (
 	"context"
 	"fmt"
-	"github.com/stretchr/testify/assert"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/muhlemmer/gu"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/types/known/timestamppb"
@@ -151,6 +152,158 @@ func TestServer_GetUserByID(t *testing.T) {
 	}
 }
 
+func TestServer_GetUserByID_Permission(t *testing.T) {
+	timeNow := time.Now().UTC()
+	newOrgOwnerEmail := fmt.Sprintf("%d@permission.get.com", timeNow.UnixNano())
+	newOrg := Tester.CreateOrganization(IamCTX, fmt.Sprintf("GetHuman%d", time.Now().UnixNano()), newOrgOwnerEmail)
+	newUserID := newOrg.CreatedAdmins[0].GetUserId()
+	type args struct {
+		ctx context.Context
+		req *user.GetUserByIDRequest
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    *user.GetUserByIDResponse
+		wantErr bool
+	}{
+		{
+			name: "System, ok",
+			args: args{
+				SystemCTX,
+				&user.GetUserByIDRequest{
+					Organization: &object.Organization{
+						Org: &object.Organization_OrgId{
+							OrgId: newOrg.GetOrganizationId(),
+						},
+					},
+					UserId: newUserID,
+				},
+			},
+			want: &user.GetUserByIDResponse{
+				User: &user.User{
+					State:              user.UserState_USER_STATE_ACTIVE,
+					Username:           "",
+					LoginNames:         nil,
+					PreferredLoginName: "",
+					Type: &user.User_Human{
+						Human: &user.HumanUser{
+							Profile: &user.HumanProfile{
+								GivenName:         "firstname",
+								FamilyName:        "lastname",
+								NickName:          gu.Ptr(""),
+								DisplayName:       gu.Ptr("firstname lastname"),
+								PreferredLanguage: gu.Ptr("und"),
+								Gender:            user.Gender_GENDER_UNSPECIFIED.Enum(),
+								AvatarUrl:         "",
+							},
+							Email: &user.HumanEmail{
+								Email: newOrgOwnerEmail,
+							},
+							Phone: &user.HumanPhone{},
+						},
+					},
+				},
+				Details: &object.Details{
+					ChangeDate:    timestamppb.New(timeNow),
+					ResourceOwner: newOrg.GetOrganizationId(),
+				},
+			},
+		},
+		{
+			name: "Instance, ok",
+			args: args{
+				IamCTX,
+				&user.GetUserByIDRequest{
+					Organization: &object.Organization{
+						Org: &object.Organization_OrgId{
+							OrgId: newOrg.GetOrganizationId(),
+						},
+					},
+					UserId: newUserID,
+				},
+			},
+			want: &user.GetUserByIDResponse{
+				User: &user.User{
+					State:              user.UserState_USER_STATE_ACTIVE,
+					Username:           "",
+					LoginNames:         nil,
+					PreferredLoginName: "",
+					Type: &user.User_Human{
+						Human: &user.HumanUser{
+							Profile: &user.HumanProfile{
+								GivenName:         "firstname",
+								FamilyName:        "lastname",
+								NickName:          gu.Ptr(""),
+								DisplayName:       gu.Ptr("firstname lastname"),
+								PreferredLanguage: gu.Ptr("und"),
+								Gender:            user.Gender_GENDER_UNSPECIFIED.Enum(),
+								AvatarUrl:         "",
+							},
+							Email: &user.HumanEmail{
+								Email: newOrgOwnerEmail,
+							},
+							Phone: &user.HumanPhone{},
+						},
+					},
+				},
+				Details: &object.Details{
+					ChangeDate:    timestamppb.New(timeNow),
+					ResourceOwner: newOrg.GetOrganizationId(),
+				},
+			},
+		},
+		{
+			name: "Org, error",
+			args: args{
+				CTX,
+				&user.GetUserByIDRequest{
+					Organization: &object.Organization{
+						Org: &object.Organization_OrgId{
+							OrgId: newOrg.GetOrganizationId(),
+						},
+					},
+					UserId: newUserID,
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "User, error",
+			args: args{
+				UserCTX,
+				&user.GetUserByIDRequest{
+					Organization: &object.Organization{
+						Org: &object.Organization_OrgId{
+							OrgId: newOrg.GetOrganizationId(),
+						},
+					},
+					UserId: newUserID,
+				},
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := Client.GetUserByID(tt.args.ctx, tt.args.req)
+			if tt.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				tt.want.User.UserId = tt.args.req.GetUserId()
+				tt.want.User.Username = newOrgOwnerEmail
+				tt.want.User.PreferredLoginName = newOrgOwnerEmail
+				tt.want.User.LoginNames = []string{newOrgOwnerEmail}
+				if human := tt.want.User.GetHuman(); human != nil {
+					human.Email.Email = newOrgOwnerEmail
+				}
+				assert.Equal(t, tt.want.User, got.User)
+			}
+		})
+	}
+}
+
 type userAttr struct {
 	UserID   string
 	Username string

internal/api/grpc/user/v2/user_integration_test.go

@@ -27,12 +27,13 @@ import (
 )
 
 var (
-	CTX     context.Context
-	IamCTX  context.Context
-	UserCTX context.Context
-	ErrCTX  context.Context
-	Tester  *integration.Tester
-	Client  user.UserServiceClient
+	CTX       context.Context
+	IamCTX    context.Context
+	UserCTX   context.Context
+	SystemCTX context.Context
+	ErrCTX    context.Context
+	Tester    *integration.Tester
+	Client    user.UserServiceClient
 )
 
 func TestMain(m *testing.M) {
@@ -45,6 +46,7 @@ func TestMain(m *testing.M) {
 
 		UserCTX = Tester.WithAuthorization(ctx, integration.Login)
 		IamCTX = Tester.WithAuthorization(ctx, integration.IAMOwner)
+		SystemCTX = Tester.WithAuthorization(ctx, integration.SystemUser)
 		CTX, ErrCTX = Tester.WithAuthorization(ctx, integration.OrgOwner), errCtx
 		Client = Tester.Client.UserV2
 		return m.Run()
@@ -552,6 +554,199 @@ func TestServer_AddHumanUser(t *testing.T) {
 	}
 }
 
+func TestServer_AddHumanUser_Permission(t *testing.T) {
+	newOrgOwnerEmail := fmt.Sprintf("%d@permission.com", time.Now().UnixNano())
+	newOrg := Tester.CreateOrganization(IamCTX, fmt.Sprintf("AddHuman%d", time.Now().UnixNano()), newOrgOwnerEmail)
+	type args struct {
+		ctx context.Context
+		req *user.AddHumanUserRequest
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    *user.AddHumanUserResponse
+		wantErr bool
+	}{
+		{
+			name: "System, ok",
+			args: args{
+				SystemCTX,
+				&user.AddHumanUserRequest{
+					Organisation: &object.Organisation{
+						Org: &object.Organisation_OrgId{
+							OrgId: newOrg.GetOrganizationId(),
+						},
+					},
+					Profile: &user.SetHumanProfile{
+						GivenName:         "Donald",
+						FamilyName:        "Duck",
+						NickName:          gu.Ptr("Dukkie"),
+						DisplayName:       gu.Ptr("Donald Duck"),
+						PreferredLanguage: gu.Ptr("en"),
+						Gender:            user.Gender_GENDER_DIVERSE.Enum(),
+					},
+					Email: &user.SetHumanEmail{},
+					Phone: &user.SetHumanPhone{},
+					Metadata: []*user.SetMetadataEntry{
+						{
+							Key:   "somekey",
+							Value: []byte("somevalue"),
+						},
+					},
+					PasswordType: &user.AddHumanUserRequest_Password{
+						Password: &user.Password{
+							Password:       "DifficultPW666!",
+							ChangeRequired: true,
+						},
+					},
+				},
+			},
+			want: &user.AddHumanUserResponse{
+				Details: &object.Details{
+					ChangeDate:    timestamppb.Now(),
+					ResourceOwner: newOrg.GetOrganizationId(),
+				},
+			},
+		},
+		{
+			name: "Instance, ok",
+			args: args{
+				IamCTX,
+				&user.AddHumanUserRequest{
+					Organisation: &object.Organisation{
+						Org: &object.Organisation_OrgId{
+							OrgId: newOrg.GetOrganizationId(),
+						},
+					},
+					Profile: &user.SetHumanProfile{
+						GivenName:         "Donald",
+						FamilyName:        "Duck",
+						NickName:          gu.Ptr("Dukkie"),
+						DisplayName:       gu.Ptr("Donald Duck"),
+						PreferredLanguage: gu.Ptr("en"),
+						Gender:            user.Gender_GENDER_DIVERSE.Enum(),
+					},
+					Email: &user.SetHumanEmail{},
+					Phone: &user.SetHumanPhone{},
+					Metadata: []*user.SetMetadataEntry{
+						{
+							Key:   "somekey",
+							Value: []byte("somevalue"),
+						},
+					},
+					PasswordType: &user.AddHumanUserRequest_Password{
+						Password: &user.Password{
+							Password:       "DifficultPW666!",
+							ChangeRequired: true,
+						},
+					},
+				},
+			},
+			want: &user.AddHumanUserResponse{
+				Details: &object.Details{
+					ChangeDate:    timestamppb.Now(),
+					ResourceOwner: newOrg.GetOrganizationId(),
+				},
+			},
+		},
+		{
+			name: "Org, error",
+			args: args{
+				CTX,
+				&user.AddHumanUserRequest{
+					Organisation: &object.Organisation{
+						Org: &object.Organisation_OrgId{
+							OrgId: newOrg.GetOrganizationId(),
+						},
+					},
+					Profile: &user.SetHumanProfile{
+						GivenName:         "Donald",
+						FamilyName:        "Duck",
+						NickName:          gu.Ptr("Dukkie"),
+						DisplayName:       gu.Ptr("Donald Duck"),
+						PreferredLanguage: gu.Ptr("en"),
+						Gender:            user.Gender_GENDER_DIVERSE.Enum(),
+					},
+					Email: &user.SetHumanEmail{},
+					Phone: &user.SetHumanPhone{},
+					Metadata: []*user.SetMetadataEntry{
+						{
+							Key:   "somekey",
+							Value: []byte("somevalue"),
+						},
+					},
+					PasswordType: &user.AddHumanUserRequest_Password{
+						Password: &user.Password{
+							Password:       "DifficultPW666!",
+							ChangeRequired: true,
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "User, error",
+			args: args{
+				UserCTX,
+				&user.AddHumanUserRequest{
+					Organisation: &object.Organisation{
+						Org: &object.Organisation_OrgId{
+							OrgId: newOrg.GetOrganizationId(),
+						},
+					},
+					Profile: &user.SetHumanProfile{
+						GivenName:         "Donald",
+						FamilyName:        "Duck",
+						NickName:          gu.Ptr("Dukkie"),
+						DisplayName:       gu.Ptr("Donald Duck"),
+						PreferredLanguage: gu.Ptr("en"),
+						Gender:            user.Gender_GENDER_DIVERSE.Enum(),
+					},
+					Email: &user.SetHumanEmail{},
+					Phone: &user.SetHumanPhone{},
+					Metadata: []*user.SetMetadataEntry{
+						{
+							Key:   "somekey",
+							Value: []byte("somevalue"),
+						},
+					},
+					PasswordType: &user.AddHumanUserRequest_Password{
+						Password: &user.Password{
+							Password:       "DifficultPW666!",
+							ChangeRequired: true,
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+	}
+	for i, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			userID := fmt.Sprint(time.Now().UnixNano() + int64(i))
+			tt.args.req.UserId = &userID
+			if email := tt.args.req.GetEmail(); email != nil {
+				email.Email = fmt.Sprintf("%s@me.now", userID)
+			}
+
+			if tt.want != nil {
+				tt.want.UserId = userID
+			}
+
+			got, err := Client.AddHumanUser(tt.args.ctx, tt.args.req)
+			if tt.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+
+			assert.Equal(t, tt.want.GetUserId(), got.GetUserId())
+			integration.AssertDetails(t, tt.want, got)
+		})
+	}
+}
+
 func TestServer_UpdateHumanUser(t *testing.T) {
 	type args struct {
 		ctx context.Context
@@ -908,6 +1103,89 @@ func TestServer_UpdateHumanUser(t *testing.T) {
 	}
 }
 
+func TestServer_UpdateHumanUser_Permission(t *testing.T) {
+	newOrgOwnerEmail := fmt.Sprintf("%d@permission.update.com", time.Now().UnixNano())
+	newOrg := Tester.CreateOrganization(IamCTX, fmt.Sprintf("UpdateHuman%d", time.Now().UnixNano()), newOrgOwnerEmail)
+	newUserID := newOrg.CreatedAdmins[0].GetUserId()
+	type args struct {
+		ctx context.Context
+		req *user.UpdateHumanUserRequest
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    *user.UpdateHumanUserResponse
+		wantErr bool
+	}{
+		{
+			name: "system, ok",
+			args: args{
+				SystemCTX,
+				&user.UpdateHumanUserRequest{
+					UserId:   newUserID,
+					Username: gu.Ptr(fmt.Sprint("system", time.Now().UnixNano()+1)),
+				},
+			},
+			want: &user.UpdateHumanUserResponse{
+				Details: &object.Details{
+					ChangeDate:    timestamppb.Now(),
+					ResourceOwner: newOrg.GetOrganizationId(),
+				},
+			},
+		},
+		{
+			name: "instance, ok",
+			args: args{
+				IamCTX,
+				&user.UpdateHumanUserRequest{
+					UserId:   newUserID,
+					Username: gu.Ptr(fmt.Sprint("instance", time.Now().UnixNano()+1)),
+				},
+			},
+			want: &user.UpdateHumanUserResponse{
+				Details: &object.Details{
+					ChangeDate:    timestamppb.Now(),
+					ResourceOwner: newOrg.GetOrganizationId(),
+				},
+			},
+		},
+		{
+			name: "org, error",
+			args: args{
+				CTX,
+				&user.UpdateHumanUserRequest{
+					UserId:   newUserID,
+					Username: gu.Ptr(fmt.Sprint("org", time.Now().UnixNano()+1)),
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "user, error",
+			args: args{
+				UserCTX,
+				&user.UpdateHumanUserRequest{
+					UserId:   newUserID,
+					Username: gu.Ptr(fmt.Sprint("user", time.Now().UnixNano()+1)),
+				},
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+
+			got, err := Client.UpdateHumanUser(tt.args.ctx, tt.args.req)
+			if tt.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+			integration.AssertDetails(t, tt.want, got)
+		})
+	}
+}
+
 func TestServer_LockUser(t *testing.T) {
 	type args struct {
 		ctx     context.Context

internal/integration/integration.go

@@ -252,6 +252,13 @@ func (s *Tester) WithInstanceAuthorization(ctx context.Context, u UserType, inst
 	return s.WithAuthorizationToken(ctx, s.Users.Get(instanceID, u).Token)
 }
 
+func (s *Tester) GetUserID(u UserType) string {
+	if u == SystemUser {
+		s.ensureSystemUser()
+	}
+	return s.Users.Get(FirstInstanceUsersKey, u).ID
+}
+
 func (s *Tester) WithAuthorizationToken(ctx context.Context, token string) context.Context {
 	md, ok := metadata.FromOutgoingContext(ctx)
 	if !ok {

internal/query/user.go

@@ -127,7 +127,7 @@ func (u *Users) RemoveNoPermission(ctx context.Context, permissionCheck domain.P
 	for i := range u.Users {
 		ctxData := authz.GetCtxData(ctx)
 		if ctxData.UserID != u.Users[i].ID {
-			if err := permissionCheck(ctx, domain.PermissionUserRead, ctxData.OrgID, u.Users[i].ID); err != nil {
+			if err := permissionCheck(ctx, domain.PermissionUserRead, u.Users[i].ResourceOwner, u.Users[i].ID); err != nil {
 				removableIndexes = append(removableIndexes, i)
 			}
 		}