Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Techdebt: Extending Express Request Object + Fixing type errors #1327

Merged
merged 15 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions api/src/__mocks__/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,6 @@ export class MockReq {
query = {};
params = {};
body = {};
files: any[] = [];
// Exists on authenticated requests. @see authentication.ts and authorization.ts
keycloak_token?: Record<string, any>;
// Exists on authenticated requests. @see authentication.ts and authorization.ts
system_user?: Record<string, any>;
}

export type ExtendedMockRes = MockRes & Response;
Expand Down
4 changes: 2 additions & 2 deletions api/src/database/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ export interface IDBConnection {
*
* const sqlStatement = SQL\`select * from table where id = ${id};\`;
*
* const connection = await getDBConnection(req['keycloak_token']);
* const connection = await getDBConnection(req.keycloak_token);
*
* try {
* await connection.open();
Expand All @@ -225,7 +225,7 @@ export interface IDBConnection {
* @param {object} keycloakToken
* @return {*} {IDBConnection}
*/
export const getDBConnection = function (keycloakToken: KeycloakUserInformation): IDBConnection {
export const getDBConnection = function (keycloakToken?: KeycloakUserInformation): IDBConnection {
if (!keycloakToken) {
throw Error('Keycloak token is undefined');
}
Expand Down
6 changes: 3 additions & 3 deletions api/src/middleware/critterbase-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export const getCritterbaseApiHostUrl = () => {
export const authorizeAndAuthenticateMiddleware: RequestHandler = async (req, _, next) => {
await authenticateRequest(req);

req['authorization_scheme'] = {
req.authorization_scheme = {
and: [
{
discriminator: 'SystemUser'
Expand Down Expand Up @@ -165,8 +165,8 @@ export const getCritterbaseProxyMiddleware = () =>
client.setHeader(
'user',
JSON.stringify({
keycloak_guid: req['system_user']?.user_guid,
username: req['system_user']?.user_identifier
keycloak_guid: req.system_user?.user_guid,
username: req.system_user?.user_identifier
})
);
},
Expand Down
2 changes: 1 addition & 1 deletion api/src/paths/administrative-activities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ GET.apiDoc = {
*/
export function getAdministrativeActivities(): RequestHandler {
return async (req, res) => {
const connection = getDBConnection(req['keycloak_token']);
const connection = getDBConnection(req.keycloak_token);

try {
// Only search for specified types if provided, otherwise search all types
Expand Down
4 changes: 2 additions & 2 deletions api/src/paths/administrative-activity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ describe('getAdministrativeActivityStanding', () => {

const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq['keycloak_token'] = {
mockReq.keycloak_token = {
idir_user_guid: 'testguid',
identity_provider: 'idir',
idir_username: 'testuser',
Expand Down Expand Up @@ -158,7 +158,7 @@ describe('getAdministrativeActivityStanding', () => {

const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq['keycloak_token'] = {
mockReq.keycloak_token = {
idir_user_guid: 'testguid',
identity_provider: 'idir',
idir_username: 'testuser',
Expand Down
4 changes: 3 additions & 1 deletion api/src/paths/administrative-activity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { HTTP400, HTTP500 } from '../errors/http-error';
import { AdministrativeActivityService } from '../services/administrative-activity-service';
import { getUserGuid } from '../utils/keycloak-utils';
import { getLogger } from '../utils/logger';
import { getKeycloakTokenFromRequest } from '../utils/request';

const defaultLog = getLogger('paths/administrative-activity-request');

Expand Down Expand Up @@ -173,7 +174,8 @@ export function getAdministrativeActivityStanding(): RequestHandler {
const connection = getAPIUserDBConnection();

try {
const userGUID = getUserGuid(req['keycloak_token']);
const keycloakToken = getKeycloakTokenFromRequest(req);
const userGUID = getUserGuid(keycloakToken);

if (!userGUID) {
throw new HTTP400('Failed to identify user');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export function approveAccessRequest(): RequestHandler {

const roleIds: number[] = req.body.roleIds || [];

const connection = getDBConnection(req['keycloak_token']);
const connection = getDBConnection(req.keycloak_token);

try {
await connection.open();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export function rejectAccessRequest(): RequestHandler {
return async (req, res) => {
const administrativeActivityId = Number(req.params.administrativeActivityId);

const connection = getDBConnection(req['keycloak_token']);
const connection = getDBConnection(req.keycloak_token);

try {
await connection.open();
Expand Down
14 changes: 8 additions & 6 deletions api/src/paths/animal/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import sinonChai from 'sinon-chai';
import { SYSTEM_ROLE } from '../../constants/roles';
import * as db from '../../database/db';
import { HTTPError } from '../../errors/http-error';
import { SystemUser } from '../../repositories/user-repository';
import { FindCrittersResponse, SurveyCritterService } from '../../services/survey-critter-service';
import { KeycloakUserInformation } from '../../utils/keycloak-utils';
import { getMockDBConnection, getRequestHandlerMocks } from '../../__mocks__/db';
import { findAnimals } from './index';

Expand Down Expand Up @@ -59,10 +61,10 @@ describe('findAnimals', () => {
sort: undefined,
order: undefined
};
mockReq['keycloak_token'] = {};
mockReq['system_user'] = {
mockReq.keycloak_token = {} as KeycloakUserInformation;
mockReq.system_user = {
role_names: [SYSTEM_ROLE.SYSTEM_ADMIN]
};
} as SystemUser;

const requestHandler = findAnimals();

Expand Down Expand Up @@ -126,10 +128,10 @@ describe('findAnimals', () => {
sort: undefined,
order: undefined
};
mockReq['keycloak_token'] = {};
mockReq['system_user'] = {
mockReq.keycloak_token = {} as KeycloakUserInformation;
mockReq.system_user = {
role_names: [SYSTEM_ROLE.PROJECT_CREATOR]
};
} as SystemUser;

const requestHandler = findAnimals();

Expand Down
7 changes: 5 additions & 2 deletions api/src/paths/animal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
makePaginationOptionsFromRequest,
makePaginationResponse
} from '../../utils/pagination';
import { getSystemUserFromRequest } from '../../utils/request';

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

Expand Down Expand Up @@ -184,16 +185,18 @@ export function findAnimals(): RequestHandler {
return async (req, res) => {
defaultLog.debug({ label: 'findAnimals' });

const connection = getDBConnection(req['keycloak_token']);
const connection = getDBConnection(req.keycloak_token);

try {
await connection.open();

const systemUserId = connection.systemUserId();

const systemUser = getSystemUserFromRequest(req);

const isUserAdmin = userHasValidRole(
[SYSTEM_ROLE.SYSTEM_ADMIN, SYSTEM_ROLE.DATA_ADMINISTRATOR],
req['system_user']['role_names']
systemUser.role_names
);

const filterFields = parseQueryParams(req);
Expand Down
8 changes: 4 additions & 4 deletions api/src/paths/codes.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import chai, { expect } from 'chai';
import { describe } from 'mocha';
import sinon from 'sinon';
import sinonChai from 'sinon-chai';
import * as db from '../database/db';
import { HTTPError } from '../errors/http-error';
import { CodeService } from '../services/code-service';
import { KeycloakUserInformation } from '../utils/keycloak-utils';
import { getMockDBConnection, getRequestHandlerMocks } from '../__mocks__/db';
import * as codes from './codes';

Expand All @@ -24,7 +24,7 @@ describe('codes', () => {

const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq.keycloak_token = {};
mockReq.keycloak_token = {} as KeycloakUserInformation;

try {
const requestHandler = codes.getAllCodes();
Expand All @@ -47,7 +47,7 @@ describe('codes', () => {

const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq.keycloak_token = {};
mockReq.keycloak_token = {} as KeycloakUserInformation;

const requestHandler = codes.getAllCodes();

Expand All @@ -66,7 +66,7 @@ describe('codes', () => {

const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq.keycloak_token = {};
mockReq.keycloak_token = {} as KeycloakUserInformation;

try {
const requestHandler = codes.getAllCodes();
Expand Down
4 changes: 2 additions & 2 deletions api/src/paths/funding-sources/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('getFundingSources', () => {
agency: null
};
// system_user would be set by the authorization-service, if this endpoint was called for real
mockReq['system_user'] = systemUser;
mockReq.system_user = systemUser;

const requestHandler = getFundingSources();

Expand Down Expand Up @@ -128,7 +128,7 @@ describe('getFundingSources', () => {
agency: null
};
// system_user would be set by the authorization-service, if this endpoint was called for real
mockReq['system_user'] = systemUser;
mockReq.system_user = systemUser;

const requestHandler = getFundingSources();

Expand Down
11 changes: 6 additions & 5 deletions api/src/paths/funding-sources/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import { Operation } from 'express-openapi';
import { SYSTEM_ROLE } from '../../constants/roles';
import { getDBConnection } from '../../database/db';
import { FundingSource, FundingSourceSupplementaryData } from '../../repositories/funding-source-repository';
import { SystemUser } from '../../repositories/user-repository';
import { authorizeRequestHandler } from '../../request-handlers/security/authorization';
import { FundingSourceService, IFundingSourceSearchParams } from '../../services/funding-source-service';
import { UserService } from '../../services/user-service';
import { getLogger } from '../../utils/logger';
import { getSystemUserFromRequest } from '../../utils/request';

const defaultLog = getLogger('paths/funding-sources/index');

Expand Down Expand Up @@ -113,7 +113,7 @@ GET.apiDoc = {
*/
export function getFundingSources(): RequestHandler {
return async (req, res) => {
const connection = getDBConnection(req['keycloak_token']);
const connection = getDBConnection(req.keycloak_token);
const filterFields: IFundingSourceSearchParams = req.query || {};
try {
await connection.open();
Expand All @@ -124,8 +124,9 @@ export function getFundingSources(): RequestHandler {

await connection.commit();

const systemUserObject: SystemUser = req['system_user'];
if (!UserService.isAdmin(systemUserObject)) {
const systemUser = getSystemUserFromRequest(req);

if (!UserService.isAdmin(systemUser)) {
// User is not an admin, strip sensitive fields from response
response = removeNonAdminFieldsFromFundingSourcesResponse(response);
}
Expand Down Expand Up @@ -260,7 +261,7 @@ POST.apiDoc = {
*/
export function postFundingSource(): RequestHandler {
return async (req, res) => {
const connection = getDBConnection(req['keycloak_token']);
const connection = getDBConnection(req.keycloak_token);
const service = new FundingSourceService(connection);
const data = req.body;
try {
Expand Down
6 changes: 3 additions & 3 deletions api/src/paths/funding-sources/{fundingSourceId}.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ GET.apiDoc = {
*/
export function getFundingSource(): RequestHandler {
return async (req, res) => {
const connection = getDBConnection(req['keycloak_token']);
const connection = getDBConnection(req.keycloak_token);

const fundingSourceId = Number(req.params.fundingSourceId);

Expand Down Expand Up @@ -309,7 +309,7 @@ PUT.apiDoc = {
*/
export function putFundingSource(): RequestHandler {
return async (req, res) => {
const connection = getDBConnection(req['keycloak_token']);
const connection = getDBConnection(req.keycloak_token);

try {
await connection.open();
Expand Down Expand Up @@ -408,7 +408,7 @@ DELETE.apiDoc = {
*/
export function deleteFundingSource(): RequestHandler {
return async (req, res) => {
const connection = getDBConnection(req['keycloak_token']);
const connection = getDBConnection(req.keycloak_token);

const fundingSourceId = Number(req.params.fundingSourceId);

Expand Down
14 changes: 8 additions & 6 deletions api/src/paths/observation/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import { SYSTEM_ROLE } from '../../constants/roles';
import * as db from '../../database/db';
import { HTTPError } from '../../errors/http-error';
import { ObservationRecordWithSamplingAndSubcountData } from '../../repositories/observation-repository/observation-repository';
import { SystemUser } from '../../repositories/user-repository';
import { ObservationService } from '../../services/observation-service';
import { KeycloakUserInformation } from '../../utils/keycloak-utils';
import { getMockDBConnection, getRequestHandlerMocks } from '../../__mocks__/db';
import { findObservations } from './index';

Expand Down Expand Up @@ -79,10 +81,10 @@ describe('findObservations', () => {
sort: undefined,
order: undefined
};
mockReq['keycloak_token'] = {};
mockReq['system_user'] = {
mockReq.keycloak_token = {} as KeycloakUserInformation;
mockReq.system_user = {
role_names: [SYSTEM_ROLE.SYSTEM_ADMIN]
};
} as SystemUser;

const requestHandler = findObservations();

Expand Down Expand Up @@ -172,10 +174,10 @@ describe('findObservations', () => {
sort: undefined,
order: undefined
};
mockReq['keycloak_token'] = {};
mockReq['system_user'] = {
mockReq.keycloak_token = {} as KeycloakUserInformation;
mockReq.system_user = {
role_names: [SYSTEM_ROLE.PROJECT_CREATOR]
};
} as SystemUser;

const requestHandler = findObservations();

Expand Down
7 changes: 5 additions & 2 deletions api/src/paths/observation/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
makePaginationOptionsFromRequest,
makePaginationResponse
} from '../../utils/pagination';
import { getSystemUserFromRequest } from '../../utils/request';

const defaultLog = getLogger('paths/observation/index');

Expand Down Expand Up @@ -169,16 +170,18 @@ export function findObservations(): RequestHandler {
return async (req, res) => {
defaultLog.debug({ label: 'getObservations' });

const connection = getDBConnection(req['keycloak_token']);
const connection = getDBConnection(req.keycloak_token);

try {
await connection.open();

const systemUserId = connection.systemUserId();

const systemUser = getSystemUserFromRequest(req);

const isUserAdmin = userHasValidRole(
[SYSTEM_ROLE.SYSTEM_ADMIN, SYSTEM_ROLE.DATA_ADMINISTRATOR],
req['system_user']['role_names']
systemUser.role_names
);

const filterFields = parseQueryParams(req);
Expand Down
2 changes: 1 addition & 1 deletion api/src/paths/project/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ POST.apiDoc = {
*/
export function createProject(): RequestHandler {
return async (req, res) => {
const connection = getDBConnection(req['keycloak_token']);
const connection = getDBConnection(req.keycloak_token);

const sanitizedProjectPostData = new PostProjectObject(req.body);

Expand Down
Loading
Loading