fix: mixed-case user resource names (#5853)
This commit is contained in:
parent
1fd6a2a385
commit
01be01f4b7
3 changed files with 81 additions and 12 deletions
|
|
@ -44,6 +44,62 @@ func TestUserResourceName(t *testing.T) {
|
|||
require.Equal(t, "users/newuser", created.Name)
|
||||
})
|
||||
|
||||
t.Run("Mixed-case username remains usable after auth", func(t *testing.T) {
|
||||
ts := NewTestService(t)
|
||||
defer ts.Cleanup()
|
||||
|
||||
user, err := ts.CreateRegularUser(ctx, "Gnammi")
|
||||
require.NoError(t, err)
|
||||
|
||||
userCtx := ts.CreateUserContext(ctx, user.ID)
|
||||
currentUser, err := ts.Service.GetCurrentUser(userCtx, &apiv1.GetCurrentUserRequest{})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, currentUser.GetUser())
|
||||
require.Equal(t, "users/Gnammi", currentUser.GetUser().Name)
|
||||
|
||||
settings, err := ts.Service.ListUserSettings(userCtx, &apiv1.ListUserSettingsRequest{
|
||||
Parent: currentUser.GetUser().Name,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, settings)
|
||||
|
||||
shortcuts, err := ts.Service.ListShortcuts(userCtx, &apiv1.ListShortcutsRequest{
|
||||
Parent: currentUser.GetUser().Name,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, shortcuts)
|
||||
})
|
||||
|
||||
t.Run("BatchGetUsers preserves mixed-case usernames", func(t *testing.T) {
|
||||
ts := NewTestService(t)
|
||||
defer ts.Cleanup()
|
||||
|
||||
user, err := ts.CreateRegularUser(ctx, "Gnammi")
|
||||
require.NoError(t, err)
|
||||
|
||||
resp, err := ts.Service.BatchGetUsers(ctx, &apiv1.BatchGetUsersRequest{
|
||||
Usernames: []string{user.Username},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, resp.Users, 1)
|
||||
require.Equal(t, "users/Gnammi", resp.Users[0].Name)
|
||||
})
|
||||
|
||||
t.Run("CreateUser rejects all-numeric usernames", func(t *testing.T) {
|
||||
ts := NewTestService(t)
|
||||
defer ts.Cleanup()
|
||||
|
||||
_, err := ts.Service.CreateUser(ctx, &apiv1.CreateUserRequest{
|
||||
User: &apiv1.User{
|
||||
Username: "123",
|
||||
Email: "123@example.com",
|
||||
Password: "password123",
|
||||
},
|
||||
})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "invalid username")
|
||||
})
|
||||
|
||||
t.Run("GetUser rejects numeric user resource names", func(t *testing.T) {
|
||||
ts := NewTestService(t)
|
||||
defer ts.Cleanup()
|
||||
|
|
|
|||
|
|
@ -2,8 +2,6 @@ package v1
|
|||
|
||||
import (
|
||||
"context"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/pkg/errors"
|
||||
|
||||
|
|
@ -26,15 +24,31 @@ func ExtractUsernameFromName(name string) (string, error) {
|
|||
if username == "" {
|
||||
return "", errors.Errorf("invalid user name %q", name)
|
||||
}
|
||||
if _, err := strconv.ParseInt(username, 10, 32); err == nil {
|
||||
return "", errors.Errorf("invalid username %q", username)
|
||||
}
|
||||
if username != strings.ToLower(username) || !base.UIDMatcher.MatchString(username) {
|
||||
return "", errors.Errorf("invalid username %q", username)
|
||||
if err := validateUsername(username); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return username, nil
|
||||
}
|
||||
|
||||
func validateUsername(username string) error {
|
||||
if username == "" || isNumericUsername(username) || !base.UIDMatcher.MatchString(username) {
|
||||
return errors.Errorf("invalid username %q", username)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func isNumericUsername(username string) bool {
|
||||
if username == "" {
|
||||
return false
|
||||
}
|
||||
for _, char := range username {
|
||||
if char < '0' || char > '9' {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// ResolveUserByName resolves a username-based user resource name to a store user.
|
||||
func ResolveUserByName(ctx context.Context, stores *store.Store, name string) (*store.User, error) {
|
||||
username, err := ExtractUsernameFromName(name)
|
||||
|
|
|
|||
|
|
@ -20,7 +20,6 @@ import (
|
|||
"google.golang.org/protobuf/types/known/emptypb"
|
||||
"google.golang.org/protobuf/types/known/timestamppb"
|
||||
|
||||
"github.com/usememos/memos/internal/base"
|
||||
"github.com/usememos/memos/internal/util"
|
||||
"github.com/usememos/memos/internal/webhook"
|
||||
v1pb "github.com/usememos/memos/proto/gen/api/v1"
|
||||
|
|
@ -76,8 +75,8 @@ func normalizeBatchUsernames(usernames []string) []string {
|
|||
uniqueUsernames := make([]string, 0, len(usernames))
|
||||
seen := make(map[string]struct{}, len(usernames))
|
||||
for _, username := range usernames {
|
||||
username = strings.TrimSpace(strings.ToLower(username))
|
||||
if username == "" || !base.UIDMatcher.MatchString(username) {
|
||||
username = strings.TrimSpace(username)
|
||||
if validateUsername(username) != nil {
|
||||
continue
|
||||
}
|
||||
if _, ok := seen[username]; ok {
|
||||
|
|
@ -177,7 +176,7 @@ func (s *APIV1Service) CreateUser(ctx context.Context, request *v1pb.CreateUserR
|
|||
roleToAssign = store.RoleUser
|
||||
}
|
||||
|
||||
if !base.UIDMatcher.MatchString(strings.ToLower(request.User.Username)) {
|
||||
if err := validateUsername(request.User.Username); err != nil {
|
||||
return nil, status.Errorf(codes.InvalidArgument, "invalid username: %s", request.User.Username)
|
||||
}
|
||||
|
||||
|
|
@ -254,7 +253,7 @@ func (s *APIV1Service) UpdateUser(ctx context.Context, request *v1pb.UpdateUserR
|
|||
if instanceGeneralSetting.DisallowChangeUsername {
|
||||
return nil, status.Errorf(codes.PermissionDenied, "permission denied: disallow change username")
|
||||
}
|
||||
if !base.UIDMatcher.MatchString(strings.ToLower(request.User.Username)) {
|
||||
if err := validateUsername(request.User.Username); err != nil {
|
||||
return nil, status.Errorf(codes.InvalidArgument, "invalid username: %s", request.User.Username)
|
||||
}
|
||||
update.Username = &request.User.Username
|
||||
|
|
|
|||
Loading…
Reference in a new issue