Merge branch 'main' into autofill/pm-5189-fix-issues-present-with-inline-menu-rendering-in-iframes

.github/workflows/test.yml

@@ -1,5 +1,4 @@
----
-name: Run tests
+name: Testing
 
 on:
   workflow_dispatch:
@@ -8,29 +7,20 @@ on:
       - "main"
       - "rc"
       - "hotfix-rc-*"
-  pull_request_target:
+  pull_request:
     types: [opened, synchronize]
 
-defaults:
-  run:
-    shell: bash
-
 jobs:
-  check-run:
-    name: Check PR run
-    uses: bitwarden/gh-actions/.github/workflows/check-run.yml@main
-
   test:
     name: Run tests
     runs-on: ubuntu-22.04
-    needs: check-run
     permissions:
       checks: write
       contents: read
       pull-requests: write
       
     steps:
-      - name: Checkout repo
+      - name: Check out repo
         uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
 
       - name: Get Node Version
@@ -75,14 +65,26 @@ jobs:
           reporter: jest-junit
           fail-on-error: true
 
+      - name: Check for Codecov secret
+        id: check-codecov-secret
+        run: |
+          if [ "${{ secrets.CODECOV_TOKEN }}" != '' ]; then
+            echo "available=true" >> $GITHUB_OUTPUT;
+          else
+            echo "available=false" >> $GITHUB_OUTPUT;
+          fi
+
       - name: Upload to codecov.io
         uses: codecov/codecov-action@54bcd8715eee62d40e33596ef5e8f0f48dbbccab # v4.1.0
+        if: steps.check-codecov-secret.outputs.available == 'true'
         env:
           CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
 
   rust:
-    name: rust - ${{ matrix.os }}
+    name: Run Rust tests on ${{ matrix.os }}
     runs-on: ${{ matrix.os || 'ubuntu-latest' }}
+    permissions:
+      contents: read
 
     strategy:
       matrix:
@@ -92,7 +94,7 @@ jobs:
           - windows-latest
 
     steps:
-      - name: Rust version check
+      - name: Check Rust version
         run: rustup --version
 
       - name: Install gnome-keyring
@@ -101,7 +103,7 @@ jobs:
           sudo apt-get update
           sudo apt-get install -y gnome-keyring dbus-x11
 
-      - name: Checkout repo
+      - name: Check out repo
         uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
 
       - name: Build

apps/browser/src/background/main.background.ts

@@ -938,7 +938,6 @@ export default class MainBackground {
       logoutCallback,
       this.stateService,
       this.authService,
-      this.authRequestService,
       this.messagingService,
     );
 

apps/desktop/src/app/accounts/settings.component.html

@@ -166,20 +166,6 @@
                   "recommendedForSecurity" | i18n
                 }}</small>
               </div>
-              <div class="form-group">
-                <div class="checkbox">
-                  <label for="approveLoginRequests">
-                    <input
-                      id="approveLoginRequests"
-                      type="checkbox"
-                      formControlName="approveLoginRequests"
-                      (change)="updateApproveLoginRequests()"
-                    />
-                    {{ "approveLoginRequests" | i18n }}
-                  </label>
-                </div>
-                <small class="help-block">{{ "approveLoginRequestDesc" | i18n }}</small>
-              </div>
             </ng-container>
           </div>
         </div>

apps/desktop/src/app/accounts/settings.component.ts

@@ -3,7 +3,7 @@ import { FormBuilder } from "@angular/forms";
 import { BehaviorSubject, Observable, Subject, firstValueFrom } from "rxjs";
 import { concatMap, debounceTime, filter, map, switchMap, takeUntil, tap } from "rxjs/operators";
 
-import { AuthRequestServiceAbstraction, PinServiceAbstraction } from "@bitwarden/auth/common";
+import { PinServiceAbstraction } from "@bitwarden/auth/common";
 import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service";
 import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
 import { PolicyType } from "@bitwarden/common/admin-console/enums";
