mirror of https://github.com/usememos/memos.git
fix(api): restrict user email visibility
This commit is contained in:
parent
acddef1f3d
commit
55aedd5d5d
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
})
|
||||
}
|
||||
|
|
@ -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:
|
||||
|
|
|
|||
Loading…
Reference in New Issue