Skip to content

Commit

Permalink
Fix Team management - Add a new member - type BCEID - Business/Basic …
Browse files Browse the repository at this point in the history
…textfield is case sensitive - FORMS-1012

Fix copy submission option not visible for submissions other than SUBMITTED state. - FORMS-1277
  • Loading branch information
bhuvan-aot committed Aug 13, 2024
1 parent c9a4a64 commit 5030693
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ defineExpose({
<span
v-if="
submission.status === 'SUBMITTED' &&
(submission.status === 'SUBMITTED' ||
submission.status === 'COMPLETED') &&
isCopyFromExistingSubmissionEnabled === true
"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ const BCEIDBASIC = {
param: 'username',
exact: true,
required: 2,
caseSensitive: false,
},
{
name: 'filterFullName',
Expand All @@ -104,6 +105,7 @@ const BCEIDBASIC = {
exact: true,
param: 'email',
required: 2,
caseSensitive: false,
},
{
name: 'filterSearch',
Expand Down Expand Up @@ -164,6 +166,7 @@ const BCEIDBUSINESS = {
param: 'username',
exact: true,
required: 2,
caseSensitive: false,
},
{
name: 'filterFullName',
Expand All @@ -185,6 +188,7 @@ const BCEIDBUSINESS = {
exact: true,
param: 'email',
required: 2,
caseSensitive: false,
},
{
name: 'filterSearch',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ const BCEIDBASIC = {
param: 'username',
exact: true,
required: 2,
caseSensitive: false,
},
{
name: 'filterFullName',
Expand All @@ -129,6 +130,7 @@ const BCEIDBASIC = {
exact: true,
param: 'email',
required: 2,
caseSensitive: false,
},
{
name: 'filterSearch',
Expand Down Expand Up @@ -189,6 +191,7 @@ const BCEIDBUSINESS = {
param: 'username',
exact: true,
required: 2,
caseSensitive: false,
},
{
name: 'filterFullName',
Expand All @@ -210,6 +213,7 @@ const BCEIDBUSINESS = {
exact: true,
param: 'email',
required: 2,
caseSensitive: false,
},
{
name: 'filterSearch',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ describe('MySubmissionsActions', () => {
);
});

it('renders if this is a submitted submission and isCopyFromExistingSubmissionEnabled is true', () => {
it('renders if this is a completed submission and isCopyFromExistingSubmissionEnabled is true', () => {
const SUBMISSION = {
status: 'SUBMITTED',
status: 'COMPLETED',
};
formStore.form = {
enableCopyExistingSubmission: true,
Expand All @@ -103,6 +103,30 @@ describe('MySubmissionsActions', () => {
);
});

it('renders if this is a completed submission and isCopyFromExistingSubmissionEnabled is true', () => {
const SUBMISSION = {
status: 'ASSIGNED',
};
formStore.form = {
enableCopyExistingSubmission: true,
};

const wrapper = shallowMount(MySubmissionsActions, {
props: {
formId: FORM_ID,
submission: SUBMISSION,
},
global: {
plugins: [pinia, router],
stubs: STUBS,
},
});

expect(wrapper.html()).not.toContain(
'trans.mySubmissionsActions.copyThisSubmission'
);
});

it('hasDeletePermission returns true if the submission has the create permission', () => {
let SUBMISSION = {
permissions: [FormPermissions.SUBMISSION_CREATE],
Expand Down
9 changes: 5 additions & 4 deletions app/src/components/idpService.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,10 @@ class IdpService {
const filterName = f.name;
const paramName = f.param;
const value = params[paramName];
if ('exact' in f) {
if ('exact' in f || 'caseSensitive' in f) {
const exact = f.exact;
q.modify(filterName, value, exact);
const caseSensitive = f.caseSensitive;
q.modify(filterName, value, exact, caseSensitive);
} else {
q.modify(filterName, value);
}
Expand Down Expand Up @@ -198,11 +199,11 @@ class IdpService {
return User.query()
.modify('filterIdpUserId', params.idpUserId)
.modify('filterIdpCode', params.idpCode)
.modify('filterUsername', params.username, false)
.modify('filterUsername', params.username, false, false)
.modify('filterFullName', params.fullName)
.modify('filterFirstName', params.firstName)
.modify('filterLastName', params.lastName)
.modify('filterEmail', params.email, false)
.modify('filterEmail', params.email, false, false)
.modify('filterSearch', params.search)
.modify('orderLastFirstAscending');
}
Expand Down
10 changes: 6 additions & 4 deletions app/src/forms/common/models/tables/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ class User extends Timestamps(Model) {
query.where('idpCode', value);
}
},
filterUsername(query, value, exact = false) {
filterUsername(query, value, exact = false, caseSensitive = true) {
if (value) {
if (exact) query.where('username', value);
if (exact && caseSensitive) query.where('username', value);
else if (exact && !caseSensitive) query.where('username', 'ilike', String(value).toLowerCase());
// ilike is postgres case insensitive like
else query.where('username', 'ilike', `%${value}%`);
}
Expand All @@ -65,9 +66,10 @@ class User extends Timestamps(Model) {
query.where('fullName', 'ilike', `%${value}%`);
}
},
filterEmail(query, value, exact = false) {
filterEmail(query, value, exact = false, caseSensitive = true) {
if (value) {
if (exact) query.where('email', value);
if (exact && caseSensitive) query.where('email', value);
else if (exact && !caseSensitive) query.where('email', 'ilike', String(value).toLowerCase());
// ilike is postgres case insensitive like
else query.where('email', 'ilike', `%${value}%`);
}
Expand Down
12 changes: 8 additions & 4 deletions app/tests/fixtures/form/identity_providers.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@
"name": "filterUsername",
"exact": true,
"param": "username",
"required": 2
"required": 2,
"caseSensitive": false
},
{
"name": "filterFullName",
Expand All @@ -98,7 +99,8 @@
"name": "filterEmail",
"exact": true,
"param": "email",
"required": 2
"required": 2,
"caseSensitive": false
},
{
"name": "filterSearch",
Expand Down Expand Up @@ -161,7 +163,8 @@
"name": "filterUsername",
"exact": true,
"param": "username",
"required": 2
"required": 2,
"caseSensitive": false
},
{
"name": "filterFullName",
Expand All @@ -182,7 +185,8 @@
"name": "filterEmail",
"exact": true,
"param": "email",
"required": 2
"required": 2,
"caseSensitive": false
},
{
"name": "filterSearch",
Expand Down
4 changes: 2 additions & 2 deletions app/tests/unit/components/idpService.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,15 +139,15 @@ describe('idpService', () => {
expect(MockModel.query).toBeCalledTimes(1);
expect(MockModel.modify).toBeCalledTimes(9);
expect(MockModel.modify).toBeCalledWith('filterIdpCode', 'idir');
expect(MockModel.modify).toBeCalledWith('filterEmail', '[email protected]', false);
expect(MockModel.modify).toBeCalledWith('filterEmail', '[email protected]', false, false);
});

it('should return a customized user search', async () => {
const s = await idpService.userSearch({ idpCode: 'bceid-business', email: '[email protected]' });
expect(s).toBeFalsy();
expect(MockModel.query).toBeCalledTimes(1);
expect(MockModel.modify).toBeCalledWith('filterIdpCode', 'bceid-business');
expect(MockModel.modify).toBeCalledWith('filterEmail', '[email protected]', true);
expect(MockModel.modify).toBeCalledWith('filterEmail', '[email protected]', true, false);
expect(MockModel.modify).toBeCalledTimes(9);
});

Expand Down
4 changes: 2 additions & 2 deletions app/tests/unit/forms/user/service.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ describe('list', () => {
expect(MockModel.modify).toBeCalledTimes(9);
expect(MockModel.modify).toBeCalledWith('filterIdpUserId', params.idpUserId);
expect(MockModel.modify).toBeCalledWith('filterIdpCode', params.idpCode);
expect(MockModel.modify).toBeCalledWith('filterUsername', params.username, false);
expect(MockModel.modify).toBeCalledWith('filterUsername', params.username, false, false);
expect(MockModel.modify).toBeCalledWith('filterFullName', params.fullName);
expect(MockModel.modify).toBeCalledWith('filterFirstName', params.firstName);
expect(MockModel.modify).toBeCalledWith('filterLastName', params.lastName);
expect(MockModel.modify).toBeCalledWith('filterEmail', params.email, false);
expect(MockModel.modify).toBeCalledWith('filterEmail', params.email, false, false);
expect(MockModel.modify).toBeCalledWith('filterSearch', params.search);
expect(MockModel.modify).toBeCalledWith('orderLastFirstAscending');
});
Expand Down
40 changes: 22 additions & 18 deletions docs/chefs-identity-provider-changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,27 @@ Within the CHEFs application a user's identity provider determines a lot of thei

A User's Identity Provider (IDP) is who vouches for them. In a simplified manner: they provide a username and password (generally) and an Identity Provder verifies them and they end up with a token. Currently for CHEFs we have 3 Identity Providers: `IDIR`, `BCeID Basic` and `BCeID Business`. `IDIR` is for employees/contractors on the BC Government. In CHEFs, the `IDIR` Identity Provider allows for greater power within CHEFs; as far as the CHEFs application is concerned IDIR is the `primary` Identity Provider.

Previously, all IDP logic was hardcoded within the frontend code and was difficult to change and maintain.
Previously, all IDP logic was hardcoded within the frontend code and was difficult to change and maintain.

**Example pseudocode:**

```
if user has idp === 'IDIR' then
if user has idp === 'IDIR' then
enable create forms button
```

By removing the hardcode, we can add in new IDPs and redefine which IDP is the `primary`. This opens up CHEFs for installations in non-BC Government environments.

## Identity Provider Table

Columns are added to the Identity Provider table to support runtime configuration.

* `primary`: boolean, which IDP is the highest level access (currently IDIR)
* `login`: boolean, if this IDP should appear as a login option (Public does not)
* `permissions`: string array, what permissions within CHEFS (not forms) does this IDP have
* `roles`: string array, what Form Roles does this IDP have (designer, owner, submitter, etc)
* `tokenmap`: json blob. this contains the mapping of IDP token fields to userInfo fields.
* `extra`: json blob. this is where non-standard configuration goes. we don't want a column for everything.
- `primary`: boolean, which IDP is the highest level access (currently IDIR)
- `login`: boolean, if this IDP should appear as a login option (Public does not)
- `permissions`: string array, what permissions within CHEFS (not forms) does this IDP have
- `roles`: string array, what Form Roles does this IDP have (designer, owner, submitter, etc)
- `tokenmap`: json blob. this contains the mapping of IDP token fields to userInfo fields.
- `extra`: json blob. this is where non-standard configuration goes. we don't want a column for everything.

### Application Permissions

Expand Down Expand Up @@ -58,6 +59,7 @@ Identity Provider also sets the scope of what roles a user can be assigned to an
```

### Extra

This is a `json` field with no predetermined structure. For BC Gov, we use it for extra functionality for the BCeID IDPs.

There are UX "enhancements" (frontend) and user search restrictions (server side) that were hardcoded, so now moved into this `json`. Any use of `extra` should assume that data fields may not exist or have null values.
Expand All @@ -78,22 +80,23 @@ Currently, `IDIR` has no data in `extra`.
},
},
userSearch: {
filters: [
{ name: 'filterIdpUserId', param: 'idpUserId', required: 0 },
{ name: 'filterIdpCode', param: 'idpCode', required: 0 },
{ name: 'filterUsername', param: 'username', required: 2, exact: true },
{ name: 'filterFullName', param: 'fullName', required: 0 },
{ name: 'filterFirstName', param: 'firstName', required: 0 },
{ name: 'filterLastName', param: 'lastName', required: 0 },
{ name: 'filterEmail', param: 'email', required: 2, exact: true },
{ name: 'filterSearch', param: 'search', required: 0 },
filters: [
{ name: 'filterIdpUserId', param: 'idpUserId', required: 0 },
{ name: 'filterIdpCode', param: 'idpCode', required: 0 },
{ name: 'filterUsername', param: 'username', required: 2, exact: true, caseSensitive: false},
{ name: 'filterFullName', param: 'fullName', required: 0 },
{ name: 'filterFirstName', param: 'firstName', required: 0 },
{ name: 'filterLastName', param: 'lastName', required: 0 },
{ name: 'filterEmail', param: 'email', required: 2, exact: true, caseSensitive: false},
{ name: 'filterSearch', param: 'search', required: 0 },
],
detail: 'Could not retrieve BCeID users. Invalid options provided.'
}
}
```

### Tokenmap

As part of the transistion to a new managed Keycloak realm, we lose the ability to do mapping of Identity Provider attributes to tokens. We do expect our User Information to be standardized and independent of the IDP, so we need to to the mapping ourselves.

The `tokenmap` is a `json` blob that is effectively a `userInfo` property name mapped to a `token` attribute. Each Identity Provider must provide a mapping so we can build out our `userInfo` object (our current user).
Expand Down Expand Up @@ -125,10 +128,11 @@ The code (both server and frontend) is confusing since `code` and `idp` fields w
Basically, be aware and cautious with `code`, `idp`, `hint` and `idpHint` until this is addressed.

## Frontend - idpStore

When the application is loaded, we query and store the Identity Providers. This can be found in `frontend/store/identityProviders.js`.

This has helper methods for building the login buttons, getting login hints, the primary IDP and getting data from `extra`. All access to the cached IDP data should come through this store.

## Backend - IdpService
Logic for new Identity Provider fields encapsulated in `components/idpService.js`. The queries and logic for parsing the token (use `tokenmap` field to transform token to userInfo). Also, `userSearch` is here as BCeID has specific requirements that are contained in the `extra` field.

Logic for new Identity Provider fields encapsulated in `components/idpService.js`. The queries and logic for parsing the token (use `tokenmap` field to transform token to userInfo). Also, `userSearch` is here as BCeID has specific requirements that are contained in the `extra` field.

0 comments on commit 5030693

Please sign in to comment.