From 76d07bdb0cfe264ffe8eab32d35cf83c89085758 Mon Sep 17 00:00:00 2001 From: Jamie Finnigan Date: Thu, 29 Oct 2020 13:50:42 -0700 Subject: [PATCH 1/3] update gajira-comment action, addressing security vulnerability --- .github/workflows/jira.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/jira.yml b/.github/workflows/jira.yml index 984276c69..cfb4c2bc4 100644 --- a/.github/workflows/jira.yml +++ b/.github/workflows/jira.yml @@ -68,7 +68,7 @@ jobs: - name: Sync comment if: github.event.action == 'created' && steps.search.outputs.issue - uses: atlassian/gajira-comment@v2.0.1 + uses: atlassian/gajira-comment@v2.0.2 with: issue: ${{ steps.search.outputs.issue }} comment: "${{ github.actor }} ${{ github.event.review.state || 'commented' }}:\n\n${{ github.event.comment.body || github.event.review.body }}\n\n${{ github.event.comment.html_url || github.event.review.html_url }}" From efa851928ad9b86d40f60290ece48f759059d3df Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Fri, 30 Oct 2020 13:23:19 -0400 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20=F0=9F=90=9B=20fixes=20issue=20where?= =?UTF-8?q?=20grant=20strings=20become=20corrupted=20in=20ui?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/addon/models/role.js | 13 +++++---- addons/api/addon/serializers/role.js | 29 ++++--------------- addons/api/tests/unit/models/role-test.js | 6 ++-- .../api/tests/unit/serializers/role-test.js | 10 +++---- .../app/components/form/role/grants/index.hbs | 8 ++--- .../app/components/form/role/grants/index.js | 10 +++++++ .../routes/scopes/scope/roles/role/grants.js | 12 ++++---- ui/core/mirage/config.js | 2 +- 8 files changed, 41 insertions(+), 49 deletions(-) diff --git a/addons/api/addon/models/role.js b/addons/api/addon/models/role.js index adc7778f5..3b428b9ba 100644 --- a/addons/api/addon/models/role.js +++ b/addons/api/addon/models/role.js @@ -1,5 +1,5 @@ import GeneratedRoleModel from '../generated/models/role'; -import { fragmentArray } from 'ember-data-model-fragments/attributes'; +import { fragmentArray, array } from 'ember-data-model-fragments/attributes'; export default class RoleModel extends GeneratedRoleModel { // =attributes @@ -18,23 +18,24 @@ export default class RoleModel extends GeneratedRoleModel { principals; /** - * Grants is read-only _most_ under normal circumstances. But grants can - * be persisted via a dedicated call to `saveGrants()`. + * Grants is read-only. But grants can be persisted via a dedicated + * call to `saveGrants(grants)`. */ - @fragmentArray('fragment-string', { readOnly: true }) grants; + @array('string', { readOnly: true }) grant_strings; // =methods /** * Saves the `grants` array on the role via the `set-grants` method. * See serializer and adapter for more information. + * @param {[string]} grants * @return {Promise} */ - saveGrants() { + saveGrants(grants) { return this.save({ adapterOptions: { method: 'set-grants', - serializeGrants: true, + grants }, }); } diff --git a/addons/api/addon/serializers/role.js b/addons/api/addon/serializers/role.js index 5efd37c0f..2fc2f3fc6 100644 --- a/addons/api/addon/serializers/role.js +++ b/addons/api/addon/serializers/role.js @@ -15,9 +15,9 @@ export default class RoleSerializer extends ApplicationSerializer { * @return {object} */ serialize(snapshot) { - const serializeGrants = get(snapshot, 'adapterOptions.serializeGrants'); + const grants = get(snapshot, 'adapterOptions.grants'); let serialized = super.serialize(...arguments); - if (serializeGrants) serialized = this.serializeWithGrants(snapshot); + if (grants) serialized = this.serializeWithGrants(snapshot, grants); const principalIDs = snapshot?.adapterOptions?.principalIDs; if (principalIDs) serialized = this.serializewithPrincipals(snapshot, principalIDs); @@ -27,14 +27,13 @@ export default class RoleSerializer extends ApplicationSerializer { /** * Returns a payload containing only the grants array. * @param {Snapshot} snapshot + * @param {[string]} grants * @return {object} */ - serializeWithGrants(snapshot) { + serializeWithGrants(snapshot, grants) { return { version: snapshot.attr('version'), - grant_strings: snapshot - .attr('grants') - .map((grant) => grant.attr('value')), + grant_strings: grants }; } @@ -51,22 +50,4 @@ export default class RoleSerializer extends ApplicationSerializer { principal_ids: principalIDs, }; } - - /** - * Converts string arrays to object arrays for use with `fragment-string`. - * E.g. `"foo"` -> `{value: "foo"}` - * @override - * @param {Model} typeClass - * @param {Object} hash - * @return {Object} - */ - normalize(modelClass, resourceHash) { - // TODO: can fragment string normalization be handled generically? - if (resourceHash.grant_strings) { - resourceHash.grants = resourceHash.grant_strings.map((value) => ({ - value, - })); - } - return super.normalize(modelClass, resourceHash); - } } diff --git a/addons/api/tests/unit/models/role-test.js b/addons/api/tests/unit/models/role-test.js index a3b593e4b..bbeb95075 100644 --- a/addons/api/tests/unit/models/role-test.js +++ b/addons/api/tests/unit/models/role-test.js @@ -17,7 +17,7 @@ module('Unit | Model | role', function (hooks) { this.server.post('/v1/roles/123abc:set-grants', (schema, request) => { const body = JSON.parse(request.requestBody); assert.deepEqual(body, { - grant_strings: ['grant1', 'grant2'], + grant_strings: ['foo', 'bar'], version: 1, }); return { id: '123abc' }; @@ -30,7 +30,7 @@ module('Unit | Model | role', function (hooks) { attributes: { name: 'Role', description: 'Description', - grants: [{ value: 'grant1' }, { value: 'grant2' }], + grant_strings: ['grant1', 'grant2'], version: 1, scope: { scope_id: 'o_1', @@ -40,7 +40,7 @@ module('Unit | Model | role', function (hooks) { }, }); const model = store.peekRecord('role', '123abc'); - await model.saveGrants(); + await model.saveGrants(['foo', 'bar']); }); test('it has an `addPrincipals` method that targets a specific POST API endpoint and serialization', async function (assert) { diff --git a/addons/api/tests/unit/serializers/role-test.js b/addons/api/tests/unit/serializers/role-test.js index 1e0624d6a..0303cd60f 100644 --- a/addons/api/tests/unit/serializers/role-test.js +++ b/addons/api/tests/unit/serializers/role-test.js @@ -11,7 +11,7 @@ module('Unit | Serializer | role', function (hooks) { const record = store.createRecord('role', { name: 'User', description: 'Description', - grants: [{ value: 'foo' }, { value: 'bar' }], + grant_strings: ['foo', 'bar'], version: 1, }); const snapshot = record._createSnapshot(); @@ -24,19 +24,19 @@ module('Unit | Serializer | role', function (hooks) { }); }); - test('it serializes only grants when `adapterOptions.serializeGrants` is true', function (assert) { + test('it serializes only grants when `adapterOptions.grants` is set', function (assert) { assert.expect(1); const store = this.owner.lookup('service:store'); const serializer = store.serializerFor('role'); const record = store.createRecord('role', { name: 'User', description: 'Description', - grants: [{ value: 'foo' }, { value: 'bar' }], + grant_strings: ['grant1', 'grant2'], version: 1, }); const snapshot = record._createSnapshot(); snapshot.adapterOptions = { - serializeGrants: true, + grants: ['foo', 'bar'], }; const serializedRecord = serializer.serialize(snapshot); assert.deepEqual(serializedRecord, { @@ -67,7 +67,7 @@ module('Unit | Serializer | role', function (hooks) { type: 'role', attributes: { name: 'Role 1', - grants: [{ value: '*' }, { value: '*' }], + grant_strings: ['*', '*'], principals: [], }, relationships: {}, diff --git a/ui/core/app/components/form/role/grants/index.hbs b/ui/core/app/components/form/role/grants/index.hbs index 8b5e97f28..bbe47e7d6 100644 --- a/ui/core/app/components/form/role/grants/index.hbs +++ b/ui/core/app/components/form/role/grants/index.hbs @@ -35,7 +35,7 @@ - {{#each @model.grants as |grant|}} + {{#each this.grants as |grant|}} {{t "form.grant.label"}} @@ -74,7 +74,7 @@ @style="secondary" @iconOnly="trash" title={{t "actions.remove"}} - {{on "click" (fn @removeGrant grant)}} + {{on "click" (fn @removeGrant grant.value)}} > {{t "actions.remove"}} diff --git a/ui/core/app/components/form/role/grants/index.js b/ui/core/app/components/form/role/grants/index.js index f1f0f8ce3..11e7d595f 100644 --- a/ui/core/app/components/form/role/grants/index.js +++ b/ui/core/app/components/form/role/grants/index.js @@ -10,6 +10,16 @@ export default class FormRoleGrantsComponent extends Component { */ @tracked newGrantString = ''; + @computed('args.model.grant_strings.[]') + get grants() { + return this.args.model.grant_strings.map(value => ({ value })); + } + + @computed('grants.@each.value') + get grantStrings() { + return this.grants.map(obj => obj.value); + } + /** * True if the grant string field is empty, false otherwise. This is used * to disable the submit button. diff --git a/ui/core/app/routes/scopes/scope/roles/role/grants.js b/ui/core/app/routes/scopes/scope/roles/role/grants.js index af56f1990..b87721d99 100644 --- a/ui/core/app/routes/scopes/scope/roles/role/grants.js +++ b/ui/core/app/routes/scopes/scope/roles/role/grants.js @@ -37,18 +37,18 @@ export default class ScopesScopeRolesRoleGrantsRoute extends Route { */ @action addGrant(role, grantString) { - role.grants.unshiftObject({ value: grantString }); + role.grant_strings.unshiftObject(grantString); } /** * Removes a grant from the role. Grant removal is not immediately permanent; * users may rollback the change via "cancel" or commit it via "save". * @param {RoleModel} role - * @param {FragmentString} grant + * @param {string} grantString */ @action - removeGrant(role, grant) { - role.grants.removeFragment(grant); + removeGrant(role, grantString) { + role.grant_strings.removeObject(grantString); } /** @@ -59,7 +59,7 @@ export default class ScopesScopeRolesRoleGrantsRoute extends Route { @loading @notifyError(({ message }) => message, { catch: true }) @notifySuccess('notifications.save-success') - async save(role) { - await role.saveGrants(); + async save(role, grants) { + await role.saveGrants(grants); } } diff --git a/ui/core/mirage/config.js b/ui/core/mirage/config.js index 50d1b5344..d2596e43f 100644 --- a/ui/core/mirage/config.js +++ b/ui/core/mirage/config.js @@ -206,7 +206,7 @@ export default function() { if (method === 'set-grants') { updatedAttrs = { version: attrs.version, - grant_strings: attrs.grant_strings + grantStrings: attrs.grantStrings }; } From 3016d855ae687656541d9e0fb7e2a15249181368 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Fri, 30 Oct 2020 15:33:08 -0400 Subject: [PATCH 3/3] =?UTF-8?q?chore:=20=F0=9F=A4=96=20rename=20grants=20t?= =?UTF-8?q?o=20grantStrings=20and=20clean=20up=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/addon/models/host-set.js | 4 ++-- addons/api/addon/models/role.js | 12 ++++++------ addons/api/addon/serializers/role.js | 11 ++++++----- addons/api/tests/unit/models/role-test.js | 4 ++-- addons/api/tests/unit/serializers/role-test.js | 4 ++-- ui/core/app/components/form/role/grants/index.js | 6 ++++++ ui/core/app/routes/scopes/scope/roles/role/grants.js | 5 +++-- 7 files changed, 27 insertions(+), 19 deletions(-) diff --git a/addons/api/addon/models/host-set.js b/addons/api/addon/models/host-set.js index 2750c0cb8..3c56d2879 100644 --- a/addons/api/addon/models/host-set.js +++ b/addons/api/addon/models/host-set.js @@ -6,8 +6,8 @@ export default class HostSetModel extends GeneratedHostSetModel { // =attributes /** - * Grants is read-only _most_ under normal circumstances. But grants can - * be persisted via a dedicated call to `saveGrants()`. + * Host IDs are read-only under normal circumstances. But these can + * be persisted via a dedicated call to `addHosts()`. */ @fragmentArray('fragment-string', { readOnly: true, diff --git a/addons/api/addon/models/role.js b/addons/api/addon/models/role.js index 3b428b9ba..be29c1c24 100644 --- a/addons/api/addon/models/role.js +++ b/addons/api/addon/models/role.js @@ -18,24 +18,24 @@ export default class RoleModel extends GeneratedRoleModel { principals; /** - * Grants is read-only. But grants can be persisted via a dedicated - * call to `saveGrants(grants)`. + * Grant strings are read-only. But grants can be persisted via a dedicated + * call to `saveGrantStrings(grants)`. */ @array('string', { readOnly: true }) grant_strings; // =methods /** - * Saves the `grants` array on the role via the `set-grants` method. + * Saves grant strings on the role via the `set-grants` method. * See serializer and adapter for more information. - * @param {[string]} grants + * @param {[string]} grantStrings * @return {Promise} */ - saveGrants(grants) { + saveGrantStrings(grantStrings) { return this.save({ adapterOptions: { method: 'set-grants', - grants + grantStrings }, }); } diff --git a/addons/api/addon/serializers/role.js b/addons/api/addon/serializers/role.js index 2fc2f3fc6..59271fe44 100644 --- a/addons/api/addon/serializers/role.js +++ b/addons/api/addon/serializers/role.js @@ -15,9 +15,10 @@ export default class RoleSerializer extends ApplicationSerializer { * @return {object} */ serialize(snapshot) { - const grants = get(snapshot, 'adapterOptions.grants'); + const grantStrings = get(snapshot, 'adapterOptions.grantStrings'); let serialized = super.serialize(...arguments); - if (grants) serialized = this.serializeWithGrants(snapshot, grants); + if (grantStrings) serialized = + this.serializeWithGrantStrings(snapshot, grantStrings); const principalIDs = snapshot?.adapterOptions?.principalIDs; if (principalIDs) serialized = this.serializewithPrincipals(snapshot, principalIDs); @@ -27,13 +28,13 @@ export default class RoleSerializer extends ApplicationSerializer { /** * Returns a payload containing only the grants array. * @param {Snapshot} snapshot - * @param {[string]} grants + * @param {[string]} grantStrings * @return {object} */ - serializeWithGrants(snapshot, grants) { + serializeWithGrantStrings(snapshot, grantStrings) { return { version: snapshot.attr('version'), - grant_strings: grants + grant_strings: grantStrings }; } diff --git a/addons/api/tests/unit/models/role-test.js b/addons/api/tests/unit/models/role-test.js index bbeb95075..9d64e9ac2 100644 --- a/addons/api/tests/unit/models/role-test.js +++ b/addons/api/tests/unit/models/role-test.js @@ -12,7 +12,7 @@ module('Unit | Model | role', function (hooks) { assert.ok(model); }); - test('it has a `saveGrants` method that targets a specific POST API endpoint and serialization', async function (assert) { + test('it has a `saveGrantStrings` method that targets a specific POST API endpoint and serialization', async function (assert) { assert.expect(1); this.server.post('/v1/roles/123abc:set-grants', (schema, request) => { const body = JSON.parse(request.requestBody); @@ -40,7 +40,7 @@ module('Unit | Model | role', function (hooks) { }, }); const model = store.peekRecord('role', '123abc'); - await model.saveGrants(['foo', 'bar']); + await model.saveGrantStrings(['foo', 'bar']); }); test('it has an `addPrincipals` method that targets a specific POST API endpoint and serialization', async function (assert) { diff --git a/addons/api/tests/unit/serializers/role-test.js b/addons/api/tests/unit/serializers/role-test.js index 0303cd60f..15aea8662 100644 --- a/addons/api/tests/unit/serializers/role-test.js +++ b/addons/api/tests/unit/serializers/role-test.js @@ -24,7 +24,7 @@ module('Unit | Serializer | role', function (hooks) { }); }); - test('it serializes only grants when `adapterOptions.grants` is set', function (assert) { + test('it serializes only grant strings when `adapterOptions.grantStrings` is set', function (assert) { assert.expect(1); const store = this.owner.lookup('service:store'); const serializer = store.serializerFor('role'); @@ -36,7 +36,7 @@ module('Unit | Serializer | role', function (hooks) { }); const snapshot = record._createSnapshot(); snapshot.adapterOptions = { - grants: ['foo', 'bar'], + grantStrings: ['foo', 'bar'], }; const serializedRecord = serializer.serialize(snapshot); assert.deepEqual(serializedRecord, { diff --git a/ui/core/app/components/form/role/grants/index.js b/ui/core/app/components/form/role/grants/index.js index 11e7d595f..a55a28093 100644 --- a/ui/core/app/components/form/role/grants/index.js +++ b/ui/core/app/components/form/role/grants/index.js @@ -10,11 +10,17 @@ export default class FormRoleGrantsComponent extends Component { */ @tracked newGrantString = ''; + /** + * @type {[object]} + */ @computed('args.model.grant_strings.[]') get grants() { return this.args.model.grant_strings.map(value => ({ value })); } + /** + * @type {[string]} + */ @computed('grants.@each.value') get grantStrings() { return this.grants.map(obj => obj.value); diff --git a/ui/core/app/routes/scopes/scope/roles/role/grants.js b/ui/core/app/routes/scopes/scope/roles/role/grants.js index b87721d99..721e93446 100644 --- a/ui/core/app/routes/scopes/scope/roles/role/grants.js +++ b/ui/core/app/routes/scopes/scope/roles/role/grants.js @@ -54,12 +54,13 @@ export default class ScopesScopeRolesRoleGrantsRoute extends Route { /** * Save an role in current scope. * @param {RoleModel} role + * @param {[string]} grantStrings */ @action @loading @notifyError(({ message }) => message, { catch: true }) @notifySuccess('notifications.save-success') - async save(role, grants) { - await role.saveGrants(grants); + async save(role, grantStrings) { + await role.saveGrantStrings(grantStrings); } }