Skip to content

Commit f86767c

Browse files
anikdhabaldevin-ai-integration[bot]bot_apk
authored
fix: correct admin password banner message and auto-sign-out after 2FA enable (calcom#28129)
* fix: correct admin password banner message to require both password length and 2FA The banner message incorrectly used 'or' implying only one condition was needed, but the code requires BOTH a password of at least 15 characters AND 2FA enabled. Updated the message to clearly state both requirements and added a hint that users need to log out and log back in after updating their security settings. Fixes calcom#9527 Co-Authored-By: unknown <> * fix: auto-sign-out INACTIVE_ADMIN users after enabling 2FA When an INACTIVE_ADMIN user enables 2FA, automatically sign them out so they can log back in with refreshed session role, dismissing the banner. This matches the existing behavior for password changes. Co-Authored-By: unknown <> * feat: add dynamic admin banner message based on inactiveAdminReason Co-Authored-By: unknown <> * Remove and add various localization strings * Update common.json * Add cookie consent checkbox message and remove entries * test: add unit tests for AdminPasswordBanner and inactiveAdminReason logic Co-Authored-By: unknown <> * fix: add expires field to session mock to fix type check Co-Authored-By: unknown <> * fix: wrap CALENDSO_ENCRYPTION_KEY mutations in try/finally to prevent env state leaks Addresses Cubic AI review feedback (confidence 9/10): when a test fails early, the CALENDSO_ENCRYPTION_KEY env var was not being restored, which could leak state into subsequent tests. Wrapped the env mutation in try/finally blocks to guarantee cleanup. Co-Authored-By: bot_apk <apk@cognition.ai> * fix: add missing 'expires' field to buildSession in AdminPasswordBanner test The Session type requires 'expires' to be a string, but buildSession was not providing it, causing a type error caught by CI type-check. Co-Authored-By: bot_apk <apk@cognition.ai> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: bot_apk <apk@cognition.ai>
1 parent f3f5523 commit f86767c

7 files changed

Lines changed: 286 additions & 11 deletions

File tree

apps/web/modules/settings/security/two-factor-auth-view.tsx

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
"use client";
22

3-
import { useState } from "react";
4-
53
import { useLocale } from "@calcom/lib/hooks/useLocale";
64
import { trpc } from "@calcom/trpc/react";
5+
import { Alert } from "@calcom/ui/components/alert";
76
import { Badge } from "@calcom/ui/components/badge";
87
import { SettingsToggle } from "@calcom/ui/components/form";
9-
import { Alert } from "@calcom/ui/components/alert";
108
import { SkeletonButton, SkeletonContainer, SkeletonText } from "@calcom/ui/components/skeleton";
11-
129
import DisableTwoFactorModal from "@components/settings/DisableTwoFactorModal";
1310
import EnableTwoFactorModal from "@components/settings/EnableTwoFactorModal";
11+
import { signOut, useSession } from "next-auth/react";
12+
import { useState } from "react";
1413

1514
const SkeletonLoader = () => {
1615
return (
@@ -27,6 +26,7 @@ const SkeletonLoader = () => {
2726

2827
const TwoFactorAuthView = () => {
2928
const utils = trpc.useUtils();
29+
const { data: sessionData } = useSession();
3030

3131
const { t } = useLocale();
3232
const { data: user, isPending } = trpc.viewer.me.get.useQuery({ includePasswordAdded: true });
@@ -63,7 +63,13 @@ const TwoFactorAuthView = () => {
6363
onOpenChange={() => setEnableModalOpen(!enableModalOpen)}
6464
onEnable={() => {
6565
setEnableModalOpen(false);
66-
utils.viewer.me.invalidate();
66+
if (sessionData?.user.role === "INACTIVE_ADMIN") {
67+
// Session role is set at login time, so we need to sign out
68+
// and sign back in to refresh the role and dismiss the banner
69+
signOut({ callbackUrl: "/auth/login" });
70+
} else {
71+
utils.viewer.me.invalidate();
72+
}
6773
}}
6874
onCancel={() => {
6975
setEnableModalOpen(false);
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
import { render, screen } from "@testing-library/react";
2+
import type { ReactNode } from "react";
3+
import type { SessionContextValue } from "next-auth/react";
4+
import { describe, expect, it, vi } from "vitest";
5+
6+
import AdminPasswordBanner from "./AdminPasswordBanner";
7+
8+
vi.mock("@calcom/lib/hooks/useLocale", () => ({
9+
useLocale: () => ({
10+
t: (key: string) => key,
11+
}),
12+
}));
13+
14+
vi.mock("next/link", () => ({
15+
default: ({ href, children, ...props }: { href: string; children: ReactNode }) => (
16+
<a href={href} {...props}>
17+
{children}
18+
</a>
19+
),
20+
}));
21+
22+
vi.mock("@calcom/ui/components/top-banner", () => ({
23+
TopBanner: ({ text, actions }: { text: string; actions?: ReactNode }) => (
24+
<div>
25+
<span>{text}</span>
26+
{actions}
27+
</div>
28+
),
29+
}));
30+
31+
function buildSession(overrides?: Partial<NonNullable<SessionContextValue["data"]>>): NonNullable<
32+
SessionContextValue["data"]
33+
> {
34+
return {
35+
expires: "2099-01-01T00:00:00.000Z",
36+
hasValidLicense: true,
37+
upId: "usr_test",
38+
profileId: 1,
39+
user: {
40+
id: 1,
41+
uuid: "uuid_test",
42+
email: "test@example.com",
43+
name: "Test User",
44+
username: "testuser",
45+
role: "INACTIVE_ADMIN",
46+
inactiveAdminReason: "both",
47+
},
48+
...overrides,
49+
};
50+
}
51+
52+
describe("AdminPasswordBanner", () => {
53+
it("does not render for non-INACTIVE_ADMIN users", () => {
54+
const data = buildSession({ user: { ...buildSession().user, role: "USER" } });
55+
const { container } = render(<AdminPasswordBanner data={data} />);
56+
expect(container).toBeEmptyDOMElement();
57+
});
58+
59+
it("renders 2FA-only message and links to 2FA settings", () => {
60+
const data = buildSession({ user: { ...buildSession().user, inactiveAdminReason: "2fa" } });
61+
62+
render(<AdminPasswordBanner data={data} />);
63+
64+
expect(screen.getByText("invalid_admin_password_2fa_only")).toBeInTheDocument();
65+
const link = screen.getByRole("link");
66+
expect(link).toHaveAttribute("href", "/settings/security/two-factor-auth");
67+
expect(link).toHaveTextContent("enable_2fa");
68+
});
69+
70+
it("renders password-only message and links to password settings", () => {
71+
const data = buildSession({ user: { ...buildSession().user, inactiveAdminReason: "password" } });
72+
73+
render(<AdminPasswordBanner data={data} />);
74+
75+
expect(screen.getByText("invalid_admin_password_password_only")).toBeInTheDocument();
76+
const link = screen.getByRole("link");
77+
expect(link).toHaveAttribute("href", "/settings/security/password");
78+
expect(link).toHaveTextContent("change_password_admin");
79+
});
80+
81+
it("defaults to both message and password settings when reason is missing", () => {
82+
const data = buildSession({ user: { ...buildSession().user, inactiveAdminReason: undefined } });
83+
84+
render(<AdminPasswordBanner data={data} />);
85+
86+
expect(screen.getByText("invalid_admin_password")).toBeInTheDocument();
87+
const link = screen.getByRole("link");
88+
expect(link).toHaveAttribute("href", "/settings/security/password");
89+
expect(link).toHaveTextContent("change_password_admin");
90+
});
91+
});

apps/web/modules/users/components/AdminPasswordBanner.tsx

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,45 @@ import { TopBanner } from "@calcom/ui/components/top-banner";
66

77
export type AdminPasswordBannerProps = { data: SessionContextValue["data"] };
88

9+
function getBannerMessageKey(reason: "both" | "password" | "2fa" | undefined): string {
10+
switch (reason) {
11+
case "password":
12+
return "invalid_admin_password_password_only";
13+
case "2fa":
14+
return "invalid_admin_password_2fa_only";
15+
case "both":
16+
default:
17+
return "invalid_admin_password";
18+
}
19+
}
20+
21+
function getBannerAction(reason: "both" | "password" | "2fa" | undefined): { href: string; labelKey: string } {
22+
switch (reason) {
23+
case "2fa":
24+
return { href: "/settings/security/two-factor-auth", labelKey: "enable_2fa" };
25+
case "password":
26+
case "both":
27+
default:
28+
return { href: "/settings/security/password", labelKey: "change_password_admin" };
29+
}
30+
}
31+
932
function AdminPasswordBanner({ data }: AdminPasswordBannerProps) {
1033
const { t } = useLocale();
1134

1235
if (data?.user.role !== "INACTIVE_ADMIN") return null;
1336

37+
const messageKey = getBannerMessageKey(data.user.inactiveAdminReason);
38+
const { href, labelKey } = getBannerAction(data.user.inactiveAdminReason);
39+
1440
return (
1541
<>
1642
<TopBanner
17-
text={t("invalid_admin_password", { user: data.user.username })}
43+
text={t(messageKey, { user: data.user.username })}
1844
variant="warning"
1945
actions={
20-
<Link href="/settings/security/password" className="border-b border-b-black">
21-
{t("change_password_admin")}
46+
<Link href={href} className="border-b border-b-black">
47+
{t(labelKey)}
2248
</Link>
2349
}
2450
/>

packages/features/auth/lib/next-auth-options.test.ts

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ vi.mock("./next-auth-custom-adapter", () => ({
8888
})),
8989
}));
9090

91+
vi.mock("@calcom/i18n/server", () => ({
92+
getTranslation: vi.fn().mockResolvedValue((key: string) => key),
93+
}));
94+
9195
vi.mock("@calcom/features/profile/repositories/ProfileRepository", () => ({
9296
ProfileRepository: {
9397
findAllProfilesForUserIncludingMovedUser: vi.fn(),
@@ -375,6 +379,128 @@ describe("CredentialsProvider authorize", () => {
375379
).rejects.toThrow(ErrorCode.IncorrectEmailPassword);
376380
});
377381
});
382+
383+
describe("Inactive admin reason", () => {
384+
it("sets reason to 'both' when password is invalid and 2FA is disabled", async () => {
385+
const { isPasswordValid } = await import("@calcom/lib/auth/isPasswordValid");
386+
vi.mocked(isPasswordValid).mockReturnValue(false);
387+
vi.mocked(verifyPassword).mockResolvedValue(true);
388+
389+
const mockUser = createMockUser({
390+
role: UserPermissionRole.ADMIN,
391+
identityProvider: IdentityProvider.CAL,
392+
twoFactorEnabled: false,
393+
});
394+
mockFindByEmailAndIncludeProfilesAndPassword.mockResolvedValue(mockUser);
395+
396+
const result = await authorizeCredentials({
397+
email: mockUser.email,
398+
password: "password123",
399+
totpCode: "",
400+
backupCode: "",
401+
});
402+
403+
expect(result?.role).toBe("INACTIVE_ADMIN");
404+
expect(result?.inactiveAdminReason).toBe("both");
405+
});
406+
407+
it("sets reason to '2fa' when password is valid and 2FA is disabled", async () => {
408+
const { isPasswordValid } = await import("@calcom/lib/auth/isPasswordValid");
409+
vi.mocked(isPasswordValid).mockReturnValue(true);
410+
vi.mocked(verifyPassword).mockResolvedValue(true);
411+
412+
const mockUser = createMockUser({
413+
role: UserPermissionRole.ADMIN,
414+
identityProvider: IdentityProvider.CAL,
415+
twoFactorEnabled: false,
416+
});
417+
mockFindByEmailAndIncludeProfilesAndPassword.mockResolvedValue(mockUser);
418+
419+
const result = await authorizeCredentials({
420+
email: mockUser.email,
421+
password: "password123",
422+
totpCode: "",
423+
backupCode: "",
424+
});
425+
426+
expect(result?.role).toBe("INACTIVE_ADMIN");
427+
expect(result?.inactiveAdminReason).toBe("2fa");
428+
});
429+
430+
it("sets reason to 'password' when password is invalid and 2FA is enabled", async () => {
431+
const originalKey = process.env.CALENDSO_ENCRYPTION_KEY;
432+
process.env.CALENDSO_ENCRYPTION_KEY = "test";
433+
434+
try {
435+
const { isPasswordValid } = await import("@calcom/lib/auth/isPasswordValid");
436+
vi.mocked(isPasswordValid).mockReturnValue(false);
437+
vi.mocked(verifyPassword).mockResolvedValue(true);
438+
439+
const { symmetricDecrypt } = await import("@calcom/lib/crypto");
440+
vi.mocked(symmetricDecrypt).mockReturnValue("a".repeat(32));
441+
442+
const { totpAuthenticatorCheck } = await import("@calcom/lib/totp");
443+
vi.mocked(totpAuthenticatorCheck).mockReturnValue(true);
444+
445+
const mockUser = createMockUser({
446+
role: UserPermissionRole.ADMIN,
447+
identityProvider: IdentityProvider.CAL,
448+
twoFactorEnabled: true,
449+
twoFactorSecret: "encrypted_secret",
450+
});
451+
mockFindByEmailAndIncludeProfilesAndPassword.mockResolvedValue(mockUser);
452+
453+
const result = await authorizeCredentials({
454+
email: mockUser.email,
455+
password: "password123",
456+
totpCode: "123456",
457+
backupCode: "",
458+
});
459+
460+
expect(result?.role).toBe("INACTIVE_ADMIN");
461+
expect(result?.inactiveAdminReason).toBe("password");
462+
} finally {
463+
process.env.CALENDSO_ENCRYPTION_KEY = originalKey;
464+
}
465+
});
466+
467+
it("does not set inactiveAdminReason when admin requirements are met", async () => {
468+
const originalKey = process.env.CALENDSO_ENCRYPTION_KEY;
469+
process.env.CALENDSO_ENCRYPTION_KEY = "test";
470+
471+
try {
472+
const { isPasswordValid } = await import("@calcom/lib/auth/isPasswordValid");
473+
vi.mocked(isPasswordValid).mockReturnValue(true);
474+
vi.mocked(verifyPassword).mockResolvedValue(true);
475+
476+
const { symmetricDecrypt } = await import("@calcom/lib/crypto");
477+
vi.mocked(symmetricDecrypt).mockReturnValue("a".repeat(32));
478+
479+
const { totpAuthenticatorCheck } = await import("@calcom/lib/totp");
480+
vi.mocked(totpAuthenticatorCheck).mockReturnValue(true);
481+
482+
const mockUser = createMockUser({
483+
role: UserPermissionRole.ADMIN,
484+
identityProvider: IdentityProvider.CAL,
485+
twoFactorEnabled: true,
486+
twoFactorSecret: "encrypted_secret",
487+
});
488+
mockFindByEmailAndIncludeProfilesAndPassword.mockResolvedValue(mockUser);
489+
490+
const result = await authorizeCredentials({
491+
email: mockUser.email,
492+
password: "password123",
493+
totpCode: "123456",
494+
backupCode: "",
495+
});
496+
497+
expect(result?.role).toBe(UserPermissionRole.ADMIN);
498+
expect(result?.inactiveAdminReason).toBeUndefined();
499+
} finally {
500+
process.env.CALENDSO_ENCRYPTION_KEY = originalKey;
501+
}
502+
});
503+
});
378504
});
379505

380506
describe("Azure AD signIn callback", () => {

packages/features/auth/lib/next-auth-options.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,8 +273,27 @@ export async function authorizeCredentials(
273273
return "INACTIVE_ADMIN";
274274
};
275275

276-
// Create a NextAuth compatible user object using our presenter
277-
return AdapterUserPresenter.fromCalUser(user, validateRole(user.role), hasActiveTeams);
276+
const role = validateRole(user.role);
277+
const baseUser = AdapterUserPresenter.fromCalUser(user, role, hasActiveTeams);
278+
279+
if (role === "INACTIVE_ADMIN") {
280+
const passwordValid = isPasswordValid(credentials.password, false, true);
281+
const has2FA = user.twoFactorEnabled;
282+
283+
let reason: "both" | "password" | "2fa";
284+
285+
if (!passwordValid && !has2FA) {
286+
reason = "both";
287+
} else if (!passwordValid) {
288+
reason = "password";
289+
} else {
290+
reason = "2fa";
291+
}
292+
293+
return { ...baseUser, inactiveAdminReason: reason };
294+
}
295+
296+
return baseUser;
278297
}
279298

280299
export const CalComCredentialsProvider = CredentialsProvider({
@@ -729,6 +748,7 @@ export const getOptions = ({
729748
locale: user?.locale,
730749
profileId: user.profile?.id ?? token.profileId ?? null,
731750
upId: user.profile?.upId ?? token.upId ?? null,
751+
inactiveAdminReason: user.inactiveAdminReason,
732752
} as JWT;
733753
}
734754

@@ -954,6 +974,7 @@ export const getOptions = ({
954974
belongsToActiveTeam: token?.belongsToActiveTeam as boolean,
955975
org: token?.org,
956976
locale: token.locale,
977+
inactiveAdminReason: token.inactiveAdminReason,
957978
},
958979
};
959980
return calendsoSession;

packages/i18n/locales/en/common.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2425,7 +2425,9 @@
24252425
"create_your_first_team_webhook_description": "Create your first webhook for this team event type",
24262426
"create_webhook_team_event_type": "Create a webhook for this team event type",
24272427
"disable_success_page": "Disable Success Page (only works if you have a redirect URL)",
2428-
"invalid_admin_password": "You are admin but you do not have a password length of at least 15 characters or no 2FA yet",
2428+
"invalid_admin_password": "You are an admin but your password must be at least 15 characters long and you must enable 2FA.",
2429+
"invalid_admin_password_password_only": "You are admin but your password must be at least 15 characters long",
2430+
"invalid_admin_password_2fa_only": "You are admin but you do not have 2FA enabled yet",
24292431
"change_password_admin": "Change your password to gain admin access",
24302432
"username_already_taken": "Username is already taken",
24312433
"assignment": "Assignment",

packages/types/next-auth.d.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ declare module "next-auth" {
4141
orgAwareUsername?: PrismaUser["username"];
4242
avatarUrl?: PrismaUser["avatarUrl"];
4343
role?: PrismaUser["role"] | "INACTIVE_ADMIN";
44+
/** Set when role is INACTIVE_ADMIN: why admin security requirements are not met */
45+
inactiveAdminReason?: "both" | "password" | "2fa";
4446
locale?: string | null;
4547
profile?: UserProfile;
4648
samlTenant?: string;
@@ -57,6 +59,7 @@ declare module "next-auth/jwt" {
5759
upId?: string;
5860
profileId?: number | null;
5961
role?: UserPermissionRole | "INACTIVE_ADMIN" | null;
62+
inactiveAdminReason?: "both" | "password" | "2fa";
6063
impersonatedBy?: {
6164
id: number;
6265
uuid: string;

0 commit comments

Comments
 (0)