enhance: add user ID based authorization

This change also changes the UI API calls to to always use user IDs
instead of usernames.

Signed-off-by: Donnie Adams <[email protected]>

Author: thedadams

pkg/api/authz/authz.go

@@ -67,7 +67,6 @@ var staticRules = map[string][]string{
 		"/api/assistants",
 		"GET /api/me",
 		"DELETE /api/me",
-		"PATCH /api/users/{id}",
 		"POST /api/llm-proxy/",
 		"POST /api/prompt",
 		"GET /api/models",

pkg/api/authz/resources.go

@@ -148,6 +148,11 @@ var apiResources = []string{
 	"GET    /api/tool-references",
 	"GET    /api/tool-references/{id}",
 	"GET    /{ui}/projects/{id}",
+	"GET    /api/users/{user_id}",
+	"PATCH  /api/users/{user_id}",
+	"GET    /api/users/{user_id}/activities",
+	"GET    /api/users/{user_id}/usage",
+	"GET    /api/users/{user_id}/total-usage",
 }
 
 type Resources struct {
@@ -191,6 +196,10 @@ func (a *Authorizer) evaluateResources(req *http.Request, vars GetVar, user user
 		ToolID:                 vars("tool_id"),
 	}
 
+	if !a.checkUser(user, vars("user_id")) {
+		return false, nil
+	}
+
 	if ok, err := a.checkAssistant(req, &resources, user); !ok || err != nil {
 		return false, err
 	}

pkg/api/authz/user.go

@@ -0,0 +1,13 @@
+package authz
+
+import (
+	"slices"
+
+	"k8s.io/apiserver/pkg/authentication/user"
+)
+
+func (a *Authorizer) checkUser(user user.Info, userID string) bool {
+	return userID == "" ||
+		userID == user.GetUID() ||
+		slices.Contains(user.GetGroups(), AdminGroup)
+}

pkg/gateway/client/user.go

@@ -80,10 +80,10 @@ func (c *Client) UserByID(ctx context.Context, id string) (*types.User, error) {
 	return u, c.decryptUser(ctx, u)
 }
 
-func (c *Client) DeleteUser(ctx context.Context, storageClient kclient.Client, username string) (*types.User, error) {
+func (c *Client) DeleteUser(ctx context.Context, storageClient kclient.Client, userID string) (*types.User, error) {
 	existingUser := new(types.User)
 	if err := c.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
-		if err := tx.Where("hashed_username = ?", hash.String(username)).First(existingUser).Error; err != nil {
+		if err := tx.Where("id = ?", userID).First(existingUser).Error; err != nil {
 			return err
 		}
 
@@ -127,10 +127,10 @@ func (c *Client) DeleteUser(ctx context.Context, storageClient kclient.Client, u
 	return existingUser, c.decryptUser(ctx, existingUser)
 }
 
-func (c *Client) UpdateUser(ctx context.Context, actingUserIsAdmin bool, updatedUser *types.User, username string) (*types.User, error) {
+func (c *Client) UpdateUser(ctx context.Context, actingUserIsAdmin bool, updatedUser *types.User, userID string) (*types.User, error) {
 	existingUser := new(types.User)
 	return existingUser, c.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
-		if err := tx.Where("hashed_username = ?", hash.String(username)).First(existingUser).Error; err != nil {
+		if err := tx.Where("id = ?", userID).First(existingUser).Error; err != nil {
 			return err
 		}
 
@@ -186,8 +186,8 @@ func (c *Client) UpdateUser(ctx context.Context, actingUserIsAdmin bool, updated
 	})
 }
 
-func (c *Client) UpdateUserInternalStatus(ctx context.Context, username string, internal bool) error {
-	return c.db.WithContext(ctx).Model(new(types.User)).Where("hashed_username = ?", hash.String(username)).Update("internal", internal).Error
+func (c *Client) UpdateUserInternalStatus(ctx context.Context, userID string, internal bool) error {
+	return c.db.WithContext(ctx).Model(new(types.User)).Where("id = ?", userID).Update("internal", internal).Error
 }
 
 func (c *Client) UpdateProfileIconIfNeeded(ctx context.Context, user *types.User, authProviderName, authProviderNamespace, authProviderURL string) error {

pkg/gateway/server/router.go

@@ -29,14 +29,14 @@ func (s *Server) AddRoutes(mux *server.Server) {
 	mux.HandleFunc("POST /api/logout-all", wrap(s.logoutAll))
 	mux.HandleFunc("GET /api/users", wrap(s.getUsers))
 	mux.HandleFunc("POST /api/encrypt-all-users", wrap(s.encryptAllUsersAndIdentities))
-	mux.HandleFunc("GET /api/users/{username_or_id}", wrap(s.getUser))
+	mux.HandleFunc("GET /api/users/{user_id}", wrap(s.getUser))
 	mux.HandleFunc("GET /api/users/{user_id}/activities", wrap(s.activitiesByUser))
 	mux.HandleFunc("GET /api/users/{user_id}/usage", wrap(s.usageForUser))
 	mux.HandleFunc("GET /api/users/{user_id}/total-usage", wrap(s.totalUsageForUser))
-	mux.HandleFunc("PATCH /api/users/{username}", wrap(s.updateUser))
-	mux.HandleFunc("POST /api/users/{username}/internal", wrap(s.markUserInternal))
-	mux.HandleFunc("POST /api/users/{username}/external", wrap(s.markUserExternal))
-	mux.HandleFunc("DELETE /api/users/{username}", wrap(s.deleteUser))
+	mux.HandleFunc("PATCH /api/users/{user_id}", wrap(s.updateUser))
+	mux.HandleFunc("POST /api/users/{user_id}/internal", wrap(s.markUserInternal))
+	mux.HandleFunc("POST /api/users/{user_id}/external", wrap(s.markUserExternal))
+	mux.HandleFunc("DELETE /api/users/{user_id}", wrap(s.deleteUser))
 	mux.HandleFunc("GET /api/active-users", wrap(s.activeUsers))
 
 	mux.HandleFunc("GET /api/token-usage", wrap(s.systemTokenUsageByUser))

pkg/gateway/server/user.go

@@ -83,26 +83,16 @@ func (s *Server) encryptAllUsersAndIdentities(apiContext api.Context) error {
 }
 
 func (s *Server) getUser(apiContext api.Context) error {
-	var (
-		getByID      = apiContext.URL.Query().Get("by-id") == "true"
-		usernameOrID = apiContext.PathValue("username_or_id")
-		user         *types.User
-	)
+	userID := apiContext.PathValue("user_id")
 
-	if usernameOrID == "" {
-		return types2.NewErrHTTP(http.StatusBadRequest, "username path parameter is required")
-	}
-
-	var err error
-	if getByID {
-		user, err = apiContext.GatewayClient.UserByID(apiContext.Context(), usernameOrID)
-	} else {
-		user, err = apiContext.GatewayClient.User(apiContext.Context(), usernameOrID)
+	if userID == "" {
+		return types2.NewErrHTTP(http.StatusBadRequest, "user_id path parameter is required")
 	}
 
+	user, err := apiContext.GatewayClient.UserByID(apiContext.Context(), userID)
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
-			return types2.NewErrNotFound("user %s not found", usernameOrID)
+			return types2.NewErrNotFound("user %s not found", userID)
 		}
 		return fmt.Errorf("failed to get user: %v", err)
 	}
@@ -111,16 +101,9 @@ func (s *Server) getUser(apiContext api.Context) error {
 }
 
 func (s *Server) updateUser(apiContext api.Context) error {
-	requestingUsername := apiContext.User.GetName()
-	actingUserIsAdmin := apiContext.UserIsAdmin()
-
-	username := apiContext.PathValue("username")
-	if username == "" {
-		return types2.NewErrHTTP(http.StatusBadRequest, "username path parameter is required")
-	}
-
-	if !actingUserIsAdmin && requestingUsername != username {
-		return types2.NewErrHTTP(http.StatusForbidden, "only admins can update other users")
+	userID := apiContext.PathValue("user_id")
+	if userID == "" {
+		return types2.NewErrHTTP(http.StatusBadRequest, "user_id path parameter is required")
 	}
 
 	user := new(types.User)
@@ -135,7 +118,7 @@ func (s *Server) updateUser(apiContext api.Context) error {
 	}
 
 	status := http.StatusInternalServerError
-	existingUser, err := apiContext.GatewayClient.UpdateUser(apiContext.Context(), actingUserIsAdmin, user, username)
+	existingUser, err := apiContext.GatewayClient.UpdateUser(apiContext.Context(), apiContext.UserIsAdmin(), user, userID)
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
 			status = http.StatusNotFound
@@ -161,14 +144,14 @@ func (s *Server) markUserExternal(apiContext api.Context) error {
 }
 
 func (s *Server) changeUserInternalStatus(apiContext api.Context, internal bool) error {
-	username := apiContext.PathValue("username")
-	if username == "" {
-		return types2.NewErrHTTP(http.StatusBadRequest, "username path parameter is required")
+	userID := apiContext.PathValue("user_id")
+	if userID == "" {
+		return types2.NewErrHTTP(http.StatusBadRequest, "user_id path parameter is required")
 	}
 
-	if err := apiContext.GatewayClient.UpdateUserInternalStatus(apiContext.Context(), username, internal); err != nil {
+	if err := apiContext.GatewayClient.UpdateUserInternalStatus(apiContext.Context(), userID, internal); err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
-			return types2.NewErrNotFound("user %s not found", username)
+			return types2.NewErrNotFound("user %s not found", userID)
 		}
 		return types2.NewErrHTTP(http.StatusInternalServerError, fmt.Sprintf("failed to update user: %v", err))
 	}
@@ -177,10 +160,10 @@ func (s *Server) changeUserInternalStatus(apiContext api.Context, internal bool)
 }
 
 func (s *Server) deleteUser(apiContext api.Context) (err error) {
-	username := apiContext.PathValue("username")
-	if username == "" {
+	userID := apiContext.PathValue("user_id")
+	if userID == "" {
 		// This is the "delete me" API
-		username = apiContext.User.GetName()
+		userID = apiContext.User.GetUID()
 		defer func() {
 			if err == nil {
 				// If everything was successful, remove the cookie so the user isn't authenticated again.
@@ -189,16 +172,16 @@ func (s *Server) deleteUser(apiContext api.Context) (err error) {
 		}()
 	}
 
-	existingUser, err := apiContext.GatewayClient.User(apiContext.Context(), username)
+	existingUser, err := apiContext.GatewayClient.UserByID(apiContext.Context(), userID)
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
-			return types2.NewErrNotFound("user %s not found", username)
+			return types2.NewErrNotFound("user %s not found", userID)
 		}
 		return fmt.Errorf("failed to get user: %v", err)
 	}
 
 	status := http.StatusInternalServerError
-	_, err = apiContext.GatewayClient.DeleteUser(apiContext.Context(), apiContext.Storage, username)
+	_, err = apiContext.GatewayClient.DeleteUser(apiContext.Context(), apiContext.Storage, userID)
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
 			status = http.StatusNotFound

ui/admin/app/components/thread/ThreadMeta.tsx

@@ -107,7 +107,7 @@ export function ThreadMeta({
 	} = useThreadTasks(isAgent ? thread.id : undefined);
 
 	const { data: user } = useSWR(
-		...UserService.getUser.swr({ username: thread.userID })
+		...UserService.getUser.swr({ userId: thread.userID })
 	);
 
 	const { dialogProps, interceptAsync } = useConfirmationDialog();

ui/admin/app/components/user/UserActionsDropdown.tsx

@@ -94,7 +94,7 @@ export function UserActionsDropdown({ user }: { user: User }) {
 						<Button
 							type="button"
 							variant="destructive"
-							onClick={() => deleteUser.execute(user.username)}
+							onClick={() => deleteUser.execute(user.id)}
 						>
 							Delete
 						</Button>

ui/admin/app/components/user/UserUpdateForm.tsx

@@ -91,7 +91,7 @@ export function UserUpdateForm({
 		) {
 			setBootstrapDialogOpen(true);
 		} else {
-			updateUser.execute(user.username, data);
+			updateUser.execute(user.id, data);
 		}
 	});
 
@@ -176,9 +176,7 @@ export function UserUpdateForm({
 						<Button
 							loading={updateUser.isLoading}
 							disabled={updateUser.isLoading}
-							onClick={() =>
-								updateUser.execute(user.username, form.getValues())
-							}
+							onClick={() => updateUser.execute(user.id, form.getValues())}
 						>
 							Confirm
 						</Button>

ui/admin/app/lib/routers/apiRoutes.ts

@@ -228,10 +228,9 @@ export const ApiRoutes = {
 	},
 	users: {
 		base: () => buildUrl("/users"),
-		updateUser: (username: string) => buildUrl(`/users/${username}`),
-		deleteUser: (username: string) => buildUrl(`/users/${username}`),
-		getOne: (username: string) =>
-			buildUrl(`/users/${username}`, { "by-id": true }),
+		updateUser: (userId: string) => buildUrl(`/users/${userId}`),
+		deleteUser: (userId: string) => buildUrl(`/users/${userId}`),
+		getOne: (userId: string) => buildUrl(`/users/${userId}`),
 	},
 	workspace: {
 		getTables: (namespace: TableNamespace, entityId: string) =>

ui/admin/app/lib/service/api/userService.ts

@@ -45,13 +45,13 @@ async function updateUser(username: string, user: Partial<User>) {
 }
 
 const handleGetUser = createFetcher(
-	z.object({ username: z.string() }),
-	async ({ username }, { signal }) => {
-		const { url } = ApiRoutes.users.getOne(username);
+	z.object({ userId: z.string() }),
+	async ({ userId }, { signal }) => {
+		const { url } = ApiRoutes.users.getOne(userId);
 		const { data } = await request<User>({ url, signal });
 		return data;
 	},
-	() => ApiRoutes.users.getOne(":username").path
+	() => ApiRoutes.users.getOne(":userId").path
 );
 
 async function deleteUser(username: string) {