Refactor login redirection/button links

Correctly handle external auth redirection in all cases

21 files changed, 77 insertions, 48 deletions

diff --git a/client/src/app/+error-page/error-page.component.html b/client/src/app/+error-page/error-page.component.html
index 070985d3c..b8b9296a3 100644
--- a/client/src/app/+error-page/error-page.component.html
+++ b/client/src/app/+error-page/error-page.component.html
@@ -31,10 +31,7 @@
       <ng-container *ngIf="type !== 'video'" i18n>You might need to login to see the resource.</ng-container>
     </div>
 
-    <a class="peertube-button-big-link orange-button mt-5" i18n routerLink="/login">
-      Login
-    </a>
-
+    <my-login-link className="peertube-button-big-link orange-button mt-5"></my-login-link>
   </div>
 
   <div *ngIf="status === 403" class="box">
diff --git a/client/src/app/+error-page/error-page.module.ts b/client/src/app/+error-page/error-page.module.ts
index 807c6f5a9..34f026ea8 100644
--- a/client/src/app/+error-page/error-page.module.ts
+++ b/client/src/app/+error-page/error-page.module.ts
@@ -1,11 +1,13 @@
 import { CommonModule } from '@angular/common'
 import { NgModule } from '@angular/core'
+import { SharedMainModule } from '@app/shared/shared-main'
 import { ErrorPageRoutingModule } from './error-page-routing.module'
 import { ErrorPageComponent } from './error-page.component'
 
 @NgModule({
   imports: [
     CommonModule,
+    SharedMainModule,
 
     ErrorPageRoutingModule
   ],
diff --git a/client/src/app/+reset-password/reset-password.component.ts b/client/src/app/+reset-password/reset-password.component.ts
index 44216f978..a6c05bbfb 100644
--- a/client/src/app/+reset-password/reset-password.component.ts
+++ b/client/src/app/+reset-password/reset-password.component.ts
@@ -45,6 +45,7 @@ export class ResetPasswordComponent extends FormReactive implements OnInit {
       .subscribe({
         next: () => {
           this.notifier.success($localize`Your password has been successfully reset!`)
+
           this.router.navigate([ '/login' ])
         },
 
diff --git a/client/src/app/+videos/+video-watch/shared/action-buttons/video-rate.component.ts b/client/src/app/+videos/+video-watch/shared/action-buttons/video-rate.component.ts
index 0fef246b3..d0c138834 100644
--- a/client/src/app/+videos/+video-watch/shared/action-buttons/video-rate.component.ts
+++ b/client/src/app/+videos/+video-watch/shared/action-buttons/video-rate.component.ts
@@ -82,7 +82,7 @@ export class VideoRateComponent implements OnInit, OnChanges, OnDestroy {
   getRatePopoverText () {
     if (this.isUserLoggedIn) return undefined
 
-    return $localize`You need to be <a href="/login">logged in</a> to rate this video.`
+    return $localize`You need to be logged in to rate this video.`
   }
 
   private checkUserRating () {
diff --git a/client/src/app/+videos/+video-watch/shared/comment/video-comment-add.component.html b/client/src/app/+videos/+video-watch/shared/comment/video-comment-add.component.html
index 6bc201f32..203e9ce62 100644
--- a/client/src/app/+videos/+video-watch/shared/comment/video-comment-add.component.html
+++ b/client/src/app/+videos/+video-watch/shared/comment/video-comment-add.component.html
@@ -74,10 +74,7 @@
       (click)="hideModals()" (key.enter)="hideModals()"
     >
 
-    <input
-      type="submit" i18n-value value="Login to comment" class="peertube-button orange-button"
-      (click)="gotoLogin()"
-    >
+    <my-login-link i18n-label label="Login to comment" className="peertube-button-link orange-button"></my-login-link>
   </div>
 </ng-template>
 
diff --git a/client/src/app/+videos/+video-watch/shared/comment/video-comment-add.component.ts b/client/src/app/+videos/+video-watch/shared/comment/video-comment-add.component.ts
index 9a9bfe710..033097084 100644
--- a/client/src/app/+videos/+video-watch/shared/comment/video-comment-add.component.ts
+++ b/client/src/app/+videos/+video-watch/shared/comment/video-comment-add.component.ts
@@ -13,7 +13,6 @@ import {
   SimpleChanges,
   ViewChild
 } from '@angular/core'
-import { Router } from '@angular/router'
 import { Notifier, User } from '@app/core'
 import { VIDEO_COMMENT_TEXT_VALIDATOR } from '@app/shared/form-validators/video-comment-validators'
 import { FormReactive, FormReactiveService } from '@app/shared/shared-forms'
@@ -52,7 +51,6 @@ export class VideoCommentAddComponent extends FormReactive implements OnChanges,
     private notifier: Notifier,
     private videoCommentService: VideoCommentService,
     private modalService: NgbModal,
-    private router: Router,
     @Inject(LOCALE_ID) private localeId: string
   ) {
     super()
@@ -161,11 +159,6 @@ export class VideoCommentAddComponent extends FormReactive implements OnChanges,
     return window.location.href
   }
 
-  gotoLogin () {
-    this.hideModals()
-    this.router.navigate([ '/login' ])
-  }
-
   cancelCommentReply () {
     this.cancel.emit(null)
     this.form.value['text'] = this.textareaElement.nativeElement.value = ''
diff --git a/client/src/app/core/auth/auth.service.ts b/client/src/app/core/auth/auth.service.ts
index ed7eabb76..6fe601d8d 100644
--- a/client/src/app/core/auth/auth.service.ts
+++ b/client/src/app/core/auth/auth.service.ts
@@ -5,11 +5,11 @@ import { HttpClient, HttpErrorResponse, HttpHeaders, HttpParams } from '@angular
 import { Injectable } from '@angular/core'
 import { Router } from '@angular/router'
 import { Notifier } from '@app/core/notification/notifier.service'
-import { logger, OAuthUserTokens, objectToUrlEncoded, peertubeLocalStorage, PluginsManager } from '@root-helpers/index'
+import { logger, OAuthUserTokens, objectToUrlEncoded, peertubeLocalStorage } from '@root-helpers/index'
 import { HttpStatusCode, MyUser as UserServerModel, OAuthClientLocal, User, UserLogin, UserRefreshToken } from '@shared/models'
 import { environment } from '../../../environments/environment'
 import { RestExtractor } from '../rest/rest-extractor.service'
-import { ServerService } from '../server'
+import { RedirectService } from '../routing'
 import { AuthStatus } from './auth-status.model'
 import { AuthUser } from './auth-user.model'
 
@@ -45,7 +45,7 @@ export class AuthService {
   private refreshingTokenObservable: Observable<any>
 
   constructor (
-    private serverService: ServerService,
+    private redirectService: RedirectService,
     private http: HttpClient,
     private notifier: Notifier,
     private hotkeysService: HotkeysService,
@@ -227,9 +227,7 @@ Ensure you have correctly configured PeerTube (config/ directory), in particular
           logger.info('Cannot refresh token -> logout...')
           this.logout()
 
-          const externalLoginUrl = PluginsManager.getDefaultLoginHref(environment.apiUrl, this.serverService.getHTMLConfig())
-          if (externalLoginUrl) window.location.href = externalLoginUrl
-          else this.router.navigate([ '/login' ])
+          this.redirectService.redirectToLogin()
 
           return observableThrowError(() => ({
             error: $localize`You need to reconnect.`
diff --git a/client/src/app/core/routing/login-guard.service.ts b/client/src/app/core/routing/login-guard.service.ts
index a949be14c..2f5a31e7f 100644
--- a/client/src/app/core/routing/login-guard.service.ts
+++ b/client/src/app/core/routing/login-guard.service.ts
@@ -1,19 +1,20 @@
 import { Injectable } from '@angular/core'
-import { ActivatedRouteSnapshot, CanActivate, CanActivateChild, Router, RouterStateSnapshot } from '@angular/router'
+import { ActivatedRouteSnapshot, CanActivate, CanActivateChild, RouterStateSnapshot } from '@angular/router'
 import { AuthService } from '../auth/auth.service'
+import { RedirectService } from './redirect.service'
 
 @Injectable()
 export class LoginGuard implements CanActivate, CanActivateChild {
 
   constructor (
-    private router: Router,
-    private auth: AuthService
+    private auth: AuthService,
+    private redirectService: RedirectService
   ) {}
 
   canActivate (route: ActivatedRouteSnapshot, state: RouterStateSnapshot) {
     if (this.auth.isLoggedIn() === true) return true
 
-    this.router.navigate([ '/login' ])
+    this.redirectService.redirectToLogin()
     return false
   }
 
diff --git a/client/src/app/core/routing/redirect.service.ts b/client/src/app/core/routing/redirect.service.ts
index 1344458d5..e239c6210 100644
--- a/client/src/app/core/routing/redirect.service.ts
+++ b/client/src/app/core/routing/redirect.service.ts
@@ -4,6 +4,8 @@ import { NavigationCancel, NavigationEnd, Router } from '@angular/router'
 import { logger } from '@root-helpers/logger'
 import { ServerService } from '../server'
 import { SessionStorageService } from '../wrappers/storage.service'
+import { PluginsManager } from '@root-helpers/plugins-manager'
+import { environment } from 'src/environments/environment'
 
 const debugLogger = debug('peertube:router:RedirectService')
 
@@ -100,6 +102,13 @@ export class RedirectService {
 
   }
 
+  redirectToLogin () {
+    const externalLoginUrl = PluginsManager.getDefaultLoginHref(environment.apiUrl, this.serverService.getHTMLConfig())
+
+    if (externalLoginUrl) window.location.href = externalLoginUrl
+    else this.router.navigate([ '/login' ])
+  }
+
   private doRedirect (redirectUrl: string, fallbackRoute?: string) {
     debugLogger('Redirecting on %s', redirectUrl)
 
diff --git a/client/src/app/core/routing/user-right-guard.service.ts b/client/src/app/core/routing/user-right-guard.service.ts
index a2ce772db..c6bd05bb6 100644
--- a/client/src/app/core/routing/user-right-guard.service.ts
+++ b/client/src/app/core/routing/user-right-guard.service.ts
@@ -1,12 +1,13 @@
 import { Injectable } from '@angular/core'
-import { ActivatedRouteSnapshot, CanActivate, CanActivateChild, Router, RouterStateSnapshot } from '@angular/router'
+import { ActivatedRouteSnapshot, CanActivate, CanActivateChild, RouterStateSnapshot } from '@angular/router'
 import { AuthService } from '../auth/auth.service'
+import { RedirectService } from './redirect.service'
 
 @Injectable()
 export class UserRightGuard implements CanActivate, CanActivateChild {
 
   constructor (
-    private router: Router,
+    private redirectService: RedirectService,
     private auth: AuthService
   ) {}
 
@@ -18,7 +19,7 @@ export class UserRightGuard implements CanActivate, CanActivateChild {
       if (user.hasRight(neededUserRight)) return true
     }
 
-    this.router.navigate([ '/login' ])
+    this.redirectService.redirectToLogin()
     return false
   }
 
diff --git a/client/src/app/menu/menu.component.html b/client/src/app/menu/menu.component.html
index abfc151e7..0fb9897b9 100644
--- a/client/src/app/menu/menu.component.html
+++ b/client/src/app/menu/menu.component.html
@@ -100,8 +100,7 @@
       </div>
 
       <div *ngIf="!isLoggedIn" class="login-buttons-block">
-        <a i18n *ngIf="!getExternalLoginHref()" routerLink="/login" class="peertube-button-link orange-button">Login</a>
-        <a i18n *ngIf="getExternalLoginHref()" [href]="getExternalLoginHref()" class="peertube-button-link orange-button">Login</a>
+        <my-login-link className="peertube-button-link orange-button w-100"></my-login-link>
 
         <a *ngIf="isRegistrationAllowed()" routerLink="/signup" class="peertube-button-link create-account-button">
           <my-signup-label [requiresApproval]="requiresApproval"></my-signup-label>
diff --git a/client/src/app/menu/menu.component.scss b/client/src/app/menu/menu.component.scss
index cd57e134e..9d5ad57c2 100644
--- a/client/src/app/menu/menu.component.scss
+++ b/client/src/app/menu/menu.component.scss
@@ -251,7 +251,8 @@ my-actor-avatar {
 .login-buttons-block {
   margin: 30px 25px 35px;
 
-  > a {
+  > a,
+  > my-login-link {
     display: block;
     width: 100%;
 
diff --git a/client/src/app/menu/menu.component.ts b/client/src/app/menu/menu.component.ts
index fc6d74cff..21354e52d 100644
--- a/client/src/app/menu/menu.component.ts
+++ b/client/src/app/menu/menu.component.ts
@@ -1,7 +1,6 @@
 import { HotkeysService } from 'angular2-hotkeys'
 import * as debug from 'debug'
 import { switchMap } from 'rxjs/operators'
-import { environment } from 'src/environments/environment'
 import { ViewportScroller } from '@angular/common'
 import { Component, OnInit, ViewChild } from '@angular/core'
 import { Router } from '@angular/router'
@@ -22,7 +21,6 @@ import { LanguageChooserComponent } from '@app/menu/language-chooser.component'
 import { QuickSettingsModalComponent } from '@app/modal/quick-settings-modal.component'
 import { PeertubeModalService } from '@app/shared/shared-main/peertube-modal/peertube-modal.service'
 import { NgbDropdown } from '@ng-bootstrap/ng-bootstrap'
-import { PluginsManager } from '@root-helpers/plugins-manager'
 import { HTMLServerConfig, ServerConfig, UserRight, VideoConstant } from '@shared/models'
 
 const debugLogger = debug('peertube:menu:MenuComponent')
@@ -135,10 +133,6 @@ export class MenuComponent implements OnInit {
       .subscribe(() => this.openQuickSettings())
   }
 
-  getExternalLoginHref () {
-    return PluginsManager.getDefaultLoginHref(environment.apiUrl, this.serverConfig)
-  }
-
   isRegistrationAllowed () {
     if (!this.serverConfig) return false
 
diff --git a/client/src/app/shared/shared-main/angular/index.ts b/client/src/app/shared/shared-main/angular/index.ts
index 4b87c2952..f72d60313 100644
--- a/client/src/app/shared/shared-main/angular/index.ts
+++ b/client/src/app/shared/shared-main/angular/index.ts
@@ -5,5 +5,6 @@ export * from './duration-formatter.pipe'
 export * from './from-now.pipe'
 export * from './infinite-scroller.directive'
 export * from './link.component'
+export * from './login-link.component'
 export * from './number-formatter.pipe'
 export * from './peertube-template.directive'
diff --git a/client/src/app/shared/shared-main/angular/link.component.html b/client/src/app/shared/shared-main/angular/link.component.html
index e61a1e085..2ea2df2b5 100644
--- a/client/src/app/shared/shared-main/angular/link.component.html
+++ b/client/src/app/shared/shared-main/angular/link.component.html
@@ -2,10 +2,10 @@
   <ng-content></ng-content>
 </ng-template>
 
-<a *ngIf="!href" [routerLink]="internalLink" [attr.title]="title" [tabindex]="tabindex">
+<a *ngIf="!href" [routerLink]="internalLink" [attr.title]="title" [tabindex]="tabindex" [ngClass]="builtClasses">
   <ng-template *ngTemplateOutlet="content"></ng-template>
 </a>
 
-<a *ngIf="href" [href]="href" [target]="target" [attr.title]="title" [tabindex]="tabindex">
+<a *ngIf="href" [href]="href" [target]="target" [attr.title]="title" [tabindex]="tabindex" [ngClass]="builtClasses">
   <ng-template *ngTemplateOutlet="content"></ng-template>
 </a>
diff --git a/client/src/app/shared/shared-main/angular/link.component.scss b/client/src/app/shared/shared-main/angular/link.component.scss
index bb86d5488..f54240d31 100644
--- a/client/src/app/shared/shared-main/angular/link.component.scss
+++ b/client/src/app/shared/shared-main/angular/link.component.scss
@@ -1,4 +1,4 @@
-a {
+.no-class {
   color: inherit;
   text-decoration: inherit;
   position: inherit;
diff --git a/client/src/app/shared/shared-main/angular/link.component.ts b/client/src/app/shared/shared-main/angular/link.component.ts
index ecbd9151c..1f5975589 100644
--- a/client/src/app/shared/shared-main/angular/link.component.ts
+++ b/client/src/app/shared/shared-main/angular/link.component.ts
@@ -1,17 +1,27 @@
-import { Component, Input } from '@angular/core'
+import { Component, Input, OnInit } from '@angular/core'
 
 @Component({
   selector: 'my-link',
   styleUrls: [ './link.component.scss' ],
   templateUrl: './link.component.html'
 })
-export class LinkComponent {
+export class LinkComponent implements OnInit {
   @Input() internalLink?: string | any[]
 
   @Input() href?: string
-  @Input() target?: string
+  @Input() target = '_self'
 
   @Input() title?: string
 
+  @Input() className?: string
+
   @Input() tabindex: string | number
+
+  builtClasses: string
+
+  ngOnInit () {
+    this.builtClasses = this.className
+      ? this.className
+      : 'no-class'
+  }
 }
diff --git a/client/src/app/shared/shared-main/angular/login-link.component.html b/client/src/app/shared/shared-main/angular/login-link.component.html
new file mode 100644
index 000000000..f90ffba68
--- /dev/null
+++ b/client/src/app/shared/shared-main/angular/login-link.component.html
@@ -0,0 +1 @@
+<my-link i18n internalLink="/login" [href]="getExternalLoginHref()" [className]="className">{{ label }}</my-link>
diff --git a/client/src/app/shared/shared-main/angular/login-link.component.ts b/client/src/app/shared/shared-main/angular/login-link.component.ts
new file mode 100644
index 000000000..7e3a773c2
--- /dev/null
+++ b/client/src/app/shared/shared-main/angular/login-link.component.ts
@@ -0,0 +1,22 @@
+import { environment } from 'src/environments/environment'
+import { Component, Input } from '@angular/core'
+import { ServerService } from '@app/core'
+import { PluginsManager } from '@root-helpers/plugins-manager'
+
+@Component({
+  selector: 'my-login-link',
+  templateUrl: './login-link.component.html'
+})
+export class LoginLinkComponent {
+  @Input() label = $localize`Login`
+
+  @Input() className?: string
+
+  constructor (private server: ServerService) {
+
+  }
+
+  getExternalLoginHref () {
+    return PluginsManager.getDefaultLoginHref(environment.apiUrl, this.server.getHTMLConfig())
+  }
+}
diff --git a/client/src/app/shared/shared-main/shared-main.module.ts b/client/src/app/shared/shared-main/shared-main.module.ts
index eb1642d97..0bdf27263 100644
--- a/client/src/app/shared/shared-main/shared-main.module.ts
+++ b/client/src/app/shared/shared-main/shared-main.module.ts
@@ -25,6 +25,7 @@ import {
   FromNowPipe,
   InfiniteScrollerDirective,
   LinkComponent,
+  LoginLinkComponent,
   NumberFormatterPipe,
   PeerTubeTemplateDirective
 } from './angular'
@@ -92,6 +93,7 @@ import { VideoChannelService } from './video-channel'
     InfiniteScrollerDirective,
     PeerTubeTemplateDirective,
     LinkComponent,
+    LoginLinkComponent,
 
     ActionDropdownComponent,
     ButtonComponent,
@@ -152,6 +154,7 @@ import { VideoChannelService } from './video-channel'
     InfiniteScrollerDirective,
     PeerTubeTemplateDirective,
     LinkComponent,
+    LoginLinkComponent,
 
     ActionDropdownComponent,
     ButtonComponent,
diff --git a/client/src/app/shared/shared-user-subscription/subscribe-button.component.ts b/client/src/app/shared/shared-user-subscription/subscribe-button.component.ts
index 7a55a6ffb..a002bf4e7 100644
--- a/client/src/app/shared/shared-user-subscription/subscribe-button.component.ts
+++ b/client/src/app/shared/shared-user-subscription/subscribe-button.component.ts
@@ -1,7 +1,6 @@
 import { concat, forkJoin, merge } from 'rxjs'
 import { Component, Input, OnChanges, OnInit } from '@angular/core'
-import { Router } from '@angular/router'
-import { AuthService, Notifier } from '@app/core'
+import { AuthService, Notifier, RedirectService } from '@app/core'
 import { Account, VideoChannel, VideoService } from '@app/shared/shared-main'
 import { FeedFormat } from '@shared/models'
 import { UserSubscriptionService } from './user-subscription.service'
@@ -27,7 +26,7 @@ export class SubscribeButtonComponent implements OnInit, OnChanges {
 
   constructor (
     private authService: AuthService,
-    private router: Router,
+    private redirectService: RedirectService,
     private notifier: Notifier,
     private userSubscriptionService: UserSubscriptionService,
     private videoService: VideoService
@@ -152,7 +151,7 @@ export class SubscribeButtonComponent implements OnInit, OnChanges {
   }
 
   gotoLogin () {
-    this.router.navigate([ '/login' ])
+    this.redirectService.redirectToLogin()
   }
 
   subscribeStatus (subscribed: boolean) {