Skip to content

Commit

Permalink
Merge pull request #135 from HHS/lock-user-access
Browse files Browse the repository at this point in the history
Lock user access
  • Loading branch information
rahearn authored Jan 29, 2021
2 parents 6f02a27 + 95a8339 commit 4c3a436
Show file tree
Hide file tree
Showing 11 changed files with 205 additions and 94 deletions.
5 changes: 5 additions & 0 deletions frontend/src/Constants.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export const SCOPE_IDS = {
SITE_ACCESS: 1,
ADMIN: 2,
READ_WRITE_ACTIVITY_REPORTS: 3,
READ_ACTIVITY_REPORTS: 4,
Expand All @@ -21,6 +22,10 @@ export const REGIONAL_SCOPES = {
};

export const GLOBAL_SCOPES = {
[SCOPE_IDS.SITE_ACCESS]: {
name: 'SITE_ACCESS',
description: 'User can login and view the TTAHUB site',
},
[SCOPE_IDS.ADMIN]: {
name: 'ADMIN',
description: 'User can view the admin panel and change user permissions (including their own)',
Expand Down
11 changes: 11 additions & 0 deletions frontend/src/__tests__/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,15 @@ describe('App', () => {
expect(await screen.findByText('HSES Login')).toBeVisible();
});
});

describe('when user is locked', () => {
beforeEach(() => {
fetchMock.get(userUrl, 403);
render(<App />);
});

it('displays the login button for now. in the future this should show the "request permissions" UI', async () => {
expect(await screen.findByText('HSES Login')).toBeVisible();
});
});
});
12 changes: 10 additions & 2 deletions src/middleware/authMiddleware.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {} from 'dotenv/config';
import ClientOAuth2 from 'client-oauth2';
import { auditLogger } from '../logger';
import { validateUserAuthForAccess } from '../services/accessValidation';

