[Feature] slack oauth support (nytimes#245)

CONTRIBUTING.md

@@ -1,3 +1,17 @@
+<!-- START doctoc generated TOC please keep comment here to allow auto update -->
+<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
+**Table of Contents**  *generated with [DocToc](https://github.com/thlorenz/doctoc)*
+
+- [Contributing to Library](#contributing-to-library)
+  - [Repo Layout](#repo-layout)
+  - [Feature Requests](#feature-requests)
+  - [Testing](#testing)
+  - [Code Conventions](#code-conventions)
+- [Contributing Documentation](#contributing-documentation)
+  - [Document Contributors](#document-contributors)
+
+<!-- END doctoc generated TOC please keep comment here to allow auto update -->
+
 # Contributing to Library
 
 ## Repo Layout

README.md

@@ -26,6 +26,7 @@ A collaborative newsroom documentation site, powered by Google Docs.
   - [Doc parsing](#doc-parsing)
   - [Listing the drive](#listing-the-drive)
   - [Auth](#auth)
+  - [User Authentication](#user-authentication)
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
@@ -205,3 +206,17 @@ The tree and file metadata are repopulated into memory on an interval (currently
 ### Auth
 
 Authentication with the Google Drive v3 api is handled by the auth.js file, which exposes a single method `getAuth`. `getAuth` will either return an already instantiated authentication client or produce a fresh one. Calling `getAuth` multiple times will not produce a new authentication client if the credentials have expired; we should build this into the auth.js file later to automatically refresh the credentials on some sort of interval to prevent them from expiring.
+
+### User Authentication
+
+Library currently supports both Slack and Google Oauth methods. As Library sites are usually intended to be internal to a set of limited users, Oauth with your organization is strongly encouraged. To use Slack Oauth, specify your Oauth strategy in your `.env` file, like so:
+```
+# Slack needs to be capitalized as per the Passport.js slack oauth docs http://www.passportjs.org/packages/passport-slack-oauth2/
+OAUTH_STRATEGY=Slack
+``` 
+You will need to provide Slack credentials, which you can do by creating a Library Oauth app in your Slack workspace. After creating the app, save the app's `CLIENT_ID` and `CLIENT_SECRET` in your `.env` file:
+```
+SLACK_CLIENT_ID=1234567890abcdefg
+SLACK_CLIENT_SECRET=09876544321qwerty
+``` 
+You will need to add a callback URL to your Slack app to allow Slack to redirect back to your Library instance after the user is authenticated. 

custom/README.md

@@ -1,3 +1,18 @@
+<!-- START doctoc generated TOC please keep comment here to allow auto update -->
+<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
+**Table of Contents**  *generated with [DocToc](https://github.com/thlorenz/doctoc)*
+
+- [Customizing Library](#customizing-library)
+  - [The `custom` Directory](#the-custom-directory)
+  - [Styles](#styles)
+  - [Text, Language, and Branding](#text-language-and-branding)
+  - [Middleware](#middleware)
+  - [Cache Client](#cache-client)
+  - [Layouts](#layouts)
+  - [Authentication](#authentication)
+
+<!-- END doctoc generated TOC please keep comment here to allow auto update -->
+
 # Customizing Library
 
 ## The `custom` Directory

package.json

@@ -44,6 +44,7 @@
     "nock": "^13.0.3",
     "nodemon": "^2.0.4",
     "nyc": "^15.1.0",
+    "passport-slack-oauth2": "^1.0.2",
     "sinon": "^9.0.2",
     "supertest": "^4.0.2",
     "valid-url": "^1.0.9"

server/userAuth.js

@@ -4,22 +4,49 @@ const passport = require('passport')
 const session = require('express-session')
 const crypto = require('crypto')
 const GoogleStrategy = require('passport-google-oauth20')
+const SlackStrategy = require('passport-slack-oauth2').Strategy
 
 const log = require('./logger')
 const {stringTemplate: template} = require('./utils')
 
 const router = require('express-promise-router')()
 const domains = new Set(process.env.APPROVED_DOMAINS.split(/,\s?/g))
 
-const md5 = (data) => crypto.createHash('md5').update(data).digest('hex')
+const authStrategies = ['google', 'Slack']
+let authStrategy = process.env.OAUTH_STRATEGY
+
+const callbackURL = process.env.REDIRECT_URL || '/auth/redirect'
+if (!authStrategies.includes(authStrategy)) {
+  log.warn(`Invalid oauth strategy ${authStrategy} specific, defaulting to google auth`)
+  authStrategy = 'google'
+}
 
-passport.use(new GoogleStrategy.Strategy({
-  clientID: process.env.GOOGLE_CLIENT_ID,
-  clientSecret: process.env.GOOGLE_CLIENT_SECRET,
-  callbackURL: '/auth/redirect',
-  userProfileURL: 'https://www.googleapis.com/oauth2/v3/userinfo',
-  passReqToCallback: true
-}, (request, accessToken, refreshToken, profile, done) => done(null, profile)))
+const isSlackOauth = authStrategy === 'Slack'
+if (isSlackOauth) {
+  passport.use(new SlackStrategy({
+    clientID: process.env.SLACK_CLIENT_ID,
+    clientSecret: process.env.SLACK_CLIENT_SECRET,
+    skipUserProfile: false,
+    callbackURL,
+    scope: ['identity.basic', 'identity.email', 'identity.avatar', 'identity.team', 'identity.email']
+  },
+  (accessToken, refreshToken, profile, done) => {
+    // optionally persist user data into a database
+    done(null, profile)
+  }
+  ))
+} else {
+  // default to google auth
+  passport.use(new GoogleStrategy.Strategy({
+    clientID: process.env.GOOGLE_CLIENT_ID,
+    clientSecret: process.env.GOOGLE_CLIENT_SECRET,
+    callbackURL,
+    userProfileURL: 'https://www.googleapis.com/oauth2/v3/userinfo',
+    passReqToCallback: true
+  }, (request, accessToken, refreshToken, profile, done) => done(null, profile)))
+}
+
+const md5 = (data) => crypto.createHash('md5').update(data).digest('hex')
 
 router.use(session({
   secret: process.env.SESSION_SECRET,
@@ -35,27 +62,28 @@ router.use(passport.session())
 passport.serializeUser((user, done) => done(null, user))
 passport.deserializeUser((obj, done) => done(null, obj))
 
-router.get('/login', passport.authenticate('google', {
+const googleLoginOptions = {
   scope: [
     'https://www.googleapis.com/auth/userinfo.email',
     'https://www.googleapis.com/auth/userinfo.profile'
   ],
   prompt: 'select_account'
-}))
+}
+
+router.get('/login', passport.authenticate(authStrategy, isSlackOauth ? {} : googleLoginOptions))
 
 router.get('/logout', (req, res) => {
   req.logout()
   res.redirect('/')
 })
 
-router.get('/auth/redirect', passport.authenticate('google'), (req, res) => {
+router.get('/auth/redirect', passport.authenticate(authStrategy, {failureRedirect: '/login'}), (req, res) => {
   res.redirect(req.session.authRedirect || '/')
 })
 
 router.use((req, res, next) => {
   const isDev = process.env.NODE_ENV === 'development'
   const passportUser = (req.session.passport || {}).user || {}
-
   if (isDev || (req.isAuthenticated() && isAuthorized(passportUser))) {
     setUserInfo(req)
     return next()
@@ -91,10 +119,11 @@ function setUserInfo(req) {
     }
     return
   }
+  const email = isSlackOauth ? req.session.passport.user.email : req.session.passport.user.emails[0].value
   req.userInfo = req.userInfo ? req.userInfo : {
-    email: req.session.passport.user.emails[0].value,
     userId: req.session.passport.user.id,
-    analyticsUserId: md5(req.session.passport.user.id + 'library')
+    analyticsUserId: md5(req.session.passport.user.id + 'library'),
+    email
   }
 }
 

test/functional/userAuth.test.js

@@ -1,10 +1,9 @@
 'use strict'
-
 const request = require('supertest')
 const {assert} = require('chai')
 const sinon = require('sinon')
 const express = require('express')
-
+const log = require('../../server/logger')
 const app = require('../../server/index')
 
 /*
@@ -24,18 +23,78 @@ const regexUser = {
 }
 
 const specificUser = {
-  emails: [{ value: '[email protected]' }],
+  emails: [{value: '[email protected]'}],
   id: '12',
   userId: '12'
 }
 
 const unauthorizedUser = {
-  emails: [{ value: '[email protected]' }],
+  emails: [{value: '[email protected]'}],
   id: '13',
   userId: '13'
 }
 
 describe('Authentication', () => {
+  describe('.env specified oauth strategy', () => {
+    const sandbox = sinon.createSandbox()
+    beforeEach(() => {
+      jest.resetModules()
+      sandbox.stub(express.request, 'isAuthenticated').returns(false)
+    })
+
+    afterEach(() => {
+      sandbox.restore()
+    })
+
+    it('should warn if there is an invalid strategy specified', () => {
+      process.env.OAUTH_STRATEGY = 'fakjkjfdz'
+      const spy = sandbox.spy(log, 'warn')
+      const appWithInvalidOauth = require('../../server/index') // need to redo app setup
+      return request(appWithInvalidOauth)
+        .get('/login')
+        .expect(302)
+        .then((res) => {
+          assert.isTrue(spy.called, 'warn was not called')
+          assert.match(res.headers.location, /google/)
+        })
+    })
+
+    it('should default to google if there is no auth strategy specified', () => {
+      process.env.OAUTH_STRATEGY = undefined
+      const appWithoutOauth = require('../../server/index') // need to redo app setup
+      return request(appWithoutOauth)
+        .get('/login')
+        .expect(302)
+        .then((res) => {
+          assert.match(res.headers.location, /google/)
+        })
+    })
+
+    it('should use slack strategy if slack is specified', () => {
+      process.env.OAUTH_STRATEGY = 'Slack'
+      process.env.SLACK_CLIENT_ID = '1234567890'
+      process.env.SLACK_CLIENT_SECRET = '1234567890'
+      const appWithSlackAuth = require('../../server/index') // need to redo app setup
+      return request(appWithSlackAuth)
+        .get('/login')
+        .expect(302)
+        .then((res) => {
+          assert.match(res.headers.location, /slack/)
+        })
+    })
+
+    it('Slack has to be capitalized, sorry', () => {
+      process.env.OAUTH_STRATEGY = 'slack'
+      const appWithSlackAuth = require('../../server/index') // need to redo app setup
+      return request(appWithSlackAuth)
+        .get('/login')
+        .expect(302)
+        .then((res) => {
+          assert.match(res.headers.location, /google/)
+        })
+    })
+  })
+
   describe('when not logged in', () => {
     beforeAll(() => sinon.stub(express.request, 'isAuthenticated').returns(false))
     afterAll(() => sinon.restore())
@@ -63,7 +122,7 @@ describe('Authentication', () => {
 
   describe('when logging in with regex-approved domain', () => {
     beforeAll(() => {
-      sinon.stub(app.request, 'session').value({ passport: { user: regexUser } })
+      sinon.stub(app.request, 'session').value({passport: {user: regexUser}})
       sinon.stub(express.request, 'user').value(regexUser)
       sinon.stub(express.request, 'userInfo').value(regexUser)
     })
@@ -78,7 +137,7 @@ describe('Authentication', () => {
 
   describe('when logging in with specified email address', () => {
     beforeAll(() => {
-      sinon.stub(app.request, 'session').value({ passport: { user: specificUser } })
+      sinon.stub(app.request, 'session').value({passport: {user: specificUser}})
       sinon.stub(express.request, 'user').value(specificUser)
       sinon.stub(express.request, 'userInfo').value(specificUser)
     })