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

[PM-13115] Allow users to disable extension content script injections by domain #11826

Merged
merged 19 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
76d68d2
add disabledInteractionsUris state to the domain settings service
jprusik Nov 1, 2024
3a1f241
add routes and ui for user disabledInteractionsUris state management
jprusik Nov 1, 2024
9a50c1b
use disabled URIs service state as a preemptive conditon to injectingโ€ฆ
jprusik Nov 1, 2024
fd9719c
move disabled domains navigation button from account security settingโ€ฆ
jprusik Nov 11, 2024
427db63
update disabled domain terminology to blocked domain terminology
jprusik Nov 11, 2024
2686be0
update copy
jprusik Nov 11, 2024
5d60897
handle blocked domains initializing with null value
jprusik Nov 15, 2024
2f8d2ac
add dismissable banner to the vault view when the active autofill tabโ€ฆ
jprusik Nov 15, 2024
05b0dc0
add autofill blocked domain indicators to autofill suggestions sectioโ€ฆ
jprusik Nov 15, 2024
1c6ec55
add BlockBrowserInjectionsByDomain feature flag and put feature behinโ€ฆ
jprusik Dec 13, 2024
d62edcf
update router config to new style
jprusik Dec 13, 2024
c339d68
update tests and cleanup
jprusik Dec 16, 2024
09095b0
use full-width-notice slot for domain script injection blocked banner
jprusik Dec 23, 2024
a77d48d
convert thrown error on content script injection block to a warning aโ€ฆ
jprusik Dec 23, 2024
b962373
simplify and enspeeden state resolution for blockedInteractionsUris
jprusik Dec 23, 2024
2b1a004
refactor feature flag state fetching and update tests
jprusik Jan 3, 2025
958a738
document domain settings service
jprusik Jan 3, 2025
f408574
Merge remote-tracking branch 'origin/main' into pm-13115
jprusik Jan 6, 2025
fe40dd8
remove vault component presentational updates
jprusik Jan 6, 2025
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
15 changes: 15 additions & 0 deletions apps/browser/src/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2324,6 +2324,9 @@
"message": "Domains",
"description": "A category title describing the concept of web domains"
},
"blockedDomains": {
"message": "Blocked domains"
},
"excludedDomains": {
"message": "Excluded domains"
},
Expand All @@ -2333,6 +2336,15 @@
"excludedDomainsDescAlt": {
"message": "Bitwarden will not ask to save login details for these domains for all logged in accounts. You must refresh the page for changes to take effect."
},
"blockedDomainsDesc": {
"message": "Autofill and other related features will not be offered for these websites. You must refresh the page for changes to take effect."
},
"autofillBlockedNotice": {
"message": "Autofill is blocked for this website. Review or change this in settings."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any way to deep link to the relevant settings page in the notice?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that this would be great to have!

Copy link
Contributor Author

@jprusik jprusik Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed; I've added a follow up task to the epic

This is used in a banner, so that component may need updates as a result.

},
"autofillBlockedTooltip": {
"message": "Autofill is blocked on this website. Review in settings."
},
"websiteItemLabel": {
"message": "Website $number$ (URI)",
"placeholders": {
Expand All @@ -2351,6 +2363,9 @@
}
}
},
"blockedDomainsSavedSuccess": {
"message": "Blocked domain changes saved"
},
"excludedDomainsSavedSuccess": {
"message": "Excluded domain changes saved"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { mock, MockProxy, mockReset } from "jest-mock-extended";
import { BehaviorSubject } from "rxjs";
import { BehaviorSubject, of } from "rxjs";

import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
Expand All @@ -14,6 +14,7 @@ import {
} from "@bitwarden/common/autofill/services/domain-settings.service";
import { InlineMenuVisibilitySetting } from "@bitwarden/common/autofill/types";
import { NeverDomains } from "@bitwarden/common/models/domain/domain-service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import {
EnvironmentService,
Region,
Expand Down Expand Up @@ -93,6 +94,7 @@ describe("OverlayBackground", () => {
let logService: MockProxy<LogService>;
let cipherService: MockProxy<CipherService>;
let autofillService: MockProxy<AutofillService>;
let configService: MockProxy<ConfigService>;
let activeAccountStatusMock$: BehaviorSubject<AuthenticationStatus>;
let authService: MockProxy<AuthService>;
let environmentMock$: BehaviorSubject<CloudEnvironment>;
Expand Down Expand Up @@ -149,11 +151,13 @@ describe("OverlayBackground", () => {
}

beforeEach(() => {
configService = mock<ConfigService>();
configService.getFeatureFlag$.mockImplementation(() => of(true));
accountService = mockAccountServiceWith(mockUserId);
fakeStateProvider = new FakeStateProvider(accountService);
showFaviconsMock$ = new BehaviorSubject(true);
neverDomainsMock$ = new BehaviorSubject({});
domainSettingsService = new DefaultDomainSettingsService(fakeStateProvider);
domainSettingsService = new DefaultDomainSettingsService(fakeStateProvider, configService);
domainSettingsService.showFavicons$ = showFaviconsMock$;
domainSettingsService.neverDomains$ = neverDomainsMock$;
logService = mock<LogService>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
DefaultDomainSettingsService,
DomainSettingsService,
} from "@bitwarden/common/autofill/services/domain-settings.service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import {
EnvironmentService,
Region,
Expand Down Expand Up @@ -61,6 +62,7 @@ describe("OverlayBackground", () => {
let overlayBackground: LegacyOverlayBackground;
const cipherService = mock<CipherService>();
const autofillService = mock<AutofillService>();
let configService: MockProxy<ConfigService>;
let activeAccountStatusMock$: BehaviorSubject<AuthenticationStatus>;
let authService: MockProxy<AuthService>;

Expand Down Expand Up @@ -92,7 +94,9 @@ describe("OverlayBackground", () => {
};

beforeEach(() => {
domainSettingsService = new DefaultDomainSettingsService(fakeStateProvider);
configService = mock<ConfigService>();
configService.getFeatureFlag$.mockImplementation(() => of(true));
domainSettingsService = new DefaultDomainSettingsService(fakeStateProvider, configService);
activeAccountStatusMock$ = new BehaviorSubject(AuthenticationStatus.Unlocked);
authService = mock<AuthService>();
authService.activeAccountStatus$ = activeAccountStatusMock$;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,4 +255,16 @@ <h2 class="box-header">{{ "additionalOptions" | i18n }}</h2>
{{ "showIdentitiesCurrentTabDesc" | i18n }}
</div>
</div>
<div class="box list" *ngIf="blockBrowserInjectionsByDomainEnabled">
<div class="box-content single-line">
<button
type="button"
class="box-content-row box-content-row-flex text-default"
routerLink="/blocked-domains"
>
<div class="row-main">{{ "blockedDomains" | i18n }}</div>
<i class="bwi bwi-angle-right bwi-lg row-sub-icon" aria-hidden="true"></i>
</button>
</div>
</div>
</main>
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
protected autoFillOverlayVisibilityOptions: any[];
protected disablePasswordManagerLink: string;
protected inlineMenuPositioningImprovementsEnabled: boolean = false;
protected blockBrowserInjectionsByDomainEnabled: boolean = false;

Check warning on line 39 in apps/browser/src/autofill/popup/settings/autofill-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/autofill/popup/settings/autofill-v1.component.ts#L39

Added line #L39 was not covered by tests
protected showInlineMenuIdentities: boolean = true;
protected showInlineMenuCards: boolean = true;
inlineMenuIsEnabled: boolean = false;
Expand Down Expand Up @@ -120,6 +121,10 @@
FeatureFlag.InlineMenuPositioningImprovements,
);

this.blockBrowserInjectionsByDomainEnabled = await this.configService.getFeatureFlag(

Check warning on line 124 in apps/browser/src/autofill/popup/settings/autofill-v1.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/autofill/popup/settings/autofill-v1.component.ts#L124

Added line #L124 was not covered by tests
FeatureFlag.BlockBrowserInjectionsByDomain,
);

this.inlineMenuIsEnabled = this.isInlineMenuEnabled();

this.showInlineMenuIdentities =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,5 +282,11 @@ <h2 bitTypography="h6">{{ "additionalOptions" | i18n }}</h2>
</bit-form-field>
</bit-card>
</bit-section>
<bit-section *ngIf="blockBrowserInjectionsByDomainEnabled">
<bit-item>
<a bit-item-content routerLink="/blocked-domains">{{ "blockedDomains" | i18n }}</a>
<i slot="end" class="bwi bwi-angle-right bwi-lg row-sub-icon" aria-hidden="true"></i>
</bit-item>
</bit-section>
</div>
</popup-page>
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@

import { BrowserApi } from "../../../platform/browser/browser-api";
import { PopOutComponent } from "../../../platform/popup/components/pop-out.component";
import { PopupFooterComponent } from "../../../platform/popup/layout/popup-footer.component";
import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component";
import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component";

Expand All @@ -67,7 +66,6 @@
JslibModule,
LinkModule,
PopOutComponent,
PopupFooterComponent,
Copy link
Contributor Author

@jprusik jprusik Dec 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PopupFooterComponent was not in use anywhere here or in the template

PopupHeaderComponent,
PopupPageComponent,
RouterModule,
Expand All @@ -87,6 +85,7 @@
protected inlineMenuVisibility: InlineMenuVisibilitySetting =
AutofillOverlayVisibility.OnFieldFocus;
protected inlineMenuPositioningImprovementsEnabled: boolean = false;
protected blockBrowserInjectionsByDomainEnabled: boolean = false;

Check warning on line 88 in apps/browser/src/autofill/popup/settings/autofill.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/autofill/popup/settings/autofill.component.ts#L88

Added line #L88 was not covered by tests
protected browserClientVendor: BrowserClientVendor = BrowserClientVendors.Unknown;
protected disablePasswordManagerURI: DisablePasswordManagerUri =
DisablePasswordManagerUris.Unknown;
Expand Down Expand Up @@ -164,6 +163,10 @@
FeatureFlag.InlineMenuPositioningImprovements,
);

this.blockBrowserInjectionsByDomainEnabled = await this.configService.getFeatureFlag(

Check warning on line 166 in apps/browser/src/autofill/popup/settings/autofill.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/autofill/popup/settings/autofill.component.ts#L166

Added line #L166 was not covered by tests
FeatureFlag.BlockBrowserInjectionsByDomain,
);

this.showInlineMenuIdentities =
this.inlineMenuPositioningImprovementsEnabled &&
(await firstValueFrom(this.autofillSettingsService.showInlineMenuIdentities$));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<popup-page>
<popup-header slot="header" pageTitle="{{ 'blockedDomains' | i18n }}" showBackButton>
<ng-container slot="end">
<app-pop-out></app-pop-out>
</ng-container>
</popup-header>

<div class="tw-bg-background-alt">
<p>{{ "blockedDomainsDesc" | i18n }}</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense for this description to have info about the type of matching that is used for blocked domains?
Something along the lines of, 'This uses host matching', potentially with a link to the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think we need that documentation, though I'm not sure if we should clarify it in the ui here. Perhaps we could explore that in PM-15997 ?

<bit-section *ngIf="!isLoading">
<bit-section-header>
<h2 bitTypography="h6">{{ "domainsTitle" | i18n }}</h2>
<span bitTypography="body2" slot="end">{{ blockedDomainsState?.length || 0 }}</span>
</bit-section-header>

<ng-container *ngIf="blockedDomainsState">
<bit-item
*ngFor="let domain of blockedDomainsState; let i = index; trackBy: trackByFunction"
>
<bit-item-content>
<bit-label *ngIf="i >= fieldsEditThreshold">{{
"websiteItemLabel" | i18n: i + 1
}}</bit-label>
<input
*ngIf="i >= fieldsEditThreshold"
#uriInput
appInputVerbatim
bitInput
id="excludedDomain{{ i }}"
inputmode="url"
name="excludedDomain{{ i }}"
type="text"
(change)="fieldChange()"
[(ngModel)]="blockedDomainsState[i]"
/>
<div id="excludedDomain{{ i }}" *ngIf="i < fieldsEditThreshold">{{ domain }}</div>
</bit-item-content>
<button
*ngIf="i < fieldsEditThreshold"
appA11yTitle="{{ 'remove' | i18n }}"
bitIconButton="bwi-minus-circle"
buttonType="danger"
size="small"
slot="end"
type="button"
(click)="removeDomain(i)"
></button>
</bit-item>
</ng-container>
<button bitLink class="tw-pt-2" type="button" linkType="primary" (click)="addNewDomain()">
<i class="bwi bwi-plus-circle bwi-fw" aria-hidden="true"></i> {{ "addDomain" | i18n }}
</button>
</bit-section>
</div>
<popup-footer slot="footer">
<button
bitButton
buttonType="primary"
type="submit"
[disabled]="dataIsPristine"
(click)="saveChanges()"
>
{{ "save" | i18n }}
</button>
</popup-footer>
</popup-page>
Loading
Loading