From bc90883f1a5e9c4ecb76ae358734b85be515af7f Mon Sep 17 00:00:00 2001
From: Chocobozzz <me@florianbigard.com>
Date: Thu, 30 Apr 2020 15:03:09 +0200
Subject: [PATCH] Handle external login errors

---
 client/src/app/login/login.component.html    | 10 +++--
 client/src/app/login/login.component.scss    |  9 ++--
 client/src/app/login/login.component.ts      |  7 ++++
 server/lib/auth.ts                           | 18 +++++---
 server/lib/client-html.ts                    |  2 +-
 server/lib/plugins/register-helpers-store.ts |  4 +-
 server/models/server/plugin.ts               | 44 +++++++++++++-------
 7 files changed, 61 insertions(+), 33 deletions(-)

diff --git a/client/src/app/login/login.component.html b/client/src/app/login/login.component.html
index b0639d8ca..a935c86c3 100644
--- a/client/src/app/login/login.component.html
+++ b/client/src/app/login/login.component.html
@@ -3,7 +3,11 @@
     Login
   </div>
 
-  <ng-container *ngIf="!isAuthenticatedWithExternalAuth">
+  <div class="alert alert-danger" i18n *ngIf="externalAuthError">
+    Sorry but there was an issue with the external login process. Please <a routerLink="/about">contact an administrator</a>.
+  </div>
+
+  <ng-container *ngIf="!externalAuthError && !isAuthenticatedWithExternalAuth">
     <div class="alert alert-info" *ngIf="signupAllowed === false" role="alert">
       <h6 class="alert-heading" i18n>
         If you are looking for an account…
@@ -63,8 +67,8 @@
       <div class="external-login-blocks" *ngIf="getExternalLogins().length !== 0">
         <div class="block-title" i18n>Or sign in with</div>
 
-        <div class="external-login-block">
-          <a *ngFor="let auth of getExternalLogins()" [href]="getAuthHref(auth)" role="button">
+        <div>
+          <a class="external-login-block" *ngFor="let auth of getExternalLogins()" [href]="getAuthHref(auth)" role="button">
             {{ auth.authDisplayName }}
           </a>
         </div>
diff --git a/client/src/app/login/login.component.scss b/client/src/app/login/login.component.scss
index ccc98c12a..db9f78f7c 100644
--- a/client/src/app/login/login.component.scss
+++ b/client/src/app/login/login.component.scss
@@ -38,7 +38,6 @@ input[type=submit] {
   }
 
   .external-login-blocks {
-    padding: 0 10px 10px 10px;
     min-width: 200px;
 
     .block-title {
@@ -46,9 +45,12 @@ input[type=submit] {
     }
 
     .external-login-block {
+      @include disable-default-a-behaviour;
+
       cursor: pointer;
       border: 1px solid #d1d7e0;
       border-radius: 5px;
+      color: var(--mainForegroundColor);
       margin: 10px 10px 0 0;
       display: flex;
       justify-content: center;
@@ -59,11 +61,6 @@ input[type=submit] {
       &:hover {
         background-color: rgba(209, 215, 224, 0.5)
       }
-
-      a {
-        @include disable-default-a-behaviour;
-        color: var(--mainForegroundColor);
-      }
     }
   }
 }
diff --git a/client/src/app/login/login.component.ts b/client/src/app/login/login.component.ts
index 5db8d3dbb..5d935cb49 100644
--- a/client/src/app/login/login.component.ts
+++ b/client/src/app/login/login.component.ts
@@ -23,7 +23,9 @@ export class LoginComponent extends FormReactive implements OnInit, AfterViewIni
 
   error: string = null
   forgotPasswordEmail = ''
+
   isAuthenticatedWithExternalAuth = false
+  externalAuthError = false
   externalLogins: string[] = []
 
   private openedForgotPasswordModal: NgbModalRef
@@ -61,6 +63,11 @@ export class LoginComponent extends FormReactive implements OnInit, AfterViewIni
       return
     }
 
