Merge branch 'artonge/PeerTube-feature/logoutUrlForAuthProviders' into 'develop'

Artonge/peer tube feature/logout url for auth providers

See merge request framasoft/peertube/PeerTube!33

7 files changed, 123 insertions, 13 deletions

diff --git a/server/lib/auth.ts b/server/lib/auth.ts
index 3f8e18633..acf0da18a 100644
--- a/server/lib/auth.ts
+++ b/server/lib/auth.ts
@@ -52,7 +52,7 @@ async function handleTokenRevocation (req: express.Request, res: express.Respons
   const token = res.locals.oauth.token
 
   res.locals.explicitLogout = true
-  await revokeToken(token)
+  const result = await revokeToken(token)
 
   // FIXME: uncomment when https://github.com/oauthjs/node-oauth2-server/pull/289 is released
   // oAuthServer.revoke(req, res, err => {
@@ -68,7 +68,7 @@ async function handleTokenRevocation (req: express.Request, res: express.Respons
   //   }
   // })
 
-  return res.json()
+  return res.json(result)
 }
 
 async function onExternalUserAuthenticated (options: {
diff --git a/server/lib/oauth-model.ts b/server/lib/oauth-model.ts
index 3273c6c2d..f7ea98b41 100644
--- a/server/lib/oauth-model.ts
+++ b/server/lib/oauth-model.ts
@@ -141,13 +141,15 @@ async function getUser (usernameOrEmail?: string, password?: string) {
   return user
 }
 
-async function revokeToken (tokenInfo: { refreshToken: string }) {
+async function revokeToken (tokenInfo: { refreshToken: string }): Promise<{ success: boolean, redirectUrl?: string }> {
   const res: express.Response = this.request.res
   const token = await OAuthTokenModel.getByRefreshTokenAndPopulateUser(tokenInfo.refreshToken)
 
   if (token) {
+    let redirectUrl: string
+
     if (res.locals.explicitLogout === true && token.User.pluginAuth && token.authName) {
-      PluginManager.Instance.onLogout(token.User.pluginAuth, token.authName, token.User)
+      redirectUrl = await PluginManager.Instance.onLogout(token.User.pluginAuth, token.authName, token.User, this.request)
     }
 
     clearCacheByToken(token.accessToken)
@@ -155,10 +157,10 @@ async function revokeToken (tokenInfo: { refreshToken: string }) {
     token.destroy()
          .catch(err => logger.error('Cannot destroy token when revoking token.', { err }))
 
-    return true
+    return { success: true, redirectUrl }
   }
 
-  return false
+  return { success: false }
 }
 
 async function saveToken (token: TokenInfo, client: OAuthClientModel, user: UserModel) {
diff --git a/server/lib/plugins/plugin-manager.ts b/server/lib/plugins/plugin-manager.ts
index 94b5ecc41..8e7491257 100644
--- a/server/lib/plugins/plugin-manager.ts
+++ b/server/lib/plugins/plugin-manager.ts
@@ -1,3 +1,4 @@
+import * as express from 'express'
 import { createReadStream, createWriteStream } from 'fs'
 import { outputFile, readJSON } from 'fs-extra'
 import { basename, join } from 'path'
@@ -166,18 +167,25 @@ export class PluginManager implements ServerHook {
 
   // ###################### External events ######################
 
-  onLogout (npmName: string, authName: string, user: MUser) {
+  async onLogout (npmName: string, authName: string, user: MUser, req: express.Request) {
     const auth = this.getAuth(npmName, authName)
 
     if (auth?.onLogout) {
       logger.info('Running onLogout function from auth %s of plugin %s', authName, npmName)
 
       try {
-        auth.onLogout(user)
+        // Force await, in case or onLogout returns a promise
+        const result = await auth.onLogout(user, req)
+
+        return typeof result === 'string'
+          ? result
+          : undefined
       } catch (err) {
         logger.warn('Cannot run onLogout function from auth %s of plugin %s.', authName, npmName, { err })
       }
     }
+
+    return undefined
   }
 
   onSettingsChanged (name: string, settings: any) {
diff --git a/server/tests/fixtures/peertube-plugin-test-external-auth-three/main.js b/server/tests/fixtures/peertube-plugin-test-external-auth-three/main.js
new file mode 100644
index 000000000..30cedccc6
--- /dev/null
+++ b/server/tests/fixtures/peertube-plugin-test-external-auth-three/main.js
@@ -0,0 +1,53 @@
+async function register ({
+  registerExternalAuth,
+  peertubeHelpers
+}) {
+  {
+    const result = registerExternalAuth({
+      authName: 'external-auth-7',
+      authDisplayName: () => 'External Auth 7',
+      onAuthRequest: (req, res) => {
+        result.userAuthenticated({
+          req,
+          res,
+          username: 'cid',
+          email: 'cid@example.com',
+          displayName: 'Cid Marquez'
+        })
+      },
+      onLogout: (user, req) => {
+        return 'https://example.com/redirectUrl'
+      }
+    })
+  }
+
+  {
+    const result = registerExternalAuth({
+      authName: 'external-auth-8',
+      authDisplayName: () => 'External Auth 8',
+      onAuthRequest: (req, res) => {
+        result.userAuthenticated({
+          req,
+          res,
+          username: 'cid',
+          email: 'cid@example.com',
+          displayName: 'Cid Marquez'
+        })
+      },
+      onLogout: (user, req) => {
+        return 'https://example.com/redirectUrl?access_token=' + req.headers['authorization'].split(' ')[1]
+      }
+    })
+  }
+}
+
+async function unregister () {
+
+}
+
+module.exports = {
+  register,
+  unregister
+}
+
+// ###########################################################################
diff --git a/server/tests/fixtures/peertube-plugin-test-external-auth-three/package.json b/server/tests/fixtures/peertube-plugin-test-external-auth-three/package.json
new file mode 100644
index 000000000..f323d189d
--- /dev/null
+++ b/server/tests/fixtures/peertube-plugin-test-external-auth-three/package.json
@@ -0,0 +1,20 @@
+{
+  "name": "peertube-plugin-test-external-auth-three",
+  "version": "0.0.1",
+  "description": "External auth three",
+  "engine": {
+    "peertube": ">=1.3.0"
+  },
+  "keywords": [
+    "peertube",
+    "plugin"
+  ],
+  "homepage": "https://github.com/Chocobozzz/PeerTube",
+  "author": "Chocobozzz",
+  "bugs": "https://github.com/Chocobozzz/PeerTube/issues",
+  "library": "./main.js",
+  "staticDirs": {},
+  "css": [],
+  "clientScripts": [],
+  "translations": {}
+}
diff --git a/server/tests/plugins/external-auth.ts b/server/tests/plugins/external-auth.ts
index 57361be05..6d907cc51 100644
--- a/server/tests/plugins/external-auth.ts
+++ b/server/tests/plugins/external-auth.ts
@@ -73,7 +73,7 @@ describe('Test external auth plugins', function () {
     server = await flushAndRunServer(1)
     await setAccessTokensToServers([ server ])
 
-    for (const suffix of [ 'one', 'two' ]) {
+    for (const suffix of [ 'one', 'two', 'three' ]) {
       await installPlugin({
         url: server.url,
         accessToken: server.accessToken,
@@ -88,7 +88,7 @@ describe('Test external auth plugins', function () {
     const config: ServerConfig = res.body
 
     const auths = config.plugin.registeredExternalAuths
-    expect(auths).to.have.lengthOf(6)
+    expect(auths).to.have.lengthOf(8)
 
     const auth2 = auths.find((a) => a.authName === 'external-auth-2')
     expect(auth2).to.exist
@@ -301,7 +301,7 @@ describe('Test external auth plugins', function () {
     const config: ServerConfig = res.body
 
     const auths = config.plugin.registeredExternalAuths
-    expect(auths).to.have.lengthOf(5)
+    expect(auths).to.have.lengthOf(7)
 
     const auth1 = auths.find(a => a.authName === 'external-auth-2')
     expect(auth1).to.not.exist
@@ -371,7 +371,7 @@ describe('Test external auth plugins', function () {
     const config: ServerConfig = res.body
 
     const auths = config.plugin.registeredExternalAuths
-    expect(auths).to.have.lengthOf(4)
+    expect(auths).to.have.lengthOf(6)
 
     const auth2 = auths.find((a) => a.authName === 'external-auth-2')
     expect(auth2).to.not.exist
@@ -380,4 +380,30 @@ describe('Test external auth plugins', function () {
   after(async function () {
     await cleanupTests([ server ])
   })
+
+  it('Should forward the redirectUrl if the plugin returns one', async function () {
+    const resLogin = await loginExternal({
+      server,
+      npmName: 'test-external-auth-three',
+      authName: 'external-auth-7',
+      username: 'cid'
+    })
+
+    const resLogout = await logout(server.url, resLogin.access_token)
+
+    expect(resLogout.body.redirectUrl).to.equal('https://example.com/redirectUrl')
+  })
+
+  it('Should call the plugin\'s onLogout method with the request', async function () {
+    const resLogin = await loginExternal({
+      server,
+      npmName: 'test-external-auth-three',
+      authName: 'external-auth-8',
+      username: 'cid'
+    })
+
+    const resLogout = await logout(server.url, resLogin.access_token)
+
+    expect(resLogout.body.redirectUrl).to.equal('https://example.com/redirectUrl?access_token=' + resLogin.access_token)
+  })
 })
diff --git a/server/types/plugins/register-server-auth.model.ts b/server/types/plugins/register-server-auth.model.ts
index 31c71b0d0..3e1a5aeba 100644
--- a/server/types/plugins/register-server-auth.model.ts
+++ b/server/types/plugins/register-server-auth.model.ts
@@ -21,7 +21,8 @@ interface RegisterServerAuthBase {
   authName: string
 
   // Called by PeerTube when a user from your plugin logged out
-  onLogout?(user: MUser): void
+  // Returns a redirectUrl sent to the client or nothing
+  onLogout?(user: MUser, req: express.Request): Promise<string>
 
   // Your plugin can hook PeerTube access/refresh token validity
   // So you can control for your plugin the user session lifetime