1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-06 00:13:28 +00:00

[PM-16171] Simplelogin alias generation only generate random words instead the domain name (#13024)

* Exposes URI property from the cipher form.
* Updates credential generator to accept the URI using a `website` attribute

---------

Co-authored-by:  Audrey  <audrey@audreyality.com>
This commit is contained in:
albertboyd5
2025-02-20 10:33:40 -06:00
committed by GitHub
parent d5764a2b5e
commit ca41ecba29
24 changed files with 282 additions and 51 deletions

View File

@@ -3038,6 +3038,30 @@
}
}
},
"forwaderInvalidOperation": {
"message": "$SERVICENAME$ refused your request. Please contact your service provider for assistance.",
"description": "Displayed when the user is forbidden from using the API by the forwarding service.",
"placeholders": {
"servicename": {
"content": "$1",
"example": "SimpleLogin"
}
}
},
"forwaderInvalidOperationWithMessage": {
"message": "$SERVICENAME$ refused your request: $ERRORMESSAGE$",
"description": "Displayed when the user is forbidden from using the API by the forwarding service with an error message.",
"placeholders": {
"servicename": {
"content": "$1",
"example": "SimpleLogin"
},
"errormessage": {
"content": "$2",
"example": "Please verify your email address to continue."
}
}
},
"forwarderNoAccountId": {
"message": "Unable to obtain $SERVICENAME$ masked email account ID.",
"description": "Displayed when the forwarding service fails to return an account ID.",

View File

@@ -8,6 +8,7 @@
<vault-cipher-form-generator
[type]="params.type"
[uri]="uri"
(valueGenerated)="onValueGenerated($event)"
></vault-cipher-form-generator>

View File

@@ -22,6 +22,7 @@ import {
})
class MockCipherFormGenerator {
@Input() type: "password" | "username";
@Input() uri: string;
@Output() valueGenerated = new EventEmitter<string>();
}

View File

