Skip to content

Commit a0a1633

Browse files
BekacruCopilot
Bekacru
and
Copilot
authored
feat(admin): add support role with permissions for user updates and enforce role change validation (#6699)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent f0ed5c2 commit a0a1633

File tree

6 files changed

+279
-9
lines changed

6 files changed

+279
-9
lines changed

‎packages/better-auth/src/api/routes/session.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -597,11 +597,23 @@ export const listSessions = <Option extends BetterAuthOptions>() =>
597597
const sessions = await ctx.context.internalAdapter.listSessions(
598598
ctx.context.session.user.id,
599599
);
600-
const activeSessions = sessions.filter((session) => {
601-
return session.expiresAt > new Date();
602-
});
600+
const activeSessions = sessions
601+
.filter((session) => {
602+
return session.expiresAt > new Date();
603+
})
604+
.map((session) => {
605+
return {
606+
...session,
607+
token: undefined, // we don't need to return the token to the client
608+
expiresAt: session.expiresAt.toISOString(),
609+
createdAt: session.createdAt.toISOString(),
610+
updatedAt: session.updatedAt.toISOString(),
611+
};
612+
});
603613
return ctx.json(
604-
activeSessions as unknown as Prettify<InferSession<Option>>[],
614+
activeSessions as unknown as Prettify<
615+
InferSession<Option> & { token: undefined }
616+
>[],
605617
);
606618
} catch (e: any) {
607619
ctx.context.logger.error(e);

‎packages/better-auth/src/plugins/admin/admin.test.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -957,6 +957,10 @@ describe("access control", async (it) => {
957957
user: ["read"],
958958
order: ["read"],
959959
});
960+
const supportAc = ac.newRole({
961+
user: ["update"],
962+
order: ["update"],
963+
});
960964

961965
const { signInWithTestUser, cookieSetter, auth, customFetchImpl } =
962966
await getTestInstance(
@@ -967,6 +971,7 @@ describe("access control", async (it) => {
967971
roles: {
968972
admin: adminAc,
969973
user: userAc,
974+
support: supportAc,
970975
},
971976
}),
972977
],
@@ -982,6 +987,14 @@ describe("access control", async (it) => {
982987
},
983988
};
984989
}
990+
if (user.name === "Support") {
991+
return {
992+
data: {
993+
...user,
994+
role: "support",
995+
},
996+
};
997+
}
985998
},
986999
},
9871000
},
@@ -1001,6 +1014,7 @@ describe("access control", async (it) => {
10011014
roles: {
10021015
admin: adminAc,
10031016
user: userAc,
1017+
support: supportAc,
10041018
},
10051019
}),
10061020
],
@@ -1012,6 +1026,104 @@ describe("access control", async (it) => {
10121026

10131027
const { headers, user } = await signInWithTestUser();
10141028

1029+
it("should not allow role updates via update-user without user:set-role", async () => {
1030+
const supportHeaders = new Headers();
1031+
const { data: supportSignUp } = await client.signUp.email(
1032+
{
1033+
email: "support@test.com",
1034+
password: "password",
1035+
name: "Support",
1036+
},
1037+
{
1038+
onSuccess: cookieSetter(supportHeaders),
1039+
},
1040+
);
1041+
const supportUserId = supportSignUp?.user.id || "";
1042+
1043+
// Non-sensitive updates should still be allowed with `user:update`
1044+
const ok = await client.admin.updateUser(
1045+
{
1046+
userId: supportUserId,
1047+
data: {
1048+
name: "Support Updated",
1049+
},
1050+
},
1051+
{
1052+
headers: supportHeaders,
1053+
},
1054+
);
1055+
expect(ok.data?.name).toBe("Support Updated");
1056+
1057+
// But attempting to update `role` must be rejected without `user:set-role`.
1058+
const res = await client.admin.updateUser(
1059+
{
1060+
userId: supportUserId,
1061+
data: {
1062+
role: "admin",
1063+
},
1064+
},
1065+
{
1066+
headers: supportHeaders,
1067+
},
1068+
);
1069+
expect(res.error?.status).toBe(403);
1070+
});
1071+
1072+
it("should reject non-existent roles via update-user", async () => {
1073+
const { data: targetUserRes } = await client.signUp.email(
1074+
{
1075+
email: "role-target@test.com",
1076+
password: "password",
1077+
name: "Role Target",
1078+
},
1079+
{
1080+
onSuccess: cookieSetter(new Headers()),
1081+
},
1082+
);
1083+
const targetUserId = targetUserRes?.user.id || "";
1084+
1085+
const res = await client.admin.updateUser(
1086+
{
1087+
userId: targetUserId,
1088+
data: {
1089+
role: "non-existent-role",
1090+
},
1091+
},
1092+
{
1093+
headers,
1094+
},
1095+
);
1096+
expect(res.error?.status).toBe(400);
1097+
});
1098+
1099+
it("should allow role updates via update-user for valid roles with user:set-role", async () => {
1100+
const { data: targetUserRes } = await client.signUp.email(
1101+
{
1102+
email: "role-valid-target@test.com",
1103+
password: "password",
1104+
name: "Role Valid Target",
1105+
},
1106+
{
1107+
onSuccess: cookieSetter(new Headers()),
1108+
},
1109+
);
1110+
const targetUserId = targetUserRes?.user.id || "";
1111+
1112+
const res = await client.admin.updateUser(
1113+
{
1114+
userId: targetUserId,
1115+
data: {
1116+
role: "support",
1117+
},
1118+
},
1119+
{
1120+
headers,
1121+
},
1122+
);
1123+
expect(res.error).toBeNull();
1124+
expect(res.data?.role).toBe("support");
1125+
});
1126+
10151127
it("should validate on the client", async () => {
10161128
const canCreateOrder = client.admin.checkRolePermission({
10171129
role: "admin",

‎packages/better-auth/src/plugins/admin/error-codes.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,5 @@ export const ADMIN_ERROR_CODES = defineErrorCodes({
2929
YOU_ARE_NOT_ALLOWED_TO_SET_NON_EXISTENT_VALUE:
3030
"You are not allowed to set a non-existent role value",
3131
YOU_CANNOT_IMPERSONATE_ADMINS: "You cannot impersonate admins",
32+
INVALID_ROLE_TYPE: "Invalid role type",
3233
});

‎packages/better-auth/src/plugins/admin/routes.ts‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,41 @@ export const adminUpdateUser = (opts: AdminOptions) =>
460460
message: ADMIN_ERROR_CODES.NO_DATA_TO_UPDATE,
461461
});
462462
}
463-
if (ctx.body.data?.role) {
464-
ctx.body.data.role = parseRoles(ctx.body.data.role);
463+
464+
// Role changes must be guarded by `user:set-role` and validated against the role allow-list.
465+
if (Object.prototype.hasOwnProperty.call(ctx.body.data, "role")) {
466+
const canSetRole = hasPermission({
467+
userId: ctx.context.session.user.id,
468+
role: ctx.context.session.user.role,
469+
options: opts,
470+
permissions: {
471+
user: ["set-role"],
472+
},
473+
});
474+
if (!canSetRole) {
475+
throw new APIError("FORBIDDEN", {
476+
message: ADMIN_ERROR_CODES.YOU_ARE_NOT_ALLOWED_TO_CHANGE_USERS_ROLE,
477+
});
478+
}
479+
480+
const roleValue = (ctx.body.data as Record<string, any>).role;
481+
const inputRoles = Array.isArray(roleValue) ? roleValue : [roleValue];
482+
for (const role of inputRoles) {
483+
if (typeof role !== "string") {
484+
throw new APIError("BAD_REQUEST", {
485+
message: ADMIN_ERROR_CODES.INVALID_ROLE_TYPE,
486+
});
487+
}
488+
if (opts.roles && !opts.roles[role as keyof typeof opts.roles]) {
489+
throw new APIError("BAD_REQUEST", {
490+
message:
491+
ADMIN_ERROR_CODES.YOU_ARE_NOT_ALLOWED_TO_SET_NON_EXISTENT_VALUE,
492+
});
493+
}
494+
}
495+
(ctx.body.data as Record<string, any>).role = parseRoles(
496+
inputRoles as string[],
497+
);
465498
}
466499
const updatedUser = await ctx.context.internalAdapter.updateUser(
467500
ctx.body.userId,

‎packages/stripe/src/routes.ts‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ export const upgradeSubscription = (options: StripeOptions) => {
182182
message: STRIPE_ERROR_CODES.SUBSCRIPTION_PLAN_NOT_FOUND,
183183
});
184184
}
185-
const subscriptionToUpdate = ctx.body.subscriptionId
185+
let subscriptionToUpdate = ctx.body.subscriptionId
186186
? await ctx.context.adapter.findOne<Subscription>({
187187
model: "subscription",
188188
where: [
@@ -205,6 +205,14 @@ export const upgradeSubscription = (options: StripeOptions) => {
205205
})
206206
: null;
207207

208+
if (
209+
ctx.body.subscriptionId &&
210+
subscriptionToUpdate &&
211+
subscriptionToUpdate.referenceId !== referenceId
212+
) {
213+
subscriptionToUpdate = null;
214+
}
215+
208216
if (ctx.body.subscriptionId && !subscriptionToUpdate) {
209217
throw new APIError("BAD_REQUEST", {
210218
message: STRIPE_ERROR_CODES.SUBSCRIPTION_NOT_FOUND,
@@ -698,7 +706,7 @@ export const cancelSubscription = (options: StripeOptions) => {
698706
},
699707
async (ctx) => {
700708
const referenceId = ctx.body?.referenceId || ctx.context.session.user.id;
701-
const subscription = ctx.body.subscriptionId
709+
let subscription = ctx.body.subscriptionId
702710
? await ctx.context.adapter.findOne<Subscription>({
703711
model: "subscription",
704712
where: [
@@ -719,6 +727,15 @@ export const cancelSubscription = (options: StripeOptions) => {
719727
),
720728
);
721729

730+
// Ensure the specified subscription belongs to the (validated) referenceId.
731+
if (
732+
ctx.body.subscriptionId &&
733+
subscription &&
734+
subscription.referenceId !== referenceId
735+
) {
736+
subscription = undefined;
737+
}
738+
722739
if (!subscription || !subscription.stripeCustomerId) {
723740
throw ctx.error("BAD_REQUEST", {
724741
message: STRIPE_ERROR_CODES.SUBSCRIPTION_NOT_FOUND,
@@ -847,7 +864,7 @@ export const restoreSubscription = (options: StripeOptions) => {
847864
async (ctx) => {
848865
const referenceId = ctx.body?.referenceId || ctx.context.session.user.id;
849866

850-
const subscription = ctx.body.subscriptionId
867+
let subscription = ctx.body.subscriptionId
851868
? await ctx.context.adapter.findOne<Subscription>({
852869
model: "subscription",
853870
where: [
@@ -872,6 +889,15 @@ export const restoreSubscription = (options: StripeOptions) => {
872889
(sub) => sub.status === "active" || sub.status === "trialing",
873890
),
874891
);
892+
893+
// Ensure the specified subscription belongs to the (validated) referenceId.
894+
if (
895+
ctx.body.subscriptionId &&
896+
subscription &&
897+
subscription.referenceId !== referenceId
898+
) {
899+
subscription = undefined;
900+
}
875901
if (!subscription || !subscription.stripeCustomerId) {
876902
throw ctx.error("BAD_REQUEST", {
877903
message: STRIPE_ERROR_CODES.SUBSCRIPTION_NOT_FOUND,

‎packages/stripe/src/stripe.test.ts‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,92 @@ describe("stripe", async () => {
227227
});
228228
});
229229

230+
it("should not allow cross-user subscriptionId operations (upgrade/cancel/restore)", async () => {
231+
const userA = {
232+
email: "user-a@email.com",
233+
password: "password",
234+
name: "User A",
235+
};
236+
const userARes = await authClient.signUp.email(userA, { throw: true });
237+
238+
const userAHeaders = new Headers();
239+
await authClient.signIn.email(userA, {
240+
throw: true,
241+
onSuccess: setCookieToHeader(userAHeaders),
242+
});
243+
await authClient.subscription.upgrade({
244+
plan: "starter",
245+
fetchOptions: { headers: userAHeaders },
246+
});
247+
248+
const userASub = await ctx.adapter.findOne<Subscription>({
249+
model: "subscription",
250+
where: [{ field: "referenceId", value: userARes.user.id }],
251+
});
252+
expect(userASub).toBeTruthy();
253+
254+
const userB = {
255+
email: "user-b@email.com",
256+
password: "password",
257+
name: "User B",
258+
};
259+
await authClient.signUp.email(userB, { throw: true });
260+
const userBHeaders = new Headers();
261+
await authClient.signIn.email(userB, {
262+
throw: true,
263+
onSuccess: setCookieToHeader(userBHeaders),
264+
});
265+
266+
mockStripe.checkout.sessions.create.mockClear();
267+
mockStripe.billingPortal.sessions.create.mockClear();
268+
mockStripe.subscriptions.list.mockClear();
269+
mockStripe.subscriptions.update.mockClear();
270+
271+
const upgradeRes = await authClient.subscription.upgrade({
272+
plan: "premium",
273+
subscriptionId: userASub!.id,
274+
fetchOptions: { headers: userBHeaders },
275+
});
276+
expect(upgradeRes.error?.message).toContain("Subscription not found");
277+
expect(mockStripe.checkout.sessions.create).not.toHaveBeenCalled();
278+
expect(mockStripe.billingPortal.sessions.create).not.toHaveBeenCalled();
279+
280+
const cancelHeaders = new Headers(userBHeaders);
281+
cancelHeaders.set("content-type", "application/json");
282+
const cancelResponse = await auth.handler(
283+
new Request("http://localhost:3000/api/auth/subscription/cancel", {
284+
method: "POST",
285+
headers: cancelHeaders,
286+
body: JSON.stringify({
287+
subscriptionId: userASub!.id,
288+
returnUrl: "/account",
289+
}),
290+
}),
291+
);
292+
expect(cancelResponse.status).toBe(400);
293+
expect((await cancelResponse.json()).message).toContain(
294+
"Subscription not found",
295+
);
296+
expect(mockStripe.billingPortal.sessions.create).not.toHaveBeenCalled();
297+
298+
const restoreHeaders = new Headers(userBHeaders);
299+
restoreHeaders.set("content-type", "application/json");
300+
const restoreResponse = await auth.handler(
301+
new Request("http://localhost:3000/api/auth/subscription/restore", {
302+
method: "POST",
303+
headers: restoreHeaders,
304+
body: JSON.stringify({
305+
subscriptionId: userASub!.id,
306+
}),
307+
}),
308+
);
309+
expect(restoreResponse.status).toBe(400);
310+
expect((await restoreResponse.json()).message).toContain(
311+
"Subscription not found",
312+
);
313+
expect(mockStripe.subscriptions.update).not.toHaveBeenCalled();
314+
});
315+
230316
it("should list active subscriptions", async () => {
231317
const userRes = await authClient.signUp.email(
232318
{

0 commit comments

Comments
 (0)