Skip to content

Commit

Permalink
Merge branch 'main' into PM-11777-browser-extension-totp-not-copied-w…
Browse files Browse the repository at this point in the history
…hen-autofilling-passkey
  • Loading branch information
dan-livefront authored Nov 1, 2024
2 parents 9a6f6c8 + a4c6731 commit b11fd25
Show file tree
Hide file tree
Showing 10 changed files with 115 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ export type OverlayBackgroundExtensionMessageHandlers = {
getCurrentTabFrameId: ({ sender }: BackgroundSenderParam) => number;
updateSubFrameData: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
triggerSubFrameFocusInRebuild: ({ sender }: BackgroundSenderParam) => void;
shouldRepositionSubFrameInlineMenuOnScroll: ({ sender }: BackgroundSenderParam) => void;
destroyAutofillInlineMenuListeners: ({
message,
sender,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -633,9 +633,7 @@ describe("OverlayBackground", () => {

it("skips updating the inline menu list if the user has the inline menu set to open on button click", async () => {
inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnButtonClick);
jest
.spyOn(overlayBackground as any, "checkIsInlineMenuListVisible")
.mockReturnValue(false);
overlayBackground["inlineMenuListPort"] = null;
tabsSendMessageSpy.mockImplementation((_tab, message, _options) => {
if (message.command === "checkFocusedFieldHasValue") {
return Promise.resolve(true);
Expand Down Expand Up @@ -2271,7 +2269,7 @@ describe("OverlayBackground", () => {
});

it("closes the list if the user has the inline menu set to show on button click and the list is open", async () => {
overlayBackground["isInlineMenuListVisible"] = true;
overlayBackground["inlineMenuListPort"] = listPortSpy;
inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnButtonClick);

sendMockExtensionMessage({ command: "openAutofillInlineMenu" }, sender);
Expand Down
20 changes: 2 additions & 18 deletions apps/browser/src/autofill/background/overlay.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,6 @@ export class OverlayBackground implements OverlayBackgroundInterface {
getCurrentTabFrameId: ({ sender }) => this.getSenderFrameId(sender),
updateSubFrameData: ({ message, sender }) => this.updateSubFrameData(message, sender),
triggerSubFrameFocusInRebuild: ({ sender }) => this.triggerSubFrameFocusInRebuild(sender),
shouldRepositionSubFrameInlineMenuOnScroll: ({ sender }) =>
this.shouldRepositionSubFrameInlineMenuOnScroll(sender),
destroyAutofillInlineMenuListeners: ({ message, sender }) =>
this.triggerDestroyInlineMenuListeners(sender.tab, message.subFrameData.frameId),
collectPageDetailsResponse: ({ message, sender }) => this.storePageDetails(message, sender),
Expand Down Expand Up @@ -1012,7 +1010,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
);

if (
!this.checkIsInlineMenuListVisible() &&
!this.inlineMenuListPort &&
(await this.getInlineMenuVisibility()) === AutofillOverlayVisibility.OnButtonClick
) {
return;
Expand Down Expand Up @@ -1825,7 +1823,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
return;
}

if (this.isInlineMenuListVisible) {
if (this.inlineMenuListPort) {
this.closeInlineMenu(sender, {
forceCloseInlineMenu: true,
overlayElement: AutofillOverlayElement.List,
Expand Down Expand Up @@ -2606,20 +2604,6 @@ export class OverlayBackground implements OverlayBackgroundInterface {
this.repositionInlineMenu$.next(sender);
}

/**
* Triggers on scroll of a frame within the tab. Will reposition the inline menu
* if the focused field is within a sub-frame and the inline menu is visible.
*
* @param sender - The sender of the message
*/
private shouldRepositionSubFrameInlineMenuOnScroll(sender: chrome.runtime.MessageSender) {
if (!this.isInlineMenuButtonVisible || sender.tab.id !== this.focusedFieldData?.tabId) {
return false;
}

return this.focusedFieldData.frameId > 0;
}

/**
* Handles determining if the inline menu should be repositioned or closed, and initiates
* the process of calculating the new position of the inline menu.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1703,6 +1703,10 @@ describe("AutofillOverlayContentService", () => {
const repositionEvents = [EVENTS.SCROLL, EVENTS.RESIZE];
repositionEvents.forEach((repositionEvent) => {
it(`sends a message trigger overlay reposition message to the background when a ${repositionEvent} event occurs`, async () => {
Object.defineProperty(globalThis, "scrollY", {
value: 10,
writable: true,
});
sendExtensionMessageSpy.mockResolvedValueOnce(true);
globalThis.dispatchEvent(new Event(repositionEvent));
await flushPromises();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1568,41 +1568,46 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
* the overlay elements on scroll or resize.
*/
private setOverlayRepositionEventListeners() {
let currentScrollY = globalThis.scrollY;
let currentScrollX = globalThis.scrollX;
let mostRecentTargetScrollY = 0;
const repositionHandler = this.useEventHandlersMemo(
throttle(this.handleOverlayRepositionEvent, 250),
AUTOFILL_OVERLAY_HANDLE_REPOSITION,
);

const eventTargetContainsFocusedField = (eventTarget: Element | Document) => {
if (!eventTarget || !this.mostRecentlyFocusedField) {
const eventTargetContainsFocusedField = (eventTarget: Element) => {
if (typeof eventTarget.contains !== "function") {
return false;
}

const activeElement = (eventTarget as Document).activeElement;
if (activeElement) {
return (
activeElement === this.mostRecentlyFocusedField ||
activeElement.contains(this.mostRecentlyFocusedField) ||
this.inlineMenuContentService?.isElementInlineMenu(activeElement as HTMLElement)
);
}

if (typeof eventTarget.contains !== "function") {
const targetScrollY = eventTarget.scrollTop;
if (targetScrollY === mostRecentTargetScrollY) {
return false;
}
return (

if (
eventTarget === this.mostRecentlyFocusedField ||
eventTarget.contains(this.mostRecentlyFocusedField)
);
) {
mostRecentTargetScrollY = targetScrollY;
return true;
}

return false;
};
const scrollHandler = this.useEventHandlersMemo(
throttle(async (event) => {
if (
eventTargetContainsFocusedField(event.target) ||
(await this.shouldRepositionSubFrameInlineMenuOnScroll())
currentScrollY !== globalThis.scrollY ||
currentScrollX !== globalThis.scrollX ||
eventTargetContainsFocusedField(event.target)
) {
repositionHandler(event);
}

currentScrollY = globalThis.scrollY;
currentScrollX = globalThis.scrollX;
}, 50),
AUTOFILL_OVERLAY_HANDLE_SCROLL,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ
const queueLength = this.mutationsQueue.length;

if (!this.domQueryService.pageContainsShadowDomElements()) {
this.domQueryService.checkPageContainsShadowDom();
this.checkPageContainsShadowDom();
}

for (let queueIndex = 0; queueIndex < queueLength; queueIndex++) {
Expand All @@ -999,6 +999,29 @@ export class CollectAutofillContentService implements CollectAutofillContentServ
this.mutationsQueue = [];
};

/**
* Handles checking if the current page contains a ShadowDOM element and
* flags that a re-collection of page details is required if it does.
*/
private checkPageContainsShadowDom() {
this.domQueryService.checkPageContainsShadowDom();
if (this.domQueryService.pageContainsShadowDomElements()) {
this.flagPageDetailsUpdateIsRequired();
}
}

/**
* Triggers several flags that indicate that a collection of page details should
* occur again on a subsequent call after a mutation has been observed in the DOM.
*/
private flagPageDetailsUpdateIsRequired() {
this.domRecentlyMutated = true;
if (this.autofillOverlayContentService) {
this.autofillOverlayContentService.pageDetailsUpdateRequired = true;
}
this.noFieldsFound = false;
}

/**
* Processes all mutation records encountered by the mutation observer.
*
Expand All @@ -1023,11 +1046,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ
(this.isAutofillElementNodeMutated(mutation.removedNodes, true) ||
this.isAutofillElementNodeMutated(mutation.addedNodes))
) {
this.domRecentlyMutated = true;
if (this.autofillOverlayContentService) {
this.autofillOverlayContentService.pageDetailsUpdateRequired = true;
}
this.noFieldsFound = false;
this.flagPageDetailsUpdateIsRequired();
return;
}

Expand Down
7 changes: 7 additions & 0 deletions libs/vault/src/cipher-form/cipher-form.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { CollectionView } from "@bitwarden/admin-console/common";
import { AuditService } from "@bitwarden/common/abstractions/audit.service";
import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service";
import { ClientType } from "@bitwarden/common/enums";
Expand Down Expand Up @@ -183,6 +184,12 @@ export default {
getClientType: () => ClientType.Browser,
},
},
{
provide: AccountService,
useValue: {
activeAccount$: new BehaviorSubject({ email: "[email protected]" }),
},
},
],
}),
componentWrapperDecorator(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ <h2 bitTypography="h6">{{ "itemDetails" | i18n }}</h2>
<bit-label>{{ "owner" | i18n }}</bit-label>
<bit-select formControlName="organizationId">
<bit-option
*ngIf="allowPersonalOwnership"
*ngIf="showPersonalOwnerOption"
[value]="null"
[label]="'selfOwnershipLabel' | i18n"
[label]="userEmail$ | async"
></bit-option>
<bit-option
*ngFor="let org of config.organizations"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import { CommonModule } from "@angular/common";
import { ComponentFixture, fakeAsync, TestBed, tick } from "@angular/core/testing";
import { ReactiveFormsModule } from "@angular/forms";
import { By } from "@angular/platform-browser";
import { mock, MockProxy } from "jest-mock-extended";
import { BehaviorSubject } from "rxjs";

import { CollectionView } from "@bitwarden/admin-console/common";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { SelectComponent } from "@bitwarden/components";

import { CipherFormConfig } from "../../abstractions/cipher-form-config.service";
import { CipherFormContainer } from "../../cipher-form-container";
Expand All @@ -20,6 +24,8 @@ describe("ItemDetailsSectionComponent", () => {
let cipherFormProvider: MockProxy<CipherFormContainer>;
let i18nService: MockProxy<I18nService>;

const activeAccount$ = new BehaviorSubject<{ email: string }>({ email: "[email protected]" });

beforeEach(async () => {
cipherFormProvider = mock<CipherFormContainer>();
i18nService = mock<I18nService>();
Expand All @@ -29,6 +35,7 @@ describe("ItemDetailsSectionComponent", () => {
providers: [
{ provide: CipherFormContainer, useValue: cipherFormProvider },
{ provide: I18nService, useValue: i18nService },
{ provide: AccountService, useValue: { activeAccount$ } },
],
}).compileComponents();

Expand Down Expand Up @@ -207,6 +214,35 @@ describe("ItemDetailsSectionComponent", () => {
});
});

describe("showPersonalOwnerOption", () => {
it("should show personal ownership when the configuration allows", () => {
component.config.mode = "edit";
component.config.allowPersonalOwnership = true;
component.config.organizations = [{ id: "134-433-22" } as Organization];
fixture.detectChanges();

const select = fixture.debugElement.query(By.directive(SelectComponent));
const { value, label } = select.componentInstance.items[0];

expect(value).toBeNull();
expect(label).toBe("[email protected]");
});

it("should show personal ownership when the control is disabled", async () => {
component.config.mode = "edit";
component.config.allowPersonalOwnership = false;
component.config.organizations = [{ id: "134-433-22" } as Organization];
await component.ngOnInit();
fixture.detectChanges();

const select = fixture.debugElement.query(By.directive(SelectComponent));

const { value, label } = select.componentInstance.items[0];
expect(value).toBeNull();
expect(label).toBe("[email protected]");
});
});

describe("showOwnership", () => {
it("should return true if ownership change is allowed or in edit mode with at least one organization", () => {
jest.spyOn(component, "allowOwnershipChange", "get").mockReturnValue(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { concatMap, map } from "rxjs";
import { CollectionView } from "@bitwarden/admin-console/common";
import { JslibModule } from "@bitwarden/angular/jslib.module";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
Expand Down Expand Up @@ -68,6 +69,9 @@ export class ItemDetailsSectionComponent implements OnInit {

protected showCollectionsControl: boolean;

/** The email address associated with the active account */
protected userEmail$ = this.accountService.activeAccount$.pipe(map((account) => account.email));

@Input({ required: true })
config: CipherFormConfig;

Expand Down Expand Up @@ -96,11 +100,23 @@ export class ItemDetailsSectionComponent implements OnInit {
return this.config.initialValues;
}

/**
* Show the personal ownership option in the Owner dropdown when:
* - Personal ownership is allowed
* - The `organizationId` control is disabled. This avoids the scenario
* where a the dropdown is empty because the user personally owns the cipher
* but cannot edit the ownership.
*/
get showPersonalOwnerOption() {
return this.allowPersonalOwnership || !this.itemDetailsForm.controls.organizationId.enabled;
}

constructor(
private cipherFormContainer: CipherFormContainer,
private formBuilder: FormBuilder,
private i18nService: I18nService,
private destroyRef: DestroyRef,
private accountService: AccountService,
) {
this.cipherFormContainer.registerChildForm("itemDetails", this.itemDetailsForm);
this.itemDetailsForm.valueChanges
Expand Down

0 comments on commit b11fd25

Please sign in to comment.