Archived
fix(auth): guard post-login status refresh
Skip opportunistic status requests when the flow's originating machine is no longer selected, and discard in-flight status results after the machine or session selection changes.\n\nRefs #74
This commit is contained in:
@@ -121,11 +121,7 @@ describe("AuthController", () => {
|
|||||||
const statusCalls: { session: Parameters<typeof defaultApi.status>[0]; machineId: string | undefined }[] = [];
|
const statusCalls: { session: Parameters<typeof defaultApi.status>[0]; machineId: string | undefined }[] = [];
|
||||||
const appliedStatuses: SessionStatus[] = [];
|
const appliedStatuses: SessionStatus[] = [];
|
||||||
const { controller, getState } = createController(
|
const { controller, getState } = createController(
|
||||||
{
|
{ selectedSession: session, authDialog: { step: "oauth", flow, machineId: "local", inputValue: "https://callback" } },
|
||||||
selectedMachine: remoteMachine("remote-2"),
|
|
||||||
selectedSession: session,
|
|
||||||
authDialog: { step: "oauth", flow, machineId: "remote-1", inputValue: "https://callback" },
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
respondOAuthFlow: (flowId, requestId, value, machineId) => {
|
respondOAuthFlow: (flowId, requestId, value, machineId) => {
|
||||||
respondCalls.push({ flowId, requestId, value, machineId });
|
respondCalls.push({ flowId, requestId, value, machineId });
|
||||||
@@ -142,12 +138,75 @@ describe("AuthController", () => {
|
|||||||
await controller.respondOAuth();
|
await controller.respondOAuth();
|
||||||
await flushMicrotasks();
|
await flushMicrotasks();
|
||||||
|
|
||||||
expect(respondCalls).toEqual([{ flowId: "flow-1", requestId: "request-1", value: "https://callback", machineId: "remote-1" }]);
|
expect(respondCalls).toEqual([{ flowId: "flow-1", requestId: "request-1", value: "https://callback", machineId: "local" }]);
|
||||||
expect(getState().authDialog).toBeUndefined();
|
expect(getState().authDialog).toBeUndefined();
|
||||||
expect(statusCalls).toEqual([{ session, machineId: "remote-1" }]);
|
expect(statusCalls).toEqual([{ session, machineId: "local" }]);
|
||||||
expect(appliedStatuses).toEqual([refreshedStatus]);
|
expect(appliedStatuses).toEqual([refreshedStatus]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("does not refresh a session from another selected machine when a flow completes", async () => {
|
||||||
|
const flow = oauthFlow({ prompt: { requestId: "request-1", message: "Enter secret", kind: "prompt", promptType: "secret" } });
|
||||||
|
const respondMachines: (string | undefined)[] = [];
|
||||||
|
const statusMachines: (string | undefined)[] = [];
|
||||||
|
const appliedStatuses: SessionStatus[] = [];
|
||||||
|
const { controller } = createController(
|
||||||
|
{
|
||||||
|
selectedMachine: remoteMachine("remote-2"),
|
||||||
|
selectedSession: sessionInfo("session-2"),
|
||||||
|
authDialog: { step: "oauth", flow, machineId: "remote-1", inputValue: "secret-value" },
|
||||||
|
},
|
||||||
|
{
|
||||||
|
respondOAuthFlow: (_flowId, _requestId, _value, machineId) => {
|
||||||
|
respondMachines.push(machineId);
|
||||||
|
return Promise.resolve(oauthFlow({ status: "complete" }));
|
||||||
|
},
|
||||||
|
status: (_session, machineId) => {
|
||||||
|
statusMachines.push(machineId);
|
||||||
|
return Promise.resolve(sessionStatus("session-2"));
|
||||||
|
},
|
||||||
|
},
|
||||||
|
(status) => { appliedStatuses.push(status); },
|
||||||
|
);
|
||||||
|
|
||||||
|
await controller.respondOAuth();
|
||||||
|
await flushMicrotasks();
|
||||||
|
|
||||||
|
expect(respondMachines).toEqual(["remote-1"]);
|
||||||
|
expect(statusMachines).toEqual([]);
|
||||||
|
expect(appliedStatuses).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not apply an auth status refresh after the selected session changes", async () => {
|
||||||
|
const flow = oauthFlow({ prompt: { requestId: "request-1", message: "Enter secret", kind: "prompt", promptType: "secret" } });
|
||||||
|
const originalSession = sessionInfo("session-1");
|
||||||
|
const statusResponse = deferred<SessionStatus>();
|
||||||
|
const statusCalls: { session: Parameters<typeof defaultApi.status>[0]; machineId: string | undefined }[] = [];
|
||||||
|
const appliedStatuses: SessionStatus[] = [];
|
||||||
|
const { controller, setState } = createController(
|
||||||
|
{
|
||||||
|
selectedMachine: remoteMachine("remote-1"),
|
||||||
|
selectedSession: originalSession,
|
||||||
|
authDialog: { step: "oauth", flow, machineId: "remote-1", inputValue: "secret-value" },
|
||||||
|
},
|
||||||
|
{
|
||||||
|
respondOAuthFlow: () => Promise.resolve(oauthFlow({ status: "complete" })),
|
||||||
|
status: (session, machineId) => {
|
||||||
|
statusCalls.push({ session, machineId });
|
||||||
|
return statusResponse.promise;
|
||||||
|
},
|
||||||
|
},
|
||||||
|
(status) => { appliedStatuses.push(status); },
|
||||||
|
);
|
||||||
|
|
||||||
|
await controller.respondOAuth();
|
||||||
|
setState({ selectedMachine: remoteMachine("remote-2"), selectedSession: sessionInfo("session-2") });
|
||||||
|
statusResponse.resolve(sessionStatus(originalSession.id));
|
||||||
|
await flushMicrotasks();
|
||||||
|
|
||||||
|
expect(statusCalls).toEqual([{ session: originalSession, machineId: "remote-1" }]);
|
||||||
|
expect(appliedStatuses).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
it("leaves the OAuth dialog ready to retry if responding fails", async () => {
|
it("leaves the OAuth dialog ready to retry if responding fails", async () => {
|
||||||
const flow = oauthFlow({ prompt: { requestId: "request-1", message: "Paste callback", kind: "manual" } });
|
const flow = oauthFlow({ prompt: { requestId: "request-1", message: "Paste callback", kind: "manual" } });
|
||||||
const { controller, getState } = createController(
|
const { controller, getState } = createController(
|
||||||
|
|||||||
@@ -295,17 +295,22 @@ export class AuthController {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private async refreshStatus(machineId = selectedMachineId(this.getState())): Promise<void> {
|
private async refreshStatus(machineId = selectedMachineId(this.getState())): Promise<void> {
|
||||||
const session = this.session();
|
const session = this.selectedSessionForMachine(machineId);
|
||||||
if (session === undefined) return;
|
if (session === undefined) return;
|
||||||
try {
|
try {
|
||||||
this.applyStatus(await this.api.status(session, machineId));
|
const status = await this.api.status(session, machineId);
|
||||||
|
const current = this.selectedSessionForMachine(machineId);
|
||||||
|
if (current?.id !== session.id || current.cwd !== session.cwd) return;
|
||||||
|
this.applyStatus(status);
|
||||||
} catch {
|
} catch {
|
||||||
// Status refresh is opportunistic after login completes.
|
// Status refresh is opportunistic after login completes.
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private session() {
|
private selectedSessionForMachine(machineId: string) {
|
||||||
const session = this.getState().selectedSession;
|
const state = this.getState();
|
||||||
|
if (selectedMachineId(state) !== machineId) return undefined;
|
||||||
|
const session = state.selectedSession;
|
||||||
if (session === undefined || session.archived === true) return undefined;
|
if (session === undefined || session.archived === true) return undefined;
|
||||||
return session;
|
return session;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user