Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .env.test
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ AWS_S3_UPLOADS_SECRET_ACCESS_KEY="dev-s3-secret-access-key"
AWS_S3_UPLOADS_FORCE_PATH_STYLE=true
FILE_UPLOAD_PROVIDER="AWS_S3"

USER_EMAIL_ALLOW_LIST='["charlie@example.com", "mfa-enable-totp@example.com", "mfa-totp-login@example.com", "mfa-backup-code@example.com", "mfa-disable-totp@example.com", "mfa-wrong-code@example.com"]'
USER_EMAIL_ALLOW_LIST='["charlie@example.com", "signup-allow@example.com", "signin-test@example.com", "signout-test@example.com", "pw-change@example.com", "pw-recovery@example.com", "mfa-enable@example.com", "mfa-login@example.com", "mfa-backup@example.com", "mfa-disable@example.com", "mfa-wrong-code@example.com"]'
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ bower_components
# Tests
**/playwright-report/
**/test-results/
tests/**/.auth/

# Compiled binary addons (https://nodejs.org/api/addons.html)
**/build/Release
Expand Down
1 change: 1 addition & 0 deletions apps/hash-external-services/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ services:
SELFSERVICE_FLOWS_VERIFICATION_UI_URL: "http://localhost:3000/verification"
SELFSERVICE_FLOWS_RECOVERY_UI_URL: "http://localhost:3000/recovery"
SELFSERVICE_FLOWS_SETTINGS_UI_URL: "http://localhost:3000/settings/security"
SELFSERVICE_FLOWS_SETTINGS_PRIVILEGED_SESSION_MAX_AGE: "${SELFSERVICE_FLOWS_SETTINGS_PRIVILEGED_SESSION_MAX_AGE:-5m}"
COURIER_SMTP_CONNECTION_URI: "smtps://test:test@mailslurper:1025/?skip_ssl_verify=true"
LOG_LEVEL: debug
LOG_FORMAT: text
Expand Down
7 changes: 6 additions & 1 deletion apps/hash-frontend/src/pages/_app.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,12 @@ AppWithTypeSystemContextProvider.getInitialProps = async (appContext) => {
* Fetch the authenticated user on the very first page load so it's available in the frontend.
* We leave it up to the client to re-fetch the user as necessary in response to user-initiated actions.
*
* @todo this is running on every page transition. make it stop or make caching work (need to create new client on request to avoid sharing user data)
* @todo this is running on every page transition β€” the response should
* be cacheable so it doesn't hit the backend on every navigation.
* Note: the server-side `apolloClient` singleton has
* `queryDeduplication: false` (see `create-apollo-client.ts`) because
* Apollo's dedup key ignores `context` and would otherwise leak one
* user's data into another concurrent SSR request.
*/
const initialAuthenticatedUserSubgraph = await apolloClient
.query<MeQuery>({
Expand Down
216 changes: 84 additions & 132 deletions apps/hash-frontend/src/pages/settings/security.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,8 @@ const SecurityPage: NextPageWithLayout = () => {
authenticatedUser?.emails[0]?.address ?? "";

const [flow, setFlow] = useState<SettingsFlow>();
const [currentPassword, setCurrentPassword] = useState("");
const [password, setPassword] = useState("");

const [isRecoveryFlow, setIsRecoveryFlow] = useState(false);
const [currentPasswordError, setCurrentPasswordError] = useState<string>();
const [totpCode, setTotpCode] = useState("");
const [showTotpSetupForm, setShowTotpSetupForm] = useState(false);
const [showTotpDisableForm, setShowTotpDisableForm] = useState(false);
Expand Down Expand Up @@ -155,9 +152,14 @@ const SecurityPage: NextPageWithLayout = () => {
return undefined;
}

return Promise.reject(error);
setErrorMessage(
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- response body is typed as SettingsFlow but non-400 responses have a different shape
error.response?.data?.ui?.messages?.[0]?.text ??
"Something went wrong. Please try again.",
);
Comment thread
TimDiekmann marked this conversation as resolved.
return undefined;
}),
[handleFlowError],
[handleFlowError, setErrorMessage],
);

useEffect(() => {
Expand All @@ -167,9 +169,6 @@ const SecurityPage: NextPageWithLayout = () => {

const initFlow = (data: SettingsFlow) => {
setFlow(data);
if (data.ui.messages?.some(({ id }) => id === 1060001)) {
setIsRecoveryFlow(true);
}
};

if (flowId) {
Expand Down Expand Up @@ -231,6 +230,51 @@ const SecurityPage: NextPageWithLayout = () => {
setShowTotpDisableForm(false);
}, [isTotpEnabled]);

const executeDisableTotp = useCallback(
async (currentFlow: SettingsFlow) => {
setDisablingTotp(true);
setShowTotpDisableForm(true);
persistFlowIdInUrl(currentFlow);

try {
// Step 1: Unlink TOTP.
const unlinkedFlow = await submitSettingsUpdate(currentFlow, {
method: "totp",
totp_unlink: true,
csrf_token: mustGetCsrfTokenFromFlow(currentFlow),
});

if (!unlinkedFlow) {
return;
}

// Step 2: Remove backup codes. Kratos enforces AAL2 as long as
// any second factor is present, so leaving orphan backup codes
// behind after unlinking TOTP would lock the user out at next
// login β€” they'd be asked for an authenticator code they no
// longer have.
const clearedFlow = await submitSettingsUpdate(unlinkedFlow, {
method: "lookup_secret",
lookup_secret_disable: true,
csrf_token: mustGetCsrfTokenFromFlow(unlinkedFlow),
});

if (!clearedFlow) {
setErrorMessage(
"TOTP was disabled, but backup codes could not be removed. " +
"Please reload the page and try again to avoid being locked out.",
);
return;
}

setShowTotpDisableForm(false);
} finally {
setDisablingTotp(false);
}
},
[submitSettingsUpdate, persistFlowIdInUrl],
);

const totpQrCodeDataUri = useMemo(() => {
for (const { attributes } of totpNodes) {
if (
Expand Down Expand Up @@ -264,61 +308,34 @@ const SecurityPage: NextPageWithLayout = () => {
const handlePasswordSubmit: FormEventHandler<HTMLFormElement> = (event) => {
event.preventDefault();

if (!flow || !password || (!isRecoveryFlow && !currentPassword)) {
if (!flow || !password) {
return;
}

let csrfToken: string;
try {
csrfToken = mustGetCsrfTokenFromFlow(flow);
} catch {
setErrorMessage("Could not find CSRF token. Please reload the page.");
return;
}

setUpdatingPassword(true);
setCurrentPasswordError(undefined);
persistFlowIdInUrl(flow);

// @todo H-6417 the password-reentry step below only works for users
// who actually have a password credential β€” SSO-only users can't
// change their password today. The SSO-compatible forced session
// refresh being designed under H-6417 is the path to fixing this
// (tracked separately as H-6418 for the password-setup UI itself).

const updatePassword = async () => {
if (!isRecoveryFlow) {
// Verify the current password by creating and submitting a refresh
// login flow. This also refreshes the session to "privileged",
// ensuring the settings update won't be rejected.
// Recovery flows are already privileged, so we don't need to refresh them.
await oryKratosClient
.createBrowserLoginFlow({ refresh: true })
.then(({ data: loginFlow }) =>
oryKratosClient.updateLoginFlow({
flow: loginFlow.id,
updateLoginFlowBody: {
method: "password",
identifier: usernameForPasswordManagers,
password: currentPassword,
csrf_token: mustGetCsrfTokenFromFlow(loginFlow),
},
}),
);
}

const nextFlow = await submitSettingsUpdate(flow, {
method: "password",
password,
csrf_token: mustGetCsrfTokenFromFlow(flow),
});

if (nextFlow) {
setCurrentPassword("");
setPassword("");
}
};

void updatePassword()
.catch((error: AxiosError) => {
if (error.response?.status === 400) {
setCurrentPasswordError("Current password is incorrect.");
return;
// No manual session refresh β€” Kratos enforces `privileged_session_max_age`
// server-side. If the session is stale, Kratos returns
// `session_refresh_required` and the error handler takes over.
// This works for both password and SSO-only users.
void submitSettingsUpdate(flow, {
method: "password",
password,
csrf_token: csrfToken,
})
.then((nextFlow) => {
if (nextFlow) {
setPassword("");
Comment thread
cursor[bot] marked this conversation as resolved.
}

void handleFlowError(error);
})
.finally(() => setUpdatingPassword(false));
};
Expand Down Expand Up @@ -393,54 +410,11 @@ const SecurityPage: NextPageWithLayout = () => {
return;
}

setDisablingTotp(true);
persistFlowIdInUrl(flow);

// @todo H-6417 force a session refresh before a security-sensitive
// change like this, rather than relying solely on Kratos's
// `privileged_session_max_age` window. The shape: queue the pending
// change, redirect to `/signin?refresh=true&return_to=…`, let the
// user reauthenticate via whichever credential they have (password,
// SSO, passkey), then resume the change on return. Must work for
// SSO-only users, so a password-reentry shim alone is not enough.

const disableSequence = async () => {
// Step 1: Unlink TOTP.
const unlinkedFlow = await submitSettingsUpdate(flow, {
method: "totp",
totp_unlink: true,
csrf_token: mustGetCsrfTokenFromFlow(flow),
});

if (!unlinkedFlow) {
return;
}

// Step 2: Unlink the `lookup_secret` credential.
// `required_aal: highest_available` treats any enrolled second
// factor (including backup codes) as enforcing AAL2, so leaving
// them behind after removing TOTP would force the user through an
// AAL2 prompt with no TOTP to answer it. If this step fails, TOTP
// is already gone β€” surface an error so the user can retry rather
// than silently landing in the orphan-AAL2 state.
const clearedFlow = await submitSettingsUpdate(unlinkedFlow, {
method: "lookup_secret",
lookup_secret_disable: true,
csrf_token: mustGetCsrfTokenFromFlow(unlinkedFlow),
});

if (!clearedFlow) {
setErrorMessage(
"TOTP was disabled, but backup codes could not be removed. " +
"Please reload the page and try again to avoid being locked out.",
);
return;
}

setShowTotpDisableForm(false);
};

void disableSequence().finally(() => setDisablingTotp(false));
// Kratos enforces `privileged_session_max_age` server-side. If the
// session is stale, the first `submitSettingsUpdate` call inside
// `executeDisableTotp` will fail with `session_refresh_required` and
// the error handler will redirect to re-authenticate.
void executeDisableTotp(flow);
};

const handleRegenerateBackupCodes = () => {
Expand All @@ -466,6 +440,11 @@ const SecurityPage: NextPageWithLayout = () => {
if (regeneratedCodes.length > 0) {
setBackupCodes(regeneratedCodes);
setShowBackupCodesModal(true);
} else {
setErrorMessage(
"Could not extract backup codes from the response. " +
"Please reload the page and try again.",
);
}
})
.finally(() => setRegeneratingBackupCodes(false));
Expand Down Expand Up @@ -535,26 +514,6 @@ const SecurityPage: NextPageWithLayout = () => {
Password
</Typography>
<Box sx={{ display: "flex", flexDirection: "column", gap: 1.5 }}>
{!isRecoveryFlow && (
<TextField
label="Current password"
type="password"
autoComplete="current-password"
placeholder="Enter your current password"
value={currentPassword}
onChange={({ target }) => {
setCurrentPassword(target.value);
setCurrentPasswordError(undefined);
}}
error={!!currentPasswordError}
helperText={
currentPasswordError ? (
<Typography>{currentPasswordError}</Typography>
) : undefined
}
required
/>
)}
<TextField
label="New password"
type="password"
Expand All @@ -574,14 +533,7 @@ const SecurityPage: NextPageWithLayout = () => {
/>
</Box>
<Box mt={1.5}>
<Button
type="submit"
disabled={
(!isRecoveryFlow && !currentPassword) ||
!password ||
updatingPassword
}
>
<Button type="submit" disabled={!password || updatingPassword}>
{updatingPassword ? "Updating password..." : "Update password"}
</Button>
</Box>
Expand Down
21 changes: 11 additions & 10 deletions apps/hash-frontend/src/pages/shared/ory-kratos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export type FlowValues = Flows[FlowNames][0];
* does not serve these paths, redirects need to be rewritten to the
* matching `uiPath` to avoid dead-ending on a 404.
*/
export const flowMetadata = {
const _flowMetadata = {
login: {
uiPath: "/signin",
kratosBrowserPath: "/self-service/login/browser",
Expand All @@ -74,17 +74,17 @@ export const flowMetadata = {
uiPath: "/settings/security",
kratosBrowserPath: "/self-service/settings/browser",
},
// `settingsWithPassword` shares its UI route and Kratos endpoint with
// `settings`. The split is purely a TypeScript-level distinction over the
// submit-body shape (see `Flows[settingsWithPassword]`), not over routing.
settingsWithPassword: {
uiPath: "/settings/security",
kratosBrowserPath: "/self-service/settings/browser",
},
verification: {
uiPath: "/verification",
kratosBrowserPath: "/self-service/verification/browser",
},
} as const;

// `settingsWithPassword` shares routes with `settings` β€” the split is
// purely a TypeScript-level distinction over the submit-body shape.
export const flowMetadata = {
..._flowMetadata,
settingsWithPassword: _flowMetadata.settings,
} as const satisfies Record<
FlowNames,
{ uiPath: string; kratosBrowserPath: string }
Expand All @@ -106,7 +106,8 @@ export const uiPathForKratosBrowserRedirect = (
try {
parsed = new URL(redirectUrl);
} catch {
// Malformed URL β€” caller will fall back to the raw redirect string.
// eslint-disable-next-line no-console
console.warn("Malformed Kratos redirect URL:", redirectUrl);
return undefined;
}

Expand All @@ -118,7 +119,7 @@ export const uiPathForKratosBrowserRedirect = (
};

export const gatherUiNodeValuesFromFlow = <T extends FlowNames>(
flow: FlowValues,
flow: Flows[T][0],
): Flows[T][1] =>
flow.ui.nodes
.map(({ attributes }) => attributes)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import { useAuthInfo } from "./auth-info-context";
import type { Flows } from "./ory-kratos";
import { flowMetadata, uiPathForKratosBrowserRedirect } from "./ory-kratos";

export const useKratosErrorHandler = <S>(props: {
flowType: keyof Flows;
setFlow: Dispatch<SetStateAction<S | undefined>>;
export const useKratosErrorHandler = <K extends keyof Flows>(props: {
flowType: K;
setFlow: Dispatch<SetStateAction<Flows[K][0] | undefined>>;
setErrorMessage: Dispatch<SetStateAction<string | undefined>>;
}) => {
const { flowType, setFlow, setErrorMessage } = props;
Expand Down
Loading
Loading