Skip to content

Commit

Permalink
BHBC-985: System user role updates (#240)
Browse files Browse the repository at this point in the history
* BHBC-985: System user role updates
- Add support for authorization in API
- Add support for database connections with the API's user in context
- Fixed bug where BCEID users weren't being parsed correctly in db.ts
- Add systemRole support to useKeycloakWrapper.tsx
- Add endpoint to fetch user information
- Incorporated Charlie's context updates to allow the API to be set as the database context user.

* BHBC-985: Updates to sql and api role checks

* - Review updates
- Remove system role checking in APi until we are ready to enforce it
  • Loading branch information
NickPhura authored Apr 20, 2021
1 parent 8b28e59 commit 01bcacd
Show file tree
Hide file tree
Showing 24 changed files with 821 additions and 126 deletions.
9 changes: 0 additions & 9 deletions api/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
"@istanbuljs/nyc-config-typescript": "~1.0.1",
"@types/chai": "~4.2.12",
"@types/express": "~4.17.0",
"@types/express-openapi": "~1.9.0",
"@types/geojson": "~7946.0.3",
"@types/gulp": "~4.0.6",
"@types/jsonwebtoken": "~8.5.0",
Expand Down
6 changes: 3 additions & 3 deletions api/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import multer from 'multer';
import { OpenAPI } from 'openapi-types';
import { rootAPIDoc } from './openapi/root-api-doc';
import { applyApiDocSecurityFilters } from './security/api-doc-security-filter';
import { authenticate } from './security/auth-utils';
import { authenticate, authorize } from './security/auth-utils';
import { getLogger } from './utils/logger';

const defaultLog = getLogger('app');
Expand Down Expand Up @@ -53,8 +53,8 @@ initialize({
},
securityHandlers: {
// applies authentication logic
Bearer: function (req, scopes) {
return authenticate(req, scopes);
Bearer: async function (req: any, scopes: string[]) {
return (await authenticate(req)) && authorize(req, scopes);
}
},
securityFilter: async (req, res) => {
Expand Down
3 changes: 2 additions & 1 deletion api/src/constants/database.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export enum SYSTEM_USER_TYPE {
export enum SYSTEM_IDENTITY_SOURCE {
DATABASE = 'DATABASE',
IDIR = 'IDIR',
BCEID = 'BCEID'
}
30 changes: 19 additions & 11 deletions api/src/database/db.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Pool, PoolClient, PoolConfig, QueryResult } from 'pg';
import { SYSTEM_USER_TYPE } from '../constants/database';
import { HTTP400, HTTP500 } from '../errors/CustomError';
import { setSystemUserContextSQL } from '../queries/user-context-queries';
import { getUserIdentifier, getUserIdentitySource } from '../utils/keycloak-utils';
import { getLogger } from '../utils/logger';
import { HTTP400, HTTP500 } from '../errors/CustomError';

const defaultLog = getLogger('database/db');

Expand Down Expand Up @@ -204,19 +204,15 @@ export const getDBConnection = function (keycloakToken: object): IDBConnection {
* Sets the _systemUserId if successful.
*/
const _setUserContext = async () => {
// Strip the `@<alias>` from the end of the username, which is added in keycloak to IDIR and BCeID usernames
const idir = _token?.['preferred_username']?.split('@')[0];
const bceid = _token?.['preferred_username']?.split('@')[0];
const userIdentifier = getUserIdentifier(_token);
const userIdentitySource = getUserIdentitySource(_token);

if (!idir && !bceid) {
throw new HTTP400('Failed to identify user ID');
if (!userIdentifier || !userIdentitySource) {
throw new HTTP400('Failed to identify authenticated user');
}

const userIdentifier = idir || bceid;
const systemUserType = (idir && SYSTEM_USER_TYPE.IDIR) || (bceid && SYSTEM_USER_TYPE.BCEID) || null;

// Set the user context for all queries made using this connection
const setSystemUserContextSQLStatement = setSystemUserContextSQL(userIdentifier, systemUserType);
const setSystemUserContextSQLStatement = setSystemUserContextSQL(userIdentifier, userIdentitySource);

if (!setSystemUserContextSQLStatement) {
throw new HTTP400('Failed to build SQL user context statement');
Expand All @@ -243,3 +239,15 @@ export const getDBConnection = function (keycloakToken: object): IDBConnection {
systemUserId: _getSystemUserID
};
};

/**
* Returns an IDBConnection where the system user context is set to the API's system user.
*
* Note: Use of this should be limited to requests that are impossible to initiated under a real user context (ie: when
* an unknown user is requesting access to BioHub and therefore does not yet have a user in the system).
*
* @return {*} {IDBConnection}
*/
export const getAPIUserDBConnection = (): IDBConnection => {
return getDBConnection({ preferred_username: 'biohub_api@database' });
};
69 changes: 69 additions & 0 deletions api/src/models/user.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { expect } from 'chai';
import { describe } from 'mocha';
import { UserObject } from './user';

describe('UserObject', () => {
describe('No values provided', () => {
let data: UserObject;

before(() => {
data = new UserObject((null as unknown) as any);
});

it('sets id', function () {
expect(data.id).to.equal(null);
});

it('sets user_identifier', function () {
expect(data.user_identifier).to.equal(null);
});

it('sets role_names', function () {
expect(data.role_names).to.eql([]);
});
});

describe('valid values provided, no roles', () => {
let data: UserObject;

const userObject = { id: 1, user_identifier: 'test name', role_names: [] };

before(() => {
data = new UserObject(userObject);
});

it('sets id', function () {
expect(data.id).to.equal(1);
});

it('sets user_identifier', function () {
expect(data.user_identifier).to.equal('test name');
});

it('sets role_names', function () {
expect(data.role_names).to.eql([]);
});
});

describe('valid values provided', () => {
let data: UserObject;

const userObject = { id: 1, user_identifier: 'test name', role_names: ['role 1', 'role 2'] };

before(() => {
data = new UserObject(userObject);
});

it('sets id', function () {
expect(data.id).to.equal(1);
});

it('sets user_identifier', function () {
expect(data.user_identifier).to.equal('test name');
});

it('sets role_names', function () {
expect(data.role_names).to.eql(['role 1', 'role 2']);
});
});
});
17 changes: 17 additions & 0 deletions api/src/models/user.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { getLogger } from '../utils/logger';

const defaultLog = getLogger('models/user');

export class UserObject {
id: number;
user_identifier: string;
role_names: string[];

constructor(obj?: any) {
defaultLog.debug({ label: 'UserObject', message: 'params', obj });

this.id = obj?.id || null;
this.user_identifier = obj?.user_identifier || null;
this.role_names = (obj?.role_names?.length && obj.role_names) || [];
}
}
93 changes: 93 additions & 0 deletions api/src/paths/user.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import { RequestHandler } from 'express';
import { Operation } from 'express-openapi';
import { READ_ROLES } from '../constants/roles';
import { getDBConnection } from '../database/db';
import { HTTP400 } from '../errors/CustomError';
import { getUserByIdSQL } from '../queries/users/user-queries';
import { getLogger } from '../utils/logger';
import { logRequest } from '../utils/path-utils';

const defaultLog = getLogger('paths/user');

export const GET: Operation = [logRequest('paths/user', 'GET'), getUser()];

GET.apiDoc = {
description: 'Get user details for the currently authenticated user.',
tags: ['user'],
security: [
{
Bearer: READ_ROLES
}
],
responses: {
200: {
description: 'User details for the currently authenticated user.',
content: {
'application/json': {
schema: {
title: 'User Response Object',
type: 'object',
properties: {
// TODO needs finalizing (here and in the user-queries.ts SQL)
}
}
}
}
},
400: {
$ref: '#/components/responses/400'
},
401: {
$ref: '#/components/responses/401'
},
403: {
$ref: '#/components/responses/401'
},
500: {
$ref: '#/components/responses/500'
},
default: {
$ref: '#/components/responses/default'
}
}
};

/**
* Get a user by its user identifier.
*
* @returns {RequestHandler}
*/
function getUser(): RequestHandler {
return async (req, res) => {
const connection = getDBConnection(req['keycloak_token']);

try {
await connection.open();

const systemUserId = connection.systemUserId();

if (!systemUserId) {
throw new HTTP400('Failed to identify system user ID');
}

const getUserSQLStatement = getUserByIdSQL(systemUserId);

if (!getUserSQLStatement) {
throw new HTTP400('Failed to build SQL get statement');
}

const response = await connection.query(getUserSQLStatement.text, getUserSQLStatement.values);

await connection.commit();

const result = (response && response.rows && response.rows[0]) || null;

return res.status(200).json(result);
} catch (error) {
defaultLog.debug({ label: 'getUser', message: 'error', error });
throw error;
} finally {
connection.release();
}
};
}
8 changes: 4 additions & 4 deletions api/src/queries/user-context-queries.test.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
import { expect } from 'chai';
import { describe } from 'mocha';
import { SYSTEM_USER_TYPE } from '../constants/database';
import { SYSTEM_IDENTITY_SOURCE } from '../constants/database';
import { setSystemUserContextSQL } from './user-context-queries';

describe('setSystemUserContextSQL', () => {
it('has empty userIdentifier', () => {
const response = setSystemUserContextSQL('', SYSTEM_USER_TYPE.IDIR);
const response = setSystemUserContextSQL('', SYSTEM_IDENTITY_SOURCE.IDIR);

expect(response).to.be.null;
});

it('identifies an IDIR user', () => {
const response = setSystemUserContextSQL('idir-user', SYSTEM_USER_TYPE.IDIR);
const response = setSystemUserContextSQL('idir-user', SYSTEM_IDENTITY_SOURCE.IDIR);

expect(response).not.to.be.null;
});

it('identifies a BCEID user', () => {
const response = setSystemUserContextSQL('bceid-user', SYSTEM_USER_TYPE.BCEID);
const response = setSystemUserContextSQL('bceid-user', SYSTEM_IDENTITY_SOURCE.BCEID);

expect(response).not.to.be.null;
});
Expand Down
14 changes: 4 additions & 10 deletions api/src/queries/user-context-queries.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,20 @@
import { SQL, SQLStatement } from 'sql-template-strings';
import { SYSTEM_USER_TYPE } from '../constants/database';
import { SYSTEM_IDENTITY_SOURCE } from '../constants/database';
import { getLogger } from '../utils/logger';

const defaultLog = getLogger('queries/template-queries');
const defaultLog = getLogger('queries/user-context-queries');

export const setSystemUserContextSQL = (
userIdentifier: string,
systemUserType: SYSTEM_USER_TYPE
systemUserType: SYSTEM_IDENTITY_SOURCE
): SQLStatement | null => {
defaultLog.debug({ label: 'setSystemUserContextSQL', message: 'params', userIdentifier, systemUserType });

if (!userIdentifier) {
return null;
}

let sqlStatement;

if (systemUserType === SYSTEM_USER_TYPE.IDIR) {
sqlStatement = SQL`select api_set_context(${userIdentifier}, null);`;
} else {
sqlStatement = SQL`select api_set_context(null, ${userIdentifier});`;
}
const sqlStatement = SQL`select api_set_context(${userIdentifier}, ${systemUserType});`;

defaultLog.debug({
label: 'postTemplateSQL',
Expand Down
31 changes: 31 additions & 0 deletions api/src/queries/users/user-queries.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { expect } from 'chai';
import { describe } from 'mocha';
import { getUserByIdSQL, getUserByUserIdentifierSQL } from './user-queries';

describe('getUserByUserIdentifierSQL', () => {
it('returns null response when null userIdentifier provided', () => {
const response = getUserByUserIdentifierSQL((null as unknown) as string);

expect(response).to.be.null;
});

it('returns non null response when valid userIdentifier provided', () => {
const response = getUserByUserIdentifierSQL('aUserName');

expect(response).to.not.be.null;
});
});

describe('getUserByIdSQL', () => {
it('returns null response when null userId provided', () => {
const response = getUserByIdSQL((null as unknown) as number);

expect(response).to.be.null;
});

it('returns non null response when valid userId provided', () => {
const response = getUserByIdSQL(1);

expect(response).to.not.be.null;
});
});
Loading

0 comments on commit 01bcacd

Please sign in to comment.