From 1b6a9869867e6d7c5b5d4aa0e812a1029e85249a Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Wed, 4 Nov 2020 19:54:04 -0800 Subject: [PATCH] chore: Refactor authentication pass between subdomains (#1619) * fix: Use get request instead of cookie to transfer token between domains * Add domain to database Add redirects to team domain when present * 30s -> 1m * fix: Avoid redirect loop if subdomain and domain set * fix: Create a transfer specific token to prevent replay requests * refactor: Move isCustomDomain out of shared as it won't work on the client --- app/components/Authenticated.js | 11 ++++-- app/models/Team.js | 1 + server/api/auth.js | 21 ++++++++++- server/auth/index.js | 25 ++++++------- server/middlewares/authentication.js | 20 ++++++----- .../20201103050534-custom-domains.js | 15 ++++++++ server/models/Team.js | 8 +++++ server/models/User.js | 36 ++++++++++++++++--- server/presenters/team.js | 1 + server/utils/domains.js | 10 +++++- server/utils/jwt.js | 20 +++++++++++ 11 files changed, 136 insertions(+), 32 deletions(-) create mode 100644 server/migrations/20201103050534-custom-domains.js diff --git a/app/components/Authenticated.js b/app/components/Authenticated.js index 4b0193f8..e585eb7a 100644 --- a/app/components/Authenticated.js +++ b/app/components/Authenticated.js @@ -21,9 +21,14 @@ const Authenticated = observer(({ auth, children }: Props) => { return ; } - // If we're authenticated but viewing a subdomain that doesn't match the - // currently authenticated team then kick the user to the teams subdomain. - if ( + // If we're authenticated but viewing a domain that doesn't match the + // current team then kick the user to the teams correct domain. + if (team.domain) { + if (team.domain !== hostname) { + window.location.href = `${team.url}${window.location.pathname}`; + return ; + } + } else if ( env.SUBDOMAINS_ENABLED && team.subdomain && isCustomSubdomain(hostname) && diff --git a/app/models/Team.js b/app/models/Team.js index 0c4a067f..73b5647b 100644 --- a/app/models/Team.js +++ b/app/models/Team.js @@ -12,6 +12,7 @@ class Team extends BaseModel { documentEmbeds: boolean; guestSignin: boolean; subdomain: ?string; + domain: ?string; url: string; @computed diff --git a/server/api/auth.js b/server/api/auth.js index 8834c85d..53a61e57 100644 --- a/server/api/auth.js +++ b/server/api/auth.js @@ -6,6 +6,7 @@ import { signin } from "../../shared/utils/routeHelpers"; import auth from "../middlewares/authentication"; import { Team } from "../models"; import { presentUser, presentTeam, presentPolicies } from "../presenters"; +import { isCustomDomain } from "../utils/domains"; const router = new Router(); @@ -68,11 +69,29 @@ router.post("auth.config", async (ctx) => { } } + if (isCustomDomain(ctx.request.hostname)) { + const team = await Team.findOne({ + where: { domain: ctx.request.hostname }, + }); + + if (team) { + ctx.body = { + data: { + name: team.name, + hostname: ctx.request.hostname, + services: filterServices(team), + }, + }; + return; + } + } + // If subdomain signin page then we return minimal team details to allow // for a custom screen showing only relevant signin options for that team. if ( process.env.SUBDOMAINS_ENABLED === "true" && - isCustomSubdomain(ctx.request.hostname) + isCustomSubdomain(ctx.request.hostname) && + !isCustomDomain(ctx.request.hostname) ) { const domain = parseDomain(ctx.request.hostname); const subdomain = domain ? domain.subdomain : undefined; diff --git a/server/auth/index.js b/server/auth/index.js index 11095f49..deb716a3 100644 --- a/server/auth/index.js +++ b/server/auth/index.js @@ -3,10 +3,10 @@ import addMonths from "date-fns/add_months"; import Koa from "koa"; import bodyParser from "koa-body"; import Router from "koa-router"; +import { AuthenticationError } from "../errors"; import auth from "../middlewares/authentication"; import validation from "../middlewares/validation"; import { Team } from "../models"; -import { getCookieDomain } from "../utils/domains"; import email from "./email"; import google from "./google"; @@ -21,23 +21,20 @@ router.use("/", email.routes()); router.get("/redirect", auth(), async (ctx) => { const user = ctx.state.user; - - // transfer access token cookie from root to subdomain - const rootToken = ctx.cookies.get("accessToken"); const jwtToken = user.getJwtToken(); - if (rootToken === jwtToken) { - ctx.cookies.set("accessToken", undefined, { - httpOnly: true, - domain: getCookieDomain(ctx.request.hostname), - }); - - ctx.cookies.set("accessToken", jwtToken, { - httpOnly: false, - expires: addMonths(new Date(), 3), - }); + if (jwtToken === ctx.params.token) { + throw new AuthenticationError("Cannot extend token"); } + // ensure that the lastActiveAt on user is updated to prevent replay requests + await user.updateActiveAt(ctx.request.ip, true); + + ctx.cookies.set("accessToken", jwtToken, { + httpOnly: false, + expires: addMonths(new Date(), 3), + }); + const team = await Team.findByPk(user.teamId); ctx.redirect(`${team.url}/home`); }); diff --git a/server/middlewares/authentication.js b/server/middlewares/authentication.js index b3fc3635..9ed0899b 100644 --- a/server/middlewares/authentication.js +++ b/server/middlewares/authentication.js @@ -1,5 +1,4 @@ // @flow -import addMinutes from "date-fns/add_minutes"; import addMonths from "date-fns/add_months"; import JWT from "jsonwebtoken"; import { AuthenticationError, UserSuspendedError } from "../errors"; @@ -62,7 +61,15 @@ export default function auth(options?: { required?: boolean } = {}) { throw new AuthenticationError("Invalid API key"); } - user = await User.findByPk(apiKey.userId); + user = await User.findByPk(apiKey.userId, { + include: [ + { + model: Team, + as: "team", + required: true, + }, + ], + }); if (!user) { throw new AuthenticationError("Invalid API key"); } @@ -134,12 +141,9 @@ export default function auth(options?: { required?: boolean } = {}) { domain, }); - ctx.cookies.set("accessToken", user.getJwtToken(), { - httpOnly: true, - expires: addMinutes(new Date(), 1), - domain, - }); - ctx.redirect(`${team.url}/auth/redirect`); + ctx.redirect( + `${team.url}/auth/redirect?token=${user.getTransferToken()}` + ); } else { ctx.cookies.set("accessToken", user.getJwtToken(), { httpOnly: false, diff --git a/server/migrations/20201103050534-custom-domains.js b/server/migrations/20201103050534-custom-domains.js new file mode 100644 index 00000000..2f564126 --- /dev/null +++ b/server/migrations/20201103050534-custom-domains.js @@ -0,0 +1,15 @@ +'use strict'; + +module.exports = { + up: async (queryInterface, Sequelize) => { + await queryInterface.addColumn('teams', 'domain', { + type: Sequelize.STRING, + allowNull: true, + unique: true + }); + }, + + down: async (queryInterface, Sequelize) => { + await queryInterface.removeColumn('teams', 'domain'); + } +}; \ No newline at end of file diff --git a/server/models/Team.js b/server/models/Team.js index 695935d8..521d0bd3 100644 --- a/server/models/Team.js +++ b/server/models/Team.js @@ -47,6 +47,11 @@ const Team = sequelize.define( }, unique: true, }, + domain: { + type: DataTypes.STRING, + allowNull: true, + unique: true, + }, slackId: { type: DataTypes.STRING, allowNull: true }, googleId: { type: DataTypes.STRING, allowNull: true }, avatarUrl: { type: DataTypes.STRING, allowNull: true }, @@ -66,6 +71,9 @@ const Team = sequelize.define( { getterMethods: { url() { + if (this.domain) { + return `https://${this.domain}`; + } if (!this.subdomain || process.env.SUBDOMAINS_ENABLED !== "true") { return process.env.URL; } diff --git a/server/models/User.js b/server/models/User.js index 701d51e7..0f552189 100644 --- a/server/models/User.js +++ b/server/models/User.js @@ -1,5 +1,6 @@ // @flow import crypto from "crypto"; +import addMinutes from "date-fns/add_minutes"; import subMinutes from "date-fns/sub_minutes"; import JWT from "jsonwebtoken"; import uuid from "uuid"; @@ -91,12 +92,12 @@ User.prototype.collectionIds = async function (options = {}) { .map((c) => c.id); }; -User.prototype.updateActiveAt = function (ip) { +User.prototype.updateActiveAt = function (ip, force = false) { const fiveMinutesAgo = subMinutes(new Date(), 5); // ensure this is updated only every few minutes otherwise // we'll be constantly writing to the DB as API requests happen - if (this.lastActiveAt < fiveMinutesAgo) { + if (this.lastActiveAt < fiveMinutesAgo || force) { this.lastActiveAt = new Date(); this.lastActiveIp = ip; return this.save({ hooks: false }); @@ -109,17 +110,42 @@ User.prototype.updateSignedIn = function (ip) { return this.save({ hooks: false }); }; -User.prototype.getJwtToken = function () { - return JWT.sign({ id: this.id }, this.jwtSecret); +// Returns a session token that is used to make API requests and is stored +// in the client browser cookies to remain logged in. +User.prototype.getJwtToken = function (expiresAt?: Date) { + return JWT.sign( + { + id: this.id, + expiresAt: expiresAt ? expiresAt.toISOString() : undefined, + type: "session", + }, + this.jwtSecret + ); }; +// Returns a temporary token that is only used for transferring a session +// between subdomains or domains. It has a short expiry and can only be used once +User.prototype.getTransferToken = function () { + return JWT.sign( + { + id: this.id, + createdAt: new Date().toISOString(), + expiresAt: addMinutes(new Date(), 1).toISOString(), + type: "transfer", + }, + this.jwtSecret + ); +}; + +// Returns a temporary token that is only used for logging in from an email +// It can only be used to sign in once and has a medium length expiry User.prototype.getEmailSigninToken = function () { if (this.service && this.service !== "email") { throw new Error("Cannot generate email signin token for OAuth user"); } return JWT.sign( - { id: this.id, createdAt: new Date().toISOString() }, + { id: this.id, createdAt: new Date().toISOString(), type: "email-signin" }, this.jwtSecret ); }; diff --git a/server/presenters/team.js b/server/presenters/team.js index c0b2b369..bc75d9d8 100644 --- a/server/presenters/team.js +++ b/server/presenters/team.js @@ -12,6 +12,7 @@ export default function present(team: Team) { documentEmbeds: team.documentEmbeds, guestSignin: team.guestSignin, subdomain: team.subdomain, + domain: team.domain, url: team.url, }; } diff --git a/server/utils/domains.js b/server/utils/domains.js index 7a2add99..aed2f12b 100644 --- a/server/utils/domains.js +++ b/server/utils/domains.js @@ -1,8 +1,16 @@ // @flow -import { stripSubdomain } from "../../shared/utils/domains"; +import { parseDomain, stripSubdomain } from "../../shared/utils/domains"; export function getCookieDomain(domain: string) { return process.env.SUBDOMAINS_ENABLED === "true" ? stripSubdomain(domain) : domain; } + +export function isCustomDomain(hostname: string) { + const parsed = parseDomain(hostname); + const main = parseDomain(process.env.URL); + return ( + parsed && main && (main.domain !== parsed.domain || main.tld !== parsed.tld) + ); +} diff --git a/server/utils/jwt.js b/server/utils/jwt.js index d1bbfa2b..c4121832 100644 --- a/server/utils/jwt.js +++ b/server/utils/jwt.js @@ -20,8 +20,24 @@ function getJWTPayload(token) { export async function getUserForJWT(token: string): Promise { const payload = getJWTPayload(token); + + // check the token is within it's expiration time + if (payload.expiresAt) { + if (new Date(payload.expiresAt) < new Date()) { + throw new AuthenticationError("Expired token"); + } + } + const user = await User.findByPk(payload.id); + if (payload.type === "transfer") { + // If the user has made a single API request since the transfer token was + // created then it's no longer valid, they'll need to sign in again. + if (user.lastActiveAt > new Date(payload.createdAt)) { + throw new AuthenticationError("Token has already been used"); + } + } + try { JWT.verify(token, user.jwtSecret); } catch (err) { @@ -34,6 +50,10 @@ export async function getUserForJWT(token: string): Promise { export async function getUserForEmailSigninToken(token: string): Promise { const payload = getJWTPayload(token); + if (payload.type !== "email-signin") { + throw new AuthenticationError("Invalid token"); + } + // check the token is within it's expiration time if (payload.createdAt) { if (new Date(payload.createdAt) < subMinutes(new Date(), 10)) {