fix: correctly handle user id passthrough on headscale actions

This commit is contained in:
Aarnav Tale
2026-05-30 17:14:28 -04:00
parent 7a62359b6a
commit 7901f37002
10 changed files with 76 additions and 143 deletions
+3 -3
View File
@@ -54,24 +54,24 @@ export default function UserMenu({
{modal === "reassign" && (
<Reassign
displayName={displayName}
headplaneUserId={user.id}
isOpen={modal === "reassign"}
role={user.role}
setIsOpen={(isOpen) => {
if (!isOpen) setModal(null);
}}
userId={user.linkedHeadscaleUser?.id ?? user.id}
/>
)}
{modal === "link" && (
<LinkUser
currentLink={currentLink}
displayName={displayName}
headplaneUserId={user.id}
headscaleUsers={linkableUsers}
isOpen={modal === "link"}
setIsOpen={(isOpen) => {
if (!isOpen) setModal(null);
}}
userId={user.linkedHeadscaleUser?.id ?? user.id}
/>
)}
{modal === "transfer" && (
@@ -81,7 +81,7 @@ export default function UserMenu({
if (!isOpen) setModal(null);
}}
targetDisplayName={displayName}
targetUserId={user.linkedHeadscaleUser?.id ?? user.id}
targetHeadplaneUserId={user.id}
/>
)}
+1 -1
View File
@@ -34,7 +34,7 @@ export default function DeleteUser({ user, machines, isOpen, setIsOpen }: Delete
</Text>
)}
<input name="action_id" type="hidden" value="delete_user" />
<input name="user_id" type="hidden" value={user.id} />
<input name="headscale_user_id" type="hidden" value={user.id} />
</DialogPanel>
</Dialog>
);
+3 -3
View File
@@ -5,7 +5,7 @@ import Title from "~/components/title";
import cn from "~/utils/cn";
interface LinkUserProps {
userId: string;
headplaneUserId: string;
displayName: string;
headscaleUsers: { id: string; name: string }[];
currentLink?: string;
@@ -14,7 +14,7 @@ interface LinkUserProps {
}
export default function LinkUser({
userId,
headplaneUserId,
displayName,
headscaleUsers,
currentLink,
@@ -34,7 +34,7 @@ export default function LinkUser({
) : (
<>
<input name="action_id" type="hidden" value="link_user" />
<input name="user_id" type="hidden" value={userId} />
<input name="headplane_user_id" type="hidden" value={headplaneUserId} />
<select
className={cn(
"w-full rounded-lg border p-2",
+3 -3
View File
@@ -8,7 +8,7 @@ import { Roles } from "~/server/web/roles";
import type { Role } from "~/server/web/roles";
interface ReassignProps {
userId: string;
headplaneUserId: string;
displayName: string;
role: Role;
isOpen: boolean;
@@ -16,7 +16,7 @@ interface ReassignProps {
}
export default function ReassignUser({
userId,
headplaneUserId,
displayName,
role,
isOpen,
@@ -38,7 +38,7 @@ export default function ReassignUser({
) : (
<>
<input name="action_id" type="hidden" value="reassign_user" />
<input name="user_id" type="hidden" value={userId} />
<input name="headplane_user_id" type="hidden" value={headplaneUserId} />
<RadioGroup className="gap-4" defaultValue={role} label="Role" name="new_role">
{Object.keys(Roles)
.filter((r) => r !== "owner")
+1 -1
View File
@@ -21,7 +21,7 @@ export default function RenameUser({ user, isOpen, setIsOpen }: RenameProps) {
update any ACL policies that may refer to this user by their old username.
</Text>
<input name="action_id" type="hidden" value="rename_user" />
<input name="user_id" type="hidden" value={user.id} />
<input name="headscale_user_id" type="hidden" value={user.id} />
<Input
defaultValue={user.name}
required
@@ -4,14 +4,14 @@ import Text from "~/components/text";
import Title from "~/components/title";
interface TransferOwnershipProps {
targetUserId: string;
targetHeadplaneUserId: string;
targetDisplayName: string;
isOpen: boolean;
setIsOpen: (isOpen: boolean) => void;
}
export default function TransferOwnership({
targetUserId,
targetHeadplaneUserId,
targetDisplayName,
isOpen,
setIsOpen,
@@ -29,7 +29,7 @@ export default function TransferOwnership({
ownership.
</Notice>
<input name="action_id" type="hidden" value="transfer_ownership" />
<input name="user_id" type="hidden" value={targetUserId} />
<input name="headplane_user_id" type="hidden" value={targetHeadplaneUserId} />
</DialogPanel>
</Dialog>
);
+22 -59
View File
@@ -1,7 +1,6 @@
import { data } from "react-router";
import { usersResource } from "~/server/headscale/live-store";
import { getOidcSubject } from "~/server/web/headscale-identity";
import { Capabilities } from "~/server/web/roles";
import type { Role } from "~/server/web/roles";
@@ -42,28 +41,28 @@ export async function userAction({ request, context }: Route.ActionArgs) {
return { message: "User created successfully" };
}
case "delete_user": {
const userId = formData.get("user_id")?.toString();
if (!userId) {
throw data("Missing `user_id` in the form data.", {
const headscaleUserId = formData.get("headscale_user_id")?.toString();
if (!headscaleUserId) {
throw data("Missing `headscale_user_id` in the form data.", {
status: 400,
});
}
await api.users.delete(userId);
await api.users.delete(headscaleUserId);
await context.hsLive.refresh(usersResource, api);
return { message: "User deleted successfully" };
}
case "rename_user": {
const userId = formData.get("user_id")?.toString();
const headscaleUserId = formData.get("headscale_user_id")?.toString();
const newName = formData.get("new_name")?.toString();
if (!userId || !newName) {
if (!headscaleUserId || !newName) {
return data({ success: false }, 400);
}
const users = await api.users.list({ id: userId });
const user = users.find((user) => user.id === userId);
const users = await api.users.list({ id: headscaleUserId });
const user = users.find((user) => user.id === headscaleUserId);
if (!user) {
throw data(`No user found with id: ${userId}`, { status: 400 });
throw data(`No user found with id: ${headscaleUserId}`, { status: 400 });
}
if (user.provider === "oidc") {
@@ -73,34 +72,20 @@ export async function userAction({ request, context }: Route.ActionArgs) {
});
}
await api.users.rename(userId, newName);
await api.users.rename(headscaleUserId, newName);
await context.hsLive.refresh(usersResource, api);
return { message: "User renamed successfully" };
}
case "reassign_user": {
const userId = formData.get("user_id")?.toString();
const headplaneUserId = formData.get("headplane_user_id")?.toString();
const newRole = formData.get("new_role")?.toString();
if (!userId || !newRole) {
throw data("Missing `user_id` or `new_role` in the form data.", {
if (!headplaneUserId || !newRole) {
throw data("Missing `headplane_user_id` or `new_role` in the form data.", {
status: 400,
});
}
const users = await api.users.list({ id: userId });
const user = users.find((user) => user.id === userId);
if (!user) {
throw data("Specified user not found", {
status: 400,
});
}
const subject = getOidcSubject(user);
if (!subject) {
throw data("Specified user is not an OIDC user or has no subject.", { status: 400 });
}
const result = await context.auth.reassignSubject(subject, newRole as Role);
const result = await context.auth.reassignUser(headplaneUserId, newRole as Role);
if (!result) {
throw data("Failed to reassign user role.", { status: 500 });
}
@@ -112,23 +97,12 @@ export async function userAction({ request, context }: Route.ActionArgs) {
throw data("Only the owner can transfer ownership.", { status: 403 });
}
const userId = formData.get("user_id")?.toString();
if (!userId) {
throw data("Missing `user_id` in the form data.", { status: 400 });
const headplaneUserId = formData.get("headplane_user_id")?.toString();
if (!headplaneUserId) {
throw data("Missing `headplane_user_id` in the form data.", { status: 400 });
}
const users = await api.users.list({ id: userId });
const user = users.find((user) => user.id === userId);
if (!user) {
throw data("Specified user not found", { status: 400 });
}
const targetSubject = getOidcSubject(user);
if (!targetSubject) {
throw data("Target user is not an OIDC user or has no subject.", { status: 400 });
}
const result = await context.auth.transferOwnership(principal.user.subject, targetSubject);
const result = await context.auth.transferOwnership(principal.user.id, headplaneUserId);
if (!result) {
throw data("Failed to transfer ownership.", { status: 500 });
}
@@ -136,26 +110,15 @@ export async function userAction({ request, context }: Route.ActionArgs) {
return { message: "Ownership transferred successfully" };
}
case "link_user": {
const userId = formData.get("user_id")?.toString();
const headplaneUserId = formData.get("headplane_user_id")?.toString();
const headscaleUserId = formData.get("headscale_user_id")?.toString();
if (!userId || !headscaleUserId) {
throw data("Missing `user_id` or `headscale_user_id` in the form data.", {
if (!headplaneUserId || !headscaleUserId) {
throw data("Missing `headplane_user_id` or `headscale_user_id` in the form data.", {
status: 400,
});
}
const users = await api.users.list({ id: userId });
const user = users.find((user) => user.id === userId);
if (!user) {
throw data("Specified user not found", { status: 400 });
}
const subject = getOidcSubject(user);
if (!subject) {
throw data("Specified user is not an OIDC user or has no subject.", { status: 400 });
}
const linked = await context.auth.linkHeadscaleUserBySubject(subject, headscaleUserId);
const linked = await context.auth.linkHeadscaleUser(headplaneUserId, headscaleUserId);
if (!linked) {
throw data("That Headscale user is already linked to another account.", { status: 409 });
}
@@ -65,12 +65,14 @@ export function makePreAuthKeyApi(
});
return;
}
// Pre-0.28: expire takes user + key string.
// Pre-0.28: expire takes the owning user's ID (a uint64 — Headscale
// rejects names with `proto: invalid value for uint64 field user`)
// plus the key string.
await transport.request({
method: "POST",
path: "v1/preauthkey/expire",
apiKey,
body: { user: key.user?.name ?? "", key: key.key },
body: { user: key.user?.id ?? "", key: key.key },
});
},
};
+18 -41
View File
@@ -77,13 +77,12 @@ export interface AuthService {
linkHeadscaleUser(userId: string, headscaleUserId: string): Promise<boolean>;
unlinkHeadscaleUser(userId: string): Promise<void>;
linkHeadscaleUserBySubject(subject: string, headscaleUserId: string): Promise<boolean>;
listUsers(): Promise<HeadplaneUser[]>;
claimedHeadscaleUserIds(): Promise<Set<string>>;
roleForSubject(subject: string): Promise<Role | undefined>;
roleForHeadscaleUser(headscaleUserId: string): Promise<Role | undefined>;
transferOwnership(currentOwnerSubject: string, newOwnerSubject: string): Promise<boolean>;
reassignSubject(subject: string, role: Role): Promise<boolean>;
transferOwnership(currentOwnerUserId: string, newOwnerUserId: string): Promise<boolean>;
reassignUser(userId: string, role: Role): Promise<boolean>;
pruneExpiredSessions(): Promise<void>;
start(): void;
stop(): void;
@@ -370,23 +369,6 @@ export function createAuthService(opts: AuthServiceOptions): AuthService {
.where(eq(users.id, userId));
}
async function linkHeadscaleUserBySubject(
subject: string,
headscaleUserId: string,
): Promise<boolean> {
const [user] = await opts.db
.select({ id: users.id })
.from(users)
.where(eq(users.sub, subject))
.limit(1);
if (!user) {
return false;
}
return linkHeadscaleUser(user.id, headscaleUserId);
}
async function listUsers(): Promise<HeadplaneUser[]> {
return opts.db.select().from(users);
}
@@ -428,13 +410,17 @@ export function createAuthService(opts: AuthServiceOptions): AuthService {
}
async function transferOwnership(
currentOwnerSubject: string,
newOwnerSubject: string,
currentOwnerUserId: string,
newOwnerUserId: string,
): Promise<boolean> {
if (currentOwnerUserId === newOwnerUserId) {
return false;
}
const [current] = await opts.db
.select()
.from(users)
.where(eq(users.sub, currentOwnerSubject))
.where(eq(users.id, currentOwnerUserId))
.limit(1);
if (!current || current.role !== "owner") {
@@ -444,10 +430,10 @@ export function createAuthService(opts: AuthServiceOptions): AuthService {
const [target] = await opts.db
.select()
.from(users)
.where(eq(users.sub, newOwnerSubject))
.where(eq(users.id, newOwnerUserId))
.limit(1);
if (!target || target.id === current.id) {
if (!target) {
return false;
}
@@ -464,24 +450,16 @@ export function createAuthService(opts: AuthServiceOptions): AuthService {
return true;
}
async function reassignSubject(subject: string, role: Role): Promise<boolean> {
const currentRole = await roleForSubject(subject);
if (currentRole === "owner") {
async function reassignUser(userId: string, role: Role): Promise<boolean> {
const [user] = await opts.db.select().from(users).where(eq(users.id, userId)).limit(1);
if (!user || user.role === "owner") {
return false;
}
await opts.db
.insert(users)
.values({
id: ulid(),
sub: subject,
role,
caps: capsForRole(role),
})
.onConflictDoUpdate({
target: users.sub,
set: { role, caps: capsForRole(role), updated_at: new Date() },
});
.update(users)
.set({ role, caps: capsForRole(role), updated_at: new Date() })
.where(eq(users.id, userId));
return true;
}
@@ -512,13 +490,12 @@ export function createAuthService(opts: AuthServiceOptions): AuthService {
findOrCreateUser,
linkHeadscaleUser,
unlinkHeadscaleUser,
linkHeadscaleUserBySubject,
listUsers,
claimedHeadscaleUserIds,
roleForSubject,
roleForHeadscaleUser,
transferOwnership,
reassignSubject,
reassignUser,
pruneExpiredSessions,
start,
stop,
+18 -27
View File
@@ -78,15 +78,9 @@ describe("linkHeadscaleUser", () => {
const user = users.find((u) => u.id === userId);
expect(user?.headscale_user_id).toBeNull();
});
test("linkHeadscaleUserBySubject works through subject lookup", async () => {
await auth.findOrCreateUser("sub-1");
const result = await auth.linkHeadscaleUserBySubject("sub-1", "hs-1");
expect(result).toBe(true);
});
});
describe("reassignSubject", () => {
describe("reassignUser", () => {
let auth: AuthService;
beforeEach(() => {
@@ -95,9 +89,9 @@ describe("reassignSubject", () => {
test("updates an existing non-owner role", async () => {
await auth.findOrCreateUser("sub-owner");
await auth.findOrCreateUser("sub-user");
const userId = await auth.findOrCreateUser("sub-user");
const result = await auth.reassignSubject("sub-user", "admin");
const result = await auth.reassignUser(userId, "admin");
expect(result).toBe(true);
const role = await auth.roleForSubject("sub-user");
@@ -105,18 +99,15 @@ describe("reassignSubject", () => {
});
test("returns false for owner demotion attempt", async () => {
await auth.findOrCreateUser("sub-owner");
const ownerId = await auth.findOrCreateUser("sub-owner");
const result = await auth.reassignSubject("sub-owner", "member");
const result = await auth.reassignUser(ownerId, "member");
expect(result).toBe(false);
});
test("creates user with role if subject doesn't exist yet (upsert behavior)", async () => {
const result = await auth.reassignSubject("sub-new", "auditor");
expect(result).toBe(true);
const role = await auth.roleForSubject("sub-new");
expect(role).toBe("auditor");
test("returns false when user does not exist", async () => {
const result = await auth.reassignUser("01ARZ3NDEKTSV4RRFFQ69G5FAV", "auditor");
expect(result).toBe(false);
});
});
@@ -128,10 +119,10 @@ describe("transferOwnership", () => {
});
test("swaps roles: owner→admin, target→owner", async () => {
await auth.findOrCreateUser("sub-owner");
await auth.findOrCreateUser("sub-target");
const ownerId = await auth.findOrCreateUser("sub-owner");
const targetId = await auth.findOrCreateUser("sub-target");
const result = await auth.transferOwnership("sub-owner", "sub-target");
const result = await auth.transferOwnership(ownerId, targetId);
expect(result).toBe(true);
expect(await auth.roleForSubject("sub-owner")).toBe("admin");
@@ -139,24 +130,24 @@ describe("transferOwnership", () => {
});
test("returns false if caller is not owner", async () => {
await auth.findOrCreateUser("sub-owner");
await auth.findOrCreateUser("sub-other");
const ownerId = await auth.findOrCreateUser("sub-owner");
const otherId = await auth.findOrCreateUser("sub-other");
const result = await auth.transferOwnership("sub-other", "sub-owner");
const result = await auth.transferOwnership(otherId, ownerId);
expect(result).toBe(false);
});
test("returns false if target doesn't exist", async () => {
await auth.findOrCreateUser("sub-owner");
const ownerId = await auth.findOrCreateUser("sub-owner");
const result = await auth.transferOwnership("sub-owner", "sub-ghost");
const result = await auth.transferOwnership(ownerId, "01ARZ3NDEKTSV4RRFFQ69G5FAV");
expect(result).toBe(false);
});
test("returns false if target is same user as owner", async () => {
await auth.findOrCreateUser("sub-owner");
const ownerId = await auth.findOrCreateUser("sub-owner");
const result = await auth.transferOwnership("sub-owner", "sub-owner");
const result = await auth.transferOwnership(ownerId, ownerId);
expect(result).toBe(false);
});
});