Federated Accounts & CSRF (#293)

.github/workflows/alpha.yml

@@ -2,7 +2,7 @@ name: 'Alpha Build'
 
 on:
   push:
-    branches: [v460/speed-up-migrate]
+    branches: [v470/slack-bot]
 
 jobs:
   build:

README.md

@@ -147,6 +147,8 @@ When using the Docker deployment, your database runs from a container. But if yo
 
 ### Version 4.7.0 (not released)
 
+- CSRF protection
+- Account federation
 - Add the ability for anonymous users to delete the boards they created under certain conditions ([#229](https://github.com/antoinejaussoin/retro-board/issues/229)).
 
 ### Version 4.6.1

backend/package.json

@@ -19,6 +19,7 @@
     "@types/bcryptjs": "^2.4.2",
     "@types/connect-redis": "0.0.17",
     "@types/crypto-js": "^4.0.2",
+    "@types/csurf": "^1.11.2",
     "@types/dotenv": "^8.2.0",
     "@types/express": "^4.17.13",
     "@types/express-mung": "^0.5.2",
@@ -49,6 +50,7 @@
     "connect-redis": "6.0.0",
     "cross-env": "7.0.3",
     "crypto-js": "^4.1.1",
+    "csurf": "^1.11.0",
     "date-fns": "^2.23.0",
     "dotenv": "^10.0.0",
     "eslint": "^7.32.0",

backend/src/admin/router.ts

@@ -1,14 +1,20 @@
 import express from 'express';
-import { getAllPasswordUsers, updateUser, getUser } from '../db/actions/users';
+import {
+  getAllPasswordUsers,
+  getPasswordIdentityByUserId,
+  updateIdentity,
+} from '../db/actions/users';
 import config from '../config';
 import { isLicenced } from '../security/is-licenced';
 import {
   AdminChangePasswordPayload,
   SelfHostingPayload,
 } from '@retrospected/common';
-import { getUserFromRequest, hashPassword } from '../utils';
+import { getIdentityFromRequest, hashPassword } from '../utils';
+import csurf from 'csurf';
 
 const router = express.Router();
+const csrfProtection = csurf();
 
 router.get('/self-hosting', async (_, res) => {
   const licenced = await isLicenced();
@@ -29,24 +35,24 @@ router.get('/self-hosting', async (_, res) => {
 });
 
 router.get('/users', async (req, res) => {
-  const authUser = await getUserFromRequest(req);
-  if (!authUser || authUser.email !== config.SELF_HOSTED_ADMIN) {
+  const identity = await getIdentityFromRequest(req);
+  if (!identity || identity.user.email !== config.SELF_HOSTED_ADMIN) {
     return res.status(403).send('You are not allowed to do this');
   }
   const users = await getAllPasswordUsers();
   res.send(users.map((u) => u.toJson()));
 });
 
-router.patch('/user', async (req, res) => {
-  const authUser = await getUserFromRequest(req);
-  if (!authUser || authUser.email !== config.SELF_HOSTED_ADMIN) {
+router.patch('/user', csrfProtection, async (req, res) => {
+  const authIdentity = await getIdentityFromRequest(req);
+  if (!authIdentity || authIdentity.user.email !== config.SELF_HOSTED_ADMIN) {
     return res.status(403).send('You are not allowed to do this');
   }
   const payload = req.body as AdminChangePasswordPayload;
-  const user = await getUser(payload.userId);
-  if (user) {
+  const identity = await getPasswordIdentityByUserId(payload.userId);
+  if (identity) {
     const hashedPassword = await hashPassword(payload.password);
-    const updatedUser = await updateUser(user.id, {
+    const updatedUser = await updateIdentity(identity.id, {
       password: hashedPassword,
     });
     if (updatedUser) {

backend/src/auth/logins/anonymous-user.ts

@@ -1,36 +1,10 @@
-import { UserEntity } from '../../db/entities';
-import { v4 } from 'uuid';
-import {
-  getUserByUsername,
-  getOrSaveUser,
-  updateUserPassword,
-} from '../../db/actions/users';
-import { hashPassword } from '../../utils';
-import { compare } from 'bcryptjs';
+import { UserIdentityEntity } from '../../db/entities';
+import { registerAnonymousUser } from '../../db/actions/users';
 
 export default async function loginAnonymous(
   username: string,
   password: string
-): Promise<UserEntity | null> {
-  const actualUsername = username.split('^')[0];
-  const existingUser = await getUserByUsername(username);
-  if (!existingUser) {
-    const hashedPassword = await hashPassword(password);
-    const user = new UserEntity(v4(), actualUsername, hashedPassword);
-    user.username = username;
-    user.language = 'en';
-
-    const dbUser = await getOrSaveUser(user);
-    return dbUser;
-  }
-
-  if (!existingUser.password) {
-    const hashedPassword = await hashPassword(password);
-    const dbUser = await updateUserPassword(existingUser.id, hashedPassword);
-    return dbUser;
-  }
-
-  const isPasswordCorrect = await compare(password, existingUser.password);
-
-  return isPasswordCorrect ? existingUser : null;
+): Promise<UserIdentityEntity | null> {
+  const identity = registerAnonymousUser(username, password);
+  return identity;
 }

backend/src/auth/logins/password-user.ts

@@ -1,12 +1,12 @@
 import { compare } from 'bcryptjs';
-import { UserEntity } from '../../db/entities';
-import { getUserByUsername } from '../../db/actions/users';
+import { UserIdentityEntity } from '../../db/entities';
+import { getIdentityByUsername } from '../../db/actions/users';
 
 export default async function loginUser(
   username: string,
   password: string
-): Promise<UserEntity | null> {
-  const user = await getUserByUsername(username);
+): Promise<UserIdentityEntity | null> {
+  const user = await getIdentityByUsername('password', username);
   if (!user || user.password === null) {
     return null;
   }

backend/src/auth/passport.ts

@@ -15,12 +15,10 @@ import {
   SLACK_CONFIG,
   OKTA_CONFIG,
 } from './config';
-import { v4 } from 'uuid';
 import { AccountType } from '@retrospected/common';
 import chalk from 'chalk';
 import loginUser from './logins/password-user';
 import loginAnonymous from './logins/anonymous-user';
-import UserEntity from '../db/entities/User';
 import {
   BaseProfile,
   TwitterProfile,
@@ -30,14 +28,17 @@ import {
   SlackProfile,
   OktaProfile,
 } from './types';
-import { getOrSaveUser } from '../db/actions/users';
+import { registerUser, UserRegistration } from '../db/actions/users';
+import { serialiseIds, UserIds, deserialiseIds } from '../utils';
 
 export default () => {
-  passport.serializeUser((user: string, cb) => {
-    cb(null, user);
+  passport.serializeUser<string>((user, cb) => {
+    // Typings are wrong
+    const actualUser = user as unknown as UserIds;
+    cb(null, serialiseIds(actualUser));
   });
-  passport.deserializeUser(async (userId: string, cb) => {
-    cb(null, userId);
+  passport.deserializeUser<string>(async (userId: string, cb) => {
+    cb(null, deserialiseIds(userId) as unknown as string);
   });
 
   function callback<TProfile, TCallback>(type: AccountType) {
@@ -48,7 +49,7 @@ export default () => {
       cb: TCallback
     ) => {
       const profile = anyProfile as unknown as BaseProfile;
-      let user: UserEntity;
+      let user: UserRegistration | null;
       switch (type) {
         case 'google':
           user = buildFromGoogleProfile(profile as GoogleProfile);
@@ -72,82 +73,120 @@ export default () => {
           throw new Error('Unknown provider: ' + type);
       }
 
-      const dbUser = await getOrSaveUser(user);
       const callback = cb as unknown as (
         error: string | null,
-        user: string
+        user: UserIds | null
       ) => void;
-      callback(null, dbUser.id);
+
+      if (!user) {
+        callback('Cannot build a user profile', null);
+        return;
+      }
+
+      const dbIdentity = await registerUser(user);
+
+      callback(null, dbIdentity.toIds());
     };
   }
 
-  function buildFromTwitterProfile(profile: TwitterProfile): UserEntity {
-    const user: UserEntity = new UserEntity(v4(), profile.displayName);
-    user.accountType = 'twitter';
-    user.language = 'en';
-    user.photo = profile.photos?.length ? profile.photos[0].value : null;
-    user.username = profile.username;
-    user.email = profile.emails.length ? profile.emails[0].value : null;
-    return user;
+  function buildFromTwitterProfile(
+    profile: TwitterProfile
+  ): UserRegistration | null {
+    const email = profile.emails.length ? profile.emails[0].value : null;
+    if (!email) {
+      return null;
+    }
+    return {
+      name: profile.displayName,
+      type: 'twitter',
+      language: 'en',
+      photo: profile.photos?.length ? profile.photos[0].value : undefined,
+      username: profile.username,
+      email,
+    };
   }
 
-  function buildFromGitHubProfile(profile: GitHubProfile): UserEntity {
+  function buildFromGitHubProfile(
+    profile: GitHubProfile
+  ): UserRegistration | null {
     const displayName =
       profile.displayName ||
       profile.username ||
       (profile.emails.length ? profile.emails[0].value : '');
-
-    const user: UserEntity = new UserEntity(v4(), displayName);
     const email =
       profile.emails && profile.emails.length ? profile.emails[0] : null;
-    user.accountType = 'github';
-    user.language = 'en';
-    user.photo = profile.photos?.length ? profile.photos[0].value : null;
-    user.username = profile.username;
-    user.email = email ? email.value : null;
-    return user;
+
+    if (!email) {
+      return null;
+    }
+
+    return {
+      name: displayName,
+      type: 'github',
+      language: 'en',
+      photo: profile.photos?.length ? profile.photos[0].value : undefined,
+      username: profile.username,
+      email: email.value,
+    };
   }
 
-  function buildFromGoogleProfile(profile: GoogleProfile): UserEntity {
-    const user: UserEntity = new UserEntity(v4(), profile.displayName);
+  function buildFromGoogleProfile(
+    profile: GoogleProfile
+  ): UserRegistration | null {
     const email = profile.emails.length ? profile.emails[0].value : null;
-    user.accountType = 'google';
-    user.language = 'en';
-    user.photo = profile.photos?.length ? profile.photos[0].value : null;
-    user.username = email;
-    user.email = email;
-    return user;
+    if (!email) {
+      return null;
+    }
+    return {
+      name: profile.displayName,
+      type: 'google',
+      language: 'en',
+      photo: profile.photos?.length ? profile.photos[0].value : undefined,
+      username: email,
+      email,
+    };
   }
 
-  function buildFromSlackProfile(profile: SlackProfile): UserEntity {
-    const user: UserEntity = new UserEntity(v4(), profile.displayName);
+  function buildFromSlackProfile(
+    profile: SlackProfile
+  ): UserRegistration | null {
     const email = profile.user.email;
-    user.accountType = 'slack';
-    user.language = 'en';
-    user.photo = profile.user.image_192;
-    user.username = email;
-    user.email = email;
-    return user;
+
+    return {
+      name: profile.displayName,
+      type: 'slack',
+      language: 'en',
+      photo: profile.user.image_192,
+      username: email,
+      email,
+      slackUserId: profile.id,
+      slackTeamId: profile.team ? profile.team.id : undefined,
+    };
   }
 
-  function buildFromMicrosoftProfile(profile: MicrosoftProfile): UserEntity {
-    const user: UserEntity = new UserEntity(v4(), profile.displayName);
+  function buildFromMicrosoftProfile(
+    profile: MicrosoftProfile
+  ): UserRegistration | null {
     const email = profile.emails[0].value;
-    user.accountType = 'microsoft';
-    user.language = 'en';
-    user.username = email;
-    user.email = email;
-    return user;
+    return {
+      name: profile.displayName,
+      type: 'microsoft',
+      language: 'en',
+      username: email,
+      email,
+    };
   }
 
-  function buildFromOktaProfile(profile: OktaProfile): UserEntity {
-    const user: UserEntity = new UserEntity(v4(), profile.fullName);
+  function buildFromOktaProfile(profile: OktaProfile): UserRegistration | null {
     const email = profile.email;
-    user.accountType = 'okta';
-    user.language = 'en';
-    user.username = email;
-    user.email = email;
-    return user;
+
+    return {
+      name: profile.fullName,
+      type: 'okta',
+      language: 'en',
+      username: email,
+      email,
+    };
   }
 
   // Adding each OAuth provider's strategy to passport
@@ -191,7 +230,7 @@ export default () => {
         password: string,
         done: (
           error: string | null,
-          user?: string,
+          user?: UserIds,
           options?: IVerifyOptions
         ) => void
       ) => {
@@ -203,12 +242,18 @@ export default () => {
           const actualUsername = username
             .replace('ANONUSER__', '')
             .replace('__ANONUSER', '');
-          const user = await loginAnonymous(actualUsername, password);
-          done(!user ? 'Anonymous account not valid' : null, user?.id);
+          const identity = await loginAnonymous(actualUsername, password);
+          done(
+            !identity ? 'Anonymous account not valid' : null,
+            identity ? identity.toIds() : undefined
+          );
         } else {
           // Regular account login
-          const user = await loginUser(username, password);
-          done(!user ? 'User cannot log in' : null, user?.id);
+          const identity = await loginUser(username, password);
+          done(
+            !identity ? 'User cannot log in' : null,
+            identity ? identity.toIds() : undefined
+          );
         }
       }
     )