Fix redirect users after login with external auth

client/src/app/+login/login.component.ts

@@ -1,6 +1,6 @@
 
 import { AfterViewInit, Component, ElementRef, OnInit, ViewChild } from '@angular/core'
-import { ActivatedRoute } from '@angular/router'
+import { ActivatedRoute, Router } from '@angular/router'
 import { AuthService, Notifier, RedirectService, SessionStorageService, UserService } from '@app/core'
 import { HooksService } from '@app/core/plugins/hooks.service'
 import { LOGIN_PASSWORD_VALIDATOR, LOGIN_USERNAME_VALIDATOR } from '@app/shared/form-validators/login-validators'
@@ -49,7 +49,8 @@ export class LoginComponent extends FormReactive implements OnInit, AfterViewIni
     private redirectService: RedirectService,
     private notifier: Notifier,
     private hooks: HooksService,
-    private storage: SessionStorageService
+    private storage: SessionStorageService,
+    private router: Router
   ) {
     super()
   }
@@ -92,7 +93,10 @@ export class LoginComponent extends FormReactive implements OnInit, AfterViewIni
       return
     }
 
-    this.storage.setItem(LoginComponent.SESSION_STORAGE_REDIRECT_URL_KEY, this.redirectService.getPreviousUrl())
+    const previousUrl = this.redirectService.getPreviousUrl()
+    if (previousUrl && previousUrl !== '/') {
+      this.storage.setItem(LoginComponent.SESSION_STORAGE_REDIRECT_URL_KEY, previousUrl)
+    }
   }
 
   ngAfterViewInit () {
@@ -157,7 +161,13 @@ The link will expire within 1 hour.`
     this.authService.login(username, null, token)
       .subscribe({
         next: () => {
-          this.redirectService.redirectToPreviousRoute(this.storage.getItem(LoginComponent.SESSION_STORAGE_REDIRECT_URL_KEY))
+          const redirectUrl = this.storage.getItem(LoginComponent.SESSION_STORAGE_REDIRECT_URL_KEY)
+          if (redirectUrl) {
+            this.storage.removeItem(LoginComponent.SESSION_STORAGE_REDIRECT_URL_KEY)
+            return this.router.navigateByUrl(redirectUrl)
+          }
+
+          this.redirectService.redirectToLatestSessionRoute()
         },
 
         error: err => {

client/src/app/core/routing/redirect.service.ts

@@ -1,9 +1,15 @@
+import * as debug from 'debug'
 import { Injectable } from '@angular/core'
 import { NavigationCancel, NavigationEnd, Router } from '@angular/router'
 import { ServerService } from '../server'
+import { SessionStorageService } from '../wrappers/storage.service'
+
+const logger = debug('peertube:router:RedirectService')
 
 @Injectable()
 export class RedirectService {
+  private static SESSION_STORAGE_LATEST_SESSION_URL_KEY = 'redirect-latest-session-url'
+
   // Default route could change according to the instance configuration
   static INIT_DEFAULT_ROUTE = '/videos/trending'
   static INIT_DEFAULT_TRENDING_ALGORITHM = 'most-viewed'
@@ -11,13 +17,16 @@ export class RedirectService {
   private previousUrl: string
   private currentUrl: string
 
+  private latestSessionUrl: string
+
   private redirectingToHomepage = false
   private defaultTrendingAlgorithm = RedirectService.INIT_DEFAULT_TRENDING_ALGORITHM
   private defaultRoute = RedirectService.INIT_DEFAULT_ROUTE
 
   constructor (
     private router: Router,
-    private serverService: ServerService
+    private serverService: ServerService,
+    private storage: SessionStorageService
   ) {
     // The config is first loaded from the cache so try to get the default route
     const config = this.serverService.getHTMLConfig()
@@ -28,12 +37,22 @@ export class RedirectService {
       this.defaultTrendingAlgorithm = config.trending.videos.algorithms.default
     }
 
+    this.latestSessionUrl = this.storage.getItem(RedirectService.SESSION_STORAGE_LATEST_SESSION_URL_KEY)
+    this.storage.removeItem(RedirectService.SESSION_STORAGE_LATEST_SESSION_URL_KEY)
+
+    logger('Loaded latest session URL %s', this.latestSessionUrl)
+
     // Track previous url
     this.currentUrl = this.router.url
     router.events.subscribe(event => {
       if (event instanceof NavigationEnd || event instanceof NavigationCancel) {
         this.previousUrl = this.currentUrl
         this.currentUrl = event.url
+
+        logger('Previous URL is %s, current URL is %s', this.previousUrl, this.currentUrl)
+        logger('Setting %s as latest URL in session storage.', this.currentUrl)
+
+        this.storage.setItem(RedirectService.SESSION_STORAGE_LATEST_SESSION_URL_KEY, this.currentUrl)
       }
     })
   }
@@ -46,26 +65,16 @@ export class RedirectService {
     return this.defaultTrendingAlgorithm
   }
 
-  getPreviousUrl () {
+  redirectToLatestSessionRoute () {
-    return this.previousUrl
+    return this.doRedirect(this.latestSessionUrl)
   }
 
   redirectToPreviousRoute (fallbackRoute?: string) {
-    const exceptions = [
+    return this.doRedirect(this.previousUrl, fallbackRoute)
-      '/verify-account',
-      '/reset-password'
-    ]
-
-    if (this.previousUrl && this.previousUrl !== '/') {
-      const isException = exceptions.find(e => this.previousUrl.startsWith(e))
-      if (!isException) return this.router.navigateByUrl(this.previousUrl)
   }
 
-    if (fallbackRoute) {
+  getPreviousUrl () {
-      return this.router.navigateByUrl(fallbackRoute)
+    return this.previousUrl
-    }
-
-    return this.redirectToHomepage()
   }
 
   redirectToHomepage (skipLocationChange = false) {
@@ -91,4 +100,32 @@ export class RedirectService {
         })
 
   }
+
+  private doRedirect (redirectUrl: string, fallbackRoute?: string) {
+    logger('Redirecting on %s', redirectUrl)
+
+    if (this.isValidRedirection(redirectUrl)) {
+      return this.router.navigateByUrl(redirectUrl)
+    }
+
+    logger('%s is not a valid redirection, try fallback route %s', redirectUrl, fallbackRoute)
+    if (fallbackRoute) {
+      return this.router.navigateByUrl(fallbackRoute)
+    }
+
+    logger('There was no fallback route, redirecting to homepage')
+    return this.redirectToHomepage()
+  }
+
+  private isValidRedirection (redirectUrl: string) {
+    const exceptions = [
+      '/verify-account',
+      '/reset-password',
+      '/login'
+    ]
+
+    if (!redirectUrl || redirectUrl === '/') return false
+
+    return exceptions.every(e => !redirectUrl.startsWith(e))
+  }
 }