provision when the account list changes, not only when you connect
Receiving users:list updated an in-memory array and did nothing else, so a person added to Officer next month reached Photos only if somebody remembered to re-run provisioning by hand. The platform already broadcasts on every create and delete, and re-sends on every sidecar registration — nothing was listening. Now it acts on it. A restart is therefore also a reconciliation, which is the cheap way to recover from any push that was missed while the sidecar was down. An automatic run is NOT allowed to key an adopted account. Keying one means resetting its password, because that is the only way Immich issues a key, and for an account that pre-dated Officer that takes away a login somebody is using — most likely in Immich's own app. Doing that unattended, to a person who did not know it was at stake, is not the same act as the owner choosing it: so the account is linked, left keyless, and named in `pending`. The deliberate /provision route still does the whole job. Tested against the case this is for: jrg.conceicao has an Immich library and no Officer account today. When that account exists, the plan adopts the library rather than building a second one beside it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
+2
-2
@@ -161,7 +161,7 @@ async function addAccount(req: Request, userId: number): Promise<Response> {
|
|||||||
// The trigger is registering an ADMIN connection, not installing the plugin: at install time there is
|
// The trigger is registering an ADMIN connection, not installing the plugin: at install time there is
|
||||||
// no credential, so there is nothing to provision with. Awaited, so the response says what happened —
|
// no credential, so there is nothing to provision with. Awaited, so the response says what happened —
|
||||||
// and, importantly, carries the temporary passwords, which exist nowhere else.
|
// and, importantly, carries the temporary passwords, which exist nowhere else.
|
||||||
const provisioning = await runProvisioning({ cfg: { base: url, key: apiKey }, configId: account.id });
|
const provisioning = await runProvisioning({ cfg: { base: url, key: apiKey }, configId: account.id, keyAdopted: true });
|
||||||
return Response.json({ account, user: result.user, provisioning });
|
return Response.json({ account, user: result.user, provisioning });
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
if (duplicateLabel(err)) return bad(`you already have an account called "${label}"`);
|
if (duplicateLabel(err)) return bad(`you already have an account called "${label}"`);
|
||||||
@@ -211,7 +211,7 @@ async function reprovision(userId: number, id: number): Promise<Response> {
|
|||||||
if (!creds) return bad('no such account', 404);
|
if (!creds) return bad('no such account', 404);
|
||||||
|
|
||||||
const cfg = { base: normalizeBase(creds.url), key: creds.apiKey };
|
const cfg = { base: normalizeBase(creds.url), key: creds.apiKey };
|
||||||
const provisioning = await runProvisioning({ cfg, configId: id });
|
const provisioning = await runProvisioning({ cfg, configId: id, keyAdopted: true });
|
||||||
const keys = await backfillAccountKeys({ cfg, configId: id });
|
const keys = await backfillAccountKeys({ cfg, configId: id });
|
||||||
|
|
||||||
return Response.json({ provisioning, keys });
|
return Response.json({ provisioning, keys });
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import { handleLockedRoute } from './locked';
|
|||||||
import { handleOfficerRoute } from './routes';
|
import { handleOfficerRoute } from './routes';
|
||||||
import { getConfig } from './upstream';
|
import { getConfig } from './upstream';
|
||||||
import { setProvisionableUsers } from './user-list';
|
import { setProvisionableUsers } from './user-list';
|
||||||
|
import { provisionForAccountChange } from './provision-run';
|
||||||
import { API_URL } from '@@/officer-url.mjs';
|
import { API_URL } from '@@/officer-url.mjs';
|
||||||
|
|
||||||
// The officer-photos sidecar. Owns the whole Immich contract for Officer: the instance URL and the API key.
|
// The officer-photos sidecar. Owns the whole Immich contract for Officer: the instance URL and the API key.
|
||||||
@@ -155,6 +156,10 @@ function handleCommand(cmd: SidecarCommand, reply: ReplyFn) {
|
|||||||
// not a question.
|
// not a question.
|
||||||
case 'users:list':
|
case 'users:list':
|
||||||
setProvisionableUsers(cmd.users);
|
setProvisionableUsers(cmd.users);
|
||||||
|
// And act on it. Detached deliberately: this reaches out to Immich and can take seconds, while the
|
||||||
|
// command handler is on the socket the platform is waiting on. Nothing replies to `users:list`, so
|
||||||
|
// there is no answer to delay — but blocking here would stall every later message on this socket.
|
||||||
|
void provisionForAccountChange();
|
||||||
break;
|
break;
|
||||||
default:
|
default:
|
||||||
reply({
|
reply({
|
||||||
|
|||||||
+108
-6
@@ -1,7 +1,13 @@
|
|||||||
import { linkPhotosAccount, listPhotosAccountLinks, setPhotosAccountKey } from '../db/queries';
|
import { OWNER_USER_ID } from 'officerdb';
|
||||||
|
import {
|
||||||
|
getActivePhotosCredentials,
|
||||||
|
linkPhotosAccount,
|
||||||
|
listPhotosAccountLinks,
|
||||||
|
setPhotosAccountKey,
|
||||||
|
} from '../db/queries';
|
||||||
import { establishKey, provision, type ProvisionOutcome } from './provision';
|
import { establishKey, provision, type ProvisionOutcome } from './provision';
|
||||||
import { getProvisionableUsers, hasProvisionableUsers } from './user-list';
|
import { getProvisionableUsers, hasProvisionableUsers } from './user-list';
|
||||||
import { invalidateConfig, type UpstreamTarget } from './upstream';
|
import { invalidateConfig, normalizeBase, type UpstreamTarget } from './upstream';
|
||||||
|
|
||||||
// The one place that turns "an admin just connected" into Immich accounts and stored keys.
|
// The one place that turns "an admin just connected" into Immich accounts and stored keys.
|
||||||
//
|
//
|
||||||
@@ -32,6 +38,12 @@ export type ProvisioningReport =
|
|||||||
log: string[];
|
log: string[];
|
||||||
/** One entry per person who now has a key. Carries a live credential — do not log it. */
|
/** One entry per person who now has a key. Carries a live credential — do not log it. */
|
||||||
handover: Handover[];
|
handover: Handover[];
|
||||||
|
/**
|
||||||
|
* Linked, but deliberately left without a key: an account that pre-dated Officer, found by a run
|
||||||
|
* that was not allowed to touch its password. They reach Photos once somebody runs the deliberate
|
||||||
|
* provision. Named rather than counted, because the owner has to know WHO to go and do.
|
||||||
|
*/
|
||||||
|
pending: string[];
|
||||||
};
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -60,11 +72,24 @@ async function linkAndKey(params: {
|
|||||||
return { userId, email, temporaryPassword };
|
return { userId, email, temporaryPassword };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* `keyAdopted` decides what happens to an account that existed before Officer did.
|
||||||
|
*
|
||||||
|
* Keying one means resetting its password, because that is the only way Immich will issue a key. For an
|
||||||
|
* account Officer just created that costs nothing — nobody has ever held that password. For an adopted one
|
||||||
|
* it takes away a login somebody is using, most likely in Immich's own app, and hands back a replacement
|
||||||
|
* that only reaches them if the owner passes it on.
|
||||||
|
*
|
||||||
|
* So it is TRUE when a person asked for it (the `/provision` route) and FALSE when the trigger was a
|
||||||
|
* background event (a new account appearing). An automatic action must not reset a password nobody
|
||||||
|
* knew was at stake; the adopted account is linked and named in `pending` instead.
|
||||||
|
*/
|
||||||
export async function runProvisioning(params: {
|
export async function runProvisioning(params: {
|
||||||
cfg: UpstreamTarget;
|
cfg: UpstreamTarget;
|
||||||
configId: number;
|
configId: number;
|
||||||
|
keyAdopted: boolean;
|
||||||
}): Promise<ProvisioningReport> {
|
}): Promise<ProvisioningReport> {
|
||||||
const { cfg, configId } = params;
|
const { cfg, configId, keyAdopted } = params;
|
||||||
|
|
||||||
// Never told, versus told there is nobody. The first means the push has not arrived or this plugin is
|
// Never told, versus told there is nobody. The first means the push has not arrived or this plugin is
|
||||||
// not entitled to it; the second is a server with only an owner.
|
// not entitled to it; the second is a server with only an owner.
|
||||||
@@ -84,18 +109,45 @@ export async function runProvisioning(params: {
|
|||||||
});
|
});
|
||||||
|
|
||||||
const handover: Handover[] = [];
|
const handover: Handover[] = [];
|
||||||
|
const pending: string[] = [];
|
||||||
const failed = [...outcome.failed];
|
const failed = [...outcome.failed];
|
||||||
|
|
||||||
for (const entry of [...outcome.linked, ...outcome.created]) {
|
// Created accounts always get a key: their password was set moments ago by this process and has never
|
||||||
const adopted = outcome.created.every((c) => c.userId !== entry.userId);
|
// been anybody's, so spending it costs nothing.
|
||||||
|
for (const entry of outcome.created) {
|
||||||
try {
|
try {
|
||||||
handover.push(await linkAndKey({ cfg, configId, adopted, ...entry }));
|
handover.push(await linkAndKey({ cfg, configId, adopted: false, ...entry }));
|
||||||
|
} catch (error) {
|
||||||
|
failed.push({ email: entry.email, error: error instanceof Error ? error.message : String(error) });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Adopted ones are recorded either way — the pairing is a fact and costs nothing to write. Whether they
|
||||||
|
// also get a key is the caller's decision, for the reason on `keyAdopted` above.
|
||||||
|
for (const entry of outcome.linked) {
|
||||||
|
try {
|
||||||
|
if (keyAdopted) {
|
||||||
|
handover.push(await linkAndKey({ cfg, configId, adopted: true, ...entry }));
|
||||||
|
} else {
|
||||||
|
await linkPhotosAccount({
|
||||||
|
userId: entry.userId,
|
||||||
|
configId,
|
||||||
|
immichUserId: entry.immichUserId,
|
||||||
|
immichEmail: entry.email,
|
||||||
|
adopted: true,
|
||||||
|
});
|
||||||
|
pending.push(entry.email);
|
||||||
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
failed.push({ email: entry.email, error: error instanceof Error ? error.message : String(error) });
|
failed.push({ email: entry.email, error: error instanceof Error ? error.message : String(error) });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
for (const line of outcome.log) console.log(`[photos:provision] ${line}`);
|
for (const line of outcome.log) console.log(`[photos:provision] ${line}`);
|
||||||
|
for (const email of pending) {
|
||||||
|
console.log(`[photos:provision] ${email} is linked but has no key — it had an Immich account already.`);
|
||||||
|
console.log(`[photos:provision] run POST /_config/${configId}/provision to give them one.`);
|
||||||
|
}
|
||||||
for (const f of failed) console.error(`[photos:provision] ${f.email}: ${f.error}`);
|
for (const f of failed) console.error(`[photos:provision] ${f.email}: ${f.error}`);
|
||||||
|
|
||||||
return {
|
return {
|
||||||
@@ -106,6 +158,7 @@ export async function runProvisioning(params: {
|
|||||||
failed,
|
failed,
|
||||||
log: outcome.log,
|
log: outcome.log,
|
||||||
handover,
|
handover,
|
||||||
|
pending,
|
||||||
};
|
};
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
const reason = error instanceof Error ? error.message : String(error);
|
const reason = error instanceof Error ? error.message : String(error);
|
||||||
@@ -114,6 +167,55 @@ export async function runProvisioning(params: {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Provision because the ACCOUNT LIST changed, rather than because somebody asked.
|
||||||
|
*
|
||||||
|
* This is what makes an Officer account created next month reach Photos without anyone remembering that a
|
||||||
|
* plugin needed telling. `users:list` arrives on every create and delete — and on every sidecar
|
||||||
|
* registration, so a restart is also a reconciliation.
|
||||||
|
*
|
||||||
|
* Three properties it has to have, because nobody is watching it run:
|
||||||
|
*
|
||||||
|
* - **Idempotent.** The plan shrinks as links are recorded, so the common case does no upstream writes
|
||||||
|
* at all and the run costs two GETs.
|
||||||
|
* - **Never destructive.** `keyAdopted: false` — see `runProvisioning`. An account that pre-dated
|
||||||
|
* Officer keeps its password; the owner is told it is waiting.
|
||||||
|
* - **Quiet about the outcome, not about the plan.** `runProvisioning` logs its plan either way, so a
|
||||||
|
* restart prints a few lines describing the world as this plugin sees it. Only the "something
|
||||||
|
* changed" summary below is suppressed when nothing did, which is what stops a run that mattered from
|
||||||
|
* reading like the dozen that did not.
|
||||||
|
*
|
||||||
|
* Never throws. It runs detached from the command handler, and a plugin failing to provision must not take
|
||||||
|
* down the socket carrying the message.
|
||||||
|
*/
|
||||||
|
export async function provisionForAccountChange(): Promise<void> {
|
||||||
|
try {
|
||||||
|
// The instance members inherit is the owner's selected one. A member who registered an Immich of their
|
||||||
|
// own is not a provisioning target — that is their connection, not a shared library.
|
||||||
|
const creds = await getActivePhotosCredentials(OWNER_USER_ID);
|
||||||
|
if (!creds) return; // nothing connected yet; connecting is itself a provisioning trigger
|
||||||
|
|
||||||
|
const report = await runProvisioning({
|
||||||
|
cfg: { base: normalizeBase(creds.url), key: creds.apiKey },
|
||||||
|
configId: creds.id,
|
||||||
|
keyAdopted: false,
|
||||||
|
});
|
||||||
|
|
||||||
|
if (!report.ran) {
|
||||||
|
console.log(`[photos:provision] not run — ${report.reason}`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (report.created === 0 && report.linked === 0 && report.failed.length === 0) return; // nothing changed
|
||||||
|
|
||||||
|
console.log(
|
||||||
|
`[photos:provision] account list changed: ${report.created} created, ${report.linked} linked, ` +
|
||||||
|
`${report.pending.length} awaiting a key`,
|
||||||
|
);
|
||||||
|
} catch (error) {
|
||||||
|
console.error('[photos:provision] automatic run failed:', error instanceof Error ? error.message : error);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export type BackfillReport = {
|
export type BackfillReport = {
|
||||||
minted: Handover[];
|
minted: Handover[];
|
||||||
/** Already had a key. Reported rather than silent, so a re-run visibly does nothing. */
|
/** Already had a key. Reported rather than silent, so a re-run visibly does nothing. */
|
||||||
|
|||||||
@@ -109,6 +109,34 @@ describe('matching', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The planned case: an Immich account that exists today with no Officer login, whose person gets an
|
||||||
|
// Officer account later. Provisioning must adopt it rather than build a second library beside it.
|
||||||
|
describe('an Immich-only account whose owner joins Officer later', () => {
|
||||||
|
const joined = [...PLATFORM, user(28, 'jorge', 'jrg.conceicao@gmail.com')];
|
||||||
|
const plan = planReconcile({
|
||||||
|
users: joined,
|
||||||
|
immichUsers: IMMICH,
|
||||||
|
existing: [
|
||||||
|
{ userId: 24, immichUserId: 'aaaa1111' },
|
||||||
|
{ userId: 26, immichUserId: 'bbbb2222' },
|
||||||
|
],
|
||||||
|
adminImmichUserId: ADMIN_ID,
|
||||||
|
});
|
||||||
|
|
||||||
|
test('links them to the photographs they already have', () => {
|
||||||
|
expect(plan.link.map((l) => l.user.id)).toEqual([28]);
|
||||||
|
expect(plan.link[0]!.immich.id).toBe('cccc3333');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('does not create a second account for the same address', () => {
|
||||||
|
expect(plan.create.map((u) => u.email)).not.toContain('jrg.conceicao@gmail.com');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('stops reporting them as belonging to nobody', () => {
|
||||||
|
expect(plan.leave).toHaveLength(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('an account deleted in Immich behind Officer’s back', () => {
|
describe('an account deleted in Immich behind Officer’s back', () => {
|
||||||
const plan = planReconcile({
|
const plan = planReconcile({
|
||||||
users: PLATFORM,
|
users: PLATFORM,
|
||||||
|
|||||||
Reference in New Issue
Block a user