diff --git a/libs/vault/src/abstractions/change-login-password.service.ts b/libs/vault/src/abstractions/change-login-password.service.ts index c89162d42be..b8981282f4b 100644 --- a/libs/vault/src/abstractions/change-login-password.service.ts +++ b/libs/vault/src/abstractions/change-login-password.service.ts @@ -4,6 +4,8 @@ export abstract class ChangeLoginPasswordService { /** * Attempts to find a well-known change password URL for the given cipher. Only works for Login ciphers with at * least one http/https URL. If no well-known change password URL is found, the first URL is returned. + * Checks each URL until the first reliable one well-known URL is found, otherwise returns the first URL. + * * Non-Login ciphers and Logins with no valid http/https URLs return null. */ abstract getChangePasswordUrl(cipher: CipherView): Promise; diff --git a/libs/vault/src/services/default-change-login-password.service.spec.ts b/libs/vault/src/services/default-change-login-password.service.spec.ts index 4805f298797..37123604e9a 100644 --- a/libs/vault/src/services/default-change-login-password.service.spec.ts +++ b/libs/vault/src/services/default-change-login-password.service.spec.ts @@ -131,13 +131,13 @@ describe("DefaultChangeLoginPasswordService", () => { const cipher = { type: CipherType.Login, login: Object.assign(new LoginView(), { - uris: [{ uri: "https://example.com" }], + uris: [{ uri: "https://example.com/" }], }), } as CipherView; const url = await service.getChangePasswordUrl(cipher); - expect(url).toBe("https://example.com"); + expect(url).toBe("https://example.com/"); }); it("should return the original URI when the well-known URL is not found", async () => { @@ -146,12 +146,42 @@ describe("DefaultChangeLoginPasswordService", () => { const cipher = { type: CipherType.Login, login: Object.assign(new LoginView(), { - uris: [{ uri: "https://example.com" }], + uris: [{ uri: "https://example.com/" }], }), } as CipherView; const url = await service.getChangePasswordUrl(cipher); - expect(url).toBe("https://example.com"); + expect(url).toBe("https://example.com/"); + }); + + it("should try the next URI if the first one fails", async () => { + mockApiService.nativeFetch.mockImplementation((request) => { + if ( + request.url.endsWith("resource-that-should-not-exist-whose-status-code-should-not-be-200") + ) { + return Promise.resolve(mockShouldNotExistResponse); + } + + if (request.url.endsWith(".well-known/change-password")) { + if (request.url.includes("working.com")) { + return Promise.resolve(mockWellKnownResponse); + } + return Promise.resolve(new Response("Not Found", { status: 404 })); + } + + throw new Error("Unexpected request"); + }); + + const cipher = { + type: CipherType.Login, + login: Object.assign(new LoginView(), { + uris: [{ uri: "https://no-wellknown.com/" }, { uri: "https://working.com/" }], + }), + } as CipherView; + + const url = await service.getChangePasswordUrl(cipher); + + expect(url).toBe("https://working.com/.well-known/change-password"); }); }); diff --git a/libs/vault/src/services/default-change-login-password.service.ts b/libs/vault/src/services/default-change-login-password.service.ts index 82f272f1ede..29818f95c0a 100644 --- a/libs/vault/src/services/default-change-login-password.service.ts +++ b/libs/vault/src/services/default-change-login-password.service.ts @@ -20,25 +20,31 @@ export class DefaultChangeLoginPasswordService implements ChangeLoginPasswordSer return null; } - // Find the first valid URL that is an HTTP or HTTPS URL - const url = cipher.login.uris + // Filter for valid URLs that are HTTP(S) + const urls = cipher.login.uris .map((m) => Utils.getUrl(m.uri)) - .find((m) => m != null && (m.protocol === "http:" || m.protocol === "https:")); + .filter((m) => m != null && (m.protocol === "http:" || m.protocol === "https:")); - if (url == null) { + if (urls.length === 0) { return null; } - const [reliable, wellKnownChangeUrl] = await Promise.all([ - this.hasReliableHttpStatusCode(url.origin), - this.getWellKnownChangePasswordUrl(url.origin), - ]); + for (const url of urls) { + const [reliable, wellKnownChangeUrl] = await Promise.all([ + this.hasReliableHttpStatusCode(url.origin), + this.getWellKnownChangePasswordUrl(url.origin), + ]); - if (!reliable || wellKnownChangeUrl == null) { - return url.origin; + // Some servers return a 200 OK for a resource that should not exist + // Which means we cannot trust the well-known URL is valid, so we skip it + // to avoid potentially sending users to a 404 page + if (reliable && wellKnownChangeUrl != null) { + return wellKnownChangeUrl; + } } - return wellKnownChangeUrl; + // No reliable well-known URL found, fallback to the first URL + return urls[0].href; } /**