From fb2d4545b914ed8bb223bacded42f280c5e86861 Mon Sep 17 00:00:00 2001 From: Stefan Benz <46600784+stebenz@users.noreply.github.com> Date: Wed, 10 Jul 2024 17:49:35 +0200 Subject: [PATCH] fix: add details to ListUsers for user results (#8255) # Which Problems Are Solved In User v2 API, the ListUsers endpoint doesn't provide the information to which organization the user belongs to. # How the Problems Are Solved Add the details to the user results from the ListUsers endpoint, so that the OrgID is also included as ResourceOwner. # Additional Changes None # Additional Context Closes #8172 --- internal/api/grpc/user/v2/query.go | 7 ++- .../grpc/user/v2/query_integration_test.go | 52 +++++++++++-------- internal/integration/client.go | 5 +- proto/zitadel/user/v2beta/user.proto | 1 + proto/zitadel/user/v2beta/user_service.proto | 1 + 5 files changed, 39 insertions(+), 27 deletions(-) diff --git a/internal/api/grpc/user/v2/query.go b/internal/api/grpc/user/v2/query.go index 5ccaea84b..2aaf0ad8e 100644 --- a/internal/api/grpc/user/v2/query.go +++ b/internal/api/grpc/user/v2/query.go @@ -60,7 +60,12 @@ func UsersToPb(users []*query.User, assetPrefix string) []*user.User { func userToPb(userQ *query.User, assetPrefix string) *user.User { return &user.User{ - UserId: userQ.ID, + UserId: userQ.ID, + Details: object.DomainToDetailsPb(&domain.ObjectDetails{ + Sequence: userQ.Sequence, + EventDate: userQ.ChangeDate, + ResourceOwner: userQ.ResourceOwner, + }), State: userStateToPb(userQ.State), Username: userQ.Username, LoginNames: userQ.LoginNames, diff --git a/internal/api/grpc/user/v2/query_integration_test.go b/internal/api/grpc/user/v2/query_integration_test.go index 9b5c74e42..b4b770481 100644 --- a/internal/api/grpc/user/v2/query_integration_test.go +++ b/internal/api/grpc/user/v2/query_integration_test.go @@ -23,7 +23,7 @@ func TestServer_GetUserByID(t *testing.T) { type args struct { ctx context.Context req *user.GetUserByIDRequest - dep func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*timestamppb.Timestamp, error) + dep func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) } tests := []struct { name string @@ -38,7 +38,7 @@ func TestServer_GetUserByID(t *testing.T) { &user.GetUserByIDRequest{ UserId: "", }, - func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*timestamppb.Timestamp, error) { + func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) { return nil, nil }, }, @@ -51,7 +51,7 @@ func TestServer_GetUserByID(t *testing.T) { &user.GetUserByIDRequest{ UserId: "unknown", }, - func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*timestamppb.Timestamp, error) { + func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) { return nil, nil }, }, @@ -62,10 +62,10 @@ func TestServer_GetUserByID(t *testing.T) { args: args{ IamCTX, &user.GetUserByIDRequest{}, - func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*timestamppb.Timestamp, error) { + func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) { resp := Tester.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) request.UserId = resp.GetUserId() - return nil, nil + return &userAttr{resp.GetUserId(), username, nil, resp.GetDetails()}, nil }, }, want: &user.GetUserByIDResponse{ @@ -106,11 +106,11 @@ func TestServer_GetUserByID(t *testing.T) { args: args{ IamCTX, &user.GetUserByIDRequest{}, - func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*timestamppb.Timestamp, error) { + func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) { resp := Tester.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) request.UserId = resp.GetUserId() - changed := Tester.SetUserPassword(ctx, resp.GetUserId(), integration.UserPassword, true) - return changed, nil + details := Tester.SetUserPassword(ctx, resp.GetUserId(), integration.UserPassword, true) + return &userAttr{resp.GetUserId(), username, details.GetChangeDate(), resp.GetDetails()}, nil }, }, want: &user.GetUserByIDResponse{ @@ -152,7 +152,7 @@ func TestServer_GetUserByID(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { username := fmt.Sprintf("%d@mouse.com", time.Now().UnixNano()) - changed, err := tt.args.dep(tt.args.ctx, username, tt.args.req) + userAttr, err := tt.args.dep(tt.args.ctx, username, tt.args.req) require.NoError(t, err) retryDuration := time.Minute if ctxDeadline, ok := CTX.Deadline(); ok { @@ -168,14 +168,15 @@ func TestServer_GetUserByID(t *testing.T) { if getErr != nil { return } - tt.want.User.UserId = tt.args.req.GetUserId() - tt.want.User.Username = username - tt.want.User.PreferredLoginName = username - tt.want.User.LoginNames = []string{username} + tt.want.User.Details = userAttr.Details + tt.want.User.UserId = userAttr.UserID + tt.want.User.Username = userAttr.Username + tt.want.User.PreferredLoginName = userAttr.Username + tt.want.User.LoginNames = []string{userAttr.Username} if human := tt.want.User.GetHuman(); human != nil { - human.Email.Email = username + human.Email.Email = userAttr.Username if tt.want.User.GetHuman().GetPasswordChanged() != nil { - human.PasswordChanged = changed + human.PasswordChanged = userAttr.Changed } } assert.Equal(ttt, tt.want.User, got.User) @@ -311,6 +312,9 @@ func TestServer_GetUserByID_Permission(t *testing.T) { if human := tt.want.User.GetHuman(); human != nil { human.Email.Email = newOrgOwnerEmail } + // details tested in GetUserByID + tt.want.User.Details = got.User.GetDetails() + assert.Equal(t, tt.want.User, got.User) } }) @@ -321,6 +325,7 @@ type userAttr struct { UserID string Username string Changed *timestamppb.Timestamp + Details *object.Details } func TestServer_ListUsers(t *testing.T) { @@ -374,7 +379,7 @@ func TestServer_ListUsers(t *testing.T) { for i, username := range usernames { resp := Tester.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) userIDs[i] = resp.GetUserId() - infos[i] = userAttr{resp.GetUserId(), username, nil} + infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} } request.Queries = append(request.Queries, InUserIDsQuery(userIDs)) return infos, nil @@ -428,8 +433,8 @@ func TestServer_ListUsers(t *testing.T) { for i, username := range usernames { resp := Tester.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) userIDs[i] = resp.GetUserId() - changed := Tester.SetUserPassword(ctx, resp.GetUserId(), integration.UserPassword, true) - infos[i] = userAttr{resp.GetUserId(), username, changed} + details := Tester.SetUserPassword(ctx, resp.GetUserId(), integration.UserPassword, true) + infos[i] = userAttr{resp.GetUserId(), username, details.GetChangeDate(), resp.GetDetails()} } request.Queries = append(request.Queries, InUserIDsQuery(userIDs)) return infos, nil @@ -485,7 +490,7 @@ func TestServer_ListUsers(t *testing.T) { for i, username := range usernames { resp := Tester.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) userIDs[i] = resp.GetUserId() - infos[i] = userAttr{resp.GetUserId(), username, nil} + infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} } request.Queries = append(request.Queries, InUserIDsQuery(userIDs)) return infos, nil @@ -581,7 +586,7 @@ func TestServer_ListUsers(t *testing.T) { for i, username := range usernames { resp := Tester.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) userIDs[i] = resp.GetUserId() - infos[i] = userAttr{resp.GetUserId(), username, nil} + infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} request.Queries = append(request.Queries, UsernameQuery(username)) } return infos, nil @@ -633,7 +638,7 @@ func TestServer_ListUsers(t *testing.T) { infos := make([]userAttr, len(usernames)) for i, username := range usernames { resp := Tester.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - infos[i] = userAttr{resp.GetUserId(), username, nil} + infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} } request.Queries = append(request.Queries, InUserEmailsQuery(usernames)) return infos, nil @@ -685,7 +690,7 @@ func TestServer_ListUsers(t *testing.T) { infos := make([]userAttr, len(usernames)) for i, username := range usernames { resp := Tester.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - infos[i] = userAttr{resp.GetUserId(), username, nil} + infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} } request.Queries = append(request.Queries, InUserEmailsQuery(usernames)) return infos, nil @@ -800,7 +805,7 @@ func TestServer_ListUsers(t *testing.T) { infos := make([]userAttr, len(usernames)) for i, username := range usernames { resp := Tester.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - infos[i] = userAttr{resp.GetUserId(), username, nil} + infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} } request.Queries = append(request.Queries, OrganizationIdQuery(orgResp.OrganizationId)) request.Queries = append(request.Queries, InUserEmailsQuery(usernames)) @@ -920,6 +925,7 @@ func TestServer_ListUsers(t *testing.T) { human.PasswordChanged = infos[i].Changed } } + tt.want.Result[i].Details = infos[i].Details } for i := range tt.want.Result { assert.Contains(ttt, got.Result, tt.want.Result[i]) diff --git a/internal/integration/client.go b/internal/integration/client.go index 90f7518fe..783eca4c9 100644 --- a/internal/integration/client.go +++ b/internal/integration/client.go @@ -17,7 +17,6 @@ import ( "google.golang.org/grpc" "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/structpb" - "google.golang.org/protobuf/types/known/timestamppb" "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/command" @@ -312,7 +311,7 @@ func (s *Tester) RegisterUserU2F(ctx context.Context, userID string) { logging.OnError(err).Fatal("create user u2f") } -func (s *Tester) SetUserPassword(ctx context.Context, userID, password string, changeRequired bool) *timestamppb.Timestamp { +func (s *Tester) SetUserPassword(ctx context.Context, userID, password string, changeRequired bool) *object.Details { resp, err := s.Client.UserV2.SetPassword(ctx, &user.SetPasswordRequest{ UserId: userID, NewPassword: &user.Password{ @@ -321,7 +320,7 @@ func (s *Tester) SetUserPassword(ctx context.Context, userID, password string, c }, }) logging.OnError(err).Fatal("set user password") - return resp.GetDetails().GetChangeDate() + return resp.GetDetails() } func (s *Tester) AddGenericOAuthProvider(t *testing.T, ctx context.Context) string { diff --git a/proto/zitadel/user/v2beta/user.proto b/proto/zitadel/user/v2beta/user.proto index 69d9f8dce..9f1fd8a8c 100644 --- a/proto/zitadel/user/v2beta/user.proto +++ b/proto/zitadel/user/v2beta/user.proto @@ -183,6 +183,7 @@ message User { example: "\"d654e6ba-70a3-48ef-a95d-37c8d8a7901a\""; } ]; + zitadel.object.v2beta.Details details = 8; UserState state = 2 [ (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { description: "current state of the user"; diff --git a/proto/zitadel/user/v2beta/user_service.proto b/proto/zitadel/user/v2beta/user_service.proto index 3a0fb2dfa..cb3a1610e 100644 --- a/proto/zitadel/user/v2beta/user_service.proto +++ b/proto/zitadel/user/v2beta/user_service.proto @@ -989,6 +989,7 @@ message GetUserByIDRequest { } message GetUserByIDResponse { + //deprecated: details is moved into user zitadel.object.v2beta.Details details = 1; zitadel.user.v2beta.User user = 2; }