From 55aedd5d5dc666bc40bf1b56dc594a1cf17c4409 Mon Sep 17 00:00:00 2001 From: memoclaw <265580040+memoclaw@users.noreply.github.com> Date: Wed, 25 Mar 2026 21:54:00 +0800 Subject: [PATCH] fix(api): restrict user email visibility --- server/router/api/v1/auth_service.go | 4 +- server/router/api/v1/instance_service.go | 3 +- .../api/v1/test/user_email_visibility_test.go | 113 ++++++++++++++++++ server/router/api/v1/user_service.go | 22 +++- 4 files changed, 133 insertions(+), 9 deletions(-) create mode 100644 server/router/api/v1/test/user_email_visibility_test.go diff --git a/server/router/api/v1/auth_service.go b/server/router/api/v1/auth_service.go index 87cc55a63..8f6b86310 100644 --- a/server/router/api/v1/auth_service.go +++ b/server/router/api/v1/auth_service.go @@ -48,7 +48,7 @@ func (s *APIV1Service) GetCurrentUser(ctx context.Context, _ *v1pb.GetCurrentUse } return &v1pb.GetCurrentUserResponse{ - User: convertUserFromStore(user), + User: convertUserFromStore(user, user), }, nil } @@ -187,7 +187,7 @@ func (s *APIV1Service) SignIn(ctx context.Context, request *v1pb.SignInRequest) } return &v1pb.SignInResponse{ - User: convertUserFromStore(existingUser), + User: convertUserFromStore(existingUser, existingUser), AccessToken: accessToken, AccessTokenExpiresAt: timestamppb.New(accessExpiresAt), }, nil diff --git a/server/router/api/v1/instance_service.go b/server/router/api/v1/instance_service.go index 3de44a199..5fb3dfcf5 100644 --- a/server/router/api/v1/instance_service.go +++ b/server/router/api/v1/instance_service.go @@ -450,5 +450,6 @@ func (s *APIV1Service) GetInstanceAdmin(ctx context.Context) (*v1pb.User, error) return nil, nil } - return convertUserFromStore(user), nil + currentUser, _ := s.fetchCurrentUser(ctx) + return convertUserFromStore(user, currentUser), nil } diff --git a/server/router/api/v1/test/user_email_visibility_test.go b/server/router/api/v1/test/user_email_visibility_test.go new file mode 100644 index 000000000..40e69f4e5 --- /dev/null +++ b/server/router/api/v1/test/user_email_visibility_test.go @@ -0,0 +1,113 @@ +package test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + apiv1 "github.com/usememos/memos/proto/gen/api/v1" +) + +func TestUserEmailVisibility(t *testing.T) { + ctx := context.Background() + + t.Run("GetUser redacts email for anonymous callers", func(t *testing.T) { + ts := NewTestService(t) + defer ts.Cleanup() + + user, err := ts.CreateRegularUser(ctx, "targetuser") + require.NoError(t, err) + + got, err := ts.Service.GetUser(ctx, &apiv1.GetUserRequest{ + Name: "users/targetuser", + }) + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, user.Username, got.Username) + require.Empty(t, got.Email) + }) + + t.Run("GetUser redacts email for other regular users", func(t *testing.T) { + ts := NewTestService(t) + defer ts.Cleanup() + + targetUser, err := ts.CreateRegularUser(ctx, "targetuser") + require.NoError(t, err) + viewer, err := ts.CreateRegularUser(ctx, "vieweruser") + require.NoError(t, err) + + viewerCtx := ts.CreateUserContext(ctx, viewer.ID) + got, err := ts.Service.GetUser(viewerCtx, &apiv1.GetUserRequest{ + Name: "users/targetuser", + }) + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, targetUser.Username, got.Username) + require.Empty(t, got.Email) + }) + + t.Run("GetUser returns email for the same user", func(t *testing.T) { + ts := NewTestService(t) + defer ts.Cleanup() + + user, err := ts.CreateRegularUser(ctx, "selfuser") + require.NoError(t, err) + + userCtx := ts.CreateUserContext(ctx, user.ID) + got, err := ts.Service.GetUser(userCtx, &apiv1.GetUserRequest{ + Name: "users/selfuser", + }) + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, user.Email, got.Email) + }) + + t.Run("GetUser returns email for admins", func(t *testing.T) { + ts := NewTestService(t) + defer ts.Cleanup() + + targetUser, err := ts.CreateRegularUser(ctx, "targetuser") + require.NoError(t, err) + admin, err := ts.CreateHostUser(ctx, "admin") + require.NoError(t, err) + + adminCtx := ts.CreateUserContext(ctx, admin.ID) + got, err := ts.Service.GetUser(adminCtx, &apiv1.GetUserRequest{ + Name: "users/targetuser", + }) + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, targetUser.Email, got.Email) + }) + + t.Run("GetCurrentUser returns email for the authenticated user", func(t *testing.T) { + ts := NewTestService(t) + defer ts.Cleanup() + + user, err := ts.CreateRegularUser(ctx, "currentuser") + require.NoError(t, err) + + userCtx := ts.CreateUserContext(ctx, user.ID) + got, err := ts.Service.GetCurrentUser(userCtx, &apiv1.GetCurrentUserRequest{}) + require.NoError(t, err) + require.NotNil(t, got) + require.NotNil(t, got.User) + require.Equal(t, user.Email, got.User.Email) + }) + + t.Run("GetInstanceProfile redacts admin email for anonymous callers", func(t *testing.T) { + ts := NewTestService(t) + defer ts.Cleanup() + + admin, err := ts.CreateHostUser(ctx, "admin") + require.NoError(t, err) + + got, err := ts.Service.GetInstanceProfile(ctx, &apiv1.GetInstanceProfileRequest{}) + require.NoError(t, err) + require.NotNil(t, got) + require.NotNil(t, got.Admin) + require.Equal(t, admin.Username, got.Admin.Username) + require.Empty(t, got.Admin.Email) + }) +} diff --git a/server/router/api/v1/user_service.go b/server/router/api/v1/user_service.go index fd0e17564..93c5fc8d9 100644 --- a/server/router/api/v1/user_service.go +++ b/server/router/api/v1/user_service.go @@ -64,7 +64,7 @@ func (s *APIV1Service) ListUsers(ctx context.Context, request *v1pb.ListUsersReq TotalSize: int32(len(users)), } for _, user := range users { - response.Users = append(response.Users, convertUserFromStore(user)) + response.Users = append(response.Users, convertUserFromStore(user, currentUser)) } return response, nil } @@ -77,7 +77,8 @@ func (s *APIV1Service) GetUser(ctx context.Context, request *v1pb.GetUserRequest if user == nil { return nil, status.Errorf(codes.NotFound, "user not found") } - return convertUserFromStore(user), nil + currentUser, _ := s.fetchCurrentUser(ctx) + return convertUserFromStore(user, currentUser), nil } func (s *APIV1Service) CreateUser(ctx context.Context, request *v1pb.CreateUserRequest) (*v1pb.User, error) { @@ -154,7 +155,7 @@ func (s *APIV1Service) CreateUser(ctx context.Context, request *v1pb.CreateUserR return nil, status.Errorf(codes.Internal, "failed to create user: %v", err) } - return convertUserFromStore(user), nil + return convertUserFromStore(user, user), nil } func (s *APIV1Service) UpdateUser(ctx context.Context, request *v1pb.UpdateUserRequest) (*v1pb.User, error) { @@ -260,7 +261,7 @@ func (s *APIV1Service) UpdateUser(ctx context.Context, request *v1pb.UpdateUserR return nil, status.Errorf(codes.Internal, "failed to update user: %v", err) } - return convertUserFromStore(updatedUser), nil + return convertUserFromStore(updatedUser, currentUser), nil } func (s *APIV1Service) DeleteUser(ctx context.Context, request *v1pb.DeleteUserRequest) (*emptypb.Empty, error) { @@ -928,7 +929,7 @@ func convertUserWebhookFromUserSetting(webhook *storepb.WebhooksUserSetting_Webh } } -func convertUserFromStore(user *store.User) *v1pb.User { +func convertUserFromStore(user *store.User, viewer *store.User) *v1pb.User { userpb := &v1pb.User{ Name: BuildUserName(user.Username), State: convertStateFromStore(user.RowStatus), @@ -936,11 +937,13 @@ func convertUserFromStore(user *store.User) *v1pb.User { UpdateTime: timestamppb.New(time.Unix(user.UpdatedTs, 0)), Role: convertUserRoleFromStore(user.Role), Username: user.Username, - Email: user.Email, DisplayName: user.Nickname, AvatarUrl: user.AvatarURL, Description: user.Description, } + if canViewerAccessUserEmail(viewer, user) { + userpb.Email = user.Email + } // Use the avatar URL instead of raw base64 image data to reduce the response size. if user.AvatarURL != "" { // Check if avatar url is base64 format. @@ -954,6 +957,13 @@ func convertUserFromStore(user *store.User) *v1pb.User { return userpb } +func canViewerAccessUserEmail(viewer, user *store.User) bool { + if viewer == nil || user == nil { + return false + } + return viewer.Role == store.RoleAdmin || viewer.ID == user.ID +} + func convertUserRoleFromStore(role store.Role) v1pb.User_Role { switch role { case store.RoleAdmin: