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: Keycloak/Auth Updates From SIMS #216

Merged
merged 6 commits into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
51 changes: 6 additions & 45 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,50 +1,11 @@
# Overview
## Links to Jira Tickets

## Links to Jira tickets
- {Include a link to all applicable Jira tickets}

- {List all applicable Jira tickets}
## Description of Changes

## Description of relevant changes
- {List all relevant code changes. Include any changes to the business workflow that might not be obvious to the reviewers of this PR.}

- {List all relevant changes, in particular anything that will help the reviewers test/verify this PR}
## Testing Notes

## PR Checklist

A list of items that are good to consider when making any changes.

_Note: this list is not exhaustive, and not all items are always applicable._

### Code

- [ ] New files/classes/functions have appropriately descriptive names and comment blocks to describe their use/behaviour
- [ ] I have avoided duplicating code when possible, moving re-usable pieces into functions
- [ ] I have avoided hard-coding values where possible and moved any re-usable constants to a constants file
- [ ] My code is as flat as possible (avoids deeply nested if/else blocks, promise chains, etc)
- [ ] My code changes account for null/undefined values and handle errors appropriately
- [ ] My code uses types/interfaces to help describe values/parameters/etc, help ensure type safety, and improve readability

### Style

- [ ] My code follows the established style conventions
- [ ] My code uses native material-ui components/icons/conventions when possible

### Documentation

- [ ] I have commented my code sufficiently, such that an unfamiliar developer could understand my code
- [ ] I have added/updated README's and related documentation, as needed

### Tests

- [ ] I have added/updated unit tests for any code I've added/updated
- [ ] I have added/updated the Postman requests/tests to account for any API endpoints I've added/updated

### Linting/Formatting

- [ ] I have run the linter and fixed any issues, as needed
_See the `lint` commands in package.json_
- [ ] I have run the formatter and fixed any issues, as needed
_See the `format` commands in package.json_

### SonarCloud

- [ ] I have addressed all SonarCloud Bugs, Vulnerabilities, Security Hotspots, and Code Smells
- {List any relevant testing considerations, necessary pre-reqs, and areas of the app to focus on. Specifically, include anything that will help the reviewers of this PR verify the code is functioning as expected.}
1 change: 0 additions & 1 deletion api/src/models/index.ts

This file was deleted.

1 change: 0 additions & 1 deletion api/src/models/models.ts

This file was deleted.

1 change: 0 additions & 1 deletion api/src/models/user/index.ts

This file was deleted.

82 changes: 0 additions & 82 deletions api/src/models/user/user.test.ts

This file was deleted.

19 changes: 0 additions & 19 deletions api/src/models/user/user.ts

This file was deleted.

