diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0b5bf52..baf3524 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -33,7 +33,7 @@ jobs: POSTGRES_PASSWORD: ${{ env.POSTGRES_PASSWORD }} options: --restart always --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5 graphql-engine: - image: hasura/graphql-engine:v1.2.0 + image: hasura/graphql-engine:v1.2.1 env: HASURA_GRAPHQL_ENABLE_TELEMETRY: 'false' HASURA_GRAPHQL_ADMIN_SECRET: ${{ env.HASURA_GRAPHQL_ADMIN_SECRET }} diff --git a/custom/emails/activate-account/html.ejs b/custom/emails/activate-account/html.ejs index eaed5a2..b72169f 100644 --- a/custom/emails/activate-account/html.ejs +++ b/custom/emails/activate-account/html.ejs @@ -22,7 +22,7 @@

Hello <%= display_name %>

Please confirm your email address by clicking the button below:

- +

Thanks,
The Team

diff --git a/custom/storage-rules/rules.yaml b/custom/storage-rules/rules.yaml index b882691..c50b2bf 100644 --- a/custom/storage-rules/rules.yaml +++ b/custom/storage-rules/rules.yaml @@ -1,16 +1,10 @@ functions: isAuthenticated: 'return !!request.auth' - isOwner: - params: - - userId - code: "return !!request.auth && userId === request.auth['x-hasura-user-id']" + isOwner: "return !!request.auth && userId === request.auth['user-id']" validToken: 'return request.query.token === resource.Metadata.token' paths: /user/:userId/:fileId: read: 'isOwner(userId) || validToken()' write: 'isOwner(userId)' - meta: - read: 'isOwner(userId)' - write: 'isOwner(userId)' - values: - description: 'request.query.description' + metadata: + description: 'request.query.description' diff --git a/docker-compose.yaml b/docker-compose.yaml index 1a3cd93..357fef0 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -8,7 +8,7 @@ services: environment: POSTGRES_PASSWORD: '${POSTGRES_PASSWORD:-postgrespassword}' graphql-engine: - image: hasura/graphql-engine:v1.2.0 + image: hasura/graphql-engine:v1.2.1 depends_on: - 'postgres' restart: always diff --git a/docs/api.md b/docs/api.md index 9fb6c0e..66525a5 100644 --- a/docs/api.md +++ b/docs/api.md @@ -5,8 +5,8 @@ | [Authentication](#authentication) | [POST /auth/register](#registration) | Account registration | | ^^ | [POST /auth/login](#login) | Login | | ^^ | [GET /auth/jwks](#jwks) | JWK Set | -| ^^ | [POST /auth/account/activate](#activation) | Activate account | -| ^^ | [POST /auth/account/delete](#delete-account) | Delete account | +| ^^ | [POST /auth/activate](#activation) | Activate account | +| ^^ | [POST /auth/delete](#delete-account) | Delete account | | ^^ | [POST /auth/change-password/request](#forgotten-password) | Forgotten password | | ^^ | [POST /auth/change-password/change](#reset-password) | Reset password | | ^^ | [POST /auth/change-email/request](#) | | diff --git a/examples/simple-hasura-minio/docker-compose.yaml b/examples/simple-hasura-minio/docker-compose.yaml index 5d0bb32..81deadc 100644 --- a/examples/simple-hasura-minio/docker-compose.yaml +++ b/examples/simple-hasura-minio/docker-compose.yaml @@ -6,7 +6,7 @@ services: volumes: - db_data:/var/lib/postgresql/data graphql-engine: - image: hasura/graphql-engine:v1.2.0 + image: hasura/graphql-engine:v1.2.1 ports: - '8080:8080' depends_on: diff --git a/package.json b/package.json index 9d9618c..dfff7de 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "docs:dev": "vuepress dev docs", "start": "ts-node -r tsconfig-paths/register src/start.ts", "test": "NODE_ENV=test jest --coverage --forceExit", - "test:ci": "NODE_ENV=ci jest --coverage --forceExit", + "test:ci": "NODE_ENV=ci jest --coverage --forceExit --runInBand", "test:watch": "NODE_ENV=test jest --watch", "report-coverage": "codecov" }, @@ -73,7 +73,7 @@ "graphql": "15.0.0", "graphql-request": "1.8.2", "graphql-tag": "2.10.3", - "hasura-cli": "1.2.0", + "hasura-cli": "1.2.1", "helmet": "3.22.0", "hibp": "9.0.0", "jose": "1.26.0", @@ -170,4 +170,4 @@ "singleQuote": true, "trailingComma": "none" } -} +} \ No newline at end of file diff --git a/src/routes/auth/account/index.ts b/src/routes/auth/account/index.ts deleted file mode 100644 index bd8c6e8..0000000 --- a/src/routes/auth/account/index.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { AUTO_ACTIVATE_NEW_USERS } from '@shared/config' -import { Router } from 'express' -import activateAccount from './activate' -import deleteAccount from './delete' - -const router = Router() - -if (!AUTO_ACTIVATE_NEW_USERS) { - router.get('/activate', activateAccount) -} - -router.post('/delete', deleteAccount) - -export default router diff --git a/src/routes/auth/account/activate.ts b/src/routes/auth/activate.ts similarity index 100% rename from src/routes/auth/account/activate.ts rename to src/routes/auth/activate.ts diff --git a/src/routes/auth/auth.test.ts b/src/routes/auth/auth.test.ts index 05847bb..c8717f9 100644 --- a/src/routes/auth/auth.test.ts +++ b/src/routes/auth/auth.test.ts @@ -13,12 +13,12 @@ import { ANONYMOUS_USERS_ENABLE } from '@shared/config' import { generateRandomString, selectAccountByEmail } from '@shared/helpers' -import { deleteMailHogEmail, mailHogSearch, registerAccount, deleteAccount } from '@test/test-utils' +import { deleteMailHogEmail, mailHogSearch, deleteAccount } from '@test/test-utils' import { JWT } from 'jose' import { Token } from '@shared/types' import { app } from '../../server' -import request from 'supertest' +import { SuperTest, Test, agent } from 'supertest' /** * Store variables in memory. @@ -31,18 +31,31 @@ let jwtToken: string const email = `${generateRandomString()}@${generateRandomString()}.com` const password = generateRandomString() -/** - * Create agent for global state. - */ +let request: SuperTest + const server = app.listen(PORT) -const agent = request(server) + +beforeAll(async () => { + request = agent(server) // * Create the SuperTest agent +}) + // * Code that is executed after any jest test file that imports test-utiles afterAll(async () => { server.close() }) +const pwndPasswordIt = HIBP_ENABLE ? it : it.skip +pwndPasswordIt('should tell the password has been pwned', async () => { + const { + status, + body: { message } + } = await request.post('/auth/register').send({ email: 'test@example.com', password: '123456' }) + expect(status).toEqual(400) + expect(message).toEqual('Password is too weak.') +}) + it('should create an account', async () => { - const { status } = await agent + const { status } = await request .post('/auth/register') .send({ email, password, user_data: { name: 'Test name' } }) expect(status).toEqual(204) @@ -52,7 +65,7 @@ it('should tell the account already exists', async () => { const { status, body: { message } - } = await agent.post('/auth/register').send({ email, password }) + } = await request.post('/auth/register').send({ email, password }) expect(status).toEqual(400) expect(message).toEqual('Account already exists.') }) @@ -61,7 +74,7 @@ it('should tell the account already exists', async () => { const manualActivationIt = !AUTO_ACTIVATE_NEW_USERS ? it : it.skip manualActivationIt('should fail to activate an user from a wrong ticket', async () => { - const { status, redirect, header } = await agent.get(`/auth/account/activate?ticket=${uuidv4()}`) + const { status, redirect, header } = await request.get(`/auth/activate?ticket=${uuidv4()}`) expect( status === 500 || (status === 302 && redirect && header?.location === REDIRECT_URL_ERROR) ).toBeTrue() @@ -79,12 +92,12 @@ manualActivationIt('should activate the account from a valid ticket', async () = } else { ticket = (await selectAccountByEmail(email)).ticket } - const { status } = await agent.get(`/auth/account/activate?ticket=${ticket}`) + const { status } = await request.get(`/auth/activate?ticket=${ticket}`) expect(status).toBeOneOf([204, 302]) }) it('should sign the user in', async () => { - const { body, status } = await agent.post('/auth/login').send({ email, password }) + const { body, status } = await request.post('/auth/login').send({ email, password }) // Save JWT token to globally scoped varaible. jwtToken = body.jwt_token expect(status).toEqual(200) @@ -99,37 +112,17 @@ it('should decode a valid custom user claim', async () => { expect(decodedJwt[JWT_CLAIMS_NAMESPACE]['x-hasura-name']).toEqual('Test name') }) -it('should delete the account', async () => { - const { status } = await agent - .post('/auth/account/delete') - .set('Authorization', `Bearer ${jwtToken}`) - expect(status).toEqual(204) -}) - -const pwndPasswordIt = HIBP_ENABLE ? it : it.skip -pwndPasswordIt('should tell the password has been pwned', async () => { - const { - status, - body: { message } - } = await agent.post('/auth/register').send({ email: 'test@example.com', password: '123456' }) - expect(status).toEqual(400) - expect(message).toEqual('Password is too weak.') +it('should logout', async () => { + const res = await request.post('/auth/logout').send() + expect(res.status).toBe(204) + await request.post('/auth/login').send({ email, password }) + await deleteAccount(request, { email, password }) }) const anonymousAccountIt = ANONYMOUS_USERS_ENABLE ? it : it.skip anonymousAccountIt('should login anonymously', async () => { - const { body, status } = await agent.post('/auth/login').send({ anonymous: true }) + const { body, status } = await request.post('/auth/login').send({ anonymous: true }) expect(status).toEqual(200) expect(body.jwt_token).toBeString() expect(body.jwt_expires_in).toBeNumber() }) - -it('should logout', async () => { - // TODO : review this test, including cookies - const account = await registerAccount(agent) - const res = await agent.post('/auth/logout').send() - expect(res.status).toBe(204) - await deleteAccount(agent, account) -}) - -// TODO test cookies diff --git a/src/routes/auth/change-email/utils.ts b/src/routes/auth/change-email/utils.ts index 3108bcb..fd7c38f 100644 --- a/src/routes/auth/change-email/utils.ts +++ b/src/routes/auth/change-email/utils.ts @@ -1,18 +1,17 @@ import { Request } from 'express' -import { selectAccountByEmail } from '@shared/helpers' +import { selectAccountByEmail, getPermissionVariablesFromCookie } from '@shared/helpers' import { emailResetSchema } from '@shared/validation' -import { getClaims } from '@shared/jwt' import Boom from '@hapi/boom' -export const getRequestInfo = async ({ - body, - headers -}: Request): Promise<{ user_id: string; new_email: string }> => { +export const getRequestInfo = async ( + req: Request +): Promise<{ user_id: string | number; new_email: string }> => { // get current user_id - const user_id = getClaims(headers.authorization)['x-hasura-user-id'] + const permission_variables = getPermissionVariablesFromCookie(req) + const user_id = permission_variables['user-id'] // validate new email - const { new_email } = await emailResetSchema.validateAsync(body) + const { new_email } = await emailResetSchema.validateAsync(req.body) // make sure new_email is not attached to an account yet let account_exists = true @@ -27,5 +26,8 @@ export const getRequestInfo = async ({ if (account_exists) { throw Boom.badRequest('Cannot use this email.') } - return { user_id, new_email } + return { + user_id, + new_email + } } diff --git a/src/routes/auth/change-password/change.ts b/src/routes/auth/change-password/change.ts index 681be8e..14de84e 100644 --- a/src/routes/auth/change-password/change.ts +++ b/src/routes/auth/change-password/change.ts @@ -1,4 +1,10 @@ -import { asyncWrapper, checkHibp, hashPassword, selectAccountByUserId } from '@shared/helpers' +import { + asyncWrapper, + checkHibp, + hashPassword, + selectAccountByUserId, + getPermissionVariablesFromCookie +} from '@shared/helpers' import { Request, Response } from 'express' import { resetPasswordWithOldPasswordSchema, @@ -10,18 +16,17 @@ import Boom from '@hapi/boom' import bcrypt from 'bcryptjs' import { request } from '@shared/request' import { v4 as uuidv4 } from 'uuid' -import { getClaims } from '@shared/jwt' import { UpdateAccountData } from '@shared/types' /** * Reset the password, either from a valid ticket or from a valid JWT and a valid password */ -async function changePassword({ body, headers }: Request, res: Response): Promise { +async function changePassword(req: Request, res: Response): Promise { let password_hash: string - if (body.ticket) { + if (req.body.ticket) { // Reset the password from { ticket, new_password } - const { ticket, new_password } = await resetPasswordWithTicketSchema.validateAsync(body) + const { ticket, new_password } = await resetPasswordWithTicketSchema.validateAsync(req.body) await checkHibp(new_password) password_hash = await hashPassword(new_password) @@ -39,10 +44,11 @@ async function changePassword({ body, headers }: Request, res: Response): Promis } } else { // Reset the password from valid JWT and { old_password, new_password } - const user_id = getClaims(headers.authorization)['x-hasura-user-id'] + const permission_variables = getPermissionVariablesFromCookie(req) + const user_id = permission_variables['user-id'] const { old_password, new_password } = await resetPasswordWithOldPasswordSchema.validateAsync( - body + req.body ) await checkHibp(new_password) diff --git a/src/routes/auth/delete.test.ts b/src/routes/auth/delete.test.ts new file mode 100644 index 0000000..00a1c03 --- /dev/null +++ b/src/routes/auth/delete.test.ts @@ -0,0 +1,29 @@ +import 'jest-extended' + +import { SuperTest, Test, agent } from 'supertest' + +import { PORT } from '@shared/config' +import { registerAccount } from '@test/test-utils' + +import { app } from '../../server' + +let request: SuperTest + +const server = app.listen(PORT) + +// * Code that is executed before any jest test file that imports this file +beforeAll(async () => { + request = agent(server) // * Create the SuperTest agent + registerAccount(request) +}) + +// * Code that is executed after any jest test file that imports test-utiles +afterAll(async () => { + server.close() +}) + +it('should delete an account', async () => { + await registerAccount(request) + const { status } = await request.post('/auth/delete') + expect(status).toEqual(204) +}) diff --git a/src/routes/auth/account/delete.ts b/src/routes/auth/delete.ts similarity index 52% rename from src/routes/auth/account/delete.ts rename to src/routes/auth/delete.ts index 405c3de..7e92ce6 100644 --- a/src/routes/auth/account/delete.ts +++ b/src/routes/auth/delete.ts @@ -1,28 +1,24 @@ import { Request, Response } from 'express' import Boom from '@hapi/boom' -import { asyncWrapper } from '@shared/helpers' +import { asyncWrapper, getPermissionVariablesFromCookie } from '@shared/helpers' import { deleteAccountByUserId } from '@shared/queries' import { request } from '@shared/request' -import { getClaims } from '@shared/jwt' import { DeleteAccountData } from '@shared/types' -import { ALLOW_USER_SELF_DELETE } from '@shared/config' -async function deleteUser({ headers }: Request, res: Response): Promise { - if (!ALLOW_USER_SELF_DELETE) { - throw Boom.notFound() - } - - const user_id = getClaims(headers.authorization)['x-hasura-user-id'] +async function deleteUser(req: Request, res: Response): Promise { + const permission_variables = getPermissionVariablesFromCookie(req) + const user_id = permission_variables['user-id'] const hasuraData = await request(deleteAccountByUserId, { user_id }) if (!hasuraData.delete_auth_accounts.affected_rows) { - throw Boom.unauthorized('Invalid or expired JWT token.') + throw Boom.unauthorized('Unable to delete account') } - // clear cookie + // clear cookies res.clearCookie('refresh_token') + res.clearCookie('permission_variables') return res.status(204).send() } diff --git a/src/routes/auth/index.ts b/src/routes/auth/index.ts index 26766aa..3233b51 100644 --- a/src/routes/auth/index.ts +++ b/src/routes/auth/index.ts @@ -1,6 +1,10 @@ -import { MFA_ENABLE, CHANGE_EMAIL_ENABLE } from '@shared/config' +import { + MFA_ENABLE, + CHANGE_EMAIL_ENABLE, + AUTO_ACTIVATE_NEW_USERS, + ALLOW_USER_SELF_DELETE +} from '@shared/config' import { Router } from 'express' -import account from './account' import changeEmail from './change-email' import getJwks from './jwks' import loginAccount from './login' @@ -10,6 +14,8 @@ import changePassword from './change-password' import providers from './providers' import registerAccount from './register' import token from './token' +import activateAccount from './activate' +import deleteAccount from './delete' const router = Router() @@ -25,13 +31,20 @@ if (CHANGE_EMAIL_ENABLE) { router.use('/change-email', changeEmail) } +if (!AUTO_ACTIVATE_NEW_USERS) { + router.get('/activate', activateAccount) +} + +if (ALLOW_USER_SELF_DELETE) { + router.post('/delete', deleteAccount) +} + router .get('/jwks', getJwks) .post('/login', loginAccount) .post('/logout', logout) .post('/register', registerAccount) .use('/token', token) - .use('/account', account) .use('/change-password', changePassword) export default router diff --git a/src/routes/auth/logout.ts b/src/routes/auth/logout.ts index 5d5ec26..3769ef2 100644 --- a/src/routes/auth/logout.ts +++ b/src/routes/auth/logout.ts @@ -17,6 +17,7 @@ interface HasuraData { async function logout({ body, cookies, signedCookies }: Request, res: Response): Promise { // clear cookie res.clearCookie('refresh_token') + res.clearCookie('permission_variables') // should we delete all refresh tokens to this user or not const { all } = await logoutSchema.validateAsync(body) @@ -35,20 +36,12 @@ async function logout({ body, cookies, signedCookies }: Request, res: Response): return res.status(204).send() } - if (!hasura_data) { + const account = hasura_data?.auth_refresh_tokens?.[0]?.account + + if (!account) { return res.status(204).send() } - const { auth_refresh_tokens } = hasura_data - - // if no accounts found - if (!auth_refresh_tokens.length) { - return res.status(204).send() - } - - // get logged out account - const { account } = auth_refresh_tokens[0] - // delete all refresh tokens for user try { await request(deleteAllAccountRefreshTokens, { diff --git a/src/routes/auth/mfa/disable.ts b/src/routes/auth/mfa/disable.ts index e055da1..2e59763 100644 --- a/src/routes/auth/mfa/disable.ts +++ b/src/routes/auth/mfa/disable.ts @@ -1,4 +1,8 @@ -import { asyncWrapper, selectAccountByUserId } from '@shared/helpers' +import { + asyncWrapper, + selectAccountByUserId, + getPermissionVariablesFromCookie +} from '@shared/helpers' import { Request, Response } from 'express' import { deleteOtpSecret } from '@shared/queries' @@ -6,12 +10,12 @@ import Boom from '@hapi/boom' import { authenticator } from 'otplib' import { mfaSchema } from '@shared/validation' import { request } from '@shared/request' -import { getClaims } from '@shared/jwt' -async function disableMfa({ headers, body }: Request, res: Response): Promise { - const { code } = await mfaSchema.validateAsync(body) +async function disableMfa(req: Request, res: Response): Promise { + const { code } = await mfaSchema.validateAsync(req.body) - const user_id = getClaims(headers.authorization)['x-hasura-user-id'] + const permission_variables = getPermissionVariablesFromCookie(req) + const user_id = permission_variables['user-id'] const { otp_secret, mfa_enabled } = await selectAccountByUserId(user_id) diff --git a/src/routes/auth/mfa/enable.ts b/src/routes/auth/mfa/enable.ts index 8fd2f80..cb46dc5 100644 --- a/src/routes/auth/mfa/enable.ts +++ b/src/routes/auth/mfa/enable.ts @@ -1,4 +1,8 @@ -import { asyncWrapper, selectAccountByUserId } from '@shared/helpers' +import { + asyncWrapper, + selectAccountByUserId, + getPermissionVariablesFromCookie +} from '@shared/helpers' import { Request, Response } from 'express' import { updateOtpStatus } from '@shared/queries' @@ -6,12 +10,12 @@ import Boom from '@hapi/boom' import { authenticator } from 'otplib' import { mfaSchema } from '@shared/validation' import { request } from '@shared/request' -import { getClaims } from '@shared/jwt' -async function enableMfa({ headers, body }: Request, res: Response): Promise { - const { code } = await mfaSchema.validateAsync(body) +async function enableMfa(req: Request, res: Response): Promise { + const { code } = await mfaSchema.validateAsync(req.body) - const user_id = getClaims(headers.authorization)['x-hasura-user-id'] + const permission_variables = getPermissionVariablesFromCookie(req) + const user_id = permission_variables['user-id'] const { otp_secret, mfa_enabled } = await selectAccountByUserId(user_id) diff --git a/src/routes/auth/mfa/generate.ts b/src/routes/auth/mfa/generate.ts index 2835989..4fb112c 100644 --- a/src/routes/auth/mfa/generate.ts +++ b/src/routes/auth/mfa/generate.ts @@ -1,14 +1,13 @@ import { Request, Response } from 'express' -import { asyncWrapper, createQR } from '@shared/helpers' - +import { asyncWrapper, createQR, getPermissionVariablesFromCookie } from '@shared/helpers' import { OTP_ISSUER } from '@shared/config' import { authenticator } from 'otplib' import { request } from '@shared/request' import { updateOtpSecret } from '@shared/queries' -import { getClaims } from '@shared/jwt' -async function generateMfa({ headers }: Request, res: Response): Promise { - const user_id = getClaims(headers.authorization)['x-hasura-user-id'] +async function generateMfa(req: Request, res: Response): Promise { + const permission_variables = getPermissionVariablesFromCookie(req) + const user_id = permission_variables['user-id'] /** * Generate OTP secret and key URI. diff --git a/src/routes/auth/token/refresh.ts b/src/routes/auth/token/refresh.ts index 31cb6f9..5feb6ab 100644 --- a/src/routes/auth/token/refresh.ts +++ b/src/routes/auth/token/refresh.ts @@ -8,7 +8,8 @@ import { newJwtExpiry, newRefreshExpiry, createHasuraJwt, - setRefreshTokenAsCookie + setCookie, + generatePermissionVariables } from '@shared/jwt' import { request } from '@shared/request' import { v4 as uuidv4 } from 'uuid' @@ -54,7 +55,9 @@ async function refreshToken({ cookies, signedCookies }: Request, res: Response): throw Boom.badImplementation('Unable to set new refresh token') } - setRefreshTokenAsCookie(res, new_refresh_token) + const permission_variables = JSON.stringify(generatePermissionVariables(account)) + + setCookie(res, new_refresh_token, permission_variables) return res.send({ jwt_token: createHasuraJwt(account), diff --git a/src/routes/auth/token/revoke.ts b/src/routes/auth/token/revoke.ts index fe2bff7..8b38cd4 100644 --- a/src/routes/auth/token/revoke.ts +++ b/src/routes/auth/token/revoke.ts @@ -1,12 +1,11 @@ import { Request, Response } from 'express' - -import { asyncWrapper } from '@shared/helpers' +import { asyncWrapper, getPermissionVariablesFromCookie } from '@shared/helpers' import { deleteAllAccountRefreshTokens } from '@shared/queries' import { request } from '@shared/request' -import { getClaims } from '@shared/jwt' -async function revokeToken({ headers }: Request, res: Response): Promise { - const user_id = getClaims(headers.authorization)['x-hasura-user-id'] +async function revokeToken(req: Request, res: Response): Promise { + const permission_variables = getPermissionVariablesFromCookie(req) + const user_id = permission_variables['user-id'] await request(deleteAllAccountRefreshTokens, { user_id }) diff --git a/src/routes/storage/delete.ts b/src/routes/storage/delete.ts index 0259ba8..0e833da 100644 --- a/src/routes/storage/delete.ts +++ b/src/routes/storage/delete.ts @@ -1,6 +1,6 @@ import { NextFunction, Request, Response } from 'express' import { - StoragePermissions, + PathConfig, createContext, getHeadObject, getKey, @@ -16,7 +16,7 @@ export const deleteFile = async ( req: Request, res: Response, _next: NextFunction, - rules: Partial, + rules: Partial, isMetadataRequest = false ): Promise => { const headObject = await getHeadObject(req) diff --git a/src/routes/storage/get.ts b/src/routes/storage/get.ts index 5d70a58..2147fb6 100644 --- a/src/routes/storage/get.ts +++ b/src/routes/storage/get.ts @@ -1,5 +1,5 @@ import { NextFunction, Request, Response } from 'express' -import { StoragePermissions, createContext, getHeadObject, getKey, hasPermission } from './utils' +import { PathConfig, createContext, getHeadObject, getKey, hasPermission } from './utils' import Boom from '@hapi/boom' import { S3_BUCKET } from '@shared/config' @@ -9,7 +9,7 @@ export const getFile = async ( req: Request, res: Response, _next: NextFunction, - rules: Partial, + rules: Partial, isMetadataRequest = false ): Promise => { const key = getKey(req) @@ -24,7 +24,7 @@ export const getFile = async ( throw Boom.forbidden() } if (isMetadataRequest) { - return res.status(200).send(headObject) + return res.status(200).send({ key, ...headObject }) } else { const params = { Bucket: S3_BUCKET as string, diff --git a/src/routes/storage/index.ts b/src/routes/storage/index.ts index 18ba265..34727a9 100644 --- a/src/routes/storage/index.ts +++ b/src/routes/storage/index.ts @@ -1,60 +1,61 @@ -import { META_PREFIX, STORAGE_RULES, StoragePermissions, containsSomeRule } from './utils' +import { OBJECT_PREFIX, META_PREFIX, STORAGE_RULES, PathConfig, containsSomeRule } from './utils' import { NextFunction, Request, Response, Router } from 'express' import { deleteFile } from './delete' -import { getFile } from './get' import { listFile } from './list' +import { getFile } from './get' import { uploadFile } from './upload' const router = Router() const createSecureMiddleware = ( fn: Function, - rules: Partial, - isMetadataRequest: boolean, - metadataParams: object = {} + rules: Partial, + isMetadataRequest: boolean ) => (req: Request, res: Response, next: NextFunction): void => - fn(req, res, next, rules, isMetadataRequest, metadataParams).catch(next) + fn(req, res, next, rules, isMetadataRequest, rules.metadata).catch(next) const createRoutes = ( path: string, - rules: Partial, - isMetadataRequest = false, - metadataParams: object = {} + rules: Partial, + isMetadataRequest = false ): Router => { const middleware = Router() + + // write, create, update if (containsSomeRule(rules, ['write', 'create', 'update'])) { - middleware.post( - path, - createSecureMiddleware(uploadFile, rules, isMetadataRequest, metadataParams) - ) + middleware.post(path, createSecureMiddleware(uploadFile, rules, isMetadataRequest)) } - if (containsSomeRule(rules, ['read', 'get'])) { - middleware.get(path, createSecureMiddleware(getFile, rules, isMetadataRequest)) - } - if (containsSomeRule(rules, ['read', 'list'])) { - middleware.get( - `${path.substring(0, path.lastIndexOf('/'))}`, - createSecureMiddleware(listFile, rules, isMetadataRequest) - ) + + // read, get, list + if (containsSomeRule(rules, ['read', 'get', 'list'])) { + if (path.endsWith('/')) { + middleware.get(path, createSecureMiddleware(listFile, rules, isMetadataRequest)) + } else { + middleware.get(path, createSecureMiddleware(getFile, rules, isMetadataRequest)) + middleware.get( + path.substring(0, path.lastIndexOf('/')), + createSecureMiddleware(listFile, rules, isMetadataRequest) + ) + } } + + // write, delete if (containsSomeRule(rules, ['write', 'delete'])) { - middleware.delete( - path, - createSecureMiddleware(deleteFile, rules, isMetadataRequest, metadataParams) - ) + middleware.delete(path, createSecureMiddleware(deleteFile, rules, isMetadataRequest)) } + return middleware } for (const path in STORAGE_RULES.paths) { const rules = STORAGE_RULES.paths[path] - router.use(createRoutes(path, rules, false, rules.meta?.values)) - if ( - containsSomeRule(rules.meta, ['read', 'write', 'get', 'create', 'update', 'delete', 'list']) - ) { - router.use(META_PREFIX, createRoutes(path, rules.meta, true, rules.meta?.values)) - } + + // create object data paths + router.use(OBJECT_PREFIX, createRoutes(path, rules, false)) + + // create meta data paths + router.use(META_PREFIX, createRoutes(path, rules, true)) } export default router diff --git a/src/routes/storage/list.ts b/src/routes/storage/list.ts index bbef0cf..f405f9d 100644 --- a/src/routes/storage/list.ts +++ b/src/routes/storage/list.ts @@ -1,5 +1,5 @@ import { NextFunction, Request, Response } from 'express' -import { StoragePermissions, createContext, getKey, hasPermission } from './utils' +import { PathConfig, createContext, getKey, hasPermission } from './utils' import Boom from '@hapi/boom' import { S3_BUCKET } from '@shared/config' @@ -10,25 +10,20 @@ export const listFile = async ( req: Request, res: Response, _next: NextFunction, - rules: Partial, + rules: Partial, isMetadataRequest = false ): Promise => { const key = getKey(req) - try { - const context = createContext(req) - if (!hasPermission([rules.list, rules.read], context)) { - throw Boom.forbidden() - } - } catch (err) { - console.log( - 'List validation did not apply to a directory (without object context). Will anyway run validation for each object in the list.' - ) + const context = createContext(req) + if (!hasPermission([rules.list, rules.read], context)) { + throw Boom.forbidden() } const params = { Bucket: S3_BUCKET as string, Prefix: key.slice(0, -1) } const list = await s3.listObjectsV2(params).promise() + if (list.Contents) { const headObjectsList = ( await Promise.all( @@ -43,8 +38,16 @@ export const listFile = async ( })) ) ).filter((resource) => hasPermission([rules.list, rules.read], createContext(req, resource))) + if (isMetadataRequest) { - return res.status(200).send(headObjectsList.map((entry) => entry.head)) + return res.status(200).send( + headObjectsList.map((entry) => { + return { + Key: entry.key, + ...entry.head + } + }) + ) } else { const archive = archiver('zip') headObjectsList.forEach((entry) => { diff --git a/src/routes/storage/storage.test.ts b/src/routes/storage/storage.test.ts index cb70fe1..36f9664 100644 --- a/src/routes/storage/storage.test.ts +++ b/src/routes/storage/storage.test.ts @@ -18,8 +18,7 @@ it('should upload a new file', async () => { Metadata: { token, description } } } = await request - .post(`/storage/user/${getUserId()}/${filePath}`) - .set('Authorization', `Bearer ${account.token}`) + .post(`/storage/o/user/${getUserId()}/${filePath}`) .query({ description: initialDescription }) .attach('file', filePath) expect(status).toEqual(200) @@ -30,49 +29,53 @@ it('should upload a new file', async () => { it('should update an existing file', async () => { const { status } = await request - .post(`/storage/user/${getUserId()}/${filePath}`) - .set('Authorization', `Bearer ${account.token}`) + .post(`/storage/o/user/${getUserId()}/${filePath}`) .attach('file', filePath) expect(status).toEqual(200) }) it('should not update an hypothetical file of another hypothetical user', async () => { const { status } = await request - .post(`/storage/user/another-user/another-file`) - .set('Authorization', `Bearer ${account.token}`) + .post(`/storage/o/user/another-user/another-file`) .attach('file', filePath) expect(status).toEqual(403) }) -it('should get file from user authentication', async () => { - const { status, text } = await request - .get(`/storage/user/${getUserId()}/${filePath}`) - .set('Authorization', `Bearer ${account.token}`) +it('should get file', async () => { + const { status, text } = await request.get(`/storage/o/user/${getUserId()}/${filePath}`) const fileData = await readFile(filePath, 'utf8') expect(status).toEqual(200) expect(text).toEqual(fileData) }) -it('should get file from the token stored in the file metadata', async () => { - const { status, text } = await request - .get(`/storage/user/${getUserId()}/${filePath}`) - .query({ token: fileToken }) - const fileData = await readFile(filePath, 'utf8') - expect(status).toEqual(200) - expect(text).toEqual(fileData) -}) +describe('Tests as an unauthenticated user', () => { + beforeAll(async () => { + await request.post(`/auth/logout`) + }) -it('should not get file without authentication nor token', async () => { - const { status } = await request.get(`/storage/user/${getUserId()}/${filePath}`) - expect(status).toEqual(403) -}) + afterAll(async () => { + await request.post('/auth/login').send({ email: account.email, password: account.password }) + }) -// TODO attempt to get the file from another authenticated user + it('should get file from the token stored in the file metadata while unauthenticated', async () => { + const { status, text } = await request + .get(`/storage/o/user/${getUserId()}/${filePath}`) + .query({ token: fileToken }) + const fileData = await readFile(filePath, 'utf8') + expect(status).toEqual(200) + expect(text).toEqual(fileData) + }) + + it('should not get file without authentication nor token', async () => { + const { status } = await request.get(`/storage/o/user/${getUserId()}/${filePath}`) + expect(status).toEqual(403) + }) + // TODO attempt to get the file from another authenticated user +}) it(`should update an existing file's metadata`, async () => { const { status } = await request - .post(`/storage/meta/user/${getUserId()}/${filePath}`) - .set('Authorization', `Bearer ${account.token}`) + .post(`/storage/m/user/${getUserId()}/${filePath}`) .query({ description: newDescription }) expect(status).toEqual(200) }) @@ -83,44 +86,34 @@ it('should get file metadata', async () => { body: { Metadata: { filename, description } } - } = await request - .get(`/storage/meta/user/${getUserId()}/${filePath}`) - .set('Authorization', `Bearer ${account.token}`) + } = await request.get(`/storage/m/user/${getUserId()}/${filePath}`) expect(status).toEqual(200) expect(filename).toEqual(filePath) expect(description).toEqual(newDescription) }) it('should get the headers of all the user files', async () => { - const { status, body } = await request - .get(`/storage/meta/user/${getUserId()}/`) - .set('Authorization', `Bearer ${account.token}`) + const { status, body } = await request.get(`/storage/m/user/${getUserId()}/`) expect(status).toEqual(200) expect(body).toBeArrayOfSize(1) }) it('should get a zip that contains all user files', async () => { - const { status, text } = await request - .get(`/storage/user/${getUserId()}/`) - .set('Authorization', `Bearer ${account.token}`) + const { status, text } = await request.get(`/storage/m/user/${getUserId()}/`) expect(status).toEqual(200) expect(text).toBeTruthy() // TODO unzip and compare the file(s) }) it('should delete file metadata', async () => { - const { status } = await request - .delete(`/storage/meta/user/${getUserId()}/${filePath}`) - .set('Authorization', `Bearer ${account.token}`) + const { status } = await request.delete(`/storage/m/user/${getUserId()}/${filePath}`) expect(status).toEqual(204) const { status: getStatus, body: { Metadata: { filename, token, description } } - } = await request - .get(`/storage/meta/user/${getUserId()}/${filePath}`) - .set('Authorization', `Bearer ${account.token}`) + } = await request.get(`/storage/m/user/${getUserId()}/${filePath}`) expect(getStatus).toEqual(200) expect(filename).toBeUndefined() expect(token).toBeString() @@ -128,8 +121,6 @@ it('should delete file metadata', async () => { }) it('should delete file', async () => { - const { status } = await request - .delete(`/storage/user/${getUserId()}/${filePath}`) - .set('Authorization', `Bearer ${account.token}`) + const { status } = await request.delete(`/storage/m/user/${getUserId()}/${filePath}`) expect(status).toEqual(204) }) diff --git a/src/routes/storage/upload.ts b/src/routes/storage/upload.ts index 5be2989..c7f0339 100644 --- a/src/routes/storage/upload.ts +++ b/src/routes/storage/upload.ts @@ -1,7 +1,7 @@ import { NextFunction, Request, Response } from 'express' import { v4 as uuidv4 } from 'uuid' import { - StoragePermissions, + PathConfig, createContext, generateMetadata, getHeadObject, @@ -19,7 +19,7 @@ export const uploadFile = async ( req: Request, res: Response, _next: NextFunction, - rules: Partial, + rules: Partial, isMetadataRequest = false, metadata: object = {} ): Promise => { @@ -56,6 +56,7 @@ export const uploadFile = async ( try { await s3.upload(upload_params).promise() } catch (err) { + console.error(err) throw Boom.badImplementation('Impossible to create or update the object.') } } else if (!isNew) { diff --git a/src/routes/storage/utils.ts b/src/routes/storage/utils.ts index 325bda6..8a6612b 100644 --- a/src/routes/storage/utils.ts +++ b/src/routes/storage/utils.ts @@ -1,4 +1,3 @@ -import { getClaims } from '@shared/jwt' import safeEval, { FunctionFactory } from 'notevil' import { v4 as uuidv4 } from 'uuid' @@ -10,10 +9,12 @@ import fs from 'fs' import path from 'path' import { s3 } from '@shared/s3' import yaml from 'js-yaml' -import { Claims } from '@shared/types' +import { PermissionVariables } from '@shared/types' +import { getPermissionVariablesFromCookie } from '@shared/helpers' -export const META_PREFIX = '/meta' -export interface StoragePermissions { +export const OBJECT_PREFIX = '/o' +export const META_PREFIX = '/m' +export interface PathConfig { read: string write: string get: string @@ -21,14 +22,13 @@ export interface StoragePermissions { create: string update: string delete: string + metadata?: { [key: string]: string } } interface StorageRules { functions?: { [key: string]: string | { params: string[]; code: string } } paths: { - [key: string]: Partial & { - meta: Partial & { values?: { [key: string]: string } } - } + [key: string]: Partial } } @@ -37,11 +37,11 @@ interface StorageRequest { query: unknown method: string params?: string - auth?: Claims + auth?: PermissionVariables } export const containsSomeRule = ( - rulesDefinition: Partial = {}, + rulesDefinition: Partial = {}, rules: (string | undefined)[] ): boolean => Object.keys(rulesDefinition).some((rule) => rules.includes(rule)) @@ -86,10 +86,11 @@ export const createContext = ( ): object => { let auth try { - auth = getClaims(req.headers.authorization) - } catch { + auth = getPermissionVariablesFromCookie(req) + } catch (err) { auth = undefined } + const variables: StorageContext = { request: { path: req.path, @@ -100,12 +101,15 @@ export const createContext = ( ...req.params, resource: s3HeadObject } + const functions = storageFunctions(variables) + return { ...functions, ...variables } } -export const hasPermission = (rules: (string | undefined)[], context: object): boolean => - rules.some((rule) => rule && !!safeEval(rule, context)) +export const hasPermission = (rules: (string | undefined)[], context: object): boolean => { + return rules.some((rule) => rule && !!safeEval(rule, context)) +} export const generateMetadata = (metadataParams: object, context: object): object => Object.entries(metadataParams).reduce<{ [key: string]: unknown }>((aggr, [key, jsCode]) => { diff --git a/src/shared/helpers.ts b/src/shared/helpers.ts index bfe90c1..cf4dc7f 100644 --- a/src/shared/helpers.ts +++ b/src/shared/helpers.ts @@ -1,4 +1,4 @@ -import { HIBP_ENABLE } from './config' +import { HIBP_ENABLE, COOKIE_SECRET } from './config' import { NextFunction, Request, Response } from 'express' import { rotateTicket as rotateTicketQuery, @@ -13,7 +13,7 @@ import bcrypt from 'bcryptjs' import { pwnedPassword } from 'hibp' import { request } from './request' import { v4 as uuidv4 } from 'uuid' -import { AccountData, QueryAccountData } from './types' +import { AccountData, QueryAccountData, PermissionVariables } from './types' /** * Create QR code. @@ -112,3 +112,9 @@ export const rotateTicket = async (ticket: string): Promise => { }) return new_ticket } + +export const getPermissionVariablesFromCookie = (req: Request): PermissionVariables => { + const { permission_variables } = COOKIE_SECRET ? req.signedCookies : req.cookies + if (!permission_variables) throw Boom.unauthorized() + return JSON.parse(permission_variables) +} diff --git a/src/shared/jwt.ts b/src/shared/jwt.ts index 85ef7d5..56aaab1 100644 --- a/src/shared/jwt.ts +++ b/src/shared/jwt.ts @@ -7,7 +7,8 @@ import { JWT_CLAIMS_NAMESPACE, JWT_REFRESH_EXPIRES_IN, DEFAULT_USER_ROLE, - DEFAULT_ANONYMOUS_ROLE + DEFAULT_ANONYMOUS_ROLE, + JWT_CUSTOM_FIELDS } from './config' import { JWK, JWKS, JWT } from 'jose' @@ -20,6 +21,12 @@ import { v4 as uuidv4 } from 'uuid' import kebabCase from 'lodash.kebabcase' import { Claims, Token, AccountData, ClaimValueType } from './types' +interface InsertRefreshTokenData { + insert_auth_refresh_tokens_one: { + account: AccountData + } +} + const RSA_TYPES = ['RS256', 'RS384', 'RS512'] const SHA_TYPES = ['HS256', 'HS384', 'HS512'] @@ -58,6 +65,40 @@ if (RSA_TYPES.includes(JWT_ALGORITHM)) { export const newJwtExpiry = JWT_EXPIRES_IN * 60 * 1000 +/** + * Create an object that contains all the permission variables of the user, + * i.e. user-id, allowed-roles, default-role and the kebab-cased columns + * of the public.tables columns defined in JWT_CUSTOM_FIELDS + * @param jwt if true, add a 'x-hasura-' prefix to the property names, and stringifies the values (required by Hasura) + */ +export function generatePermissionVariables( + { default_role, account_roles = [], user }: AccountData, + jwt = false +): { [key: string]: ClaimValueType } { + const prefix = jwt ? 'x-hasura-' : '' + const role = user.is_anonymous ? DEFAULT_ANONYMOUS_ROLE : default_role || DEFAULT_USER_ROLE + const accountRoles = account_roles.map(({ role: roleName }) => roleName) + + if (!accountRoles.includes(role)) { + accountRoles.push(role) + } + + return { + [`${prefix}user-id`]: user.id, + [`${prefix}allowed-roles`]: accountRoles, + [`${prefix}default-role`]: role, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...JWT_CUSTOM_FIELDS.reduce<{ [key: string]: ClaimValueType }>((aggr: any, cursor) => { + aggr[`${prefix}${kebabCase(cursor)}`] = jwt + ? typeof user[cursor] === 'string' + ? user[cursor] + : JSON.stringify(user[cursor] ?? null) + : user[cursor] + return aggr + }, {}) + } +} + /** * * Creates a JWKS store. Only works with RSA algorithms. Raises an error otherwise * @returns JWKS store @@ -111,7 +152,11 @@ export function newRefreshExpiry(): number { * @param res Express Response * @param refresh_token Refresh token to be set */ -export const setRefreshTokenAsCookie = (res: Response, refresh_token: string): void => { +export const setCookie = ( + res: Response, + refresh_token: string, + permission_variables: string +): void => { // converting JWT_REFRESH_EXPIRES_IN from minutes to milliseconds const maxAge = JWT_REFRESH_EXPIRES_IN * 60 * 1000 @@ -121,6 +166,13 @@ export const setRefreshTokenAsCookie = (res: Response, refresh_token: string): v maxAge, signed: Boolean(COOKIE_SECRET) }) + + // set permission variables cookie + res.cookie('permission_variables', permission_variables, { + httpOnly: true, + maxAge, + signed: Boolean(COOKIE_SECRET) + }) } /** @@ -137,46 +189,26 @@ export const setRefreshToken = async ( if (!refresh_token) { refresh_token = uuidv4() } - await request(insertRefreshToken, { + + const insert_account_data = (await request(insertRefreshToken, { refresh_token_data: { account_id: accountId, refresh_token, expires_at: new Date(newRefreshExpiry()) } - }) + })) as InsertRefreshTokenData - setRefreshTokenAsCookie(res, refresh_token) + const { account } = insert_account_data.insert_auth_refresh_tokens_one + + const permission_variables = JSON.stringify(generatePermissionVariables(account)) + + setCookie(res, refresh_token, permission_variables) } /** * Create JWT token. - * @param id Required v4 UUID string. - * @param defaultRole Defaults to "user". - * @param roles Defaults to ["user"]. */ -export function createHasuraJwt({ default_role, account_roles = [], user }: AccountData): string { - const role = user.is_anonymous ? DEFAULT_ANONYMOUS_ROLE : default_role || DEFAULT_USER_ROLE - const accountRoles = account_roles.map(({ role: roleName }) => roleName) - - if (!accountRoles.includes(role)) { - accountRoles.push(role) - } - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { id, ...customFields } = user - return sign({ - [JWT_CLAIMS_NAMESPACE]: { - 'x-hasura-user-id': id, - 'x-hasura-allowed-roles': accountRoles, - 'x-hasura-default-role': role, - // Add custom fields based on the user fields fetched from the GQL query - ...Object.entries(customFields).reduce<{ [k: string]: ClaimValueType }>( - (aggr, [key, value]) => ({ - ...aggr, - [`x-hasura-${kebabCase(key)}`]: - typeof value === 'string' ? value : JSON.stringify(value ?? null) - }), - {} - ) - } +export const createHasuraJwt = (accountData: AccountData): string => + sign({ + [JWT_CLAIMS_NAMESPACE]: generatePermissionVariables(accountData, true) }) -} diff --git a/src/shared/queries.ts b/src/shared/queries.ts index c48e0f0..3cfb538 100644 --- a/src/shared/queries.ts +++ b/src/shared/queries.ts @@ -97,10 +97,13 @@ export const selectAccountByTicket = gql` export const insertRefreshToken = gql` mutation($refresh_token_data: auth_refresh_tokens_insert_input!) { - insert_auth_refresh_tokens(objects: [$refresh_token_data]) { - affected_rows + insert_auth_refresh_tokens_one(object: $refresh_token_data) { + account { + ...accountFragment + } } } + ${accountFragment} ` export const selectRefreshToken = gql` diff --git a/src/shared/types.ts b/src/shared/types.ts index 71893d3..19ee9d4 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -20,6 +20,16 @@ export interface Claims { [key: string]: ClaimValueType } +/** + * PermissionVariables interface. + */ +export interface PermissionVariables { + 'user-id': string + 'default-role': string + 'allowed-roles': string[] + [key: string]: ClaimValueType +} + /** * Token interface. */ diff --git a/src/test/test-utils.ts b/src/test/test-utils.ts index c72f24f..578fd5b 100644 --- a/src/test/test-utils.ts +++ b/src/test/test-utils.ts @@ -95,7 +95,7 @@ export const registerAccount = async (agent: SuperTest): Promise => { // * Delete the account - await agent.post('/auth/account/delete').set('Authorization', `Bearer ${account.token}`) + await agent.post('/auth/delete') // * Remove any message sent to this account await deleteEmailsOfAccount(account.email) } diff --git a/yarn.lock b/yarn.lock index d14c792..a08088d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5372,10 +5372,9 @@ hash.js@^1.0.0, hash.js@^1.0.3: inherits "^2.0.3" minimalistic-assert "^1.0.1" -hasura-cli@1.2.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/hasura-cli/-/hasura-cli-1.2.0.tgz#a1ca61266cbc57141ce4b1d18b230b24b4b32106" - integrity sha512-WIpiELAP1rcOF7F5yIRUiWzfF3E6j6mRKxCgc97+OU/X6NiNqTdGYU6FwzEJQ45Iw0AddBPOiwgZFvynN8zUAA== +hasura-cli@1.2.1: + version "1.2.1" + resolved "https://registry.yarnpkg.com/hasura-cli/-/hasura-cli-1.2.1.tgz#f25d94d3c10b074ae41346b58e9fdee1529b7bfe" dependencies: axios "^0.19.0" chalk "^2.4.2"