+    if (snapshot.queryParams.externalAuthError) {
+      this.externalAuthError = true
+      return
+    }
+
     this.buildForm({
       username: this.loginValidatorsService.LOGIN_USERNAME,
       password: this.loginValidatorsService.LOGIN_PASSWORD
diff --git a/server/lib/auth.ts b/server/lib/auth.ts
index 1fa896f6e..7c1dd1139 100644
--- a/server/lib/auth.ts
+++ b/server/lib/auth.ts
@@ -83,10 +83,13 @@ async function onExternalUserAuthenticated (options: {
     return
   }
 
-  if (!isAuthResultValid(npmName, authName, authResult)) return
-
   const { res } = authResult
 
+  if (!isAuthResultValid(npmName, authName, authResult)) {
+    res.redirect('/login?externalAuthError=true')
+    return
+  }
+
   logger.info('Generating auth bypass token for %s in auth %s of plugin %s.', authResult.username, authName, npmName)
 
   const bypassToken = await generateRandomString(32)
@@ -238,24 +241,27 @@ function proxifyExternalAuthBypass (req: express.Request, res: express.Response)
 
 function isAuthResultValid (npmName: string, authName: string, result: RegisterServerAuthenticatedResult) {
   if (!isUserUsernameValid(result.username)) {
-    logger.error('Auth method %s of plugin %s did not provide a valid username.', authName, npmName, { result })
+    logger.error('Auth method %s of plugin %s did not provide a valid username.', authName, npmName, { username: result.username })
     return false
   }
 
   if (!result.email) {
-    logger.error('Auth method %s of plugin %s did not provide a valid email.', authName, npmName, { result })
+    logger.error('Auth method %s of plugin %s did not provide a valid email.', authName, npmName, { email: result.email })
     return false
   }
 
   // role is optional
   if (result.role && !isUserRoleValid(result.role)) {
-    logger.error('Auth method %s of plugin %s did not provide a valid role.', authName, npmName, { result })
+    logger.error('Auth method %s of plugin %s did not provide a valid role.', authName, npmName, { role: result.role })
     return false
   }
 
   // display name is optional
   if (result.displayName && !isUserDisplayNameValid(result.displayName)) {
-    logger.error('Auth method %s of plugin %s did not provide a valid display name.', authName, npmName, { result })
+    logger.error(
+      'Auth method %s of plugin %s did not provide a valid display name.',
+      authName, npmName, { displayName: result.displayName }
+    )
     return false
   }
 
diff --git a/server/lib/client-html.ts b/server/lib/client-html.ts
index 572bd03bd..4a4b0d12f 100644
--- a/server/lib/client-html.ts
+++ b/server/lib/client-html.ts
@@ -119,7 +119,7 @@ export class ClientHtml {
       // Save locale in cookies
       res.cookie('clientLanguage', lang, {
         secure: WEBSERVER.SCHEME === 'https',
-        sameSite: true,
+        sameSite: 'none',
         maxAge: 1000 * 3600 * 24 * 90 // 3 months
       })
 
diff --git a/server/lib/plugins/register-helpers-store.ts b/server/lib/plugins/register-helpers-store.ts
index a3ec7ef6a..e337b1cb0 100644
--- a/server/lib/plugins/register-helpers-store.ts
+++ b/server/lib/plugins/register-helpers-store.ts
@@ -230,9 +230,9 @@ export class RegisterHelpersStore {
 
   private buildSettingsManager (): PluginSettingsManager {
     return {
-      getSetting: (name: string) => PluginModel.getSetting(this.plugin.name, this.plugin.type, name),
+      getSetting: (name: string) => PluginModel.getSetting(this.plugin.name, this.plugin.type, name, this.settings),
 
-      getSettings: (names: string[]) => PluginModel.getSettings(this.plugin.name, this.plugin.type, names),
+      getSettings: (names: string[]) => PluginModel.getSettings(this.plugin.name, this.plugin.type, names, this.settings),
 
       setSetting: (name: string, value: string) => PluginModel.setSetting(this.plugin.name, this.plugin.type, name, value),
 
diff --git a/server/models/server/plugin.ts b/server/models/server/plugin.ts
index 83c873c5b..3f88ac26d 100644
--- a/server/models/server/plugin.ts
+++ b/server/models/server/plugin.ts
@@ -1,5 +1,10 @@
+import * as Bluebird from 'bluebird'
+import { FindAndCountOptions, json } from 'sequelize'
 import { AllowNull, Column, CreatedAt, DataType, DefaultScope, Is, Model, Table, UpdatedAt } from 'sequelize-typescript'
-import { getSort, throwIfNotValid } from '../utils'
+import { MPlugin, MPluginFormattable } from '@server/typings/models'
+import { PeerTubePlugin } from '../../../shared/models/plugins/peertube-plugin.model'
+import { PluginType } from '../../../shared/models/plugins/plugin.type'
+import { RegisterServerSettingOptions } from '../../../shared/models/plugins/register-server-setting.model'
 import {
   isPluginDescriptionValid,
   isPluginHomepage,
@@ -7,12 +12,7 @@ import {
   isPluginTypeValid,
   isPluginVersionValid
 } from '../../helpers/custom-validators/plugins'
-import { PluginType } from '../../../shared/models/plugins/plugin.type'
-import { PeerTubePlugin } from '../../../shared/models/plugins/peertube-plugin.model'
-import { FindAndCountOptions, json } from 'sequelize'
-import { RegisterServerSettingOptions } from '../../../shared/models/plugins/register-server-setting.model'
-import * as Bluebird from 'bluebird'
-import { MPlugin, MPluginFormattable } from '@server/typings/models'
+import { getSort, throwIfNotValid } from '../utils'
 
 @DefaultScope(() => ({
   attributes: {
@@ -112,7 +112,7 @@ export class PluginModel extends Model<PluginModel> {
     return PluginModel.findOne(query)
   }
 
-  static getSetting (pluginName: string, pluginType: PluginType, settingName: string) {
+  static getSetting (pluginName: string, pluginType: PluginType, settingName: string, registeredSettings: RegisterServerSettingOptions[]) {
     const query = {
       attributes: [ 'settings' ],
       where: {
@@ -123,13 +123,23 @@ export class PluginModel extends Model<PluginModel> {
 
     return PluginModel.findOne(query)
       .then(p => {
-        if (!p || !p.settings) return undefined
+        if (!p || p.settings === undefined) {
+          const registered = registeredSettings.find(s => s.name === settingName)
+          if (!registered || registered.default === undefined) return undefined
+
+          return registered.default
+        }
 
         return p.settings[settingName]
       })
   }
 
-  static getSettings (pluginName: string, pluginType: PluginType, settingNames: string[]) {
+  static getSettings (
+    pluginName: string,
+    pluginType: PluginType,
+    settingNames: string[],
+    registeredSettings: RegisterServerSettingOptions[]
+  ) {
     const query = {
       attributes: [ 'settings' ],
       where: {
@@ -140,13 +150,17 @@ export class PluginModel extends Model<PluginModel> {
 
     return PluginModel.findOne(query)
       .then(p => {
-        if (!p || !p.settings) return {}
+        const result: { [settingName: string ]: string | boolean } = {}
 
-        const result: { [settingName: string ]: string } = {}
+        for (const name of settingNames) {
+          if (!p || p.settings[name] === undefined) {
+            const registered = registeredSettings.find(s => s.name === name)
 
-        for (const key of Object.keys(p.settings)) {
-          if (settingNames.includes(key)) {
-            result[key] = p.settings[key]
+            if (registered?.default !== undefined) {
+              result[name] = registered.default
+            }
+          } else {
+            result[name] = p.settings[name]
           }
         }
 
-- 
2.41.0