Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prioritise entirely supported flows for UIA #3402

Merged
merged 5 commits into from
May 24, 2023
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
43 changes: 43 additions & 0 deletions spec/unit/interactive-auth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -517,4 +517,47 @@ describe("InteractiveAuth", () => {
expect(ia.getEmailSid()).toEqual(sid);
});
});

it("should prioritise shorter flows", async () => {
const doRequest = jest.fn();
const stateUpdated = jest.fn();

const ia = new InteractiveAuth({
matrixClient: getFakeClient(),
doRequest: doRequest,
stateUpdated: stateUpdated,
requestEmailToken: jest.fn(),
authData: {
session: "sessionId",
flows: [{ stages: [AuthType.Recaptcha, AuthType.Password] }, { stages: [AuthType.Password] }],
params: {},
},
});

// @ts-ignore
ia.chooseStage();
expect(ia.getChosenFlow()?.stages).toEqual([AuthType.Password]);
});

it("should prioritise flows with entirely supported stages", async () => {
const doRequest = jest.fn();
const stateUpdated = jest.fn();

const ia = new InteractiveAuth({
matrixClient: getFakeClient(),
doRequest: doRequest,
stateUpdated: stateUpdated,
requestEmailToken: jest.fn(),
authData: {
session: "sessionId",
flows: [{ stages: ["com.devture.shared_secret_auth"] }, { stages: [AuthType.Password] }],
params: {},
},
supportedStages: [AuthType.Password],
});

// @ts-ignore
ia.chooseStage();
expect(ia.getChosenFlow()?.stages).toEqual([AuthType.Password]);
});
});
33 changes: 29 additions & 4 deletions src/interactive-auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const EMAIL_STAGE_TYPE = "m.login.email.identity";
const MSISDN_STAGE_TYPE = "m.login.msisdn";

export interface UIAFlow {
stages: AuthType[];
stages: Array<AuthType | string>;
}

export interface IInputs {
Expand Down Expand Up @@ -156,6 +156,14 @@ interface IOpts {
*/
emailSid?: string;

/**
* If specified, will prefer flows which entirely consist of listed stages.
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
* These should normally be of type AuthTypes but can be string when supporting custom auth stages.
*
* This can be used to avoid needing the fallback mechanism.
*/
supportedStages?: Array<AuthType | string>;

/**
* Called with the new auth dict to submit the request.
* Also passes a second deprecated arg which is a flag set to true if this request is a background request.
Expand All @@ -176,7 +184,7 @@ interface IOpts {
* m.login.email.identity:
* * emailSid: string, the sid of the active email auth session
*/
stateUpdated(nextStage: AuthType, status: IStageStatus): void;
stateUpdated(nextStage: AuthType | string, status: IStageStatus): void;

/**
* A function that takes the email address (string), clientSecret (string), attempt number (int) and
Expand Down Expand Up @@ -216,6 +224,7 @@ export class InteractiveAuth {
private readonly busyChangedCallback?: IOpts["busyChanged"];
private readonly stateUpdatedCallback: IOpts["stateUpdated"];
private readonly requestEmailTokenCallback: IOpts["requestEmailToken"];
private readonly supportedStages?: Set<string>;

private data: IAuthData;
private emailSid?: string;
Expand Down Expand Up @@ -243,6 +252,7 @@ export class InteractiveAuth {
if (opts.sessionId) this.data.session = opts.sessionId;
this.clientSecret = opts.clientSecret || this.matrixClient.generateClientSecret();
this.emailSid = opts.emailSid;
if (opts.supportedStages !== undefined) this.supportedStages = new Set(opts.supportedStages);
}

/**
Expand Down Expand Up @@ -571,7 +581,7 @@ export class InteractiveAuth {
* @returns login type
* @throws {@link NoAuthFlowFoundError} If no suitable authentication flow can be found
*/
private chooseStage(): AuthType | undefined {
private chooseStage(): AuthType | string | undefined {
if (this.chosenFlow === null) {
this.chosenFlow = this.chooseFlow();
}
Expand All @@ -581,6 +591,17 @@ export class InteractiveAuth {
return nextStage;
}

// Returns a low number for flows we consider best. Counts increase for longer flows and even more so
// for flows which contain stages not listed in `supportedStages`.
private scoreFlow(flow: UIAFlow): number {
let score = flow.stages.length;
if (this.supportedStages !== undefined) {
// Add 10 points to the score for each unsupported stage in the flow.
score += flow.stages.filter((stage) => !this.supportedStages!.has(stage)).length * 10;
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
}
return score;
}

/**
* Pick one of the flows from the returned list
* If a flow using all of the inputs is found, it will
Expand All @@ -603,6 +624,10 @@ export class InteractiveAuth {
const haveEmail = Boolean(this.inputs.emailAddress) || Boolean(this.emailSid);
const haveMsisdn = Boolean(this.inputs.phoneCountry) && Boolean(this.inputs.phoneNumber);

// Flows are not represented in a significant order, so we can choose any we support best
// Sort flows based on how many unsupported stages they contain ascending
flows.sort((a, b) => this.scoreFlow(a) - this.scoreFlow(b));

for (const flow of flows) {
let flowHasEmail = false;
let flowHasMsisdn = false;
Expand Down Expand Up @@ -633,7 +658,7 @@ export class InteractiveAuth {
* @internal
* @returns login type
*/
private firstUncompletedStage(flow: UIAFlow): AuthType | undefined {
private firstUncompletedStage(flow: UIAFlow): AuthType | string | undefined {
const completed = this.data.completed || [];
return flow.stages.find((stageType) => !completed.includes(stageType));
}
Expand Down