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

redirect to login on 401, display 403 variant #3632

Merged
merged 1 commit into from
Jan 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/src/app/+accounts/accounts.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class AccountsComponent implements OnInit, OnDestroy {
switchMap(accountId => this.accountService.getAccount(accountId)),
tap(account => this.onAccount(account)),
switchMap(account => this.videoChannelService.listAccountVideoChannels(account)),
catchError(err => this.restExtractor.redirectTo404IfNotFound(err, [
catchError(err => this.restExtractor.redirectTo404IfNotFound(err, 'other', [
HttpStatusCode.BAD_REQUEST_400,
HttpStatusCode.NOT_FOUND_404
]))
Expand Down
15 changes: 12 additions & 3 deletions client/src/app/+page-not-found/page-not-found.component.html
Original file line number Diff line number Diff line change
@@ -1,23 +1,32 @@
<div class="root">
<div *ngIf="status === 404" class="box">
<div *ngIf="status !== 403 && status !== 418" class="box">
<strong>{{ status }}.</strong>
<span class="ml-1 text-muted" i18n>That's an error.</span>

<div class="text mt-4" i18n>
We couldn't find any ressource tied to the URL {{ pathname }} you were looking for.
We couldn't find any {{ getRessourceName() }} tied to the URL {{ pathname }} you were looking for.
</div>

<div class="text-muted mt-4">
<span i18n="Possible reasons preceding a list of reasons a `Not Found` error page may occur">Possible reasons:</span>

<ul>
<li i18n>The page may have been moved or deleted</li>
<li i18n>You may have used an outdated or broken link</li>
<li i18n>The {{ getRessourceName() }} may have been moved or deleted</li>
<li i18n>You may have typed the address or URL incorrectly</li>
</ul>
</div>
</div>

<div *ngIf="status === 403" class="box">
<strong>{{ status }}.</strong>
<span class="ml-1 text-muted" i18n>You are not authorized here.</span>

<div class="text mt-4" i18n>
You might need to check your account is allowed by the {{ getRessourceName() }} or instance owner.
</div>
</div>

<div *ngIf="status === 418" class="box">
<strong>{{ status }}.</strong>
<span class="ml-1 text-muted">I'm a teapot.</span>
Expand Down
22 changes: 20 additions & 2 deletions client/src/app/+page-not-found/page-not-found.component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Component, OnInit } from '@angular/core'
import { Title } from '@angular/platform-browser'
import { Router } from '@angular/router'
import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes'

@Component({
Expand All @@ -9,10 +10,16 @@ import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes'
})
export class PageNotFoundComponent implements OnInit {
status = HttpStatusCode.NOT_FOUND_404
type: string

public constructor (
private titleService: Title
) {}
private titleService: Title,
private router: Router
) {
const state = this.router.getCurrentNavigation()?.extras.state
Chocobozzz marked this conversation as resolved.
Show resolved Hide resolved
this.type = state?.type || this.type
this.status = state?.obj.status || this.status
}

ngOnInit () {
if (this.pathname.includes('teapot')) {
Expand All @@ -25,10 +32,21 @@ export class PageNotFoundComponent implements OnInit {
return window.location.pathname
}

getRessourceName () {
switch (this.type) {
case 'video':
return $localize`video`
default:
return $localize`ressource`
}
}

getMascotName () {
switch (this.status) {
case HttpStatusCode.I_AM_A_TEAPOT_418:
return 'happy'
case HttpStatusCode.FORBIDDEN_403:
return 'arguing'
case HttpStatusCode.NOT_FOUND_404:
default:
return 'defeated'
Expand Down
2 changes: 1 addition & 1 deletion client/src/app/+video-channels/video-channels.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class VideoChannelsComponent implements OnInit, OnDestroy {
map(params => params[ 'videoChannelName' ]),
distinctUntilChanged(),
switchMap(videoChannelName => this.videoChannelService.getVideoChannel(videoChannelName)),
catchError(err => this.restExtractor.redirectTo404IfNotFound(err, [
catchError(err => this.restExtractor.redirectTo404IfNotFound(err, 'other', [
HttpStatusCode.BAD_REQUEST_400,
HttpStatusCode.NOT_FOUND_404
]))
Expand Down
13 changes: 5 additions & 8 deletions client/src/app/+videos/+video-watch/video-watch.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ export class VideoWatchComponent implements OnInit, OnDestroy {
this.videoCaptionService.listCaptions(videoId)
])
.pipe(
// If 401, the video is private or blocked so redirect to 404
// If 400, 403 or 404, the video is private or blocked so redirect to 404
catchError(err => {
if (err.body.errorCode === ServerErrorCode.DOES_NOT_RESPECT_FOLLOW_CONSTRAINTS && err.body.originUrl) {
const search = window.location.search
Expand All @@ -416,9 +416,8 @@ export class VideoWatchComponent implements OnInit, OnDestroy {
$localize`Redirection`
).then(res => {
if (res === false) {
return this.restExtractor.redirectTo404IfNotFound(err, [
return this.restExtractor.redirectTo404IfNotFound(err, 'video', [
HttpStatusCode.BAD_REQUEST_400,
HttpStatusCode.UNAUTHORIZED_401,
HttpStatusCode.FORBIDDEN_403,
HttpStatusCode.NOT_FOUND_404
])
Expand All @@ -428,9 +427,8 @@ export class VideoWatchComponent implements OnInit, OnDestroy {
})
}

return this.restExtractor.redirectTo404IfNotFound(err, [
return this.restExtractor.redirectTo404IfNotFound(err, 'video', [
HttpStatusCode.BAD_REQUEST_400,
HttpStatusCode.UNAUTHORIZED_401,
HttpStatusCode.FORBIDDEN_403,
HttpStatusCode.NOT_FOUND_404
])
Expand Down Expand Up @@ -464,10 +462,9 @@ export class VideoWatchComponent implements OnInit, OnDestroy {

this.playlistService.getVideoPlaylist(playlistId)
.pipe(
// If 401, the video is private or blocked so redirect to 404
catchError(err => this.restExtractor.redirectTo404IfNotFound(err, [
// If 400 or 403, the video is private or blocked so redirect to 404
catchError(err => this.restExtractor.redirectTo404IfNotFound(err, 'video', [
HttpStatusCode.BAD_REQUEST_400,
HttpStatusCode.UNAUTHORIZED_401,
HttpStatusCode.FORBIDDEN_403,
HttpStatusCode.NOT_FOUND_404
]))
Expand Down
4 changes: 2 additions & 2 deletions client/src/app/core/rest/rest-extractor.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ export class RestExtractor {
return observableThrowError(errorObj)
}

redirectTo404IfNotFound (obj: { status: number }, status = [ HttpStatusCode.NOT_FOUND_404 ]) {
redirectTo404IfNotFound (obj: { status: number }, type: 'video' | 'other', status = [ HttpStatusCode.NOT_FOUND_404 ]) {
if (obj && obj.status && status.indexOf(obj.status) !== -1) {
// Do not use redirectService to avoid circular dependencies
this.router.navigate([ '/404' ], { skipLocationChange: true })
this.router.navigate([ '/404' ], { state: { type, obj }, skipLocationChange: true })
}

return observableThrowError(obj)
Expand Down
19 changes: 14 additions & 5 deletions client/src/app/shared/shared-main/auth/auth-interceptor.service.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import { Observable, throwError as observableThrowError } from 'rxjs'
import { Observable, of, throwError as observableThrowError } from 'rxjs'
import { catchError, switchMap } from 'rxjs/operators'
import { HTTP_INTERCEPTORS, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/common/http'
import { HTTP_INTERCEPTORS, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest, HttpErrorResponse } from '@angular/common/http'
import { Injectable, Injector } from '@angular/core'
import { AuthService } from '@app/core/auth/auth.service'
import { Router } from '@angular/router'
import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes'

@Injectable()
export class AuthInterceptor implements HttpInterceptor {
private authService: AuthService

// https://github.com/angular/angular/issues/18224#issuecomment-316957213
constructor (private injector: Injector) {}
constructor (private injector: Injector, private router: Router) {}

intercept (req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
if (this.authService === undefined) {
Expand All @@ -22,9 +24,11 @@ export class AuthInterceptor implements HttpInterceptor {
// Catch 401 errors (refresh token expired)
return next.handle(authReq)
.pipe(
catchError(err => {
if (err.status === 401 && err.error && err.error.code === 'invalid_token') {
catchError((err: HttpErrorResponse) => {
if (err.status === HttpStatusCode.UNAUTHORIZED_401 && err.error && err.error.code === 'invalid_token') {
return this.handleTokenExpired(req, next)
} else if (err.status === HttpStatusCode.UNAUTHORIZED_401) {
return this.handleNotAuthenticated(err)
}

return observableThrowError(err)
Expand All @@ -51,6 +55,11 @@ export class AuthInterceptor implements HttpInterceptor {
// Clone the request to add the new header
return req.clone({ headers: req.headers.set('Authorization', authHeaderValue) })
}

private handleNotAuthenticated (err: HttpErrorResponse, path = '/login'): Observable<any> {
this.router.navigateByUrl(path)
return of(err.message)
}
}

export const AUTH_INTERCEPTOR_PROVIDER = {
Expand Down