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
16 changes: 16 additions & 0 deletions .changeset/29161-impersonation-banner-admin-name.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
'@object-ui/app-shell': patch
'@object-ui/i18n': patch
---

fix(app-shell): the impersonation banner names the administrator by name or email, never by a raw user id

Clause-②: yes (widening)
Metadata-protocol: none

An impersonated session carries the administrator only as `session.impersonatedBy`, a `sys_user` id, and the banner printed that id ("Started by administrator 963Jhis…"). Neither the administrator nor anyone watching the screen could tell who started the impersonation.

- **Named from the user's row.** The banner reads the administrator's `sys_user` row once (`GET /api/v1/data/sys_user/ID`, sent as the session the console holds) and names them by their name, else their email: "Started by administrator Dev Admin."
- **An honest sentence when the row cannot be read.** The read is made as the impersonated user, whose row scope may not reach the administrator's row. A refused, missing or failed read, or a row with neither a name nor an email, shows "Started by an administrator." The raw id is never shown, and the sentence is held back until the read answers.
- **The remediation overlay is unchanged.** It shares the exit with the banner but names nobody, so it issues no read.
- **New locale key** in every `@object-ui/i18n` pack: `impersonation.banner.startedByUnnamed`.
88 changes: 82 additions & 6 deletions ui/packages/app-shell/src/layout/ImpersonationBanner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,24 @@
* whose `/home` showed no sign of impersonation at all, so a home that could
* not carry the indicator would have reproduced the bug.
*
* ## The administrator is named by a person's name, never by an id
*
* The session carries the administrator only as `impersonatedBy`, a `sys_user`
* id, which tells neither the administrator nor anyone watching the screen who
* started this. The banner therefore reads that one `sys_user` row and names it
* by its name, else its email. The read is made AS the impersonated user (that
* is the session the console holds), and their row scope may not reach the
* administrator's row: a refused, missing or failed read says "an
* administrator" instead. ⛔ The raw id is never shown, and nothing is shown for
* the administrator until the read has answered, so the sentence never flips
* from an id or a guess to the name.
*
* The read is a raw authenticated `GET` on the `/api/v1` lane, the lane the
* exit uses: this banner mounts in `ConsoleShell`, above `AdapterProvider`, so
* the data adapter does not exist here. It lives in the banner rather than in
* `useImpersonationExit` because the remediation overlay shares that hook and
* names nobody, so it must not issue the read.
*
* ## The exit fails LOUDLY
*
* `POST /auth/admin/stop-impersonating` restores the administrator from the
Expand All @@ -34,18 +52,70 @@
* the operator would go back to ordinary work believing they were themselves.
*/

import { useEffect, useState } from 'react';
import { UserCog } from 'lucide-react';
import { Button } from '@object-ui/components';
import { createAuthenticatedFetch } from '@object-ui/auth';
import { GetDataResponseSchema } from '@objectstack/spec/api';
import { useObjectTranslation } from '@object-ui/i18n';
import { PERSON_DISPLAY_FIELDS, rowLabel } from '../utils/approverIdentity.js';
import { useImpersonationExit } from './useImpersonationExit.js';

/**
* How the banner names the administrator: `{ name }` once the `sys_user` read
* answered with a name or email, `null` once it answered without one (refused,
* missing, failed), `undefined` while it is still being asked.
*/
type AdministratorName = { name: string } | null | undefined;

/**
* Read the administrator's `sys_user` row once per administrator id. See the
* module header for why the read is made here and how a refusal reads.
*/
function useAdministratorName(adminId: string | undefined): AdministratorName {
// The answer is kept with the id it answers for, so a later impersonation by
// another administrator never shows the previous one's name.
const [answer, setAnswer] = useState<{ id: string; name: { name: string } | null } | null>(null);

useEffect(() => {
if (!adminId) return;
let cancelled = false;
void (async () => {
let name: string | null = null;
try {
const baseUrl = import.meta.env.VITE_SERVER_URL || '';
const res = await createAuthenticatedFetch()(
`${baseUrl}/api/v1/data/sys_user/${encodeURIComponent(adminId)}`,
{ method: 'GET', headers: { Accept: 'application/json' }, credentials: 'include' },
);
if (res.ok) {
// The data route answers the spec's `GetDataResponse` body; anything
// else names nobody.
const body = GetDataResponseSchema.safeParse(await res.json());
if (body.success) name = rowLabel(body.data.record, PERSON_DISPLAY_FIELDS) || null;
}
} catch {
// A failed read names nobody; the banner says "an administrator".
}
if (!cancelled) setAnswer({ id: adminId, name: name ? { name } : null });
})();
return () => {
cancelled = true;
};
}, [adminId]);

if (!adminId || answer === null || answer.id !== adminId) return undefined;
return answer.name;
}

