Merge branch 'autofill/pm-8869-regression-autofill-broken-safari' into autofill/pm-8027-inline-menu-appears-within-input-fields-that-do-not-relate-to-user-login

This commit is contained in:
Cesar Gonzalez 2024-06-14 10:08:37 -05:00
commit 09f69e774c
No known key found for this signature in database
GPG Key ID: 3381A5457F8CCECF
21 changed files with 81 additions and 142 deletions

View File

@ -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

View File

@ -21,6 +21,7 @@ import {
nodeIsFormElement,
nodeIsInputElement,
sendExtensionMessage,
requestIdleCallbackPolyfill,
} from "../utils";
import { AutofillOverlayContentService } from "./abstractions/autofill-overlay-content.service";
@ -1055,7 +1056,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
}
if (!this.mutationsQueue.length) {
globalThis.requestIdleCallback(this.processMutations, { timeout: 500 });
requestIdleCallbackPolyfill(this.processMutations, { timeout: 500 });
}
this.mutationsQueue.push(mutations);
};
@ -1192,7 +1193,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
continue;
}
globalThis.requestIdleCallback(
requestIdleCallbackPolyfill(
// We are setting this item to a -1 index because we do not know its position in the DOM.
// This value should be updated with the next call to collect page details.
() => void this.buildAutofillFieldItem(node as ElementWithOpId<FormFieldElement>, -1),

View File

@ -1,6 +1,20 @@
import { AutofillPort } from "../enums/autofill-port.enums";
import { FillableFormFieldElement, FormFieldElement } from "../types";
/**
* Polyfills the requestIdleCallback API with a setTimeout fallback.
*
* @param callback - The callback function to run when the browser is idle.
* @param options - The options to pass to the requestIdleCallback function.
*/
export function requestIdleCallbackPolyfill(callback: () => void, options?: Record<string, any>) {
if ("requestIdleCallback" in globalThis) {
return globalThis.requestIdleCallback(() => callback(), options);
}
return globalThis.setTimeout(() => callback(), 1);
}
/**
* Generates a random string of characters that formatted as a custom element name.
*/

View File

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

View File

@ -2,10 +2,10 @@ import { Injectable } from "@angular/core";
import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import { FormBuilder } from "@angular/forms";
import {
Observable,
combineLatest,
distinctUntilChanged,
map,
Observable,
startWith,
switchMap,
tap,
@ -104,6 +104,11 @@ export class VaultPopupListFiltersService {
map(
(filters) => (ciphers: CipherView[]) =>
ciphers.filter((cipher) => {
// Vault popup lists never shows deleted ciphers
if (cipher.isDeleted) {
return false;
}
if (filters.cipherType !== null && cipher.type !== filters.cipherType) {
return false;
}

View File

@ -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>

View File

@ -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();

View File

@ -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": {

View File

@ -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,
});
}
}

View File

@ -1,6 +1,6 @@
{
"name": "@bitwarden/web-vault",
"version": "2024.6.0",
"version": "2024.6.1",
"scripts": {
"build:oss": "webpack",
"build:bit": "webpack -c ../../bitwarden_license/bit-web/webpack.config.js",

View File

@ -69,9 +69,9 @@ export class VaultCollectionRowComponent {
if (this.collection instanceof CollectionAdminView) {
// Only show AddAccess if unmanaged and allowAdminAccessToAllCollectionItems is disabled
return (
!this.organization.allowAdminAccessToAllCollectionItems &&
!this.organization?.allowAdminAccessToAllCollectionItems &&
this.collection.unmanaged &&
this.organization.canEditUnmanagedCollections()
this.organization?.canEditUnmanagedCollections()
);
}

View File

@ -93,7 +93,7 @@
</ng-container>
<bit-search
*ngIf="organization?.isProviderUser"
*ngIf="restrictProviderAccessFlag && organization?.isProviderUser && !organization?.isMember"
class="tw-grow"
[ngModel]="searchText"
(ngModelChange)="onSearchTextChanged($event)"

View File

@ -68,7 +68,7 @@ export class VaultHeaderComponent implements OnInit {
protected organizations$ = this.organizationService.organizations$;
protected flexibleCollectionsV1Enabled = false;
private restrictProviderAccessFlag = false;
protected restrictProviderAccessFlag = false;
constructor(
private organizationService: OrganizationService,
@ -220,7 +220,11 @@ export class VaultHeaderComponent implements OnInit {
}
get canCreateCipher(): boolean {
if (this.organization?.isProviderUser && this.restrictProviderAccessFlag) {
if (
this.organization?.isProviderUser &&
this.restrictProviderAccessFlag &&
!this.organization?.isMember
) {
return false;
}
return true;

View File

@ -166,7 +166,11 @@ export class VaultComponent implements OnInit, OnDestroy {
}
protected get hideVaultFilters(): boolean {
return this.restrictProviderAccessEnabled && this.organization?.isProviderUser;
return (
this.restrictProviderAccessEnabled &&
this.organization?.isProviderUser &&
!this.organization?.isMember
);
}
private searchText$ = new Subject<string>();
@ -352,6 +356,16 @@ export class VaultComponent implements OnInit, OnDestroy {
}
let ciphers;
// Restricted providers (who are not members) do not have access org cipher endpoint below
// Return early to avoid 404 response
if (
this.restrictProviderAccessEnabled &&
!organization.isMember &&
organization.isProviderUser
) {
return [];
}
if (this.flexibleCollectionsV1Enabled) {
// Flexible collections V1 logic.
// If the user can edit all ciphers for the organization then fetch them ALL.

View File

@ -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())

View File

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

View File

@ -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.

View File

@ -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");

View File

@ -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");

View File

@ -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:

2
package-lock.json generated
View File

@ -251,7 +251,7 @@
},
"apps/web": {
"name": "@bitwarden/web-vault",
"version": "2024.6.0"
"version": "2024.6.1"
},
"libs/admin-console": {
"name": "@bitwarden/admin-console",