@@ -15,6 +15,7 @@ import { PopupPageComponent } from "../../../../../platform/popup/layout/popup-p
export interface GeneratorDialogParams {
type: "password" | "username";
uri?: string;
}
export interface GeneratorDialogResult {
@@ -60,11 +61,15 @@ export class VaultGeneratorDialogComponent {
*/
protected generatedValue: string = "";
protected uri: string;
constructor(
@Inject(DIALOG_DATA) protected params: GeneratorDialogParams,
private dialogRef: DialogRef<GeneratorDialogResult>,
private i18nService: I18nService,
) {}
) {
this.uri = params.uri;
}
/**
* Close the dialog without selecting a value.

View File

@@ -28,9 +28,9 @@ export class BrowserCipherFormGenerationService implements CipherFormGenerationS
return result.generatedValue;
}
async generateUsername(): Promise<string> {
async generateUsername(uri: string): Promise<string> {
const dialogRef = VaultGeneratorDialogComponent.open(this.dialogService, this.overlay, {
data: { type: "username" },
data: { type: "username", uri: uri },
});
const result = await firstValueFrom(dialogRef.closed);

View File

@@ -111,6 +111,30 @@
}
}
},
"forwaderInvalidOperation": {
"message": "$SERVICENAME$ refused your request. Please contact your service provider for assistance.",
"description": "Displayed when the user is forbidden from using the API by the forwarding service.",
"placeholders": {
"servicename": {
"content": "$1",
"example": "SimpleLogin"
}
}
},
"forwaderInvalidOperationWithMessage": {
"message": "$SERVICENAME$ refused your request: $ERRORMESSAGE$",
"description": "Displayed when the user is forbidden from using the API by the forwarding service with an error message.",
"placeholders": {
"servicename": {
"content": "$1",
"example": "SimpleLogin"
},
"errormessage": {
"content": "$2",
"example": "Please verify your email address to continue."
}
}
},
"forwarderNoAccountId": {
"message": "Unable to obtain $SERVICENAME$ masked email account ID.",
"description": "Displayed when the forwarding service fails to return an account ID.",

View File

@@ -2640,6 +2640,30 @@
}
}
},
"forwaderInvalidOperation": {
"message": "$SERVICENAME$ refused your request. Please contact your service provider for assistance.",
"description": "Displayed when the user is forbidden from using the API by the forwarding service.",
"placeholders": {
"servicename": {
"content": "$1",
"example": "SimpleLogin"
}
}
},
"forwaderInvalidOperationWithMessage": {
"message": "$SERVICENAME$ refused your request: $ERRORMESSAGE$",
"description": "Displayed when the user is forbidden from using the API by the forwarding service with an error message.",
"placeholders": {
"servicename": {
"content": "$1",
"example": "SimpleLogin"
},
"errormessage": {
"content": "$2",
"example": "Please verify your email address to continue."
}
}
},
"forwarderNoAccountId": {
"message": "Unable to obtain $SERVICENAME$ masked email account ID.",
"description": "Displayed when the forwarding service fails to return an account ID.",

View File

@@ -5,6 +5,7 @@
<ng-container bitDialogContent>
<vault-cipher-form-generator
[type]="params.type"
[uri]="uri"
(valueGenerated)="onValueGenerated($event)"
disableMargin
></vault-cipher-form-generator>

View File

@@ -23,6 +23,7 @@ import {
})
class MockCipherFormGenerator {
@Input() type: "password" | "username";
@Input() uri?: string;
@Output() valueGenerated = new EventEmitter<string>();
}

View File

@@ -10,6 +10,7 @@ import { CipherFormGeneratorComponent } from "@bitwarden/vault";
export interface WebVaultGeneratorDialogParams {
type: "password" | "username";
uri?: string;
}
export interface WebVaultGeneratorDialogResult {
@@ -48,11 +49,15 @@ export class WebVaultGeneratorDialogComponent {
*/
protected generatedValue: string = "";
protected uri: string;
constructor(
@Inject(DIALOG_DATA) protected params: WebVaultGeneratorDialogParams,
private dialogRef: DialogRef<WebVaultGeneratorDialogResult>,
private i18nService: I18nService,
) {}
) {
this.uri = params.uri;
}
/**
* Close the dialog without selecting a value.

View File

@@ -26,9 +26,9 @@ export class WebCipherFormGenerationService implements CipherFormGenerationServi
return result.generatedValue;
}
async generateUsername(): Promise<string> {
async generateUsername(uri: string): Promise<string> {
const dialogRef = WebVaultGeneratorDialogComponent.open(this.dialogService, {
data: { type: "username" },
data: { type: "username", uri: uri },
});
const result = await firstValueFrom(dialogRef.closed);

View File

@@ -6967,6 +6967,30 @@
}
}
},
"forwaderInvalidOperation": {
"message": "$SERVICENAME$ refused your request. Please contact your service provider for assistance.",
"description": "Displayed when the user is forbidden from using the API by the forwarding service.",
"placeholders": {
"servicename": {
"content": "$1",
"example": "SimpleLogin"
}
}
},
"forwaderInvalidOperationWithMessage": {
"message": "$SERVICENAME$ refused your request: $ERRORMESSAGE$",
"description": "Displayed when the user is forbidden from using the API by the forwarding service with an error message.",
"placeholders": {
"servicename": {
"content": "$1",
"example": "SimpleLogin"
},
"errormessage": {
"content": "$2",
"example": "Please verify your email address to continue."
}
}
},
"forwarderNoAccountId": {
"message": "Unable to obtain $SERVICENAME$ masked email account ID.",
"description": "Displayed when the forwarding service fails to return an account ID.",

View File

@@ -51,52 +51,52 @@ describe("RestClient", () => {
expect(api.nativeFetch).toHaveBeenCalledWith(expectedRpc.fetchRequest);
});
it.each([[401] /*,[403]*/])(
"throws an invalid token error when HTTP status is %i",
async (status) => {
const client = new RestClient(api, i18n);
const request: IntegrationRequest = { website: null };
const response = mock<Response>({ status, statusText: null });
api.nativeFetch.mockResolvedValue(response);
it.each([
[401, "forwaderInvalidToken"],
[403, "forwaderInvalidOperation"],
])("throws an invalid token error when HTTP status is %i", async (status, messageKey) => {
const client = new RestClient(api, i18n);
const request: IntegrationRequest = { website: null };
const response = mock<Response>({ status, statusText: null });
api.nativeFetch.mockResolvedValue(response);
const result = client.fetchJson(rpc, request);
const result = client.fetchJson(rpc, request);
await expect(result).rejects.toEqual("forwaderInvalidToken");
},
);
await expect(result).rejects.toEqual(messageKey);
});
it.each([
[401, null, null],
[401, undefined, undefined],
[401, undefined, null],
[403, null, null],
[403, undefined, undefined],
[403, undefined, null],
[401, null, null, "forwaderInvalidToken"],
[401, undefined, undefined, "forwaderInvalidToken"],
[401, undefined, null, "forwaderInvalidToken"],
[403, null, null, "forwaderInvalidOperation"],
[403, undefined, undefined, "forwaderInvalidOperation"],
[403, undefined, null, "forwaderInvalidOperation"],
])(
"throws an invalid token error when HTTP status is %i, message is %p, and error is %p",
async (status) => {
async (status, message, error, messageKey) => {
const client = new RestClient(api, i18n);
const request: IntegrationRequest = { website: null };
const response = mock<Response>({
status,
text: () => Promise.resolve(`{ "message": null, "error": null }`),
text: () => Promise.resolve(JSON.stringify({ message, error })),
});
api.nativeFetch.mockResolvedValue(response);
const result = client.fetchJson(rpc, request);
await expect(result).rejects.toEqual("forwaderInvalidToken");
await expect(result).rejects.toEqual(messageKey);
},
);
it.each([
[401, "message"],
[403, "message"],
[401, "error"],
[403, "error"],
[401, "message", "forwaderInvalidTokenWithMessage"],
[403, "message", "forwaderInvalidOperationWithMessage"],
[401, "error", "forwaderInvalidTokenWithMessage"],
[403, "error", "forwaderInvalidOperationWithMessage"],
])(
"throws an invalid token detailed error when HTTP status is %i and the payload has a %s",
async (status, property) => {
async (status, property, messageKey) => {
const client = new RestClient(api, i18n);
const request: IntegrationRequest = { website: null };
const response = mock<Response>({
@@ -107,18 +107,17 @@ describe("RestClient", () => {
const result = client.fetchJson(rpc, request);
await expect(result).rejects.toEqual("forwaderInvalidTokenWithMessage");
expect(i18n.t).toHaveBeenCalledWith(
"forwaderInvalidTokenWithMessage",
"mock",
"expected message",
);
await expect(result).rejects.toEqual(messageKey);
expect(i18n.t).toHaveBeenCalledWith(messageKey, "mock", "expected message");
},
);
it.each([[401], [403]])(
it.each([
[401, "forwaderInvalidTokenWithMessage"],
[403, "forwaderInvalidOperationWithMessage"],
])(
"throws an invalid token detailed error when HTTP status is %i and the payload has a %s",
async (status) => {
async (status, messageKey) => {
const client = new RestClient(api, i18n);
const request: IntegrationRequest = { website: null };
const response = mock<Response>({
@@ -130,12 +129,8 @@ describe("RestClient", () => {
const result = client.fetchJson(rpc, request);
await expect(result).rejects.toEqual("forwaderInvalidTokenWithMessage");
expect(i18n.t).toHaveBeenCalledWith(
"forwaderInvalidTokenWithMessage",
"mock",
"that happened: expected message",
);
await expect(result).rejects.toEqual(messageKey);
expect(i18n.t).toHaveBeenCalledWith(messageKey, "mock", "that happened: expected message");
},
);

View File

@@ -46,10 +46,14 @@ export class RestClient {
}
private async detectCommonErrors(response: Response): Promise<[string, string] | undefined> {
if (response.status === 401 || response.status === 403) {
if (response.status === 401) {
const message = await this.tryGetErrorMessage(response);
const key = message ? "forwaderInvalidTokenWithMessage" : "forwaderInvalidToken";
return [key, message];
} else if (response.status === 403) {
const message = await this.tryGetErrorMessage(response);
const key = message ? "forwaderInvalidOperationWithMessage" : "forwaderInvalidOperation";
return [key, message];
} else if (response.status >= 400) {
const message = await this.tryGetErrorMessage(response);
const key = message ? "forwarderError" : "forwarderUnknownError";

View File

@@ -71,6 +71,12 @@ export class CredentialGeneratorComponent implements OnInit, OnDestroy {
@Input()
userId: UserId | null;
/**
* The website associated with the credential generation request.
*/
@Input()
website: string | null = null;
/** Emits credentials created from a generation request. */
@Output()
readonly onGenerated = new EventEmitter<GeneratedCredential>();
@@ -514,7 +520,7 @@ export class CredentialGeneratorComponent implements OnInit, OnDestroy {
* origin in the debugger.
*/
protected async generate(requestor: string) {
this.generate$.next({ source: requestor });
this.generate$.next({ source: requestor, website: this.website });
}
private toOptions(algorithms: AlgorithmInfo[]) {

View File

@@ -77,6 +77,12 @@ export class UsernameGeneratorComponent implements OnInit, OnDestroy {
@Input()
userId: UserId | null;
/**
* The website associated with the credential generation request.
*/
@Input()
website: string | null = null;
/** Emits credentials created from a generation request. */
@Output()
readonly onGenerated = new EventEmitter<GeneratedCredential>();
@@ -435,7 +441,7 @@ export class UsernameGeneratorComponent implements OnInit, OnDestroy {
* origin in the debugger.
*/
protected async generate(requestor: string) {
this.generate$.next({ source: requestor });
this.generate$.next({ source: requestor, website: this.website });
}
private toOptions(algorithms: AlgorithmInfo[]) {

View File

@@ -9,6 +9,7 @@ export abstract class CipherFormGenerationService {
/**
* Generates a random username. Called when the user clicks the "Generate Username" button in the UI.
* @param uri The URI associated with the username generation request.
*/
abstract generateUsername(): Promise<string | null>;
abstract generateUsername(uri: string): Promise<string | null>;
}

View File

@@ -51,6 +51,12 @@ export abstract class CipherFormContainer {
group: Exclude<CipherForm[K], undefined>,
): void;
/**
* The website that the component publishes to edit email and username workflows.
* Returns `null` when the cipher isn't bound to a website.
*/
abstract get website(): string | null;
/**
* Method to update the cipherView with the new values. This method should be called by the child form components
* @param updateFn - A function that takes the current cipherView and returns the updated cipherView

View File

@@ -0,0 +1,90 @@
import { ChangeDetectorRef } from "@angular/core";
import { ComponentFixture, TestBed } from "@angular/core/testing";
import { ReactiveFormsModule } from "@angular/forms";
import { ViewCacheService } from "@bitwarden/angular/platform/abstractions/view-cache.service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { ToastService } from "@bitwarden/components";
import { CipherFormService } from "../abstractions/cipher-form.service";
import { CipherFormCacheService } from "../services/default-cipher-form-cache.service";
import { CipherFormComponent } from "./cipher-form.component";
describe("CipherFormComponent", () => {
let component: CipherFormComponent;
let fixture: ComponentFixture<CipherFormComponent>;
beforeEach(async () => {
await TestBed.configureTestingModule({
imports: [CipherFormComponent, ReactiveFormsModule],
providers: [
{ provide: ChangeDetectorRef, useValue: {} },
{ provide: I18nService, useValue: { t: (key: string) => key } },
{ provide: ToastService, useValue: { showToast: jest.fn() } },
{ provide: CipherFormService, useValue: { saveCipher: jest.fn() } },
{
provide: CipherFormCacheService,
useValue: { init: jest.fn(), getCachedCipherView: jest.fn() },
},
{ provide: ViewCacheService, useValue: { signal: jest.fn(() => () => null) } },
{ provide: ConfigService, useValue: {} },
],
}).compileComponents();
});
beforeEach(() => {
fixture = TestBed.createComponent(CipherFormComponent);
component = fixture.componentInstance;
fixture.detectChanges();
});
it("should create the component", () => {
expect(component).toBeTruthy();
});
describe("website", () => {
it("should return null if updatedCipherView is null", () => {
component["updatedCipherView"] = null as any;
expect(component.website).toBeNull();
});
it("should return null if updatedCipherView.login is undefined", () => {
component["updatedCipherView"] = new CipherView();
delete component["updatedCipherView"].login;
expect(component.website).toBeNull();
});
it("should return null if updatedCipherView.login is null", () => {
component["updatedCipherView"] = new CipherView();
component["updatedCipherView"].login = null as any;
expect(component.website).toBeNull();
});
it("should return null if updatedCipherView.login.uris is undefined", () => {
component["updatedCipherView"] = new CipherView();
component["updatedCipherView"].login = { uris: undefined } as any;
expect(component.website).toBeNull();
});
it("should return null if updatedCipherView.login.uris is null", () => {
component["updatedCipherView"] = new CipherView();
component["updatedCipherView"].login = { uris: null } as any;
expect(component.website).toBeNull();
});
it("should return null if updatedCipherView.login.uris is an empty array", () => {
component["updatedCipherView"] = new CipherView();
component["updatedCipherView"].login = { uris: [] } as any;
expect(component.website).toBeNull();
});
it("should return updatedCipherView if login.uris contains at least one URI", () => {
component["updatedCipherView"] = new CipherView();
component["updatedCipherView"].login = { uris: [{ uri: "https://example.com" }] } as any;
expect(component.website).toEqual("https://example.com");
});
});
});

View File

@@ -133,6 +133,10 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci
*/
protected updatedCipherView: CipherView | null;
get website(): string | null {
return this.updatedCipherView?.login?.uris?.[0]?.uri ?? null;
}
protected loading: boolean = true;
CipherType = CipherType;

View File

@@ -6,6 +6,7 @@
></tools-password-generator>
<tools-username-generator
*ngIf="type === 'username'"
[website]="uri"
[disableMargin]="disableMargin"
(onGenerated)="onCredentialGenerated($event)"
(onAlgorithm)="onAlgorithmSelected($event)"

View File

@@ -21,6 +21,9 @@ export class CipherFormGeneratorComponent {
@Input()
onAlgorithmSelected: (selected: AlgorithmInfo) => void;
@Input()
uri: string = "";
/**
* The type of generator form to show.
*/

View File

@@ -46,7 +46,10 @@ describe("LoginDetailsSectionComponent", () => {
beforeEach(async () => {
getInitialCipherView.mockClear();
cipherFormContainer = mock<CipherFormContainer>({ getInitialCipherView });
cipherFormContainer = mock<CipherFormContainer>({
getInitialCipherView,
website: "example.com",
});
generationService = mock<CipherFormGenerationService>();
auditService = mock<AuditService>();

View File

@@ -243,7 +243,9 @@ export class LoginDetailsSectionComponent implements OnInit {
* TODO: Browser extension needs a means to cache the current form so values are not lost upon navigating to the generator.
*/
generateUsername = async () => {
const newUsername = await this.generationService.generateUsername();
const newUsername = await this.generationService.generateUsername(
this.cipherFormContainer.website,
);
if (newUsername) {
this.loginDetailsForm.controls.username.patchValue(newUsername);
}