export const hsesAuth = new ClientOAuth2({
clientId: process.env.AUTH_CLIENT_ID,
Expand Down Expand Up @@ -45,9 +46,16 @@ export default async function authMiddleware(req, res, next) {
auditLogger.warn(`Bypassing authentication in authMiddleware - using User ${process.env.CURRENT_USER_ID}`);
req.session.userId = process.env.CURRENT_USER_ID;
}
if (!req.session.userId) {
let userId = null;
if (req.session) {
userId = req.session.userId;
}
if (!userId) {
res.sendStatus(401);
} else {
} else if (await validateUserAuthForAccess(userId)) {
next();
} else {
auditLogger.warn(`User ${userId} denied access due to missing SITE_ACCESS`);
res.sendStatus(403);
}
}
35 changes: 33 additions & 2 deletions src/middleware/authMiddleware.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {} from 'dotenv/config';
import { UNAUTHORIZED } from 'http-codes';

import db, { User, Permission, sequelize } from '../models';
import authMiddleware, { login } from './authMiddleware';
import SCOPES from './scopeConstants';

describe('authMiddleware', () => {
const ORIGINAL_ENV = process.env;
Expand All @@ -13,12 +14,36 @@ describe('authMiddleware', () => {

afterAll(() => {
process.env = ORIGINAL_ENV; // restore original env
db.sequelize.close();
});

const mockUser = {
id: 63,
name: 'Auth Middleware',
hsesUserId: '63',
permissions: [{
userId: 63,
regionId: 14,
scopeId: SCOPES.SITE_ACCESS,
}],
};

const setupUser = async (user) => (
sequelize.transaction(async (transaction) => {
await User.destroy({ where: { id: user.id } }, { transaction });
await User.create(user, {
include: [{ model: Permission, as: 'permissions' }],
transaction,
});
})
);

it('should allow access if user data is present', async () => {
await setupUser(mockUser);

const mockNext = jest.fn();
const mockSession = jest.fn();
mockSession.userId = 2;
mockSession.userId = mockUser.id;
const mockRequest = {
path: '/api/endpoint',
session: mockSession,
Expand Down Expand Up @@ -77,6 +102,12 @@ describe('authMiddleware', () => {

it('bypass authorization if variables are set for UAT or accessibility testing', async () => {
// auth is bypassed if non-prod NODE_ENV and BYPASS_AUTH = 'true', needed for cucumber and axe
const user = {
...mockUser,
id: process.env.CURRENT_USER_ID,
hsesUserId: process.env.CURRENT_USER_ID,
};
await setupUser(user);
process.env.NODE_ENV = 'development';
process.env.BYPASS_AUTH = 'true';

Expand Down
2 changes: 1 addition & 1 deletion src/middleware/userAdminAccessMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import handleErrors from '../lib/apiErrorHandler';
export default async function userAdminAccessMiddleware(req, res, next) {
try {
const { userId } = req.session;
if ((await validateUserAuthForAdmin(req))) {
if ((await validateUserAuthForAdmin(userId))) {
auditLogger.info(`User ${userId} successfully checked ADMIN access`);
} else {
auditLogger.error(`User ${userId} attempted to access an ADMIN route without permission`);
Expand Down
11 changes: 8 additions & 3 deletions src/routes/files/handlers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,24 @@ const ORIGINAL_ENV = process.env;
jest.mock('../../lib/s3Uploader');

const mockUser = {
id: 200,
id: process.env.CURRENT_USER_ID,
homeRegionId: 1,
permissions: [
{
userId: 200,
userId: process.env.CURRENT_USER_ID,
regionId: 5,
scopeId: SCOPES.READ_WRITE_REPORTS,
},
{
userId: 200,
userId: process.env.CURRENT_USER_ID,
regionId: 6,
scopeId: SCOPES.READ_WRITE_REPORTS,
},
{
userId: process.env.CURRENT_USER_ID,
regionId: 14,
scopeId: SCOPES.SITE_ACCESS,
},
],
};

Expand Down
46 changes: 35 additions & 11 deletions src/seeders/20201124160449-users.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
const SCOPES = {
SITE_ACCESS: 1,
ADMIN: 2,
READ_WRITE_REPORTS: 3,
READ_REPORTS: 4,
APPROVE_REPORTS: 5,
};

const {
ADMIN, READ_WRITE_REPORTS, READ_REPORTS, APPROVE_REPORTS,
} = SCOPES;
const SITE_ACCESS = 1;
const ADMIN = 2;
const READ_WRITE_REPORTS = 3;
const READ_REPORTS = 4;
const APPROVE_REPORTS = 5;

const permissions = [
{
userId: 1,
scopeId: SITE_ACCESS,
regionId: 14,
},
{
userId: 1,
scopeId: ADMIN,
regionId: 14,
},
{
userId: 3,
scopeId: SITE_ACCESS,
regionId: 14,
},
{
userId: 3,
regionId: 2,
Expand All @@ -36,6 +40,11 @@ const permissions = [
regionId: 3,
scopeId: READ_WRITE_REPORTS,
},
{
userId: 4,
scopeId: SITE_ACCESS,
regionId: 14,
},
{
userId: 4,
regionId: 14,
Expand All @@ -51,16 +60,31 @@ const permissions = [
regionId: 4,
scopeId: APPROVE_REPORTS,
},
{
userId: 5,
scopeId: SITE_ACCESS,
regionId: 14,
},
{
userId: 5,
regionId: 1,
scopeId: READ_WRITE_REPORTS,
},
{
userId: 6,
scopeId: SITE_ACCESS,
regionId: 14,
},
{
userId: 6,
regionId: 14,
scopeId: ADMIN,
},
{
userId: 7,
scopeId: SITE_ACCESS,
regionId: 14,
},
{
userId: 7,
regionId: 14,
Expand Down
30 changes: 23 additions & 7 deletions src/services/accessValidation.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { User, Permission, sequelize } from '../models';
import { auditLogger as logger } from '../logger';
import SCOPES from '../middleware/scopeConstants';

const { ADMIN } = SCOPES;
const { SITE_ACCESS, ADMIN } = SCOPES;

const namespace = 'SERVICE:ACCESS_VALIDATION';

Expand Down Expand Up @@ -36,16 +36,32 @@ export default function findOrCreateUser(data) {
}));
}

export async function validateUserAuthForAdmin(req) {
const { userId } = req.session;
export async function validateUserAuthForAccess(userId) {
try {
const userPermissions = await Permission.findAll({
attributes: ['scopeId'],
where: { userId },
const userPermission = await Permission.findOne({
where: {
userId,
scopeId: SITE_ACCESS,
},
});
return userPermissions.some((permission) => (permission.scopeId === ADMIN));
return userPermission !== null;
} catch (error) {
logger.error(`${JSON.stringify({ ...logContext })} - Access error - ${error}`);
throw error;
}
}

export async function validateUserAuthForAdmin(userId) {
try {
const userPermission = await Permission.findOne({
where: {
userId,
scopeId: ADMIN,
},
});
return userPermission !== null;
} catch (error) {
logger.error(`${JSON.stringify({ ...logContext })} - ADMIN Access error - ${error}`);
throw error;
}
}
Loading

0 comments on commit 4c3a436

Please sign in to comment.