Refactored components to make the redirectUrl more generic, fixed code review comments

apps/browser/src/auth/popup/home.component.ts

@@ -82,14 +82,15 @@ export class HomeComponent implements OnInit, OnDestroy {
     this.loginService.setEmail(this.formGroup.value.email);
     this.loginService.setRememberEmail(this.formGroup.value.rememberEmail);
 
-    // const decodedRedirectUrl = decodeURIComponent(this.route.snapshot.queryParams.redirectUrl);
-    // console.log(decodedRedirectUrl, this.route);
-    this.router.navigate(["login"], {
-      queryParams: {
+    const queryParams: { email: string; redirectUrl?: string } = {
       email: this.formGroup.value.email,
-        redirectUrl: this.route.snapshot.queryParams.redirectUrl,
-      },
-    });
+    };
+
+    if (this.route.snapshot.queryParams.redirectUrl) {
+      queryParams.redirectUrl = decodeURIComponent(this.route.snapshot.queryParams.redirectUrl);
+    }
+
+    this.router.navigate(["login"], { queryParams });
   }
 
   get selfHostedDomain() {

apps/browser/src/auth/popup/lock.component.ts

@@ -50,8 +50,8 @@ export class LockComponent extends BaseLockComponent {
     policyService: InternalPolicyService,
     passwordStrengthService: PasswordStrengthServiceAbstraction,
     private authService: AuthService,
-    dialogService: DialogServiceAbstraction,
-    route: ActivatedRoute
+    route: ActivatedRoute,
+    dialogService: DialogServiceAbstraction
   ) {
     super(
       router,
@@ -70,8 +70,8 @@ export class LockComponent extends BaseLockComponent {
       policyApiService,
       policyService,
       passwordStrengthService,
-      dialogService,
-      route
+      route,
+      dialogService
     );
     this.successRoute = "/tabs/current";
     this.isInitialLockScreen = (window as any).previousPopupUrl == null;

apps/web/src/app/auth/lock.component.ts

@@ -63,8 +63,8 @@ export class LockComponent extends BaseLockComponent {
       policyApiService,
       policyService,
       passwordStrengthService,
-      dialogService,
-      route
+      route,
+      dialogService
     );
   }
 

libs/angular/src/auth/components/lock.component.ts

@@ -41,8 +41,7 @@ export class LockComponent implements OnInit, OnDestroy {
   biometricLock: boolean;
   biometricText: string;
   hideInput: boolean;
-  redirectPath: string;
-  sessionId: string;
+  redirectUrl: string;
 
   protected successRoute = "vault";
   protected forcePasswordResetRoute = "update-temp-password";
@@ -72,21 +71,11 @@ export class LockComponent implements OnInit, OnDestroy {
     protected policyApiService: PolicyApiServiceAbstraction,
     protected policyService: InternalPolicyService,
     protected passwordStrengthService: PasswordStrengthServiceAbstraction,
-    protected dialogService: DialogServiceAbstraction,
-    protected route: ActivatedRoute
+    protected route: ActivatedRoute,
+    protected dialogService: DialogServiceAbstraction
   ) {}
 
   async ngOnInit() {
-    this.route?.queryParams.subscribe((params) => {
-      this.redirectPath = params?.redirectPath;
-      this.sessionId = params?.sessionId;
-    });
-
-    //use redirectPath to redirect to a specific page after successful login
-    if (this.redirectPath) {
-      this.successRoute = this.redirectPath;
-    }
-
     this.stateService.activeAccount$
       .pipe(
         concatMap(async () => {
@@ -279,6 +268,13 @@ export class LockComponent implements OnInit, OnDestroy {
     await this.stateService.setEverBeenUnlocked(true);
     this.messagingService.send("unlocked");
 
+    // The `redirectUrl` parameter determines the target route after a successful login.
+    // If provided in the URL's query parameters, the user will be redirected
+    // to the specified path once they are authenticated.
+    if (this.route.snapshot.queryParams.redirectUrl) {
+      this.successRoute = decodeURIComponent(this.route.snapshot.queryParams.redirectUrl);
+    }
+
     if (evaluatePasswordAfterUnlock) {
       try {
         // If we do not have any saved policies, attempt to load them from the service
@@ -304,11 +300,7 @@ export class LockComponent implements OnInit, OnDestroy {
     if (this.onSuccessfulSubmit != null) {
       await this.onSuccessfulSubmit();
     } else if (this.router != null) {
-      this.router.navigate([this.successRoute], {
-        queryParams: {
-          sessionId: this.sessionId,
-        },
-      });
+      this.router.navigateByUrl(this.successRoute);
     }
   }
 

libs/angular/src/auth/components/login.component.ts

@@ -1,7 +1,8 @@
 import { Directive, ElementRef, NgZone, OnInit, ViewChild } from "@angular/core";
 import { FormBuilder, Validators } from "@angular/forms";
 import { ActivatedRoute, Router } from "@angular/router";
-import { take } from "rxjs/operators";
+import { Subject } from "rxjs";
+import { take, takeUntil } from "rxjs/operators";
 
 import { DevicesApiServiceAbstraction } from "@bitwarden/common/abstractions/devices/devices-api.service.abstraction";
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
@@ -39,8 +40,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit
   showLoginWithDevice: boolean;
   validatedEmail = false;
   paramEmailSet = false;
-  redirectPath: string;
-  sessionId: string;
+  redirectUrl: string;
 
   formGroup = this.formBuilder.group({
     email: ["", [Validators.required, Validators.email]],
@@ -52,6 +52,8 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit
   protected successRoute = "vault";
   protected forcePasswordResetRoute = "update-temp-password";
 
+  private destroy$ = new Subject<void>();
+
   get loggedEmail() {
     return this.formGroup.value.email;
   }
@@ -82,21 +84,18 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit
   }
 
   async ngOnInit() {
-    this.route?.queryParams.subscribe((params) => {
-      if (params != null) {
-        const queryParamsEmail = params["email"];
-        this.redirectPath = params?.["redirectPath"];
-        this.sessionId = params?.["sessionId"];
+    this.route?.queryParams.pipe(takeUntil(this.destroy$)).subscribe((params) => {
+      if (!params) {
+        return;
+      }
+
+      const queryParamsEmail = params.email;
+
       if (queryParamsEmail != null && queryParamsEmail.indexOf("@") > -1) {
         this.formGroup.get("email").setValue(queryParamsEmail);
         this.loginService.setEmail(queryParamsEmail);
         this.paramEmailSet = true;
       }
-        //use redirectPath to redirect to a specific page after successful login
-        if (this.redirectPath) {
-          this.successRoute = this.redirectPath;
-        }
-      }
     });
     let email = this.loginService.getEmail();
 
@@ -114,7 +113,20 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit
     this.formGroup.get("rememberEmail")?.setValue(rememberEmail);
   }
 
+  ngOnDestroy() {
+    this.destroy$.next();
+    this.destroy$.complete();
+  }
+
   async submit(showToast = true) {
+    // The `redirectUrl` parameter determines the target route after a successful login.
+    // If provided in the URL's query parameters, the user will be redirected
+    // to the specified path once they are authenticated.
+    if (this.route.snapshot.queryParams.redirectUrl) {
+      this.redirectUrl = decodeURIComponent(this.route.snapshot.queryParams.redirectUrl);
+      this.successRoute = this.redirectUrl;
+    }
+
     const data = this.formGroup.value;
 
     await this.setupCaptcha();
@@ -152,8 +164,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit
         } else {
           this.router.navigate([this.twoFactorRoute], {
             queryParams: {
-              redirectPath: this.redirectPath,
-              sessionId: this.sessionId,
+              redirectUrl: this.redirectUrl,
             },
           });
         }
@@ -170,11 +181,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit
         if (this.onSuccessfulLoginNavigate != null) {
           this.onSuccessfulLoginNavigate();
         } else {
-          this.router.navigate([this.successRoute], {
-            queryParams: {
-              sessionId: this.sessionId,
-            },
-          });
+          this.router.navigateByUrl(this.successRoute);
         }
       }
     } catch (e) {

libs/angular/src/auth/components/two-factor.component.ts

@@ -39,8 +39,7 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI
   formPromise: Promise<any>;
   emailPromise: Promise<any>;
   identifier: string = null;
-  redirectPath: string;
-  sessionId: string;
+  redirectUrl: string;
   onSuccessfulLogin: () => Promise<any>;
   onSuccessfulLoginNavigate: () => Promise<any>;
 
@@ -76,15 +75,6 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI
       if (qParams.identifier != null) {
         this.identifier = qParams.identifier;
       }
-
-      if (qParams.redirectPath != null) {
-        this.redirectPath = qParams.redirectPath;
-        this.successRoute = this.redirectPath;
-      }
-
-      if (qParams.sessionId != null) {
-        this.sessionId = qParams.sessionId;
-      }
     });
 
     if (this.needsLock) {
@@ -203,6 +193,13 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI
   }
 
   async doSubmit() {
+    // The `redirectUrl` parameter determines the target route after a successful login.
+    // If provided in the URL's query parameters, the user will be redirected
+    // to the specified path once they are authenticated.
+    if (this.route.snapshot.queryParams.redirectUrl) {
+      this.successRoute = decodeURIComponent(this.route.snapshot.queryParams.redirectUrl);
+    }
+
     this.formPromise = this.authService.logInTwoFactor(
       new TokenTwoFactorRequest(this.selectedProviderType, this.token, this.remember),
       this.captchaToken
@@ -228,12 +225,7 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI
       await this.onSuccessfulLoginNavigate();
     } else {
       this.loginService.clearValues();
-      this.router.navigate([this.successRoute], {
-        queryParams: {
-          identifier: this.identifier,
-          sessionId: this.sessionId,
-        },
-      });
+      this.router.navigateByUrl(this.successRoute);
     }
   }