export function ImpersonationBanner() {
const { t } = useObjectTranslation();
// The exit itself — request, token adoption, outcome — is shared with the
// remediation overlay, which covers this banner when the session is held
// (objectui#13208). See `useImpersonationExit`.
const { impersonatedBy, impersonatedName, stop, stopping, stopError, notRestored } =
useImpersonationExit();
const administrator = useAdministratorName(impersonatedBy);

// Nothing to say on an ordinary session. Below every hook, so the hook order
// is stable across the transition out of impersonation.
Expand All @@ -65,12 +135,18 @@ export function ImpersonationBanner() {
user: impersonatedName,
})}
</span>{' '}
<span className="opacity-80">
{t('impersonation.banner.startedBy', {
defaultValue: 'Started by administrator {{admin}}.',
admin: impersonatedBy,
})}
</span>
{administrator === undefined ? null : (
<span className="opacity-80" data-testid="impersonation-started-by">
{administrator
? t('impersonation.banner.startedBy', {
defaultValue: 'Started by administrator {{admin}}.',
admin: administrator.name,
})
: t('impersonation.banner.startedByUnnamed', {
defaultValue: 'Started by an administrator.',
})}
</span>
)}
</p>
<Button
size="sm"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,26 @@ function tokenBoundClient(): AuthClient {
} as unknown as AuthClient;
}

/** Stub `fetch` for the stop-impersonating call. */
const STOP_PATH = '/api/v1/auth/admin/stop-impersonating';

/** The administrator's `sys_user` row, read by the banner to name them. */
const ADMIN_READ_PATH = '/api/v1/data/sys_user/usr_admin';