17 changes: 12 additions & 5 deletions api/src/paths/administrative/access-request/update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import sinon from 'sinon';
import sinonChai from 'sinon-chai';
import * as db from '../../../database/db';
import { HTTPError } from '../../../errors/http-error';
import { Models } from '../../../models';
import { SystemUserExtended } from '../../../repositories/user-repository';
import { AdministrativeService } from '../../../services/administrative-service';
import { GCNotifyService } from '../../../services/gcnotify-service';
import { UserService } from '../../../services/user-service';
Expand Down Expand Up @@ -68,12 +68,19 @@ describe('updateAccessRequest', () => {

const systemUserId = 4;
const existingRoleIds = [1, 2];
const mockSystemUser: Models.user.UserObject = {
id: systemUserId,
user_identifier: '',
const mockSystemUser: SystemUserExtended = {
system_user_id: systemUserId,
user_identity_source_id: 2,
user_identifier: 'username',
user_guid: '',
identity_source: '',
record_end_date: '',
record_effective_date: '2023-12-08',
create_date: '2023-12-08 14:37:41.315999-08',
create_user: 1,
update_date: null,
update_user: null,
revision_count: 0,
identity_source: 'identitySource',
role_ids: existingRoleIds,
role_names: []
};
Expand Down
2 changes: 1 addition & 1 deletion api/src/paths/administrative/access-request/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export function updateAccessRequest(): RequestHandler {

if (rolesIdsToAdd?.length) {
// Add any missing roles (if any)
await userService.addUserSystemRoles(systemUserObject.id, rolesIdsToAdd);
await userService.addUserSystemRoles(systemUserObject.system_user_id, rolesIdsToAdd);
}

// Update the access request record status
Expand Down
2 changes: 1 addition & 1 deletion api/src/paths/dwc/spatial/download.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ describe('download', () => {
});

it('returns 200', async () => {
const datasetID = 'AAA-BBB-CCC-123';
const datasetID = '123-456-789';
const dbConnectionObj = getMockDBConnection({ commit: sinon.stub(), release: sinon.stub() });
sinon.stub(db, 'getAPIUserDBConnection').returns(dbConnectionObj);

Expand Down
2 changes: 1 addition & 1 deletion api/src/paths/dwc/submission/{datasetId}/artifacts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe('getArtifactsByDatasetId', () => {
const mockResponse = {
artifact_id: 1,
create_date: '1970-01-01T00:00:00.000Z',
description: 'aaa',
description: 'description',
file_name: 'Lecture 5 - Partial Fraction.pdf',
file_size: 2405262,
file_type: 'Report',
Expand Down
6 changes: 3 additions & 3 deletions api/src/paths/dwc/submission/{datasetId}/related.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ describe('getRelatedDatasetsByDatasetId', () => {

const submissionServiceStub = sinon
.stub(SubmissionService.prototype, 'findRelatedDatasetsByDatasetId')
.resolves([{ datasetId: 'aaa' }, { datasetId: 'bbb' }] as RelatedDataset[]);
.resolves([{ datasetId: '123' }, { datasetId: '456' }] as RelatedDataset[]);

const isSystemUserAdminStub = sinon.stub(UserService.prototype, 'isSystemUserAdmin').resolves(true);

Expand All @@ -307,13 +307,13 @@ describe('getRelatedDatasetsByDatasetId', () => {
expect(mockRes.jsonValue).to.eql({
datasetsWithSupplementaryData: [
{
datasetId: 'aaa',
datasetId: '123',
supplementaryData: {
isPendingReview: true
}
},
{
datasetId: 'bbb',
datasetId: '456',
supplementaryData: {
isPendingReview: false
}
Expand Down
6 changes: 3 additions & 3 deletions api/src/paths/submission/intake.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('intake', () => {
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq.body = {
id: 'aaaa',
id: '123-456-789',
type: 'submission',
properties: {},
features: []
Expand Down Expand Up @@ -61,7 +61,7 @@ describe('intake', () => {
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq.body = {
id: 'aaaa',
id: '123-456-789',
type: 'submission',
properties: {},
features: []
Expand Down Expand Up @@ -105,7 +105,7 @@ describe('intake', () => {
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq.body = {
id: 'aaaa',
id: '123-456-789',
type: 'submission',
properties: { additionalInformation: 'test' },
features: []
Expand Down
21 changes: 14 additions & 7 deletions api/src/paths/user/add.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import sinonChai from 'sinon-chai';
import { SYSTEM_IDENTITY_SOURCE } from '../../constants/database';
import * as db from '../../database/db';
import { HTTPError } from '../../errors/http-error';
import { Models } from '../../models';
import { SystemUserExtended } from '../../repositories/user-repository';
import { UserService } from '../../services/user-service';
import { getMockDBConnection, getRequestHandlerMocks } from '../../__mocks__/db';
import * as user from './add';
Expand Down Expand Up @@ -46,7 +46,7 @@ describe('user', () => {
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq.body = {
userGuid: 'aaaa',
userGuid: '123-456-789',
identitySource: SYSTEM_IDENTITY_SOURCE.IDIR,
roleId: 1
};
Expand Down Expand Up @@ -94,7 +94,7 @@ describe('user', () => {
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq.body = {
userGuid: 'aaaa',
userGuid: '123-456-789',
userIdentifier: 'username',
roleId: 1
};
Expand All @@ -118,7 +118,7 @@ describe('user', () => {
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq.body = {
userGuid: 'aaaa',
userGuid: '123-456-789',
userIdentifier: 'username',
identitySource: SYSTEM_IDENTITY_SOURCE.IDIR
};
Expand All @@ -142,18 +142,25 @@ describe('user', () => {
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq.body = {
userGuid: 'aaaa',
userGuid: '123-456-789',
userIdentifier: 'username',
identitySource: SYSTEM_IDENTITY_SOURCE.IDIR,
roleId: 1
};

const mockUserObject: Models.user.UserObject = {
id: 1,
const mockUserObject: SystemUserExtended = {
system_user_id: 1,
user_identity_source_id: 2,
user_identifier: '',
user_guid: '',
identity_source: '',
record_effective_date: '',
record_end_date: '',
create_date: 'string',
create_user: 1,
update_date: null,
update_user: null,
revision_count: 1,
role_ids: [1],
role_names: []
};
Expand Down
2 changes: 1 addition & 1 deletion api/src/paths/user/add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export function addSystemRoleUser(): RequestHandler {
const userObject = await userService.ensureSystemUser(userGuid, userIdentifier, identitySource);

if (userObject) {
await userService.addUserSystemRoles(userObject.id, [roleId]);
await userService.addUserSystemRoles(userObject.system_user_id, [roleId]);
}

await connection.commit();
Expand Down
14 changes: 11 additions & 3 deletions api/src/paths/user/list.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { describe } from 'mocha';
import sinon from 'sinon';
import sinonChai from 'sinon-chai';
import * as db from '../../database/db';
import { SystemUserExtended } from '../../repositories/user-repository';
import { UserService } from '../../services/user-service';
import { getMockDBConnection, getRequestHandlerMocks } from '../../__mocks__/db';
import * as users from './list';
Expand All @@ -22,13 +23,20 @@ describe('users', () => {

sinon.stub(db, 'getDBConnection').returns(mockDBConnection);

const mockResponse = [
const mockResponse: SystemUserExtended[] = [
{
id: 1,
system_user_id: 1,
user_identity_source_id: 2,
user_identifier: 'identifier',
user_guid: 'aaaa',
user_guid: '123-456-789',
identity_source: 'idir',
record_effective_date: '',
record_end_date: '',
create_user: 1,
create_date: '',
update_user: null,
update_date: null,
revision_count: 0,
role_ids: [1, 2],
role_names: ['System Admin', 'Project Lead']
}
Expand Down
Loading
Loading