From 725f41ef1b57b09300dec9375cbc9645a26cefe7 Mon Sep 17 00:00:00 2001 From: Simon Larsen Date: Fri, 13 Oct 2023 13:25:38 +0100 Subject: [PATCH] make auth httponly --- Accounts/src/Utils/Login.ts | 3 - CommonServer/Middleware/UserAuthorization.ts | 23 ++++---- CommonServer/Utils/Cookie.ts | 28 ++++++++++ CommonServer/Utils/StartServer.ts | 3 +- CommonServer/package-lock.json | 31 +++++++++++ CommonServer/package.json | 2 + CommonUI/src/Utils/API/API.ts | 3 - CommonUI/src/Utils/Cookie.ts | 55 +++++++++++++++++++ CommonUI/src/Utils/ModelAPI/ModelAPI.ts | 2 - CommonUI/src/Utils/User.ts | 51 +++-------------- Dashboard/src/Pages/Init/Init.tsx | 1 - Dashboard/src/Pages/Logout/Logout.tsx | 7 ++- Identity/API/Authentication.ts | 46 ++++++++++++---- Identity/API/SSO.ts | 8 +++ .../src/Components/MasterPage/MasterPage.tsx | 4 -- StatusPage/src/Utils/User.ts | 9 --- config.example.env | 4 -- 17 files changed, 184 insertions(+), 96 deletions(-) create mode 100644 CommonServer/Utils/Cookie.ts create mode 100644 CommonUI/src/Utils/Cookie.ts diff --git a/Accounts/src/Utils/Login.ts b/Accounts/src/Utils/Login.ts index 04a2cd536b..14976ad823 100644 --- a/Accounts/src/Utils/Login.ts +++ b/Accounts/src/Utils/Login.ts @@ -16,9 +16,6 @@ export default abstract class LoginUtil { User ) as User; - const token: string = value['token'] as string; - - UserUtil.setAccessToken(token); UserUtil.setEmail(user.email as Email); UserUtil.setUserId(user.id as ObjectID); UserUtil.setName(user.name || new Name('')); diff --git a/CommonServer/Middleware/UserAuthorization.ts b/CommonServer/Middleware/UserAuthorization.ts index 25a69364f9..c6fd7ef165 100644 --- a/CommonServer/Middleware/UserAuthorization.ts +++ b/CommonServer/Middleware/UserAuthorization.ts @@ -29,6 +29,7 @@ import SsoAuthorizationException from 'Common/Types/Exception/SsoAuthorizationEx import JSONWebTokenData from 'Common/Types/JsonWebTokenData'; import logger from '../Utils/Logger'; import Exception from 'Common/Types/Exception/Exception'; +import CookieUtil from '../Utils/Cookie'; export default class UserMiddleware { /* @@ -41,16 +42,8 @@ export default class UserMiddleware { public static getAccessToken(req: ExpressRequest): string | null { let accessToken: string | null = null; - if (req.headers['authorization']) { - accessToken = req.headers['authorization'] as string; - } - - if (req.query['accessToken']) { - accessToken = req.query['accessToken'] as string; - } - - if (accessToken?.includes(' ')) { - accessToken = accessToken.split(' ')[1] || ''; + if(CookieUtil.getCookie(req, "user-token")){ + accessToken = CookieUtil.getCookie(req, "user-token"); } return accessToken; @@ -59,10 +52,14 @@ export default class UserMiddleware { public static getSsoTokens(req: ExpressRequest): Dictionary { const ssoTokens: Dictionary = {}; - for (const key of Object.keys(req.headers)) { + // get sso tokens from cookies. + + const cookies: Dictionary = CookieUtil.getAllCookies(req); + + for (const key of Object.keys(cookies)) { if (key.startsWith('sso-')) { const value: string | undefined | Array = - req.headers[key]; + cookies[key]; let projectId: string | undefined = undefined; try { @@ -80,7 +77,7 @@ export default class UserMiddleware { typeof value === 'string' && typeof projectId === 'string' ) { - ssoTokens[projectId] = req.headers[key] as string; + ssoTokens[projectId] = cookies[key] as string; } } } diff --git a/CommonServer/Utils/Cookie.ts b/CommonServer/Utils/Cookie.ts new file mode 100644 index 0000000000..1a5deebc19 --- /dev/null +++ b/CommonServer/Utils/Cookie.ts @@ -0,0 +1,28 @@ +import { CookieOptions } from "express"; +import { ExpressRequest, ExpressResponse } from "./Express"; + +export default class CookieUtil { + // set cookie with express response + + public static setCookie(res: ExpressResponse, name: string, value: string, options: CookieOptions) { + res.cookie(name, value, options); + } + + // get cookie with express request + + public static getCookie(req: ExpressRequest, name: string) { + return req.cookies[name]; + } + + // delete cookie with express response + + public static removeCookie(res: ExpressResponse, name: string) { + res.clearCookie(name); + } + + // get all cookies with express request + public static getAllCookies(req: ExpressRequest) { + return req.cookies; + } + +} \ No newline at end of file diff --git a/CommonServer/Utils/StartServer.ts b/CommonServer/Utils/StartServer.ts index 35ba47b5cd..41b0b3454e 100644 --- a/CommonServer/Utils/StartServer.ts +++ b/CommonServer/Utils/StartServer.ts @@ -34,7 +34,7 @@ import HTTPResponse from 'Common/Types/API/HTTPResponse'; import HTTPErrorResponse from 'Common/Types/API/HTTPErrorResponse'; import ServerException from 'Common/Types/Exception/ServerException'; import zlib from 'zlib'; -// import OpenTelemetrySDK from "./OpenTelemetry"; +import CookieParser from 'cookie-parser'; // Make sure we have stack trace for debugging. Error.stackTraceLimit = Infinity; @@ -44,6 +44,7 @@ const app: ExpressApplication = Express.getExpressApp(); app.disable('x-powered-by'); app.set('port', process.env['PORT']); app.set('view engine', 'ejs'); +app.use(CookieParser()); const jsonBodyParserMiddleware: Function = ExpressJson({ limit: '50mb', diff --git a/CommonServer/package-lock.json b/CommonServer/package-lock.json index b9d35d2f07..f053c6d764 100644 --- a/CommonServer/package-lock.json +++ b/CommonServer/package-lock.json @@ -23,6 +23,7 @@ "axios": "^1.3.3", "bullmq": "^3.6.6", "Common": "file:../Common", + "cookie-parser": "^1.4.6", "cors": "^2.8.5", "cron-parser": "^4.8.1", "dotenv": "^16.0.0", @@ -49,6 +50,7 @@ }, "devDependencies": { "@faker-js/faker": "^6.3.1", + "@types/cookie-parser": "^1.4.4", "@types/cors": "^2.8.12", "@types/express": "^4.17.13", "@types/jest": "^27.4.1", @@ -3660,6 +3662,15 @@ "resolved": "https://registry.npmjs.org/@types/cookie/-/cookie-0.4.1.tgz", "integrity": "sha512-XW/Aa8APYr6jSVVA1y/DEIZX0/GMKLEVekNG727R8cs56ahETkRAy/3DR7+fJyh7oUgGwNQaRfXCun0+KbWY7Q==" }, + "node_modules/@types/cookie-parser": { + "version": "1.4.4", + "resolved": "https://registry.npmjs.org/@types/cookie-parser/-/cookie-parser-1.4.4.tgz", + "integrity": "sha512-Var+aj5I6ZgIqsQ05N2V8q5OBrFfZXtIGWWDSrEYLIbMw758obagSwdGcLCjwh1Ga7M7+wj0SDIAaAC/WT7aaA==", + "dev": true, + "dependencies": { + "@types/express": "*" + } + }, "node_modules/@types/cookies": { "version": "0.7.7", "resolved": "https://registry.npmjs.org/@types/cookies/-/cookies-0.7.7.tgz", @@ -4935,6 +4946,26 @@ "node": ">= 0.6" } }, + "node_modules/cookie-parser": { + "version": "1.4.6", + "resolved": "https://registry.npmjs.org/cookie-parser/-/cookie-parser-1.4.6.tgz", + "integrity": "sha512-z3IzaNjdwUC2olLIB5/ITd0/setiaFMLYiZJle7xg5Fe9KWAceil7xszYfHHBtDFYLSgJduS2Ty0P1uJdPDJeA==", + "dependencies": { + "cookie": "0.4.1", + "cookie-signature": "1.0.6" + }, + "engines": { + "node": ">= 0.8.0" + } + }, + "node_modules/cookie-parser/node_modules/cookie": { + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.4.1.tgz", + "integrity": "sha512-ZwrFkGJxUR3EIoXtO+yVE69Eb7KlixbaeAWfBQB9vVsNn/o+Yw69gBWSSDK825hQNdN+wF8zELf3dFNl/kxkUA==", + "engines": { + "node": ">= 0.6" + } + }, "node_modules/cookie-signature": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.0.6.tgz", diff --git a/CommonServer/package.json b/CommonServer/package.json index ea35bc9183..42d8025005 100644 --- a/CommonServer/package.json +++ b/CommonServer/package.json @@ -25,6 +25,7 @@ "axios": "^1.3.3", "bullmq": "^3.6.6", "Common": "file:../Common", + "cookie-parser": "^1.4.6", "cors": "^2.8.5", "cron-parser": "^4.8.1", "dotenv": "^16.0.0", @@ -51,6 +52,7 @@ }, "devDependencies": { "@faker-js/faker": "^6.3.1", + "@types/cookie-parser": "^1.4.4", "@types/cors": "^2.8.12", "@types/express": "^4.17.13", "@types/jest": "^27.4.1", diff --git a/CommonUI/src/Utils/API/API.ts b/CommonUI/src/Utils/API/API.ts index 60f5f1c561..c023ff2abc 100644 --- a/CommonUI/src/Utils/API/API.ts +++ b/CommonUI/src/Utils/API/API.ts @@ -68,9 +68,6 @@ class BaseAPI extends API { let defaultHeaders: Headers = this.getDefaultHeaders(); const headers: Headers = {}; - if (User.isLoggedIn()) { - headers['Authorization'] = 'Basic ' + User.getAccessToken(); - } const globalPermissionsHash: string = LocalStorage.getItem( 'global-permissions-hash' diff --git a/CommonUI/src/Utils/Cookie.ts b/CommonUI/src/Utils/Cookie.ts new file mode 100644 index 0000000000..d2886bb166 --- /dev/null +++ b/CommonUI/src/Utils/Cookie.ts @@ -0,0 +1,55 @@ +import URL from 'Common/Types/API/URL'; +import Email from 'Common/Types/Email'; +import { JSONObject, JSONValue } from 'Common/Types/JSON'; +import Typeof from 'Common/Types/Typeof'; +import JSONFunctions from 'Common/Types/JSONFunctions'; +import UniversalCookies from 'universal-cookie'; +import Route from 'Common/Types/API/Route'; + +export default class Cookie { + public static setItem(key: string, value: JSONValue | Email | URL, options?: { + httpOnly?: boolean | undefined, + path: Route, + } | undefined): void { + if (typeof value === Typeof.Object) { + // if of type jsonobject. + value = JSON.stringify( + JSONFunctions.serializeValue(value as JSONValue) as JSONObject + ); + } + + const cookies: UniversalCookies = new UniversalCookies(); + cookies.set(key, value as string, { + httpOnly: options?.httpOnly || false, + path: options?.path ? options.path.toString() : '/' + }); + } + + public static getItem(key: string): JSONValue { + + const cookies: UniversalCookies = new UniversalCookies(); + const value: JSONValue = cookies.get(key) as JSONValue; + + try { + if (value) { + return JSONFunctions.deserializeValue( + JSONFunctions.parse(value?.toString()) + ); + } + return value; + } catch (err) { + return value; + } + } + + public static removeItem(key: string): void { + const cookies: UniversalCookies = new UniversalCookies(); + cookies.remove(key); + } + + // check if cookie exists + public static exists(key: string): boolean { + const cookies: UniversalCookies = new UniversalCookies(); + return Boolean(cookies.get(key)); + } +} diff --git a/CommonUI/src/Utils/ModelAPI/ModelAPI.ts b/CommonUI/src/Utils/ModelAPI/ModelAPI.ts index e281a597f5..de15fe4e90 100644 --- a/CommonUI/src/Utils/ModelAPI/ModelAPI.ts +++ b/CommonUI/src/Utils/ModelAPI/ModelAPI.ts @@ -17,7 +17,6 @@ import Dictionary from 'Common/Types/Dictionary'; import ProjectUtil from '../Project'; import Sort from './Sort'; import Project from 'Model/Models/Project'; -import User from '../User'; import Navigation from '../Navigation'; export class ModelAPIHttpResponse< @@ -319,7 +318,6 @@ export default class ModelAPI { headers = { ...headers, - ...User.getAllSsoTokens(), }; if (requestOptions && requestOptions.isMultiTenantRequest) { diff --git a/CommonUI/src/Utils/User.ts b/CommonUI/src/Utils/User.ts index c3fe8864d6..8610874802 100644 --- a/CommonUI/src/Utils/User.ts +++ b/CommonUI/src/Utils/User.ts @@ -8,9 +8,6 @@ import BadDataException from 'Common/Types/Exception/BadDataException'; import Dictionary from 'Common/Types/Dictionary'; export default class User { - public static getAccessToken(): string { - return LocalStorage.getItem('access_token') as string; - } public static setProfilePicId(id: ObjectID | null): void { if (!id) { @@ -31,20 +28,6 @@ export default class User { ); } - public static setAccessToken(token: string): void { - LocalStorage.setItem('access_token', token); - } - - public static setSsoToken(projectId: ObjectID, token: string): void { - LocalStorage.setItem('sso_' + projectId.toString(), token); - } - - public static getSsoToken(projectId: ObjectID): string | null { - return LocalStorage.getItem('sso_' + projectId.toString()) as - | string - | null; - } - public static isCardRegistered(): boolean { return Boolean(LocalStorage.getItem('cardRegistered')); } @@ -69,7 +52,11 @@ export default class User { LocalStorage.setItem('user_name', name.toString()); } - public static getEmail(): Email { + public static getEmail(): Email | null { + if(!LocalStorage.getItem('user_email')){ + return null; + } + return new Email(LocalStorage.getItem('user_email') as string); } @@ -94,29 +81,6 @@ export default class User { LocalStorage.setItem('project', project); } - public static getAllSsoTokens(): Dictionary { - const localStorageItems: Dictionary = - LocalStorage.getAllItems(); - const result: Dictionary = {}; - - let numberOfTokens: number = 1; - - for (const key in localStorageItems) { - if (!localStorageItems[key]) { - continue; - } - - if (key.startsWith('sso_')) { - result['sso-' + numberOfTokens] = localStorageItems[ - key - ] as string; - numberOfTokens++; - } - } - - return result; - } - public static getProject(): JSONObject { return LocalStorage.getItem('project') as JSONObject; } @@ -146,10 +110,11 @@ export default class User { } public static isLoggedIn(): boolean { - return LocalStorage.getItem('access_token') ? true : false; + return !!this.getEmail(); } - public static logout(): void { + public static async logout(): Promise { + // TODO: Clear all cookies here. LocalStorage.clear(); } diff --git a/Dashboard/src/Pages/Init/Init.tsx b/Dashboard/src/Pages/Init/Init.tsx index 263784287b..e5bfd80939 100644 --- a/Dashboard/src/Pages/Init/Init.tsx +++ b/Dashboard/src/Pages/Init/Init.tsx @@ -49,7 +49,6 @@ const Init: FunctionComponent = ( return Navigation.navigate(RouteMap[PageMap.LOGOUT] as Route); } - User.setSsoToken(decodedtoken.projectId, sso_token); } }, []); diff --git a/Dashboard/src/Pages/Logout/Logout.tsx b/Dashboard/src/Pages/Logout/Logout.tsx index 9cec322b8b..f05096a1b7 100644 --- a/Dashboard/src/Pages/Logout/Logout.tsx +++ b/Dashboard/src/Pages/Logout/Logout.tsx @@ -1,4 +1,4 @@ -import React, { FunctionComponent, ReactElement, useEffect } from 'react'; +import React, { FunctionComponent, ReactElement } from 'react'; import PageComponentProps from '../PageComponentProps'; import Page from 'CommonUI/src/Components/Page/Page'; import Route from 'Common/Types/API/Route'; @@ -9,13 +9,14 @@ import UserUtil from 'CommonUI/src/Utils/User'; import Navigation from 'CommonUI/src/Utils/Navigation'; import { ACCOUNTS_URL } from 'CommonUI/src/Config'; import UiAnalytics from 'CommonUI/src/Utils/Analytics'; +import useAsyncEffect from 'use-async-effect'; const Logout: FunctionComponent = ( _props: PageComponentProps ): ReactElement => { - useEffect(() => { + useAsyncEffect(async () => { UiAnalytics.logout(); - UserUtil.logout(); + await UserUtil.logout(); Navigation.navigate(ACCOUNTS_URL); }, []); diff --git a/Identity/API/Authentication.ts b/Identity/API/Authentication.ts index b59c92adf8..be5a0504e3 100644 --- a/Identity/API/Authentication.ts +++ b/Identity/API/Authentication.ts @@ -35,6 +35,7 @@ import AccessTokenService from 'CommonServer/Services/AccessTokenService'; import Hostname from 'Common/Types/API/Hostname'; import Protocol from 'Common/Types/API/Protocol'; import DatabaseConfig from 'CommonServer/DatabaseConfig'; +import CookieUtil from 'CommonServer/Utils/Cookie'; const router: ExpressRouter = Express.getRouter(); @@ -183,11 +184,15 @@ router.post( OneUptimeDate.getSecondsInDays(new PositiveNumber(30)) ); - return Response.sendEntityResponse(req, res, savedUser, User, { - miscData: { - token, - }, + // Set a cookie with token. + CookieUtil.setCookie(res, 'user-token', token, { + maxAge: OneUptimeDate.getSecondsInDays( + new PositiveNumber(30) + ), + httpOnly: true, }); + + return Response.sendEntityResponse(req, res, savedUser, User); } return Response.sendErrorResponse( @@ -481,6 +486,24 @@ router.post( } ); +router.post( + '/logout', + async ( + req: ExpressRequest, + res: ExpressResponse, + next: NextFunction + ): Promise => { + try { + CookieUtil.removeCookie(res, 'user-token'); + + // remove all sso cookies as well. + + return Response.sendEmptyResponse(req, res); + } catch (err) { + return next(err); + } + }); + router.post( '/login', async ( @@ -553,16 +576,19 @@ router.post( OneUptimeDate.getSecondsInDays(new PositiveNumber(30)) ); + // Set a cookie with token. + CookieUtil.setCookie(res, 'user-token', token, { + maxAge: OneUptimeDate.getSecondsInDays( + new PositiveNumber(30) + ), + httpOnly: true, + }); + return Response.sendEntityResponse( req, res, alreadySavedUser, - User, - { - miscData: { - token, - }, - } + User ); } } diff --git a/Identity/API/SSO.ts b/Identity/API/SSO.ts index f4206c0951..7e6d26999f 100644 --- a/Identity/API/SSO.ts +++ b/Identity/API/SSO.ts @@ -31,6 +31,7 @@ import Exception from 'Common/Types/Exception/Exception'; import Hostname from 'Common/Types/API/Hostname'; import Protocol from 'Common/Types/API/Protocol'; import DatabaseConfig from 'CommonServer/DatabaseConfig'; +import CookieUtil from 'CommonServer/Utils/Cookie'; const router: ExpressRouter = Express.getRouter(); @@ -345,6 +346,13 @@ router.post( const httpProtocol: Protocol = await DatabaseConfig.getHttpProtocol(); + CookieUtil.setCookie(res, 'sso-'+req.params['projectId'].toString(), token, { + maxAge: OneUptimeDate.getSecondsInDays( + new PositiveNumber(30) + ), + httpOnly: true, + }); + return Response.redirect( req, res, diff --git a/StatusPage/src/Components/MasterPage/MasterPage.tsx b/StatusPage/src/Components/MasterPage/MasterPage.tsx index 4b2c4c0d07..7a0c4470cc 100644 --- a/StatusPage/src/Components/MasterPage/MasterPage.tsx +++ b/StatusPage/src/Components/MasterPage/MasterPage.tsx @@ -32,7 +32,6 @@ import Link from 'Common/Types/Link'; import JSONWebTokenData from 'Common/Types/JsonWebTokenData'; import JSONWebToken from 'CommonUI/src/Utils/JsonWebToken'; import Route from 'Common/Types/API/Route'; -import User from '../../Utils/User'; import LoginUtil from '../../Utils/Login'; import StatusPageUtil from '../../Utils/StatusPage'; @@ -94,7 +93,6 @@ const DashboardMasterPage: FunctionComponent = ( } LoginUtil.login({ - token: sso_token, user: { ...decodedtoken, _id: decodedtoken.userId }, }); @@ -105,8 +103,6 @@ const DashboardMasterPage: FunctionComponent = ( return Navigation.navigate(logoutRoute); } - User.setAccessToken(decodedtoken.statusPageId!, sso_token); - if (Navigation.getQueryStringByName('redirectUrl')) { Navigation.navigate( new Route(Navigation.getQueryStringByName('redirectUrl')!) diff --git a/StatusPage/src/Utils/User.ts b/StatusPage/src/Utils/User.ts index cfe9e1b97d..d77f52b1d3 100644 --- a/StatusPage/src/Utils/User.ts +++ b/StatusPage/src/Utils/User.ts @@ -4,15 +4,6 @@ import ObjectID from 'Common/Types/ObjectID'; import Name from 'Common/Types/Name'; export default class User { - public static getAccessToken(statusPageId: ObjectID): string { - return LocalStorage.getItem( - statusPageId.toString() + 'access_token' - ) as string; - } - - public static setAccessToken(statusPageId: ObjectID, token: string): void { - LocalStorage.setItem(statusPageId.toString() + 'access_token', token); - } public static setUserId(statusPageId: ObjectID, userId: ObjectID): void { LocalStorage.setItem( diff --git a/config.example.env b/config.example.env index 354bc5fb6a..40a9c9ea75 100644 --- a/config.example.env +++ b/config.example.env @@ -127,10 +127,6 @@ INTERNAL_SMTP_DKIM_PUBLIC_KEY_AS_BASE64= INTERNAL_SMTP_EMAIL=test@yourcompany.com INTERNAL_SMTP_SENDING_DOMAIN=yourcompany.com -# Licensing Database -AIRTABLE_API_KEY= -AIRTABLE_BASE_ID= - # Plans # This is in the format of PlanName,PlanIdFromBillingProvider,MonthlySubscriptionPlanAmountInUSD,YearlySubscriptionPlanAmountInUSD,Order,TrialPeriodInDays # Enterprise plan will have -1 which means custom pricing.