From 8ea3b7951237417c17b17523459d3694e57b4351 Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Tue, 21 May 2024 17:28:10 -0500 Subject: [PATCH 1/8] [PM-8289] Inline menu content script does not update when user updates setting (#9279) * [PM-8289] Inline menu content script does not update whne user updates setting * [PM-8289] Fixing issue present within Jest tests * [PM-8289] Triggering a reload of autofill scripts when a user logs into their account --- .../popup/settings/autofill.component.ts | 25 ---- .../services/autofill.service.spec.ts | 113 ++++++++++++++---- .../src/autofill/services/autofill.service.ts | 43 ++++++- .../collect-autofill-content.service.ts | 4 + .../src/background/runtime.background.ts | 1 + 5 files changed, 134 insertions(+), 52 deletions(-) diff --git a/apps/browser/src/autofill/popup/settings/autofill.component.ts b/apps/browser/src/autofill/popup/settings/autofill.component.ts index 67cc25f2276..1c6583331f4 100644 --- a/apps/browser/src/autofill/popup/settings/autofill.component.ts +++ b/apps/browser/src/autofill/popup/settings/autofill.component.ts @@ -105,11 +105,7 @@ export class AutofillComponent implements OnInit { } async updateAutoFillOverlayVisibility() { - const previousAutoFillOverlayVisibility = await firstValueFrom( - this.autofillSettingsService.inlineMenuVisibility$, - ); await this.autofillSettingsService.setInlineMenuVisibility(this.autoFillOverlayVisibility); - await this.handleUpdatingAutofillOverlayContentScripts(previousAutoFillOverlayVisibility); await this.requestPrivacyPermission(); } @@ -181,27 +177,6 @@ export class AutofillComponent implements OnInit { BrowserApi.createNewTab(this.disablePasswordManagerLink); } - private async handleUpdatingAutofillOverlayContentScripts( - previousAutoFillOverlayVisibility: number, - ) { - const autofillOverlayPreviouslyDisabled = - previousAutoFillOverlayVisibility === AutofillOverlayVisibility.Off; - const autofillOverlayCurrentlyDisabled = - this.autoFillOverlayVisibility === AutofillOverlayVisibility.Off; - - if (!autofillOverlayPreviouslyDisabled && !autofillOverlayCurrentlyDisabled) { - const tabs = await BrowserApi.tabsQuery({}); - tabs.forEach((tab) => - BrowserApi.tabSendMessageData(tab, "updateAutofillOverlayVisibility", { - autofillOverlayVisibility: this.autoFillOverlayVisibility, - }), - ); - return; - } - - await this.autofillService.reloadAutofillScripts(); - } - async requestPrivacyPermission() { if ( this.autoFillOverlayVisibility === AutofillOverlayVisibility.Off || diff --git a/apps/browser/src/autofill/services/autofill.service.spec.ts b/apps/browser/src/autofill/services/autofill.service.spec.ts index 158fde6a567..24b1c90b3fa 100644 --- a/apps/browser/src/autofill/services/autofill.service.spec.ts +++ b/apps/browser/src/autofill/services/autofill.service.spec.ts @@ -1,5 +1,5 @@ -import { mock, mockReset } from "jest-mock-extended"; -import { of } from "rxjs"; +import { mock, mockReset, MockProxy } from "jest-mock-extended"; +import { BehaviorSubject, of } from "rxjs"; import { UserVerificationService } from "@bitwarden/common/auth/services/user-verification/user-verification.service"; import { AutofillOverlayVisibility } from "@bitwarden/common/autofill/constants"; @@ -8,6 +8,7 @@ import { DefaultDomainSettingsService, DomainSettingsService, } from "@bitwarden/common/autofill/services/domain-settings.service"; +import { InlineMenuVisibilitySetting } from "@bitwarden/common/autofill/types"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EventType } from "@bitwarden/common/enums"; import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service"; @@ -45,7 +46,7 @@ import { createChromeTabMock, createGenerateFillScriptOptionsMock, } from "../spec/autofill-mocks"; -import { triggerTestFailure } from "../spec/testing-utils"; +import { flushPromises, triggerTestFailure } from "../spec/testing-utils"; import { AutoFillOptions, @@ -64,7 +65,8 @@ const mockEquivalentDomains = [ describe("AutofillService", () => { let autofillService: AutofillService; const cipherService = mock(); - const autofillSettingsService = mock(); + let inlineMenuVisibilityMock$!: BehaviorSubject; + let autofillSettingsService: MockProxy; const mockUserId = Utils.newGuid() as UserId; const accountService: FakeAccountService = mockAccountServiceWith(mockUserId); const fakeStateProvider: FakeStateProvider = new FakeStateProvider(accountService); @@ -79,6 +81,9 @@ describe("AutofillService", () => { beforeEach(() => { scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService); + inlineMenuVisibilityMock$ = new BehaviorSubject(AutofillOverlayVisibility.OnFieldFocus); + autofillSettingsService = mock(); + (autofillSettingsService as any).inlineMenuVisibility$ = inlineMenuVisibilityMock$; autofillService = new AutofillService( cipherService, autofillSettingsService, @@ -142,17 +147,92 @@ describe("AutofillService", () => { // eslint-disable-next-line no-restricted-syntax expect(chrome.runtime.onConnect.addListener).toHaveBeenCalledWith(expect.any(Function)); }); + + describe("handle inline menu visibility change", () => { + beforeEach(async () => { + await autofillService.loadAutofillScriptsOnInstall(); + jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue([tab1, tab2]); + jest.spyOn(BrowserApi, "tabSendMessageData").mockImplementation(); + jest.spyOn(autofillService, "reloadAutofillScripts").mockImplementation(); + }); + + it("returns early if the setting is being initialized", async () => { + await flushPromises(); + + expect(BrowserApi.tabsQuery).toHaveBeenCalledTimes(1); + expect(BrowserApi.tabSendMessageData).not.toHaveBeenCalled(); + }); + + it("returns early if the previous setting is equivalent to the new setting", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus); + await flushPromises(); + + expect(BrowserApi.tabsQuery).toHaveBeenCalledTimes(1); + expect(BrowserApi.tabSendMessageData).not.toHaveBeenCalled(); + }); + + describe("updates the inline menu visibility setting", () => { + it("when changing the inline menu from on focus of field to on button click", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnButtonClick); + await flushPromises(); + + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + tab1, + "updateAutofillOverlayVisibility", + { autofillOverlayVisibility: AutofillOverlayVisibility.OnButtonClick }, + ); + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + tab2, + "updateAutofillOverlayVisibility", + { autofillOverlayVisibility: AutofillOverlayVisibility.OnButtonClick }, + ); + }); + + it("when changing the inline menu from button click to field focus", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnButtonClick); + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus); + await flushPromises(); + + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + tab1, + "updateAutofillOverlayVisibility", + { autofillOverlayVisibility: AutofillOverlayVisibility.OnFieldFocus }, + ); + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + tab2, + "updateAutofillOverlayVisibility", + { autofillOverlayVisibility: AutofillOverlayVisibility.OnFieldFocus }, + ); + }); + }); + + describe("reloads the autofill scripts", () => { + it("when changing the inline menu from a disabled setting to an enabled setting", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.Off); + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus); + await flushPromises(); + + expect(autofillService.reloadAutofillScripts).toHaveBeenCalled(); + }); + + it("when changing the inline menu from a enabled setting to a disabled setting", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus); + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.Off); + await flushPromises(); + + expect(autofillService.reloadAutofillScripts).toHaveBeenCalled(); + }); + }); + }); }); describe("reloadAutofillScripts", () => { - it("disconnects and removes all autofill script ports", () => { - const port1 = mock({ - disconnect: jest.fn(), - }); - const port2 = mock({ - disconnect: jest.fn(), - }); + it("re-injects the autofill scripts in all tabs and disconnects all connected ports", () => { + const port1 = mock(); + const port2 = mock(); autofillService["autofillScriptPortsSet"] = new Set([port1, port2]); + jest.spyOn(autofillService as any, "injectAutofillScriptsInAllTabs"); + jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true); // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises @@ -161,17 +241,6 @@ describe("AutofillService", () => { expect(port1.disconnect).toHaveBeenCalled(); expect(port2.disconnect).toHaveBeenCalled(); expect(autofillService["autofillScriptPortsSet"].size).toBe(0); - }); - - it("re-injects the autofill scripts in all tabs", () => { - autofillService["autofillScriptPortsSet"] = new Set([mock()]); - jest.spyOn(autofillService as any, "injectAutofillScriptsInAllTabs"); - jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true); - - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - autofillService.reloadAutofillScripts(); - expect(autofillService["injectAutofillScriptsInAllTabs"]).toHaveBeenCalled(); }); }); diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index 80829ee7141..5348ca5b9ab 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/apps/browser/src/autofill/services/autofill.service.ts @@ -1,4 +1,5 @@ -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, startWith } from "rxjs"; +import { pairwise } from "rxjs/operators"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -70,10 +71,12 @@ export default class AutofillService implements AutofillServiceInterface { */ async loadAutofillScriptsOnInstall() { BrowserApi.addListener(chrome.runtime.onConnect, this.handleInjectedScriptPortConnection); - - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.injectAutofillScriptsInAllTabs(); + void this.injectAutofillScriptsInAllTabs(); + this.autofillSettingsService.inlineMenuVisibility$ + .pipe(startWith(undefined), pairwise()) + .subscribe(([previousSetting, currentSetting]) => + this.handleInlineMenuVisibilityChange(previousSetting, currentSetting), + ); } /** @@ -2090,4 +2093,34 @@ export default class AutofillService implements AutofillServiceInterface { } } } + + /** + * Updates the autofill inline menu visibility setting in all active tabs + * when the InlineMenuVisibilitySetting observable is updated. + * + * @param previousSetting - The previous setting value + * @param currentSetting - The current setting value + */ + private async handleInlineMenuVisibilityChange( + previousSetting: InlineMenuVisibilitySetting, + currentSetting: InlineMenuVisibilitySetting, + ) { + if (previousSetting === undefined || previousSetting === currentSetting) { + return; + } + + const inlineMenuPreviouslyDisabled = previousSetting === AutofillOverlayVisibility.Off; + const inlineMenuCurrentlyDisabled = currentSetting === AutofillOverlayVisibility.Off; + if (!inlineMenuPreviouslyDisabled && !inlineMenuCurrentlyDisabled) { + const tabs = await BrowserApi.tabsQuery({}); + tabs.forEach((tab) => + BrowserApi.tabSendMessageData(tab, "updateAutofillOverlayVisibility", { + autofillOverlayVisibility: currentSetting, + }), + ); + return; + } + + await this.reloadAutofillScripts(); + } } diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.ts index 7c49a3d9881..909d8584304 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.ts @@ -1349,6 +1349,10 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte } const cachedAutofillFieldElement = this.autofillFieldElements.get(formFieldElement); + if (!cachedAutofillFieldElement) { + continue; + } + cachedAutofillFieldElement.viewable = true; void this.autofillOverlayContentService?.setupAutofillOverlayListenerOnField( diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index 5473664e3b0..388060d88c9 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -188,6 +188,7 @@ export default class RuntimeBackground { if (msg.command === "loggedIn") { await this.sendBwInstalledMessageToVault(); + await this.autofillService.reloadAutofillScripts(); } if (this.lockedVaultPendingNotifications?.length > 0) { From b0cc48085380c227049cb4158626a5a42d5a70f0 Mon Sep 17 00:00:00 2001 From: Alex Urbina <42731074+urbinaalex17@users.noreply.github.com> Date: Tue, 21 May 2024 16:34:19 -0600 Subject: [PATCH 2/8] FIX: version-auto-bump.yml trigger parameters (#9298) --- .github/workflows/version-auto-bump.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/version-auto-bump.yml b/.github/workflows/version-auto-bump.yml index d0d028e570f..f10abee300d 100644 --- a/.github/workflows/version-auto-bump.yml +++ b/.github/workflows/version-auto-bump.yml @@ -27,9 +27,9 @@ jobs: env: GH_TOKEN: ${{ steps.retrieve-bot-secrets.outputs.github-pat-bitwarden-devops-bot-repo-scope }} run: | - echo '{"cut_rc_branch": "false", \ - "bump_browser": "false", \ - "bump_cli": "false", \ - "bump_desktop": "true", \ + echo '{"cut_rc_branch": "false", + "bump_browser": "false", + "bump_cli": "false", + "bump_desktop": "true", "bump_web": "false"}' | \ gh workflow run version-bump.yml --json --repo bitwarden/clients From ace524c8c7a1a7cb8211143f64226d76052647c4 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 22 May 2024 08:05:48 -0400 Subject: [PATCH 3/8] [deps] Platform: Update @types/jquery to v3.5.30 (#9148) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 08365b8200f..ccb7bd8ba78 100644 --- a/package-lock.json +++ b/package-lock.json @@ -99,7 +99,7 @@ "@types/firefox-webext-browser": "111.0.5", "@types/inquirer": "8.2.10", "@types/jest": "29.5.12", - "@types/jquery": "3.5.29", + "@types/jquery": "3.5.30", "@types/jsdom": "21.1.6", "@types/koa": "2.14.0", "@types/koa__multer": "2.0.7", @@ -10340,9 +10340,9 @@ "dev": true }, "node_modules/@types/jquery": { - "version": "3.5.29", - "resolved": "https://registry.npmjs.org/@types/jquery/-/jquery-3.5.29.tgz", - "integrity": "sha512-oXQQC9X9MOPRrMhPHHOsXqeQDnWeCDT3PelUIg/Oy8FAbzSZtFHRjc7IpbfFVmpLtJ+UOoywpRsuO5Jxjybyeg==", + "version": "3.5.30", + "resolved": "https://registry.npmjs.org/@types/jquery/-/jquery-3.5.30.tgz", + "integrity": "sha512-nbWKkkyb919DOUxjmRVk8vwtDb0/k8FKncmUKFi+NY+QXqWltooxTrswvz4LspQwxvLdvzBN1TImr6cw3aQx2A==", "dev": true, "dependencies": { "@types/sizzle": "*" diff --git a/package.json b/package.json index 45c2d73cb91..1850de265ea 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,7 @@ "@types/firefox-webext-browser": "111.0.5", "@types/inquirer": "8.2.10", "@types/jest": "29.5.12", - "@types/jquery": "3.5.29", + "@types/jquery": "3.5.30", "@types/jsdom": "21.1.6", "@types/koa": "2.14.0", "@types/koa__multer": "2.0.7", From 588663f80f53f7c74f33dd55dc8b16d562ee9a03 Mon Sep 17 00:00:00 2001 From: vinith-kovan <156108204+vinith-kovan@users.noreply.github.com> Date: Wed, 22 May 2024 17:49:24 +0530 Subject: [PATCH 4/8] [PM 4983] migrate vault timeout input component (#8661) * migrate vault timeout input component * migrate vault timeout input component --- .../vault-timeout-input.component.html | 68 +++++++------------ 1 file changed, 25 insertions(+), 43 deletions(-) diff --git a/apps/web/src/app/settings/vault-timeout-input.component.html b/apps/web/src/app/settings/vault-timeout-input.component.html index 3791fa7e543..4911b1d57db 100644 --- a/apps/web/src/app/settings/vault-timeout-input.component.html +++ b/apps/web/src/app/settings/vault-timeout-input.component.html @@ -1,47 +1,29 @@
-
- - - {{ + + {{ "vaultTimeout" | i18n }} + + + + {{ ((canLockVault$ | async) ? "vaultTimeoutDesc" : "vaultTimeoutLogoutDesc") | i18n - }} -
-
- -
-
- - {{ "hours" | i18n }} -
-
- - {{ "minutes" | i18n }} -
-
- - - {{ "vaultCustomTimeoutMinimum" | i18n }} - + }} + +
+ + {{ "customVaultTimeout" | i18n }} + + {{ "hours" | i18n }} + + + + {{ "minutes" | i18n }} +
+ + {{ "vaultCustomTimeoutMinimum" | i18n }} +
From ca62f0cfa347f68bc281815372b79a9d9092cb91 Mon Sep 17 00:00:00 2001 From: vinith-kovan <156108204+vinith-kovan@users.noreply.github.com> Date: Wed, 22 May 2024 18:04:48 +0530 Subject: [PATCH 5/8] [AC-2405] migrate SCIM component (#8781) * migrating scim authentication component * migrating scim authentication component * migrating scim authentication component --- .../organizations/manage/scim.component.html | 84 ++++++------------- .../organizations/manage/scim.component.ts | 81 ++++++++---------- 2 files changed, 60 insertions(+), 105 deletions(-) diff --git a/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/scim.component.html b/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/scim.component.html index 6724b4a0e01..e30883515e0 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/scim.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/scim.component.html @@ -1,39 +1,21 @@ -

{{ "scimDescription" | i18n }}

+

{{ "scimDescription" | i18n }}

- {{ "loading" | i18n }} + {{ "loading" | i18n }}
-
-
-
- - -
- {{ "scimEnabledCheckboxDescHelpText" | i18n }} -
-
-
- + + + + {{ "scimEnabledCheckboxDesc" | i18n }} + {{ "scimEnabledCheckboxDescHelpText" | i18n }} + {{ "scimUrl" | i18n }} @@ -41,7 +23,7 @@ type="button" bitSuffix bitIconButton="bwi-clone" - (click)="copyScimUrl()" + [bitAction]="copyScimUrl" [appA11yTitle]="'copyScimUrl' | i18n" > @@ -54,44 +36,32 @@ formControlName="clientSecret" id="clientSecret" /> - - - - - - - + + {{ "scimApiKeyHelperText" | i18n }} -
diff --git a/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/scim.component.ts b/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/scim.component.ts index 8e8db457e50..55ae318e982 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/scim.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/scim.component.ts @@ -14,7 +14,6 @@ import { OrganizationApiKeyRequest } from "@bitwarden/common/admin-console/model import { OrganizationConnectionRequest } from "@bitwarden/common/admin-console/models/request/organization-connection.request"; import { ScimConfigRequest } from "@bitwarden/common/admin-console/models/request/scim-config.request"; import { OrganizationConnectionResponse } from "@bitwarden/common/admin-console/models/response/organization-connection.response"; -import { ApiKeyResponse } from "@bitwarden/common/auth/models/response/api-key.response"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -29,8 +28,6 @@ export class ScimComponent implements OnInit { loading = true; organizationId: string; existingConnectionId: string; - formPromise: Promise>; - rotatePromise: Promise; enabled = new FormControl(false); showScimSettings = false; showScimKey = false; @@ -82,11 +79,11 @@ export class ScimComponent implements OnInit { }); } - async copyScimUrl() { + copyScimUrl = async () => { this.platformUtilsService.copyToClipboard(await this.getScimEndpointUrl()); - } + }; - async rotateScimKey() { + rotateScimKey = async () => { const confirmed = await this.dialogService.openSimpleDialog({ title: { key: "rotateScimKey" }, content: { key: "rotateScimKeyWarning" }, @@ -102,62 +99,50 @@ export class ScimComponent implements OnInit { request.type = OrganizationApiKeyType.Scim; request.masterPasswordHash = "N/A"; - this.rotatePromise = this.organizationApiService.rotateApiKey(this.organizationId, request); + const response = await this.organizationApiService.rotateApiKey(this.organizationId, request); + this.formData.setValue({ + endpointUrl: await this.getScimEndpointUrl(), + clientSecret: response.apiKey, + }); + this.platformUtilsService.showToast("success", null, this.i18nService.t("scimApiKeyRotated")); + }; - try { - const response = await this.rotatePromise; - this.formData.setValue({ - endpointUrl: await this.getScimEndpointUrl(), - clientSecret: response.apiKey, - }); - this.platformUtilsService.showToast("success", null, this.i18nService.t("scimApiKeyRotated")); - } catch { - // Logged by appApiAction, do nothing - } - - this.rotatePromise = null; - } - - async copyScimKey() { + copyScimKey = async () => { this.platformUtilsService.copyToClipboard(this.formData.get("clientSecret").value); - } + }; - async submit() { - try { - const request = new OrganizationConnectionRequest( - this.organizationId, - OrganizationConnectionType.Scim, - true, - new ScimConfigRequest(this.enabled.value), + submit = async () => { + const request = new OrganizationConnectionRequest( + this.organizationId, + OrganizationConnectionType.Scim, + true, + new ScimConfigRequest(this.enabled.value), + ); + let response: OrganizationConnectionResponse; + + if (this.existingConnectionId == null) { + response = await this.apiService.createOrganizationConnection(request, ScimConfigApi); + } else { + response = await this.apiService.updateOrganizationConnection( + request, + ScimConfigApi, + this.existingConnectionId, ); - if (this.existingConnectionId == null) { - this.formPromise = this.apiService.createOrganizationConnection(request, ScimConfigApi); - } else { - this.formPromise = this.apiService.updateOrganizationConnection( - request, - ScimConfigApi, - this.existingConnectionId, - ); - } - const response = (await this.formPromise) as OrganizationConnectionResponse; - await this.setConnectionFormValues(response); - this.platformUtilsService.showToast("success", null, this.i18nService.t("scimSettingsSaved")); - } catch (e) { - // Logged by appApiAction, do nothing } - this.formPromise = null; - } + await this.setConnectionFormValues(response); + this.platformUtilsService.showToast("success", null, this.i18nService.t("scimSettingsSaved")); + }; async getScimEndpointUrl() { const env = await firstValueFrom(this.environmentService.environment$); return env.getScimUrl() + "/" + this.organizationId; } - toggleScimKey() { + toggleScimKey = () => { this.showScimKey = !this.showScimKey; document.getElementById("clientSecret").focus(); - } + }; private async setConnectionFormValues(connection: OrganizationConnectionResponse) { this.existingConnectionId = connection?.id; From a5bfff891bb2d5024cd7263a255fa062d4439ab3 Mon Sep 17 00:00:00 2001 From: vinith-kovan <156108204+vinith-kovan@users.noreply.github.com> Date: Wed, 22 May 2024 18:50:06 +0530 Subject: [PATCH 6/8] migrate policy edit component (#8914) --- .../policies/policies.component.ts | 36 ++++---- .../policies/policy-edit.component.html | 71 ++++++--------- .../policies/policy-edit.component.ts | 87 ++++++++++--------- 3 files changed, 88 insertions(+), 106 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/policies/policies.component.ts b/apps/web/src/app/admin-console/organizations/policies/policies.component.ts index 6f8bb27671f..782db231c69 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policies.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policies.component.ts @@ -1,18 +1,19 @@ import { Component, OnInit, ViewChild, ViewContainerRef } from "@angular/core"; -import { ActivatedRoute, Router } from "@angular/router"; +import { ActivatedRoute } from "@angular/router"; +import { lastValueFrom } from "rxjs"; import { first } from "rxjs/operators"; -import { ModalService } from "@bitwarden/angular/services/modal.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { PolicyResponse } from "@bitwarden/common/admin-console/models/response/policy.response"; +import { DialogService } from "@bitwarden/components"; import { PolicyListService } from "../../core/policy-list.service"; import { BasePolicy } from "../policies"; -import { PolicyEditComponent } from "./policy-edit.component"; +import { PolicyEditComponent, PolicyEditDialogResult } from "./policy-edit.component"; @Component({ selector: "app-org-policies", @@ -33,11 +34,10 @@ export class PoliciesComponent implements OnInit { constructor( private route: ActivatedRoute, - private modalService: ModalService, private organizationService: OrganizationService, private policyApiService: PolicyApiServiceAbstraction, private policyListService: PolicyListService, - private router: Router, + private dialogService: DialogService, ) {} async ngOnInit() { @@ -83,21 +83,17 @@ export class PoliciesComponent implements OnInit { } async edit(policy: BasePolicy) { - const [modal] = await this.modalService.openViewRef( - PolicyEditComponent, - this.editModalRef, - (comp) => { - comp.policy = policy; - comp.organizationId = this.organizationId; - comp.policiesEnabledMap = this.policiesEnabledMap; - // eslint-disable-next-line rxjs-angular/prefer-takeuntil - comp.onSavedPolicy.subscribe(() => { - modal.close(); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.load(); - }); + const dialogRef = PolicyEditComponent.open(this.dialogService, { + data: { + policy: policy, + organizationId: this.organizationId, + policiesEnabledMap: this.policiesEnabledMap, }, - ); + }); + + const result = await lastValueFrom(dialogRef.closed); + if (result === PolicyEditDialogResult.Saved) { + await this.load(); + } } } diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit.component.html b/apps/web/src/app/admin-console/organizations/policies/policy-edit.component.html index 5f5dc1d85ef..063a8759e6f 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit.component.html +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit.component.html @@ -1,49 +1,28 @@ -