@@ -90,7 +90,6 @@ export class SettingsComponent implements OnInit {
     biometric: false,
     autoPromptBiometrics: false,
     requirePasswordOnStart: false,
-    approveLoginRequests: false,
     // Account Preferences
     clearClipboard: [null],
     minimizeOnCopyToClipboard: false,
@@ -135,7 +134,6 @@ export class SettingsComponent implements OnInit {
     private biometricStateService: BiometricStateService,
     private desktopAutofillSettingsService: DesktopAutofillSettingsService,
     private pinService: PinServiceAbstraction,
-    private authRequestService: AuthRequestServiceAbstraction,
     private logService: LogService,
     private nativeMessagingManifestService: NativeMessagingManifestService,
   ) {
@@ -275,8 +273,6 @@ export class SettingsComponent implements OnInit {
       requirePasswordOnStart: await firstValueFrom(
         this.biometricStateService.requirePasswordOnStart$,
       ),
-      approveLoginRequests:
-        (await this.authRequestService.getAcceptAuthRequests(this.currentUserId)) ?? false,
       clearClipboard: await firstValueFrom(this.autofillSettingsService.clearClipboardDelay$),
       minimizeOnCopyToClipboard: await firstValueFrom(this.desktopSettingsService.minimizeOnCopy$),
       enableFavicons: await firstValueFrom(this.domainSettingsService.showFavicons$),
@@ -722,13 +718,6 @@ export class SettingsComponent implements OnInit {
     );
   }
 
-  async updateApproveLoginRequests() {
-    await this.authRequestService.setAcceptAuthRequests(
-      this.form.value.approveLoginRequests,
-      this.currentUserId,
-    );
-  }
-
   ngOnDestroy() {
     this.destroy$.next();
     this.destroy$.complete();

apps/desktop/src/locales/en/messages.json

@@ -2401,9 +2401,6 @@
   "denyLogIn": {
     "message": "Deny login"
   },
-  "approveLoginRequests": {
-    "message": "Approve login requests"
-  },
   "logInConfirmedForEmailOnDevice": {
     "message": "Login confirmed for $EMAIL$ on $DEVICE$",
     "placeholders": {
@@ -2438,9 +2435,6 @@
   "thisRequestIsNoLongerValid": {
     "message": "This request is no longer valid."
   },
-  "approveLoginRequestDesc": {
-    "message": "Use this device to approve login requests made from other devices."
-  },
   "confirmLoginAtemptForMail": {
     "message": "Confirm login attempt for $EMAIL$",
     "placeholders": {

apps/desktop/src/vault/app/vault/vault.component.ts

@@ -8,7 +8,7 @@ import {
   ViewContainerRef,
 } from "@angular/core";
 import { ActivatedRoute, Router } from "@angular/router";
-import { firstValueFrom, Subject, takeUntil } from "rxjs";
+import { Subject, takeUntil } from "rxjs";
 import { first } from "rxjs/operators";
 
 import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref";
@@ -16,7 +16,6 @@ import { ModalService } from "@bitwarden/angular/services/modal.service";
 import { VaultFilter } from "@bitwarden/angular/vault/vault-filter/models/vault-filter.model";
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
-import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
 import { EventType } from "@bitwarden/common/enums";
 import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service";
@@ -32,7 +31,6 @@ import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
 import { DialogService } from "@bitwarden/components";
 import { PasswordRepromptService } from "@bitwarden/vault";
 
-import { AuthRequestServiceAbstraction } from "../../../../../../libs/auth/src/common/abstractions";
 import { SearchBarService } from "../../../app/layout/search/search-bar.service";
 import { GeneratorComponent } from "../../../app/tools/generator.component";
 import { invokeMenu, RendererMenuItem } from "../../../utils";
@@ -107,8 +105,6 @@ export class VaultComponent implements OnInit, OnDestroy {
     private apiService: ApiService,
     private dialogService: DialogService,
     private billingAccountProfileStateService: BillingAccountProfileStateService,
-    private authRequestService: AuthRequestServiceAbstraction,
-    private accountService: AccountService,
   ) {}
 
   async ngOnInit() {
@@ -226,15 +222,11 @@ export class VaultComponent implements OnInit, OnDestroy {
     this.searchBarService.setEnabled(true);
     this.searchBarService.setPlaceholderText(this.i18nService.t("searchVault"));
 
-    const userId = (await firstValueFrom(this.accountService.activeAccount$)).id;
-    const approveLoginRequests = await this.authRequestService.getAcceptAuthRequests(userId);
-    if (approveLoginRequests) {
-      const authRequest = await this.apiService.getLastAuthRequest();
-      if (authRequest != null) {
-        this.messagingService.send("openLoginApproval", {
-          notificationId: authRequest.id,
-        });
-      }
+    const authRequest = await this.apiService.getLastAuthRequest();
+    if (authRequest != null) {
+      this.messagingService.send("openLoginApproval", {
+        notificationId: authRequest.id,
+      });
     }
   }
 

apps/web/src/app/vault/org-vault/vault.component.ts

@@ -311,10 +311,6 @@ export class VaultComponent implements OnInit, OnDestroy {
 
     this.editableCollections$ = this.allCollectionsWithoutUnassigned$.pipe(
       map((collections) => {
-        // If restricted, providers can not add items to any collections or edit those items
-        if (this.organization.isProviderUser && this.restrictProviderAccessEnabled) {
-          return [];
-        }
         // Users that can edit all ciphers can implicitly add to / edit within any collection
         if (
           this.organization.canEditAllCiphers(
@@ -356,10 +352,6 @@ export class VaultComponent implements OnInit, OnDestroy {
         }
         let ciphers;
 
-        if (organization.isProviderUser && this.restrictProviderAccessEnabled) {
-          return [];
-        }
-
         if (this.flexibleCollectionsV1Enabled) {
           // Flexible collections V1 logic.
           // If the user can edit all ciphers for the organization then fetch them ALL.
@@ -488,10 +480,6 @@ export class VaultComponent implements OnInit, OnDestroy {
       organization$,
     ]).pipe(
       map(([filter, collection, organization]) => {
-        if (organization.isProviderUser && this.restrictProviderAccessEnabled) {
-          return collection != undefined || filter.collectionId === Unassigned;
-        }
-
         return (
           (filter.collectionId === Unassigned &&
             !organization.canEditUnassignedCiphers(this.restrictProviderAccessEnabled)) ||

bitwarden_license/bit-cli/src/admin-console/device-approval/device-approval.program.ts

@@ -26,7 +26,9 @@ export class DeviceApprovalProgram extends BaseProgram {
 
   private deviceApprovalCommand() {
     return new Command("device-approval")
-      .description("Manage device approvals")
+      .description(
+        "Manage device approval requests sent to organizations that use SSO with trusted devices.",
+      )
       .addCommand(this.listCommand())
       .addCommand(this.approveCommand())
       .addCommand(this.approveAllCommand())

libs/angular/src/services/jslib-services.module.ts

@@ -802,7 +802,6 @@ const safeProviders: SafeProvider[] = [
       LOGOUT_CALLBACK,
       StateServiceAbstraction,
       AuthServiceAbstraction,
-      AuthRequestServiceAbstraction,
       MessagingServiceAbstraction,
     ],
   }),

libs/auth/src/common/abstractions/auth-request.service.abstraction.ts

@@ -10,20 +10,6 @@ export abstract class AuthRequestServiceAbstraction {
   /** Emits an auth request id when an auth request has been approved. */
   authRequestPushNotification$: Observable<string>;
 
-  /**
-   * Returns true if the user has chosen to allow auth requests to show on this client.
-   * Intended to prevent spamming the user with auth requests.
-   * @param userId The user id.
-   * @throws If `userId` is not provided.
-   */
-  abstract getAcceptAuthRequests: (userId: UserId) => Promise<boolean>;
-  /**
-   * Sets whether to allow auth requests to show on this client for this user.
-   * @param accept Whether to allow auth requests to show on this client.
-   * @param userId The user id.
-   * @throws If `userId` is not provided.
-   */
-  abstract setAcceptAuthRequests: (accept: boolean, userId: UserId) => Promise<void>;
   /**
    * Returns an admin auth request for the given user if it exists.
    * @param userId The user id.

libs/auth/src/common/services/auth-request/auth-request.service.spec.ts

@@ -62,15 +62,6 @@ describe("AuthRequestService", () => {
     });
   });
 
-  describe("AcceptAuthRequests", () => {
-    it("returns an error when userId isn't provided", async () => {
-      await expect(sut.getAcceptAuthRequests(undefined)).rejects.toThrow("User ID is required");
-      await expect(sut.setAcceptAuthRequests(true, undefined)).rejects.toThrow(
-        "User ID is required",
-      );
-    });
-  });
-
   describe("AdminAuthRequest", () => {
     it("returns an error when userId isn't provided", async () => {
       await expect(sut.getAdminAuthRequest(undefined)).rejects.toThrow("User ID is required");

libs/auth/src/common/services/auth-request/auth-request.service.ts

@@ -22,20 +22,6 @@ import { MasterKey, UserKey } from "@bitwarden/common/types/key";
 
 import { AuthRequestServiceAbstraction } from "../../abstractions/auth-request.service.abstraction";
 
-/**
- * Disk-local to maintain consistency between tabs (even though
- * approvals are currently only available on desktop). We don't
- * want to clear this on logout as it's a user preference.
- */
-export const ACCEPT_AUTH_REQUESTS_KEY = new UserKeyDefinition<boolean>(
-  AUTH_REQUEST_DISK_LOCAL,
-  "acceptAuthRequests",
-  {
-    deserializer: (value) => value ?? false,
-    clearOn: [],
-  },
-);
-
 /**
  * Disk-local to maintain consistency between tabs. We don't want to
  * clear this on logout since admin auth requests are long-lived.
@@ -64,25 +50,6 @@ export class AuthRequestService implements AuthRequestServiceAbstraction {
     this.authRequestPushNotification$ = this.authRequestPushNotificationSubject.asObservable();
   }
 
-  async getAcceptAuthRequests(userId: UserId): Promise<boolean> {
-    if (userId == null) {
-      throw new Error("User ID is required");
-    }
-
-    const value = await firstValueFrom(
-      this.stateProvider.getUser(userId, ACCEPT_AUTH_REQUESTS_KEY).state$,
-    );
-    return value;
-  }
-
-  async setAcceptAuthRequests(accept: boolean, userId: UserId): Promise<void> {
-    if (userId == null) {
-      throw new Error("User ID is required");
-    }
-
-    await this.stateProvider.setUserState(ACCEPT_AUTH_REQUESTS_KEY, accept, userId);
-  }
-
   async getAdminAuthRequest(userId: UserId): Promise<AdminAuthRequestStorable | null> {
     if (userId == null) {
       throw new Error("User ID is required");

libs/common/src/admin-console/models/domain/organization.ts

@@ -195,10 +195,18 @@ export class Organization {
   }
 
   canEditUnassignedCiphers(restrictProviderAccessFlagEnabled: boolean) {
-    if (this.isProviderUser) {
-      return !restrictProviderAccessFlagEnabled;
+    // Providers can access items until the restrictProviderAccess flag is enabled
+    // After the flag is enabled and removed, this block will be deleted
+    // so that they permanently lose access to items
+    if (this.isProviderUser && !restrictProviderAccessFlagEnabled) {
+      return true;
     }
-    return this.isAdmin || this.permissions.editAnyCollection;
+
+    return (
+      this.type === OrganizationUserType.Admin ||
+      this.type === OrganizationUserType.Owner ||
+      this.permissions.editAnyCollection
+    );
   }
 
   canEditAllCiphers(
@@ -210,8 +218,11 @@ export class Organization {
       return this.isAdmin || this.permissions.editAnyCollection;
     }
 
-    if (this.isProviderUser) {
-      return !restrictProviderAccessFlagEnabled;
+    // Providers can access items until the restrictProviderAccess flag is enabled
+    // After the flag is enabled and removed, this block will be deleted
+    // so that they permanently lose access to items
+    if (this.isProviderUser && !restrictProviderAccessFlagEnabled) {
+      return true;
     }
 
     // Post Flexible Collections V1, the allowAdminAccessToAllCollectionItems flag can restrict admins

libs/common/src/services/notifications.service.ts

@@ -4,7 +4,6 @@ import { firstValueFrom } from "rxjs";
 
 import { LogoutReason } from "@bitwarden/auth/common";
 
-import { AuthRequestServiceAbstraction } from "../../../auth/src/common/abstractions";
 import { ApiService } from "../abstractions/api.service";
 import { NotificationsService as NotificationsServiceAbstraction } from "../abstractions/notifications.service";
 import { AuthService } from "../auth/abstractions/auth.service";
@@ -21,8 +20,7 @@ import { EnvironmentService } from "../platform/abstractions/environment.service
 import { LogService } from "../platform/abstractions/log.service";
 import { MessagingService } from "../platform/abstractions/messaging.service";
 import { StateService } from "../platform/abstractions/state.service";
-import { SyncService } from "../platform/sync/sync.service";
-import { UserId } from "../types/guid";
+import { SyncService } from "../vault/abstractions/sync/sync.service.abstraction";
 
 export class NotificationsService implements NotificationsServiceAbstraction {
   private signalrConnection: signalR.HubConnection;
@@ -41,7 +39,6 @@ export class NotificationsService implements NotificationsServiceAbstraction {
     private logoutCallback: (logoutReason: LogoutReason) => Promise<void>,
     private stateService: StateService,
     private authService: AuthService,
-    private authRequestService: AuthRequestServiceAbstraction,
     private messagingService: MessagingService,
   ) {
     this.environmentService.environment$.subscribe(() => {
@@ -205,12 +202,9 @@ export class NotificationsService implements NotificationsServiceAbstraction {
         break;
       case NotificationType.AuthRequest:
         {
-          const userId = await this.stateService.getUserId();
-          if (await this.authRequestService.getAcceptAuthRequests(userId as UserId)) {
-            this.messagingService.send("openLoginApproval", {
-              notificationId: notification.payload.id,
-            });
-          }
+          this.messagingService.send("openLoginApproval", {
+            notificationId: notification.payload.id,
+          });
         }
         break;
       default: