From 5ff644db87e3d0e63bd085ae9b4ec175eff16029 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Wed, 30 Sep 2026 09:38:11 +0100 Subject: [PATCH] fix(settings): tell an SSO account its password lives at the identity provider (#850) The Change password card offered its three fields to an account with no password in OpenRisk, which only learned so after submitting (409 no_local_password). The card now reads has_password from /auth/me and shows the existing managed-by-IdP message up front, a skeleton while it finds out, and the old form plus the server's correction when it cannot tell. Signed-off-by: alex-dembele --- .../src/features/auth/ChangePasswordCard.tsx | 20 +++++-- .../__tests__/changePasswordCard.test.tsx | 58 ++++++++++++++++++- frontend/src/features/auth/authService.ts | 10 +++- .../src/features/auth/useHasLocalPassword.ts | 21 +++++++ 4 files changed, 101 insertions(+), 8 deletions(-) create mode 100644 frontend/src/features/auth/useHasLocalPassword.ts diff --git a/frontend/src/features/auth/ChangePasswordCard.tsx b/frontend/src/features/auth/ChangePasswordCard.tsx index 2a4fba30..f8c20162 100644 --- a/frontend/src/features/auth/ChangePasswordCard.tsx +++ b/frontend/src/features/auth/ChangePasswordCard.tsx @@ -7,6 +7,11 @@ // policy, as registration and reset: a password this card calls acceptable is // one the API accepts. The current password is the proof of identity, so a // wrong one is reported on that field and nowhere else. +// +// An account that signs in through an identity provider has no password here +// (#850): it is told so up front instead of being handed a form that can only +// fail. If that cannot be read, the form shows and the server's +// no_local_password answer switches the card, as before. import { useState } from 'react'; import { useForm, useWatch } from 'react-hook-form'; @@ -18,11 +23,13 @@ import { toast } from 'sonner'; import { useNavigate } from 'react-router'; import { Button, Field, Input, useSuccessFeedback } from '../../shared/ds'; +import { SkeletonRows } from '../../shared/ui'; import { useUIStore } from '../../store/uiStore'; import { useI18n } from '../../hooks/useI18n'; import { useAuthStore } from '../../hooks/useAuthStore'; import { setAccessToken } from '../../lib/session'; import { PasswordStrength } from './PasswordStrength'; +import { useHasLocalPassword } from './useHasLocalPassword'; import { changePassword, type ChangePasswordErrorBody, @@ -59,7 +66,10 @@ export function ChangePasswordCard() { const [visible, setVisible] = useState(false); const [acceptable, setAcceptable] = useState(false); const [assessment, setAssessment] = useState(null); - const [managedByIdp, setManagedByIdp] = useState(null); + const [refusedByServer, setRefusedByServer] = useState(null); + const hasPassword = useHasLocalPassword(); + // The server's own refusal wins; otherwise what /auth/me said. + const managedByIdp = refusedByServer ?? (hasPassword.data === false ? '' : null); const { feedback, flashSuccess } = useSuccessFeedback(); const { @@ -107,7 +117,7 @@ export function ChangePasswordCard() { setError('next', { message: body.error }); return; case 'no_local_password': - setManagedByIdp(body.error ?? ''); + setRefusedByServer(body.error ?? ''); return; default: if (isAxiosError(err) && err.response?.status === 429) { @@ -138,10 +148,12 @@ export function ChangePasswordCard() {

{t('accountSecurity.title')}

- {!managedByIdp && toggle} + {managedByIdp === null && !hasPassword.isLoading && toggle} - {managedByIdp !== null ? ( + {hasPassword.isLoading && refusedByServer === null ? ( + + ) : managedByIdp !== null ? (

rtlRender({ui}); +const render = (ui: React.ReactElement) => + rtlRender( + + {ui} + , + ); const changePassword = vi.fn(); +const fetchHasLocalPassword = vi.fn(); vi.mock('../authService', async () => { const actual = await vi.importActual('../authService'); - return { ...actual, changePassword: (...a: unknown[]) => changePassword(...a) }; + return { + ...actual, + changePassword: (...a: unknown[]) => changePassword(...a), + fetchHasLocalPassword: (...a: unknown[]) => fetchHasLocalPassword(...a), + }; }); // The meter talks to the server and loads zxcvbn; here it only reports a verdict. @@ -38,7 +51,7 @@ function axiosError(status: number, data: unknown) { } async function fill(current: string, next: string, confirm = next) { - await userEvent.type(screen.getByLabelText(/^Current password/), current); + await userEvent.type(await screen.findByLabelText(/^Current password/), current); await userEvent.type(screen.getByLabelText(/^New password/), next); await userEvent.type(screen.getByLabelText(/^Confirm the new password/), confirm); await userEvent.click(screen.getByTestId('change-password-submit')); @@ -46,6 +59,8 @@ async function fill(current: string, next: string, confirm = next) { beforeEach(() => { changePassword.mockReset(); + fetchHasLocalPassword.mockReset(); + fetchHasLocalPassword.mockResolvedValue(true); toastSuccess.mockReset(); useUIStore.getState().setLang('en'); }); @@ -105,4 +120,41 @@ describe('ChangePasswordCard', () => { ); expect(screen.queryByTestId('change-password-submit')).not.toBeInTheDocument(); }); + + describe('an account without a password in OpenRisk (#850)', () => { + it('says the identity provider owns the password and offers no fields', async () => { + fetchHasLocalPassword.mockResolvedValue(false); + render(); + + expect(await screen.findByTestId('password-managed-by-idp')).toHaveTextContent( + /identity provider/i, + ); + expect(screen.queryByLabelText(/^Current password/)).not.toBeInTheDocument(); + expect(screen.queryByTestId('change-password-submit')).not.toBeInTheDocument(); + expect(changePassword).not.toHaveBeenCalled(); + }); + + it('shows a skeleton, not the form, while it finds out', () => { + fetchHasLocalPassword.mockReturnValue(new Promise(() => {})); + render(); + + expect(screen.queryByLabelText(/^Current password/)).not.toBeInTheDocument(); + expect(screen.queryByTestId('password-managed-by-idp')).not.toBeInTheDocument(); + }); + + it('falls back to the form when it cannot tell, and the server still corrects it', async () => { + fetchHasLocalPassword.mockRejectedValue(new Error('offline')); + changePassword.mockRejectedValue( + axiosError(409, { code: 'no_local_password', error: 'Managed by your identity provider.' }), + ); + render(); + // One retry (1 s) before the card gives up and shows the form. + await screen.findByLabelText(/^Current password/, {}, { timeout: 3000 }); + await fill('anything-at-all', 'Violet-Kilimanjaro-Anchor-2026!'); + + expect(await screen.findByTestId('password-managed-by-idp')).toHaveTextContent( + 'Managed by your identity provider.', + ); + }); + }); }); diff --git a/frontend/src/features/auth/authService.ts b/frontend/src/features/auth/authService.ts index 7ceeb5a7..5a2912e6 100644 --- a/frontend/src/features/auth/authService.ts +++ b/frontend/src/features/auth/authService.ts @@ -241,8 +241,16 @@ export interface DisableMFAErrorBody { export type DisableMFAProof = 'password' | 'code'; export async function fetchDisableMFAProof(): Promise { + return (await fetchHasLocalPassword()) === false ? 'code' : 'password'; +} + +/** + * Whether the signed-in account has a password in OpenRisk (#754, #850). False + * for an identity-provider account; null when the server did not say. + */ +export async function fetchHasLocalPassword(): Promise { const { data } = await api.get<{ has_password?: boolean }>('/auth/me'); - return data?.has_password === false ? 'code' : 'password'; + return typeof data?.has_password === 'boolean' ? data.has_password : null; } /** diff --git a/frontend/src/features/auth/useHasLocalPassword.ts b/frontend/src/features/auth/useHasLocalPassword.ts new file mode 100644 index 00000000..52088c9c --- /dev/null +++ b/frontend/src/features/auth/useHasLocalPassword.ts @@ -0,0 +1,21 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// +// #850 — does this account have a password in OpenRisk at all? + +import { useQuery } from '@tanstack/react-query'; +import { fetchHasLocalPassword } from './authService'; + +/** + * Read fresh when a password surface mounts. `data` is null when the server did + * not say; callers treat that like "has a password" and let the server's + * no_local_password answer correct them. + */ +export function useHasLocalPassword() { + return useQuery({ + queryKey: ['auth', 'has-local-password'], + queryFn: fetchHasLocalPassword, + staleTime: 0, + retry: 1, + }); +}