Merge pull request #831 from coralproject/secrets-fixes

Improvements and fixes to JWT configuration

config.js

@@ -35,11 +35,22 @@ const CONFIG = {
   // cleared when the user is logged out.
   JWT_CLEAR_COOKIE_LOGOUT: process.env.TALK_JWT_CLEAR_COOKIE_LOGOUT ? process.env.TALK_JWT_CLEAR_COOKIE_LOGOUT !== 'FALSE' : true,
 
+  // JWT_DISABLE_AUDIENCE when TRUE will disable the audience claim (aud) from tokens.
+  JWT_DISABLE_AUDIENCE: process.env.TALK_JWT_DISABLE_AUDIENCE === 'TRUE',
+
   // JWT_AUDIENCE is the value for the audience claim for the tokens that will be
   // verified when decoding. If `JWT_AUDIENCE` is not in the environment, then it
   // will default to `talk`.
   JWT_AUDIENCE: process.env.TALK_JWT_AUDIENCE || 'talk',
 
+  // JWT_DISABLE_ISSUER when TRUE will disable the issuer claim (iss) from tokens.
+  JWT_DISABLE_ISSUER: process.env.TALK_JWT_DISABLE_ISSUER === 'TRUE',
+
+  // JWT_USER_ID_CLAIM is the claim which stores the user's id. This may be a deep
+  // object delimited using dot notation. Example `user.id` would store it like:
+  // {user: {id}} on the claims object. (Default `sub`)
+  JWT_USER_ID_CLAIM: process.env.TALK_JWT_USER_ID_CLAIM || 'sub',
+
   // JWT_ISSUER is the value for the issuer for the tokens that will be verified
   // when decoding. If `JWT_ISSUER` is not in the environment, then it will try
   // `TALK_ROOT_URL`, otherwise, it will be undefined.
@@ -130,20 +141,28 @@ if (process.env.NODE_ENV === 'test' && !CONFIG.ROOT_URL) {
 
 if (CONFIG.JWT_SECRETS) {
   CONFIG.JWT_SECRETS = JSON.parse(CONFIG.JWT_SECRETS);
-}
-
-if (process.env.NODE_ENV === 'test' && !CONFIG.JWT_SECRET) {
-  CONFIG.JWT_SECRET = 'keyboard cat';
 } else if (!CONFIG.JWT_SECRET) {
-  throw new Error(
-    'TALK_JWT_SECRET must be provided in the environment to sign/verify tokens'
-  );
+  if (process.env.NODE_ENV === 'test') {
+    if (!CONFIG.JWT_ALG.startsWith('HS')) {
+      throw new Error('Providing a asymmetric signing/verfying algorithm without a corresponding secret is not permitted');
+    }
+
+    CONFIG.JWT_SECRET = 'keyboard cat';
+  } else {
+    throw new Error(
+      'TALK_JWT_SECRET must be provided in the environment to sign/verify tokens'
+    );
+  }
 }
 
-// If this is not employing a HMAC based signing method, then we need to turn
-// the secret into a buffer.
-if (!CONFIG.JWT_ALG.startsWith('HS')) {
-  CONFIG.JWT_SECRET = Buffer.from(CONFIG.JWT_SECRET);
+// Disable the audience claim if requested.
+if (CONFIG.JWT_DISABLE_AUDIENCE) {
+  CONFIG.JWT_AUDIENCE = undefined;
+}
+
+// Disable the issuer claim if requested.
+if (CONFIG.JWT_DISABLE_ISSUER) {
+  CONFIG.JWT_ISSUER = undefined;
 }
 
 //------------------------------------------------------------------------------

docs/_docs/02-01-configuration.md

@@ -77,15 +77,43 @@ The following are configuration shared with every type of secret used.
   tokens. (Default `process.env.TALK_ROOT_URL`)
 - `TALK_JWT_AUDIENCE` (_optional_) - the audience (`aud`) claim for login JWT
   tokens. (Default `talk`)
+
+**You must also specify secrets as either the `TALK_JWT_SECRET` or the `TALK_JWT_SECRETS`
+variable. Refer to the [Secrets Documentation]({{ "/docs/running/secrets/" | absolute_url }})
+on the contents of those variables.**
+
+#### Advanced
+
+These are advanced settings for fine tuning the auth integration, and
+is not needed in most situations.
+
 - `TALK_JWT_COOKIE_NAME` (_optional_) - the name of the cookie to extract the
   JWT from (Default `authorization`)
 - `TALK_JWT_CLEAR_COOKIE_LOGOUT` (_optional_) - when `FALSE`, Talk will not
   clear the cookie with name `TALK_JWT_COOKIE_NAME` when logging out (Default
   `TRUE`)
+- `TALK_JWT_DISABLE_AUDIENCE` (_optional_) - when `TRUE`, Talk will not verify or sign JWT's
+  with an audience (`aud`) claim, even if the `TALK_JWT_AUDIENCE` config is set. (Default `FALSE`)
+- `TALK_JWT_DISABLE_ISSUER` (_optional_) - when `TRUE`, Talk will not verify or sign JWT's
+  with an issuer (`iss`) claim, even if the `TALK_JWT_ISSUER` config is set. (Default `FALSE`)
+- `TALK_JWT_USER_ID_CLAIM` (_optional_) - specify the claim using dot notation for where the
+  user id should be stored/read to/from. Example `user.id` would store it like: `{user: {id}}`
+  on the claims object. (Default `sub`)
 
-**You must also specify secrets as either the `TALK_JWT_SECRET` or the `TALK_JWT_SECRETS`
-variable. Refer to the [Secrets Documentation]({{ "/docs/running/secrets/" | absolute_url }})
-on the contents of those variables.**
+When integrating with an external authentication system, the following JWT claims
+will be used:
+
+```js
+{
+  "jti": "<the unique token identifier>",    // *required* unique id used for blacklisting
+  "aud": TALK_JWT_AUDIENCE,                  // *optional* if TALK_JWT_DISABLE_AUDIENCE === 'TRUE', *required* otherwise
+  "iss": TALK_JWT_ISSUER,                    // *optional* if TALK_JWT_DISABLE_ISSUER === 'TRUE', *required* otherwise
+
+  [TALK_JWT_USER_ID_CLAIM]: "<the user id>", // *required* the id of the user
+  // Note, if TALK_JWT_USER_ID_CLAIM contains '.', it will be used to deliniate an object, for example
+  // `user.id` would store it like: `{user: {id}}`
+}
+```
 
 ### Email
 

docs/_docs/02-02-secrets.md

@@ -60,6 +60,11 @@ must have their newlines replaced with `\\n`, this is to ensure that the
 newlines are preserved after JSON decoding. Not doing so will result in parsing
 errors.
 
+To assist with this process, we have developed a tool that can generate new
+certificates that match our required format: [coralcert](https://github.com/coralproject/coralcert).
+This tool can generate RSA and ECDSA certificates, check it's [README](https://github.com/coralproject/coralcert)
+for more details.
+
 ## Authentication Types
 
 Talk also supports two methods of providing authenticationd details.

secrets.js

@@ -22,6 +22,10 @@ if (JWT_SECRETS) {
       throw new Error('when multiple keys are specified, kid\'s must be specified');
     }
 
+    if (typeof secret.kid !== 'string' || secret.kid.length === 0) {
+      throw new Error('kid must be a unique string');
+    }
+
     // HMAC secrets do not have public/private keys.
     if (JWT_ALG.startsWith('HS')) {
       return new jwt.SharedSecret(secret, JWT_ALG);
@@ -34,7 +38,7 @@ if (JWT_SECRETS) {
     return new jwt.AsymmetricSecret(secret, JWT_ALG);
   }));
 
-  debug(`loaded ${JWT_SECRET.length} ${JWT_ALG.startsWith('HS') ? 'shared' : 'asymmetric'} secrets`);
+  debug(`loaded ${JWT_SECRETS.length} ${JWT_ALG.startsWith('HS') ? 'shared' : 'asymmetric'} secrets`);
 } else if (JWT_SECRET) {
   if (JWT_ALG.startsWith('HS')) {
     module.exports.jwt = new jwt.SharedSecret({

services/jwt.js

@@ -1,4 +1,5 @@
 const jwt = require('jsonwebtoken');
+const uniq = require('lodash/uniq');
 
 /**
  * MultiSecret will take many secrets and provide a unified interface for
@@ -6,6 +7,12 @@ const jwt = require('jsonwebtoken');
  */
 class MultiSecret {
   constructor(secrets) {
+    this.kids = secrets.map(({kid}) => kid);
+
+    if (uniq(this.kids).length !== secrets.length) {
+      throw new Error('Duplicate kid\'s cannot be used to construct a MultiSecret');
+    }
+
     this.secrets = secrets;
   }
 
@@ -86,7 +93,11 @@ class Secret {
 /**
  * SharedSecret is the HMAC based secret that's used for signing/verifying.
  */
-function SharedSecret({kid = undefined, secret}, algorithm) {
+function SharedSecret({kid = undefined, secret = null}, algorithm) {
+  if (secret === null || secret.length === 0) {
+    throw new Error('Secret cannot have a zero length');
+  }
+
   return new Secret({
     kid,
     signingKey: secret,
@@ -101,7 +112,7 @@ function SharedSecret({kid = undefined, secret}, algorithm) {
  */
 function AsymmetricSecret({kid = undefined, private: privateKey, public: publicKey}, algorithm) {
   publicKey = Buffer.from(publicKey.replace(/\\n/g, '\n'));
-  privateKey = privateKey ? Buffer.from(privateKey.replace(/\\n/g, '\n')) : null;
+  privateKey = privateKey && privateKey.length > 0 ? Buffer.from(privateKey.replace(/\\n/g, '\n')) : null;
 
   return new Secret({
     kid,

services/passport.js

@@ -1,4 +1,5 @@
 const passport = require('passport');
+const {set, get} = require('lodash');
 const UsersService = require('./users');
 const SettingsService = require('./settings');
 const TokensService = require('./tokens');
@@ -23,21 +24,26 @@ const {
   RECAPTCHA_SECRET,
   RECAPTCHA_ENABLED,
   JWT_COOKIE_NAME,
-  JWT_CLEAR_COOKIE_LOGOUT
+  JWT_CLEAR_COOKIE_LOGOUT,
+  JWT_USER_ID_CLAIM,
 } = require('../config');
 
 const {
-  jwt: JWT_SECRET
+  jwt
 } = require('../secrets');
 
 // GenerateToken will sign a token to include all the authorization information
 // needed for the front end.
 const GenerateToken = (user) => {
-  return JWT_SECRET.sign({}, {
+  const claims = {};
+
+  // Set the user id.
+  set(claims, JWT_USER_ID_CLAIM, user.id);
+
+  return jwt.sign(claims, {
     jwtid: uuid.v4(),
     expiresIn: JWT_EXPIRY,
     issuer: JWT_ISSUER,
-    subject: user.id,
     audience: JWT_AUDIENCE,
     algorithm: JWT_ALG
   });
@@ -191,11 +197,13 @@ const CheckBlacklisted = async (jwt) => {
 
   // Check to see if this is a PAT.
   if (jwt.pat) {
-    return TokensService.validate(jwt.sub, jwt.jti);
+    return TokensService.validate(get(jwt, JWT_USER_ID_CLAIM), jwt.jti);
   }
 
   // It wasn't a PAT! Check to see if it is valid anyways.
-  return checkGeneralTokenBlacklist(jwt);
+  await checkGeneralTokenBlacklist(jwt);
+
+  return null;
 };
 
 const JwtStrategy = require('passport-jwt').Strategy;
@@ -214,7 +222,7 @@ let cookieExtractor = function(req) {
 // Override the JwtVerifier method on the JwtStrategy so we can pack the
 // original token into the payload.
 JwtStrategy.JwtVerifier = (token, secretOrKey, options, callback) => {
-  return JWT_SECRET.verify(token, options, (err, jwt) => {
+  return jwt.verify(token, options, (err, jwt) => {
     if (err) {
       return callback(err);
     }
@@ -236,7 +244,7 @@ passport.use(new JwtStrategy({
 
   // Use the secret passed in which is loaded from the environment. This can be
   // a certificate (loaded) or a HMAC key.
-  secretOrKey: JWT_SECRET,
+  secretOrKey: jwt,
 
   // Verify the issuer.
   issuer: JWT_ISSUER,
@@ -257,11 +265,14 @@ passport.use(new JwtStrategy({
   try {
 
     // Check to see if the token has been revoked
-    await CheckBlacklisted(jwt);
+    let user = await CheckBlacklisted(jwt);
 
-    // Try to get the user from the database or crack it from the token and
-    // plugin integrations.
-    let user = await UsersService.findOrCreateByIDToken(jwt.sub, {token, jwt});
+    if (user === null) {
+
+      // Try to get the user from the database or crack it from the token and
+      // plugin integrations.
+      user = await UsersService.findOrCreateByIDToken(get(jwt, JWT_USER_ID_CLAIM), {token, jwt});
+    }
 
     // Attach the JWT to the request.
     req.jwt = jwt;

services/tokens.js

@@ -1,10 +1,12 @@
 const errors = require('../errors');
 const UserModel = require('../models/user');
 const uuid = require('uuid');
+const {set} = require('lodash');
 
 const {
   JWT_ISSUER,
-  JWT_AUDIENCE
+  JWT_AUDIENCE,
+  JWT_USER_ID_CLAIM,
 } = require('../config');
 
 const {
@@ -30,10 +32,11 @@ module.exports = class TokenService {
       jti: uuid.v4(),
       iss: JWT_ISSUER,
       aud: JWT_AUDIENCE,
-      sub: userID,
       pat: true
     };
 
+    set(payload, JWT_USER_ID_CLAIM, userID);
+
     // Sign the payload.
     const jwt = JWT_SECRET.sign(payload, {});
 
@@ -93,7 +96,7 @@ module.exports = class TokenService {
     // Find the user.
     let user = await UserModel.findOne({
       id: userID
-    }).select('tokens');
+    });
     if (!user || !user.tokens) {
       throw new errors.ErrAuthentication('user does not exist');
     }
@@ -108,6 +111,8 @@ module.exports = class TokenService {
     if (!token.active) {
       throw new errors.ErrAuthentication('token is not active');
     }
+
+    return user;
   }
 
   /**