fix(api): align resource IDs with AIP conventions

Validate new user-provided IDs using the AIP-122 format while retaining legacy UID compatibility. Correct resource annotations and canonical names returned by user stats.
This commit is contained in:
johnnyjoygh
2026-07-18 11:12:28 +08:00
parent 715306ea66
commit 84776cc106
45 changed files with 368 additions and 192 deletions
+6 -8
View File
@@ -19,7 +19,7 @@ var PublicMethods = map[string]struct{}{
"/memos.api.v1.InstanceService/BatchGetInstanceSettings": {},
// User Service - public user profiles and stats
"/memos.api.v1.UserService/CreateUser": {}, // Allow first user registration
"/memos.api.v1.UserService/CreateUser": {}, // Registration policy is enforced in UserService
"/memos.api.v1.UserService/GetUser": {},
"/memos.api.v1.UserService/BatchGetUsers": {},
"/memos.api.v1.UserService/GetUserAvatar": {},
@@ -51,9 +51,8 @@ func IsPublicMethod(procedure string) bool {
// anonymous callers even when the instance is private (no InstanceURL configured).
//
// It is the minimum required to render the sign-in page, authenticate, and follow
// share links. Every entry here MUST also exist in PublicMethods. CreateUser is
// intentionally excluded and handled separately (allowed only during first-run
// setup, while the instance has no users yet).
// share links, and register when instance settings permit it. Every entry here
// MUST also exist in PublicMethods.
var AuthBootstrapMethods = map[string]struct{}{
// Auth Service - sign-in and token refresh.
"/memos.api.v1.AuthService/SignIn": {},
@@ -67,14 +66,13 @@ var AuthBootstrapMethods = map[string]struct{}{
// Identity Provider Service - SSO buttons on the sign-in page.
"/memos.api.v1.IdentityProviderService/ListIdentityProviders": {},
// User Service - CreateUser applies registration and password-auth settings.
"/memos.api.v1.UserService/CreateUser": {},
// Memo sharing - share-token access stays public even on a private instance.
"/memos.api.v1.MemoService/GetMemoByShare": {},
}
// createUserProcedure is the CreateUser endpoint. On a private instance it is
// served to anonymous callers only while no user exists yet (initial admin setup).
const createUserProcedure = "/memos.api.v1.UserService/CreateUser"
// IsAuthBootstrapMethod reports whether an anonymous request to procedure is one
// of the fixed endpoints allowed while the instance is private.
func IsAuthBootstrapMethod(procedure string) bool {
+2 -3
View File
@@ -103,10 +103,11 @@ func TestAuthBootstrapMethodsAreSubsetOfPublic(t *testing.T) {
// TestAuthBootstrapClassification verifies which endpoints remain reachable by
// anonymous callers on a private instance (no InstanceURL configured).
func TestAuthBootstrapClassification(t *testing.T) {
// Reachable while private: sign-in flow, instance metadata, SSO, share links.
// Reachable while private: sign-in flow, registration, instance metadata, SSO, share links.
bootstrap := []string{
"/memos.api.v1.AuthService/SignIn",
"/memos.api.v1.AuthService/RefreshToken",
"/memos.api.v1.UserService/CreateUser",
"/memos.api.v1.InstanceService/GetInstanceProfile",
"/memos.api.v1.InstanceService/GetInstanceSetting",
"/memos.api.v1.InstanceService/BatchGetInstanceSettings",
@@ -120,14 +121,12 @@ func TestAuthBootstrapClassification(t *testing.T) {
}
// Public on an open instance, but gated on a private one: browsing and profiles.
// CreateUser is gated here too; it is allowed separately only during first-run setup.
gatedWhilePrivate := []string{
"/memos.api.v1.MemoService/ListMemos",
"/memos.api.v1.MemoService/GetMemo",
"/memos.api.v1.MemoService/ListMemoComments",
"/memos.api.v1.UserService/GetUser",
"/memos.api.v1.UserService/ListAllUserStats",
"/memos.api.v1.UserService/CreateUser",
}
for _, method := range gatedWhilePrivate {
t.Run("gated/"+method, func(t *testing.T) {
+1 -1
View File
@@ -112,7 +112,7 @@ func (s *APIV1Service) SignIn(ctx context.Context, request *v1pb.SignInRequest)
//
// Lookup goes through the user_identity table so that userInfo.Identifier is never used
// as the local username key. On the miss path, a local user is created with a
// UUID-based local username (see deriveSSOUsername) and the (provider, extern_uid)
// UUID-backed local username (see deriveSSOUsername) and the (provider, extern_uid)
// linkage is inserted in the same flow. When currentUser is provided by a caller
// outside AuthService.SignIn, the lookup miss path binds the external identity to
// that existing user instead. If the linkage insert loses a race on the unique
+4 -21
View File
@@ -25,7 +25,6 @@ var ErrUnauthenticated = errors.New("authentication required")
// governs only authentication and anonymous access.
type Authorizer struct {
authenticator *auth.Authenticator
store *store.Store
profile *profile.Profile
}
@@ -34,7 +33,6 @@ type Authorizer struct {
func NewAuthorizer(store *store.Store, secret string, profile *profile.Profile) *Authorizer {
return &Authorizer{
authenticator: auth.NewAuthenticator(store, secret),
store: store,
profile: profile,
}
}
@@ -54,7 +52,7 @@ func (a *Authorizer) Authenticate(ctx context.Context, authHeader string) *auth.
// - Anonymous + protected method: denied.
// - Anonymous + public method, open instance: permitted.
// - Anonymous + public method, private instance (no InstanceURL): permitted only
// for the auth-bootstrap set, plus CreateUser during first-run setup.
// for the auth-bootstrap set.
func (a *Authorizer) CheckAccess(ctx context.Context, procedure string, result *auth.AuthResult) error {
if result != nil {
return nil
@@ -69,22 +67,7 @@ func (a *Authorizer) CheckAccess(ctx context.Context, procedure string, result *
}
// allowedOnPrivateInstance reports whether an anonymous request to a public
// procedure is still permitted while the instance is private. It allows the
// auth-bootstrap set, plus CreateUser while the instance has no users yet
// (first-run admin setup).
func (a *Authorizer) allowedOnPrivateInstance(ctx context.Context, procedure string) bool {
if IsAuthBootstrapMethod(procedure) {
return true
}
if procedure == createUserProcedure {
return a.noUsersExist(ctx)
}
return false
}
// noUsersExist reports whether the instance has no users yet (fresh install).
func (a *Authorizer) noUsersExist(ctx context.Context) bool {
limitOne := 1
users, err := a.store.ListUsers(ctx, &store.FindUser{Limit: &limitOne})
return err == nil && len(users) == 0
// procedure is still permitted while the instance is private.
func (*Authorizer) allowedOnPrivateInstance(_ context.Context, procedure string) bool {
return IsAuthBootstrapMethod(procedure)
}
+3 -2
View File
@@ -12,8 +12,7 @@ import (
// TestAuthorizerCheckAccess exercises the method-level access policy matrix.
//
// The store-backed first-run CreateUser branch is covered by integration tests;
// every case here is decided without touching the store, so a nil store is safe.
// Every case here is decided without touching the store, so a nil store is safe.
func TestAuthorizerCheckAccess(t *testing.T) {
ctx := context.Background()
authenticated := &auth.AuthResult{AccessToken: "token"}
@@ -25,6 +24,7 @@ func TestAuthorizerCheckAccess(t *testing.T) {
protectedMethod = "/memos.api.v1.MemoService/CreateMemo"
publicMethod = "/memos.api.v1.MemoService/ListMemos"
bootstrapMethod = "/memos.api.v1.AuthService/SignIn"
createUser = "/memos.api.v1.UserService/CreateUser"
shareMethod = "/memos.api.v1.MemoService/GetMemoByShare"
)
@@ -41,6 +41,7 @@ func TestAuthorizerCheckAccess(t *testing.T) {
{"anonymous allowed on public method, open instance", openInstance, publicMethod, nil, false},
{"anonymous denied on public method, private instance", privateInstance, publicMethod, nil, true},
{"anonymous allowed on bootstrap method, private instance", privateInstance, bootstrapMethod, nil, false},
{"anonymous allowed to register on private instance", privateInstance, createUser, nil, false},
{"anonymous allowed on share access, private instance", privateInstance, shareMethod, nil, false},
}
for _, c := range cases {
+3 -3
View File
@@ -136,14 +136,14 @@ func ExtractIdentityProviderUIDFromName(name string) (string, error) {
// ValidateAndGenerateUID validates a user-provided UID or generates a new one.
// If provided is empty, a new shortuuid is generated.
// If provided is non-empty, it is validated against base.UIDMatcher.
// If provided is non-empty, it is validated as a user-provided resource ID.
func ValidateAndGenerateUID(provided string) (string, error) {
uid := strings.TrimSpace(provided)
if uid == "" {
return shortuuid.New(), nil
}
if !base.UIDMatcher.MatchString(uid) {
return "", status.Errorf(codes.InvalidArgument, "invalid ID format: must be 1-36 characters, alphanumeric and hyphens only, cannot start or end with hyphen")
if !base.ResourceIDMatcher.MatchString(uid) {
return "", status.Errorf(codes.InvalidArgument, "invalid resource ID: must be 1-63 characters, start with a lowercase letter, contain only lowercase letters, digits, or hyphens, and end with a letter or digit")
}
return uid, nil
}
@@ -0,0 +1,38 @@
package v1
import (
"strings"
"testing"
)
func TestValidateAndGenerateUIDValidatesUserProvidedResourceIDs(t *testing.T) {
tests := []struct {
name string
provided string
wantError bool
}{
{name: "lowercase", provided: "memo-1"},
{name: "maximum length", provided: "a" + strings.Repeat("b", 62)},
{name: "digit first", provided: "1-memo", wantError: true},
{name: "uppercase", provided: "Memo", wantError: true},
{name: "too long", provided: "a" + strings.Repeat("b", 63), wantError: true},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
uid, err := ValidateAndGenerateUID(test.provided)
if test.wantError {
if err == nil {
t.Fatalf("ValidateAndGenerateUID(%q) succeeded, want error", test.provided)
}
return
}
if err != nil {
t.Fatalf("ValidateAndGenerateUID(%q) returned error: %v", test.provided, err)
}
if uid != test.provided {
t.Fatalf("ValidateAndGenerateUID(%q) = %q", test.provided, uid)
}
})
}
}
+4 -5
View File
@@ -8,13 +8,12 @@ import (
// deriveSSOUsername produces the local username for a new SSO-created user.
//
// The current policy is to use a standard UUID string directly. This keeps the
// username independent of IdP profile fields and avoids availability probes or
// retry loops around concurrent first-time logins.
// The current policy prefixes a UUID with a letter so the generated value
// follows the same AIP-compatible format as user-selected usernames.
func deriveSSOUsername() (string, error) {
username := util.GenUUID()
username := "user-" + util.GenUUID()
if err := validateWritableUsername(username); err != nil {
return "", errors.Wrap(err, "generated UUID did not satisfy username constraints")
return "", errors.Wrap(err, "generated username did not satisfy username constraints")
}
return username, nil
}
+19
View File
@@ -0,0 +1,19 @@
package v1
import (
"strings"
"testing"
)
func TestDeriveSSOUsername(t *testing.T) {
username, err := deriveSSOUsername()
if err != nil {
t.Fatalf("deriveSSOUsername() returned error: %v", err)
}
if !strings.HasPrefix(username, "user-") {
t.Fatalf("deriveSSOUsername() = %q, want user- prefix", username)
}
if err := validateWritableUsername(username); err != nil {
t.Fatalf("deriveSSOUsername() produced invalid username %q: %v", username, err)
}
}
+9 -9
View File
@@ -14,10 +14,10 @@ import (
apiv1 "github.com/usememos/memos/server/router/api/v1"
)
// TestAuthorizerPrivateInstanceFirstRun verifies the private-instance access policy
// against a real store: anonymous CreateUser is permitted only until the first user
// exists, bootstrap methods stay open, and other public methods are gated.
func TestAuthorizerPrivateInstanceFirstRun(t *testing.T) {
// TestAuthorizerPrivateInstanceRegistration verifies that registration and other
// bootstrap methods stay reachable anonymously while unrelated public methods are
// gated. CreateUser enforces the instance registration settings in the service.
func TestAuthorizerPrivateInstanceRegistration(t *testing.T) {
ctx := context.Background()
ts := NewTestService(t)
defer ts.Cleanup()
@@ -35,17 +35,17 @@ func TestAuthorizerPrivateInstanceFirstRun(t *testing.T) {
// Anonymous request with no Authorization header resolves to no identity.
require.Nil(t, authorizer.Authenticate(ctx, ""))
// Fresh instance (no users): first-run CreateUser is allowed, and so are the
// bootstrap methods; browsing is still gated.
require.NoError(t, authorizer.CheckAccess(ctx, createUser, nil), "first-run CreateUser should be allowed")
// Registration and other bootstrap methods are allowed; browsing is still gated.
require.NoError(t, authorizer.CheckAccess(ctx, createUser, nil))
require.NoError(t, authorizer.CheckAccess(ctx, signIn, nil))
require.NoError(t, authorizer.CheckAccess(ctx, getMemoShare, nil))
require.ErrorIs(t, authorizer.CheckAccess(ctx, listMemos, nil), apiv1.ErrUnauthenticated)
// Once a user exists, anonymous CreateUser is denied while bootstrap stays open.
// Once a user exists, CreateUser remains reachable so UserService can enforce
// disallow_user_registration and disallow_password_auth.
_, err := ts.CreateHostUser(ctx, "host")
require.NoError(t, err)
require.ErrorIs(t, authorizer.CheckAccess(ctx, createUser, nil), apiv1.ErrUnauthenticated, "CreateUser must be denied once a user exists")
require.NoError(t, authorizer.CheckAccess(ctx, createUser, nil))
require.NoError(t, authorizer.CheckAccess(ctx, signIn, nil))
}
@@ -256,6 +256,22 @@ func TestCreateUserRegistration(t *testing.T) {
require.Contains(t, err.Error(), "invalid username")
})
t.Run("CreateUser requires user_id to match username", func(t *testing.T) {
ts := NewTestService(t)
defer ts.Cleanup()
_, err := ts.Service.CreateUser(ctx, &apiv1.CreateUserRequest{
User: &apiv1.User{
Username: "alice",
Email: "alice@example.com",
Password: "password123",
},
UserId: "bob",
})
require.Error(t, err)
require.Contains(t, err.Error(), "user_id must match user.username")
})
t.Run("UpdateUser rejects empty password", func(t *testing.T) {
ts := NewTestService(t)
defer ts.Cleanup()
@@ -159,6 +159,29 @@ func TestGetUserStats_MemoUpdatedTimestamps(t *testing.T) {
)
}
func TestGetUserStats_PinnedMemoUsesCanonicalResourceName(t *testing.T) {
ctx := context.Background()
ts := NewTestService(t)
defer ts.Cleanup()
user, err := ts.CreateHostUser(ctx, "pinned-stats-user")
require.NoError(t, err)
userCtx := ts.CreateUserContext(ctx, user.ID)
memo, err := ts.Store.CreateMemo(ctx, &store.Memo{
UID: "pinned-stats-memo",
CreatorID: user.ID,
Content: "pinned",
Visibility: store.Public,
})
require.NoError(t, err)
pinned := true
require.NoError(t, ts.Store.UpdateMemo(ctx, &store.UpdateMemo{ID: memo.ID, Pinned: &pinned}))
resp, err := ts.Service.GetUserStats(userCtx, &v1pb.GetUserStatsRequest{Name: fmt.Sprintf("users/%s", user.Username)})
require.NoError(t, err)
require.Equal(t, []string{"memos/pinned-stats-memo"}, resp.PinnedMemos)
}
func TestListAllUserStats_FilterExcludesPrivateMemos(t *testing.T) {
ctx := context.Background()
+2 -14
View File
@@ -27,24 +27,12 @@ func parseUsernameFromName(name string) (string, error) {
}
func validateWritableUsername(username string) error {
if username == "" || isNumericUsername(username) || !base.UIDMatcher.MatchString(username) {
return errors.Errorf("invalid username %q", username)
if !base.ResourceIDMatcher.MatchString(username) {
return errors.New("invalid username: must be 1-63 characters, start with a lowercase letter, contain only lowercase letters, digits, or hyphens, and end with a letter or digit")
}
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 := parseUsernameFromName(name)
@@ -1,6 +1,7 @@
package v1
import (
"strings"
"testing"
)
@@ -15,16 +16,36 @@ func TestValidateWritableUsername(t *testing.T) {
username: "alice",
},
{
name: "mixed case",
username: "Alice",
name: "mixed case",
username: "Alice",
wantError: true,
},
{
name: "hyphenated",
username: "alice-smith",
},
{
name: "uuid",
username: "550e8400-e29b-41d4-a716-446655440000",
name: "one character",
username: "a",
},
{
name: "maximum length",
username: "a" + strings.Repeat("b", 62),
},
{
name: "too long",
username: "a" + strings.Repeat("b", 63),
wantError: true,
},
{
name: "digit first",
username: "1alice",
wantError: true,
},
{
name: "hyphen last",
username: "alice-",
wantError: true,
},
{
name: "empty",
+5 -2
View File
@@ -187,8 +187,11 @@ func (s *APIV1Service) CreateUser(ctx context.Context, request *v1pb.CreateUserR
if request.User == nil {
return nil, status.Errorf(codes.InvalidArgument, "user is required")
}
if request.UserId != "" && request.UserId != request.User.Username {
return nil, status.Errorf(codes.InvalidArgument, "user_id must match user.username")
}
if err := validateWritableUsername(request.User.Username); err != nil {
return nil, status.Errorf(codes.InvalidArgument, "invalid username: %s", request.User.Username)
return nil, status.Errorf(codes.InvalidArgument, "%v", err)
}
if err := validatePassword(request.User.Password); err != nil {
return nil, status.Errorf(codes.InvalidArgument, "%v", err)
@@ -322,7 +325,7 @@ func (s *APIV1Service) UpdateUser(ctx context.Context, request *v1pb.UpdateUserR
return nil, status.Errorf(codes.PermissionDenied, "permission denied: disallow change username")
}
if err := validateWritableUsername(request.User.Username); err != nil {
return nil, status.Errorf(codes.InvalidArgument, "invalid username: %s", request.User.Username)
return nil, status.Errorf(codes.InvalidArgument, "%v", err)
}
update.Username = &request.User.Username
case "display_name":
+5 -5
View File
@@ -92,7 +92,7 @@ func (s *APIV1Service) ListAllUserStats(ctx context.Context, request *v1pb.ListA
}
userMemoStatMap := make(map[int32]*v1pb.UserStats)
pinnedMemoIDsByUserID := make(map[int32][]int32)
pinnedMemoUIDsByUserID := make(map[int32][]string)
limit := 1000
offset := 0
memoFind.Limit = &limit
@@ -156,7 +156,7 @@ func (s *APIV1Service) ListAllUserStats(ctx context.Context, request *v1pb.ListA
// Track pinned memos
if memo.Pinned {
pinnedMemoIDsByUserID[memo.CreatorID] = append(pinnedMemoIDsByUserID[memo.CreatorID], memo.ID)
pinnedMemoUIDsByUserID[memo.CreatorID] = append(pinnedMemoUIDsByUserID[memo.CreatorID], memo.UID)
}
}
@@ -178,8 +178,8 @@ func (s *APIV1Service) ListAllUserStats(ctx context.Context, request *v1pb.ListA
return nil, status.Errorf(codes.Internal, "failed to resolve user stats name")
}
userMemoStat.Name = fmt.Sprintf("%s/stats", BuildUserName(username))
for _, memoID := range pinnedMemoIDsByUserID[userID] {
userMemoStat.PinnedMemos = append(userMemoStat.PinnedMemos, fmt.Sprintf("%s/memos/%d", BuildUserName(username), memoID))
for _, memoUID := range pinnedMemoUIDsByUserID[userID] {
userMemoStat.PinnedMemos = append(userMemoStat.PinnedMemos, MemoNamePrefix+memoUID)
}
userMemoStats = append(userMemoStats, userMemoStat)
}
@@ -270,7 +270,7 @@ func (s *APIV1Service) GetUserStats(ctx context.Context, request *v1pb.GetUserSt
}
}
if memo.Pinned {
pinnedMemos = append(pinnedMemos, fmt.Sprintf("%s/memos/%d", BuildUserName(user.Username), memo.ID))
pinnedMemos = append(pinnedMemos, MemoNamePrefix+memo.UID)
}
}