bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-17 00:33:44 +00:00
Code Issues Releases Wiki Activity

[PM-1426] Refactor uri matching (#5003)

Browse Source 
* Move URI matching logic into uriView

* Fix url parsing: always assign default protocol, otherwise no protocol with port is parsed incorrectly

* Codescene: refactor domain matching logic
This commit is contained in:
Thomas Rittson
2023-04-06 13:30:26 +10:00
committed by GitHub
parent 576d85b268
commit 7899b25ab3
16 changed files with 268 additions and 218 deletions
Download Patch File Download Diff File Expand all files Collapse all files

128
libs/common/src/vault/models/view/login-uri-view.spec.ts
View File

@@ -1,4 +1,5 @@
import { UriMatchType } from "../../../enums";
import { Utils } from "../../../misc/utils";
import { LoginUriView } from "./login-uri.view";
@@ -25,6 +26,18 @@ const testData = [
},
];
const exampleUris = {
standard: "https://www.exampleapp.com.au:4000/userauth/login.html",
standardRegex: "https://www.exampleapp.com.au:[0-9]*/[A-Za-z]+/login.html",
standardNotMatching: "https://www.exampleapp.com.au:4000/userauth123/login.html",
subdomain: "https://www.auth.exampleapp.com.au",
differentDomain: "https://www.exampleapp.co.uk/subpage",
differentHost: "https://www.exampleapp.com.au/userauth/login.html",
equivalentDomains: () =>
new Set(["exampleapp.com.au", "exampleapp.com", "exampleapp.co.uk", "example.com"]),
noEquivalentDomains: () => new Set<string>(),
};
describe("LoginUriView", () => {
it("isWebsite() given an invalid domain should return false", async () => {
const uri = new LoginUriView();
@@ -63,4 +76,119 @@ describe("LoginUriView", () => {
Object.assign(uri, { match: UriMatchType.Host, uri: "someprotocol://bitwarden.com" });
expect(uri.canLaunch).toBe(false);
});
describe("uri matching", () => {
describe("using domain matching", () => {
it("matches the same domain", () => {
const uri = uriFactory(UriMatchType.Domain, exampleUris.standard);
const actual = uri.matchesUri(exampleUris.subdomain, exampleUris.noEquivalentDomains());
expect(actual).toBe(true);
});
it("matches equivalent domains", () => {
const uri = uriFactory(UriMatchType.Domain, exampleUris.standard);
const actual = uri.matchesUri(exampleUris.differentDomain, exampleUris.equivalentDomains());
expect(actual).toBe(true);
});
it("does not match a different domain", () => {
const uri = uriFactory(UriMatchType.Domain, exampleUris.standard);
const actual = uri.matchesUri(
exampleUris.differentDomain,
exampleUris.noEquivalentDomains()
);
expect(actual).toBe(false);
});
// Actual integration test with the real blacklist, not ideal
it("does not match domains that are blacklisted", () => {
const googleEquivalentDomains = new Set(["google.com", "script.google.com"]);
const uri = uriFactory(UriMatchType.Domain, "google.com");
const actual = uri.matchesUri("script.google.com", googleEquivalentDomains);
expect(actual).toBe(false);
});
});
describe("using host matching", () => {
it("matches the same host", () => {
const uri = uriFactory(UriMatchType.Host, Utils.getHost(exampleUris.standard));
const actual = uri.matchesUri(exampleUris.standard, exampleUris.noEquivalentDomains());
expect(actual).toBe(true);
});
it("does not match a different host", () => {
const uri = uriFactory(UriMatchType.Host, Utils.getHost(exampleUris.differentDomain));
const actual = uri.matchesUri(exampleUris.standard, exampleUris.noEquivalentDomains());
expect(actual).toBe(false);
});
});
describe("using exact matching", () => {
it("matches if both uris are the same", () => {
const uri = uriFactory(UriMatchType.Exact, exampleUris.standard);
const actual = uri.matchesUri(exampleUris.standard, exampleUris.noEquivalentDomains());
expect(actual).toBe(true);
});
it("does not match if the uris are different", () => {
const uri = uriFactory(UriMatchType.Exact, exampleUris.standard);
const actual = uri.matchesUri(
exampleUris.standard + "#",
exampleUris.noEquivalentDomains()
);
expect(actual).toBe(false);
});
});
describe("using startsWith matching", () => {
it("matches if the target URI starts with the saved URI", () => {
const uri = uriFactory(UriMatchType.StartsWith, exampleUris.standard);
const actual = uri.matchesUri(
exampleUris.standard + "#bookmark",
exampleUris.noEquivalentDomains()
);
expect(actual).toBe(true);
});
it("does not match if the start of the uri is not the same", () => {
const uri = uriFactory(UriMatchType.StartsWith, exampleUris.standard);
const actual = uri.matchesUri(
exampleUris.standard.slice(1),
exampleUris.noEquivalentDomains()
);
expect(actual).toBe(false);
});
});
describe("using regular expression matching", () => {
it("matches if the regular expression matches", () => {
const uri = uriFactory(UriMatchType.RegularExpression, exampleUris.standard);
const actual = uri.matchesUri(exampleUris.standardRegex, exampleUris.noEquivalentDomains());
expect(actual).toBe(false);
});
it("does not match if the regular expression does not match", () => {
const uri = uriFactory(UriMatchType.RegularExpression, exampleUris.standardNotMatching);
const actual = uri.matchesUri(exampleUris.standardRegex, exampleUris.noEquivalentDomains());
expect(actual).toBe(false);
});
});
describe("using never matching", () => {
it("does not match even if uris are identical", () => {
const uri = uriFactory(UriMatchType.Never, exampleUris.standard);
const actual = uri.matchesUri(exampleUris.standard, exampleUris.noEquivalentDomains());
expect(actual).toBe(false);
});
});
});
});
function uriFactory(match: UriMatchType, uri: string) {
const loginUri = new LoginUriView();
loginUri.match = match;
loginUri.uri = uri;
return loginUri;
}

56
libs/common/src/vault/models/view/login-uri.view.ts
View File

@@ -129,4 +129,60 @@ export class LoginUriView implements View {
static fromJSON(obj: Partial<Jsonify<LoginUriView>>): LoginUriView {
return Object.assign(new LoginUriView(), obj);
}
matchesUri(
targetUri: string,
equivalentDomains: Set<string>,
defaultUriMatch: UriMatchType = null
): boolean {
if (!this.uri || !targetUri) {
return false;
}
let matchType = this.match ?? defaultUriMatch;
matchType ??= UriMatchType.Domain;
const targetDomain = Utils.getDomain(targetUri);
const matchDomains = equivalentDomains.add(targetDomain);
switch (matchType) {
case UriMatchType.Domain:
return this.matchesDomain(targetUri, matchDomains);
case UriMatchType.Host: {
const urlHost = Utils.getHost(targetUri);
return urlHost != null && urlHost === Utils.getHost(this.uri);
}
case UriMatchType.Exact:
return targetUri === this.uri;
case UriMatchType.StartsWith:
return targetUri.startsWith(this.uri);
case UriMatchType.RegularExpression:
try {
const regex = new RegExp(this.uri, "i");
return regex.test(targetUri);
} catch (e) {
// Invalid regex
return false;
}
case UriMatchType.Never:
return false;
default:
break;
}
return false;
}
private matchesDomain(targetUri: string, matchDomains: Set<string>) {
if (targetUri == null || this.domain == null || !matchDomains.has(this.domain)) {
return false;
}
if (Utils.DomainMatchBlacklist.has(this.domain)) {
const domainUrlHost = Utils.getHost(targetUri);
return !Utils.DomainMatchBlacklist.get(this.domain).has(domainUrlHost);
}
return true;
}
}

14
libs/common/src/vault/models/view/login.view.ts
View File

@@ -1,6 +1,6 @@
import { Jsonify } from "type-fest";
import { LoginLinkedId as LinkedId } from "../../../enums";
import { LoginLinkedId as LinkedId, UriMatchType } from "../../../enums";
import { linkedFieldOption } from "../../../misc/linkedFieldOption.decorator";
import { Utils } from "../../../misc/utils";
import { Login } from "../domain/login";
@@ -63,6 +63,18 @@ export class LoginView extends ItemView {
return this.uris != null && this.uris.length > 0;
}
matchesUri(
targetUri: string,
equivalentDomains: Set<string>,
defaultUriMatch: UriMatchType = null
): boolean {
if (this.uris == null) {
return false;
}
return this.uris.some((uri) => uri.matchesUri(targetUri, equivalentDomains, defaultUriMatch));
}
static fromJSON(obj: Partial<Jsonify<LoginView>>): LoginView {
const passwordRevisionDate =
obj.passwordRevisionDate == null ? null : new Date(obj.passwordRevisionDate);

4
libs/common/src/vault/services/cipher.service.spec.ts
View File

@@ -5,7 +5,6 @@ import { ApiService } from "../../abstractions/api.service";
import { CryptoService } from "../../abstractions/crypto.service";
import { EncryptService } from "../../abstractions/encrypt.service";
import { I18nService } from "../../abstractions/i18n.service";
import { LogService } from "../../abstractions/log.service";
import { SearchService } from "../../abstractions/search.service";
import { SettingsService } from "../../abstractions/settings.service";
import { StateService } from "../../abstractions/state.service";
@@ -28,7 +27,6 @@ describe("Cipher Service", () => {
let cipherFileUploadService: SubstituteOf<CipherFileUploadService>;
let i18nService: SubstituteOf<I18nService>;
let searchService: SubstituteOf<SearchService>;
let logService: SubstituteOf<LogService>;
let encryptService: SubstituteOf<EncryptService>;
let cipherService: CipherService;
@@ -41,7 +39,6 @@ describe("Cipher Service", () => {
cipherFileUploadService = Substitute.for<CipherFileUploadService>();
i18nService = Substitute.for<I18nService>();
searchService = Substitute.for<SearchService>();
logService = Substitute.for<LogService>();
encryptService = Substitute.for<EncryptService>();
cryptoService.encryptToBytes(Arg.any(), Arg.any()).resolves(ENCRYPTED_BYTES);
@@ -53,7 +50,6 @@ describe("Cipher Service", () => {
apiService,
i18nService,
() => searchService,
logService,
stateService,
encryptService,
cipherFileUploadService

94
libs/common/src/vault/services/cipher.service.ts
View File

@@ -1,17 +1,13 @@
import { firstValueFrom } from "rxjs";
import { ApiService } from "../../abstractions/api.service";
import { CryptoService } from "../../abstractions/crypto.service";
import { EncryptService } from "../../abstractions/encrypt.service";
import { I18nService } from "../../abstractions/i18n.service";
import { LogService } from "../../abstractions/log.service";
import { SearchService } from "../../abstractions/search.service";
import { SettingsService } from "../../abstractions/settings.service";
import { StateService } from "../../abstractions/state.service";
import { FieldType, UriMatchType } from "../../enums";
import { sequentialize } from "../../misc/sequentialize";
import { Utils } from "../../misc/utils";
import { AccountSettingsSettings } from "../../models/domain/account";
import Domain from "../../models/domain/domain-base";
import { EncArrayBuffer } from "../../models/domain/enc-array-buffer";
import { EncString } from "../../models/domain/enc-string";
@@ -58,7 +54,6 @@ export class CipherService implements CipherServiceAbstraction {
private apiService: ApiService,
private i18nService: I18nService,
private searchService: () => SearchService,
private logService: LogService,
private stateService: StateService,
private encryptService: EncryptService,
private cipherFileUploadService: CipherFileUploadService
@@ -399,37 +394,9 @@ export class CipherService implements CipherServiceAbstraction {
return Promise.resolve([]);
}
const domain = Utils.getDomain(url);
const eqDomainsPromise =
domain == null
? Promise.resolve([])
: firstValueFrom(this.settingsService.settings$).then(
(settings: AccountSettingsSettings) => {
let matches: any[] = [];
settings?.equivalentDomains?.forEach((eqDomain: any) => {
if (eqDomain.length && eqDomain.indexOf(domain) >= 0) {
matches = matches.concat(eqDomain);
}
});
if (!matches.length) {
matches.push(domain);
}
return matches;
}
);
const result = await Promise.all([eqDomainsPromise, this.getAllDecrypted()]);
const matchingDomains = result[0];
const ciphers = result[1];
if (defaultMatch == null) {
defaultMatch = await this.stateService.getDefaultUriMatch();
if (defaultMatch == null) {
defaultMatch = UriMatchType.Domain;
}
}
const equivalentDomains = this.settingsService.getEquivalentDomains(url);
const ciphers = await this.getAllDecrypted();
defaultMatch ??= await this.stateService.getDefaultUriMatch();
return ciphers.filter((cipher) => {
if (cipher.deletedDate != null) {
@@ -439,59 +406,8 @@ export class CipherService implements CipherServiceAbstraction {
return true;
}
if (url != null && cipher.type === CipherType.Login && cipher.login.uris != null) {
for (let i = 0; i < cipher.login.uris.length; i++) {
const u = cipher.login.uris[i];
if (u.uri == null) {
continue;
}
const match = u.match == null ? defaultMatch : u.match;
switch (match) {
case UriMatchType.Domain:
if (domain != null && u.domain != null && matchingDomains.indexOf(u.domain) > -1) {
if (Utils.DomainMatchBlacklist.has(u.domain)) {
const domainUrlHost = Utils.getHost(url);
if (!Utils.DomainMatchBlacklist.get(u.domain).has(domainUrlHost)) {
return true;
}
} else {
return true;
}
}
break;
case UriMatchType.Host: {
const urlHost = Utils.getHost(url);
if (urlHost != null && urlHost === Utils.getHost(u.uri)) {
return true;
}
break;
}
case UriMatchType.Exact:
if (url === u.uri) {
return true;
}
break;
case UriMatchType.StartsWith:
if (url.startsWith(u.uri)) {
return true;
}
break;
case UriMatchType.RegularExpression:
try {
const regex = new RegExp(u.uri, "i");
if (regex.test(url)) {
return true;
}
} catch (e) {
this.logService.error(e);
}
break;
case UriMatchType.Never:
default:
break;
}
}
if (cipher.type === CipherType.Login && cipher.login !== null) {
return cipher.login.matchesUri(url, equivalentDomains, defaultMatch);
}
return false;