function jsonResponse(body: unknown, status = 200): Response {
return new Response(JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' } });
}

/**
* The data route's answer for the administrator's row: the spec's
* `GetDataResponse` body, unwrapped, as a live server answers it.
*/
function adminRow(fields: Record<string, unknown>): Response {
return jsonResponse({ object: 'sys_user', id: 'usr_admin', record: { id: 'usr_admin', ...fields } });
}

/** Stub `fetch` for the stop-impersonating call and the administrator read. */
function stubStopEndpoint(
respond: (url: string) => Response,
respond: (url: string) => Response | Promise<Response>,
): { urls: string[]; bearers: (string | null)[] } {
const urls: string[] = [];
const bearers: (string | null)[] = [];
Expand Down Expand Up @@ -138,34 +155,91 @@ describe('ImpersonationBanner (#4467)', () => {

it('names BOTH parties while the session is impersonated', async () => {
TokenStorage.set(IMPERSONATED_TOKEN);
const seen = stubStopEndpoint((url) =>
url.endsWith(ADMIN_READ_PATH) ? adminRow({ name: 'Dev Admin', email: 'admin@objectos.ai' }) : new Response('{}'),
);
renderBanner();

const banner = await screen.findByTestId('impersonation-banner');
// The impersonated user — whose name every write is recorded under.
expect(banner.textContent).toMatch(/RT8 Dave/);
// The administrator who started it, as the session reports them.
expect(banner.textContent).toMatch(/usr_admin/);
// The administrator who started it, by name: the session carries only the id.
const startedBy = await screen.findByTestId('impersonation-started-by');
expect(startedBy.textContent).toBe('Started by administrator Dev Admin.');
expect(banner.textContent).not.toMatch(/usr_admin/);
// The read went out as the session the console holds: the impersonated user's.
const read = seen.urls.findIndex((u) => u.endsWith(ADMIN_READ_PATH));
expect(read).toBeGreaterThanOrEqual(0);
expect(seen.bearers[read]).toBe(`Bearer ${IMPERSONATED_TOKEN}`);
// And an exit.
expect(screen.getByTestId('impersonation-stop')).toBeTruthy();
});

it('names the administrator by email when their row carries no name', async () => {
TokenStorage.set(IMPERSONATED_TOKEN);
stubStopEndpoint((url) =>
url.endsWith(ADMIN_READ_PATH) ? adminRow({ name: '', email: 'admin@objectos.ai' }) : new Response('{}'),
);
renderBanner();

const startedBy = await screen.findByTestId('impersonation-started-by');
expect(startedBy.textContent).toBe('Started by administrator admin@objectos.ai.');
});

it.each([
// What a user outside the administrator's organization gets: the row scope
// hides the row, and the route answers it as missing.
['outside the reader\'s row scope', () => jsonResponse({ error: 'Record usr_admin not found in sys_user', code: 'RECORD_NOT_FOUND', object: 'sys_user' }, 404)],
['refused', () => jsonResponse({ error: 'Permission denied', code: 'PERMISSION_DENIED' }, 403)],
['answered off the GetDataResponse shape', () => jsonResponse({ success: true, data: { name: 'Dev Admin' } })],
['answered without a name or email', () => adminRow({})],
['a network failure', () => {
throw new TypeError('Failed to fetch');
}],
])('says "an administrator", never the id, when the read is %s', async (_case, answer) => {
TokenStorage.set(IMPERSONATED_TOKEN);
stubStopEndpoint((url) => (url.endsWith(ADMIN_READ_PATH) ? answer() : new Response('{}')));
renderBanner();

const startedBy = await screen.findByTestId('impersonation-started-by');
expect(startedBy.textContent).toBe('Started by an administrator.');
expect(screen.getByTestId('impersonation-banner').textContent).not.toMatch(/usr_admin/);
});

it('shows no administrator at all while the read is still out', async () => {
TokenStorage.set(IMPERSONATED_TOKEN);
stubStopEndpoint((url) => {
if (url.endsWith(ADMIN_READ_PATH)) return new Promise<Response>(() => {});
return new Response('{}');
});
renderBanner();

const banner = await screen.findByTestId('impersonation-banner');
await new Promise((r) => setTimeout(r, 0));
expect(screen.queryByTestId('impersonation-started-by')).toBeNull();
expect(banner.textContent).not.toMatch(/usr_admin/);
});

it('stop: calls the endpoint, adopts the restored token, refreshes identity, banner goes', async () => {
TokenStorage.set(IMPERSONATED_TOKEN);
const seen = stubStopEndpoint(() =>
const seen = stubStopEndpoint((url) =>
// The server hands the ADMINISTRATOR's session back the same way it
// handed the impersonated one out: a rotation on the response header.
new Response('{}', { status: 200, headers: { 'set-auth-token': ADMIN_TOKEN } }),
// handed the impersonated one out: a rotation on the stop's response
// header, and on no other route.
url.endsWith(STOP_PATH)
? new Response('{}', { status: 200, headers: { 'set-auth-token': ADMIN_TOKEN } })
: new Response('{}'),
);
renderBanner();

await screen.findByTestId('impersonation-banner');
await userEvent.click(screen.getByTestId('impersonation-stop'));

await waitFor(() => expect(screen.queryByTestId('impersonation-banner')).toBeNull());
expect(seen.urls.some((u) => u.endsWith('/api/v1/auth/admin/stop-impersonating'))).toBe(true);
expect(seen.urls.some((u) => u.endsWith(STOP_PATH))).toBe(true);
// The request went out AS the impersonated user (that is the session we
// hold); the administrator is restored from the `admin_session` cookie.
expect(seen.bearers[0]).toBe(`Bearer ${IMPERSONATED_TOKEN}`);
expect(seen.bearers[seen.urls.findIndex((u) => u.endsWith(STOP_PATH))]).toBe(`Bearer ${IMPERSONATED_TOKEN}`);
// The rotation was adopted — this is the lane fix doing the work.
expect(TokenStorage.get()).toBe(ADMIN_TOKEN);
});
Expand Down Expand Up @@ -208,8 +282,10 @@ describe('ImpersonationBanner (#4467)', () => {

it('a LATER impersonation, after a stop that worked, is not reported as a failed stop', async () => {
TokenStorage.set(IMPERSONATED_TOKEN);
stubStopEndpoint(() =>
new Response('{}', { status: 200, headers: { 'set-auth-token': ADMIN_TOKEN } }),
stubStopEndpoint((url) =>
url.endsWith(STOP_PATH)
? new Response('{}', { status: 200, headers: { 'set-auth-token': ADMIN_TOKEN } })
: new Response('{}'),
);
renderBanner();

Expand Down
1 change: 1 addition & 0 deletions ui/packages/i18n/src/locales/ar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const ar = {
banner: {
message: 'أنت تنتحل هوية {{user}} — يُسجَّل كل إجراء باسمه.',
startedBy: 'بدأها المسؤول {{admin}}.',
startedByUnnamed: 'بدأها أحد المسؤولين.',
stop: 'إنهاء انتحال الهوية',
stopping: 'جارٍ الإنهاء…',
stopFailed: 'تعذّر إنهاء انتحال الهوية: {{reason}}',
Expand Down
1 change: 1 addition & 0 deletions ui/packages/i18n/src/locales/de.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const de = {
banner: {
message: 'Sie handeln als {{user}} — jede Aktion wird dieser Person zugeschrieben.',
startedBy: 'Gestartet von Administrator {{admin}}.',
startedByUnnamed: 'Von einem Administrator gestartet.',
stop: 'Identitätswechsel beenden',
stopping: 'Wird beendet…',
stopFailed: 'Identitätswechsel konnte nicht beendet werden: {{reason}}',
Expand Down
1 change: 1 addition & 0 deletions ui/packages/i18n/src/locales/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const en = {
banner: {
message: 'You are impersonating {{user}} — every action is recorded as them.',
startedBy: 'Started by administrator {{admin}}.',
startedByUnnamed: 'Started by an administrator.',
stop: 'Stop impersonating',
stopping: 'Stopping…',
stopFailed: 'Could not stop impersonating: {{reason}}',
Expand Down
1 change: 1 addition & 0 deletions ui/packages/i18n/src/locales/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const es = {
banner: {
message: 'Está suplantando a {{user}}: todas las acciones se registran a su nombre.',
startedBy: 'Iniciado por el administrador {{admin}}.',
startedByUnnamed: 'Iniciado por un administrador.',
stop: 'Dejar de suplantar',
stopping: 'Finalizando…',
stopFailed: 'No se pudo dejar de suplantar: {{reason}}',
Expand Down
1 change: 1 addition & 0 deletions ui/packages/i18n/src/locales/fr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const fr = {
banner: {
message: 'Vous incarnez {{user}} — chaque action est enregistrée en son nom.',
startedBy: 'Démarré par l’administrateur {{admin}}.',
startedByUnnamed: 'Démarré par un administrateur.',
stop: 'Arrêter l’usurpation',
stopping: 'Arrêt en cours…',
stopFailed: 'Impossible d’arrêter l’usurpation : {{reason}}',
Expand Down
1 change: 1 addition & 0 deletions ui/packages/i18n/src/locales/ja.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const ja = {
banner: {
message: '{{user}} になりすまして操作しています。すべての操作はこのユーザーとして記録されます。',
startedBy: '管理者 {{admin}} が開始しました。',
startedByUnnamed: '管理者が開始しました。',
stop: 'なりすましを終了',
stopping: '終了しています…',
stopFailed: 'なりすましを終了できませんでした: {{reason}}',
Expand Down
1 change: 1 addition & 0 deletions ui/packages/i18n/src/locales/ko.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const ko = {
banner: {
message: '{{user}} 계정을 대행하고 있습니다. 모든 작업이 해당 사용자로 기록됩니다.',
startedBy: '관리자 {{admin}}이(가) 시작했습니다.',
startedByUnnamed: '관리자가 시작했습니다.',
stop: '대행 종료',
stopping: '종료하는 중…',
stopFailed: '대행을 종료할 수 없습니다: {{reason}}',
Expand Down
1 change: 1 addition & 0 deletions ui/packages/i18n/src/locales/pt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const pt = {
banner: {
message: 'Você está personificando {{user}} — todas as ações são registradas em nome dele.',
startedBy: 'Iniciado pelo administrador {{admin}}.',
startedByUnnamed: 'Iniciado por um administrador.',
stop: 'Parar personificação',
stopping: 'Encerrando…',
stopFailed: 'Não foi possível parar a personificação: {{reason}}',
Expand Down
1 change: 1 addition & 0 deletions ui/packages/i18n/src/locales/ru.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const ru = {
banner: {
message: 'Вы работаете от имени {{user}} — все действия записываются на этого пользователя.',
startedBy: 'Начато администратором {{admin}}.',
startedByUnnamed: 'Начато администратором.',
stop: 'Прекратить работу от чужого имени',
stopping: 'Завершение…',
stopFailed: 'Не удалось прекратить работу от чужого имени: {{reason}}',
Expand Down
1 change: 1 addition & 0 deletions ui/packages/i18n/src/locales/zh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const zh = {
banner: {
message: '您正在以 {{user}} 的身份操作 —— 所有操作都会记录为该用户。',
startedBy: '由管理员 {{admin}} 发起。',
startedByUnnamed: '由管理员发起。',
stop: '结束模拟登录',
stopping: '正在结束…',
stopFailed: '无法结束模拟登录:{{reason}}',
Expand Down
Loading