Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplelogin alias generation only generate random words instead the domain name #13024

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

<vault-cipher-form-generator
[type]="params.type"
[uri]="uri"
(valueGenerated)="onValueGenerated($event)"
></vault-cipher-form-generator>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
})
class MockCipherFormGenerator {
@Input() type: "password" | "username";
@Input() uri: string;
@Output() valueGenerated = new EventEmitter<string>();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { PopupPageComponent } from "../../../../../platform/popup/layout/popup-p

export interface GeneratorDialogParams {
type: "password" | "username";
uri?: string;
}

export interface GeneratorDialogResult {
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<ng-container bitDialogContent>
<vault-cipher-form-generator
[type]="params.type"
[uri]="uri"
(valueGenerated)="onValueGenerated($event)"
disableMargin
></vault-cipher-form-generator>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
})
class MockCipherFormGenerator {
@Input() type: "password" | "username";
@Input() uri?: string;
@Output() valueGenerated = new EventEmitter<string>();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { CipherFormGeneratorComponent } from "@bitwarden/vault";

export interface WebVaultGeneratorDialogParams {
type: "password" | "username";
uri?: string;
}

export interface WebVaultGeneratorDialogResult {
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
audreyality marked this conversation as resolved.
Show resolved Hide resolved

/** Emits credentials created from a generation request. */
@Output()
readonly onGenerated = new EventEmitter<GeneratedCredential>();
Expand Down Expand Up @@ -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[]) {
Expand Down
audreyality marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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;
audreyality marked this conversation as resolved.
Show resolved Hide resolved

/** Emits credentials created from a generation request. */
@Output()
readonly onGenerated = new EventEmitter<GeneratedCredential>();
Expand Down Expand Up @@ -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[]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
audreyality marked this conversation as resolved.
Show resolved Hide resolved
}
6 changes: 6 additions & 0 deletions libs/vault/src/cipher-form/cipher-form-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ export abstract class CipherFormContainer {
group: Exclude<CipherForm[K], undefined>,
): void;

/**
* Returns the current cipherView
* If config.originalCipher is not settled before getCipherView(), it returns null
*/
abstract get getCipherView(): CipherView | null;
audreyality marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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
Expand Down
Original file line number Diff line number Diff line change
@@ -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("getCipherView", () => {
it("should return null if updatedCipherView is null", () => {
component["updatedCipherView"] = null as any;
expect(component.getCipherView).toBeNull();
});

it("should return null if updatedCipherView.login is undefined", () => {
component["updatedCipherView"] = new CipherView();
delete component["updatedCipherView"].login;
expect(component.getCipherView).toBeNull();
});

it("should return null if updatedCipherView.login is null", () => {
component["updatedCipherView"] = new CipherView();
component["updatedCipherView"].login = null as any;
expect(component.getCipherView).toBeNull();
});

it("should return null if updatedCipherView.login.uris is undefined", () => {
component["updatedCipherView"] = new CipherView();
component["updatedCipherView"].login = { uris: undefined } as any;
expect(component.getCipherView).toBeNull();
});

it("should return null if updatedCipherView.login.uris is null", () => {
component["updatedCipherView"] = new CipherView();
component["updatedCipherView"].login = { uris: null } as any;
expect(component.getCipherView).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.getCipherView).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.getCipherView).toEqual(component["updatedCipherView"]);
});
});
});
13 changes: 12 additions & 1 deletion libs/vault/src/cipher-form/components/cipher-form.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,18 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci
* by child components via the `patchCipher` method.
* @protected
*/
protected updatedCipherView: CipherView | null;
protected updatedCipherView: CipherView = new CipherView();

get getCipherView(): CipherView | null {
if (
!this.updatedCipherView ||
!this.updatedCipherView?.login?.uris ||
this.updatedCipherView.login.uris.length === 0
) {
return null;
}
return this.updatedCipherView;
}

protected loading: boolean = true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
></tools-password-generator>
<tools-username-generator
*ngIf="type === 'username'"
[website]="uri"
[disableMargin]="disableMargin"
(onGenerated)="onCredentialGenerated($event)"
(onAlgorithm)="onAlgorithmSelected($event)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ export class CipherFormGeneratorComponent {
@Input()
onAlgorithmSelected: (selected: AlgorithmInfo) => void;

@Input()
uri: string = "";

/**
* The type of generator form to show.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,15 @@ describe("LoginDetailsSectionComponent", () => {

beforeEach(async () => {
getInitialCipherView.mockClear();
cipherFormContainer = mock<CipherFormContainer>({ getInitialCipherView });
cipherFormContainer = mock<CipherFormContainer>({
getInitialCipherView,
getCipherView: {
login: {
uris: [{ uri: "example.com" }],
username: "existing-username",
},
} as CipherView,
});

generationService = mock<CipherFormGenerationService>();
auditService = mock<AuditService>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 cipherView = this.cipherFormContainer.getCipherView;
const uri = cipherView?.login.uris[0]?.uri;
const newUsername = await this.generationService.generateUsername(uri);
if (newUsername) {
this.loginDetailsForm.controls.username.patchValue(newUsername);
}
Expand Down
Loading