From d0a505558bfe261ede08ab4cecd2b667d4362904 Mon Sep 17 00:00:00 2001 From: Dharini Jeeva Date: Wed, 20 Sep 2023 14:55:05 -0700 Subject: [PATCH] =?UTF-8?q?refactor:=20=F0=9F=92=A1sb=20dynamic=20credenti?= =?UTF-8?q?al=20type=20secret=20fields=20and=20how=20we=20serialize=20(#19?= =?UTF-8?q?07)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: 💡 secret fields and how we serialize --- .../addon/generated/models/storage-bucket.js | 7 +++ addons/api/addon/models/storage-bucket.js | 20 ++++++--- .../api/addon/serializers/storage-bucket.js | 15 ++++--- .../tests/unit/models/storage-bucket-test.js | 2 + .../unit/serializers/storage-bucket-test.js | 43 ++++++++++++++----- .../scopes/scope/storage-buckets.js | 1 - .../routes/scopes/scope/storage-buckets.js | 9 +--- .../scopes/scope/storage-buckets/new.js | 20 +++++++-- .../scopes/scope/storage-buckets/new.hbs | 2 +- .../storage-buckets/storage-bucket/index.hbs | 2 +- 10 files changed, 85 insertions(+), 36 deletions(-) diff --git a/addons/api/addon/generated/models/storage-bucket.js b/addons/api/addon/generated/models/storage-bucket.js index 335f2bffe..7b855d801 100644 --- a/addons/api/addon/generated/models/storage-bucket.js +++ b/addons/api/addon/generated/models/storage-bucket.js @@ -73,6 +73,13 @@ export default class GeneratedStorageBucketModel extends BaseModel { }) region; + //this field is not used in the UI, but this is kept here to replicate the API + @attr('string', { + readOnly: true, + isNestedAttribute: true, + }) + secrets_hmac; + @attr('boolean', { isNestedAttribute: true, description: 'If set to `true`, credential rotation is not performed.', diff --git a/addons/api/addon/models/storage-bucket.js b/addons/api/addon/models/storage-bucket.js index ee98d055a..6f7d1b28e 100644 --- a/addons/api/addon/models/storage-bucket.js +++ b/addons/api/addon/models/storage-bucket.js @@ -22,6 +22,9 @@ export const TYPES_CREDENTIALS = Object.freeze([ ]); export default class StorageBucketModel extends GeneratedStorageBucketModel { + // attributes + + #credentialType; /** * True if the storage bucket is a plugin. * @type {boolean} @@ -31,18 +34,21 @@ export default class StorageBucketModel extends GeneratedStorageBucketModel { } /** - * If the storage bucket has role_arn, return dynamic - * otherwise return the default static type. + * Gets a value from credentialType * @type {string} */ get credentialType() { - if (this.role_arn && !this.access_key_id) { - return TYPE_CREDENTIAL_DYNAMIC; - } else { - return TYPE_CREDENTIAL_STATIC; - } + if (!this.#credentialType) return 'static'; + return this.#credentialType; } + /** + * Sets a value to credentialType + * @type {string} + */ + set credentialType(type) { + this.#credentialType = type; + } /** * True if the storage bucket is an unknown type. * @type {boolean} diff --git a/addons/api/addon/serializers/storage-bucket.js b/addons/api/addon/serializers/storage-bucket.js index cb3941094..9f9da0aa6 100644 --- a/addons/api/addon/serializers/storage-bucket.js +++ b/addons/api/addon/serializers/storage-bucket.js @@ -49,17 +49,22 @@ export default class StorageBucketSerializer extends ApplicationSerializer { json.attributes[key] = null; } + //json.secrets is only present, if there are any updates to the secret fields + if (options.isNestedSecret && json.secrets) { + if (!fieldsByCredentialType[credentialType].includes(key)) { + delete json.secrets; + } + } return value; } normalize(typeClass, hash, ...rest) { const normalizedHash = structuredClone(hash); + const normalized = super.normalize(typeClass, normalizedHash, ...rest); // Remove secret fields so we don't track them after creating/updating - if (normalizedHash?.secrets?.access_key_id) { - delete normalizedHash.secrets.access_key_id; - delete normalizedHash.secrets.secret_access_key; - } - return super.normalize(typeClass, normalizedHash, ...rest); + normalized.data.attributes.access_key_id = null; + normalized.data.attributes.secret_access_key = null; + return normalized; } } diff --git a/addons/api/tests/unit/models/storage-bucket-test.js b/addons/api/tests/unit/models/storage-bucket-test.js index 095820500..81ffa60e3 100644 --- a/addons/api/tests/unit/models/storage-bucket-test.js +++ b/addons/api/tests/unit/models/storage-bucket-test.js @@ -51,10 +51,12 @@ module('Unit | Model | storage bucket', function (hooks) { const modelA = store.createRecord('storage-bucket', { role_arn: 'test-role-arn', access_key_id: null, + credentialType: TYPE_CREDENTIAL_DYNAMIC, }); const modelB = store.createRecord('storage-bucket', { role_arn: null, access_key_id: 'test-access-key-id', + credentialType: TYPE_CREDENTIAL_STATIC, }); assert.strictEqual(modelA.credentialType, TYPE_CREDENTIAL_DYNAMIC); assert.strictEqual(modelB.credentialType, TYPE_CREDENTIAL_STATIC); diff --git a/addons/api/tests/unit/serializers/storage-bucket-test.js b/addons/api/tests/unit/serializers/storage-bucket-test.js index e9f3c3cdb..17c5ae4f4 100644 --- a/addons/api/tests/unit/serializers/storage-bucket-test.js +++ b/addons/api/tests/unit/serializers/storage-bucket-test.js @@ -8,6 +8,8 @@ import { setupTest } from 'ember-qunit'; import { TYPE_STORAGE_BUCKET_PLUGIN_AWS_S3, TYPE_STORAGE_BUCKET_PLUGIN, + TYPE_CREDENTIAL_STATIC, + TYPE_CREDENTIAL_DYNAMIC, } from 'api/models/storage-bucket'; module('Unit | Serializer | storage bucket', function (hooks) { @@ -27,6 +29,7 @@ module('Unit | Serializer | storage bucket', function (hooks) { const store = this.owner.lookup('service:store'); const record = store.createRecord('storage-bucket', { compositeType: TYPE_STORAGE_BUCKET_PLUGIN_AWS_S3, + credentialType: TYPE_CREDENTIAL_STATIC, name: 'AWS', description: 'this has an aws plugin', bucket_name: 'bucketname', @@ -36,10 +39,13 @@ module('Unit | Serializer | storage bucket', function (hooks) { access_key_id: 'foobars', secret_access_key: 'testing', disable_credential_rotation: true, - role_arn: null, - role_external_id: null, - role_session_name: null, - role_tags: null, + role_arn: 'test', + role_external_id: 'test', + role_session_name: 'test', + role_tags: [ + { key: 'Project', value: 'Automation' }, + { key: 'foo', value: 'bar' }, + ], }); const expectedResult = { name: 'AWS', @@ -55,6 +61,7 @@ module('Unit | Serializer | storage bucket', function (hooks) { role_external_id: null, role_session_name: null, role_tags: null, + secrets_hmac: null, }, secrets: { access_key_id: 'foobars', @@ -64,11 +71,12 @@ module('Unit | Serializer | storage bucket', function (hooks) { assert.deepEqual(record.serialize(), expectedResult); }); - test('it serializes a new dynamic aws plugin as expected', async function (assert) { + test('it serializes a new aws plugin with dynamic credentials as expected', async function (assert) { assert.expect(1); const store = this.owner.lookup('service:store'); const record = store.createRecord('storage-bucket', { compositeType: TYPE_STORAGE_BUCKET_PLUGIN_AWS_S3, + credentialType: TYPE_CREDENTIAL_DYNAMIC, name: 'AWS', description: 'this has an aws plugin', bucket_name: 'bucketname', @@ -100,21 +108,22 @@ module('Unit | Serializer | storage bucket', function (hooks) { role_external_id: 'Example987', role_session_name: 'my-session', role_tags: { Project: 'Automation', foo: 'bar' }, + secrets_hmac: null, }, }; assert.deepEqual(record.serialize(), expectedResult); }); - test('it serializes when updating a storage bucket with static credential correctly', async function (assert) { + + test('it serializes when updating a dynamic type credential storage bucket with static credentials correctly', async function (assert) { assert.expect(1); const store = this.owner.lookup('service:store'); const serializer = store.serializerFor('storage-bucket'); store.push({ data: { - id: '1', + id: '100', type: 'storage-bucket', attributes: { type: TYPE_STORAGE_BUCKET_PLUGIN, - compositeType: TYPE_STORAGE_BUCKET_PLUGIN_AWS_S3, name: 'AWS', description: 'this has an aws plugin', bucket_name: 'bucketname', @@ -125,10 +134,18 @@ module('Unit | Serializer | storage bucket', function (hooks) { secret_access_key: 'testing', disable_credential_rotation: true, version: 1, + role_arn: 'role_arn_test', + role_external_id: 'role_external_id_test', + role_session_name: 'role_session_test', + role_tags: [ + { key: 'Project', value: 'Automation' }, + { key: 'foo', value: 'bar' }, + ], }, }, }); - const record = store.peekRecord('storage-bucket', '1'); + const record = store.peekRecord('storage-bucket', '100'); + record.credentialType = TYPE_CREDENTIAL_STATIC; const snapshot = record._createSnapshot(); const serializedRecord = serializer.serialize(snapshot); @@ -144,6 +161,7 @@ module('Unit | Serializer | storage bucket', function (hooks) { role_external_id: null, role_session_name: null, role_tags: null, + secrets_hmac: null, }, secrets: { access_key_id: 'foobars', @@ -154,7 +172,7 @@ module('Unit | Serializer | storage bucket', function (hooks) { assert.deepEqual(serializedRecord, expectedResult); }); - test('it serializes when updating a storage bucket with dynamic credential correctly', async function (assert) { + test('it serializes when updating a static type credential storage bucket with dynamic credentials correctly', async function (assert) { assert.expect(1); const store = this.owner.lookup('service:store'); const serializer = store.serializerFor('storage-bucket'); @@ -164,7 +182,6 @@ module('Unit | Serializer | storage bucket', function (hooks) { type: 'storage-bucket', attributes: { type: TYPE_STORAGE_BUCKET_PLUGIN, - compositeType: TYPE_STORAGE_BUCKET_PLUGIN_AWS_S3, name: 'AWS', description: 'this has an aws plugin', bucket_name: 'bucketname', @@ -172,6 +189,8 @@ module('Unit | Serializer | storage bucket', function (hooks) { worker_filter: 'workerfilter', region: 'eu-west-1', disable_credential_rotation: true, + access_key_id: 'test', + secret_access_key: 'test', version: 1, role_arn: 'arn', role_external_id: 'Example987', @@ -184,6 +203,7 @@ module('Unit | Serializer | storage bucket', function (hooks) { }, }); const record = store.peekRecord('storage-bucket', '1'); + record.credentialType = TYPE_CREDENTIAL_DYNAMIC; const snapshot = record._createSnapshot(); const serializedRecord = serializer.serialize(snapshot); @@ -199,6 +219,7 @@ module('Unit | Serializer | storage bucket', function (hooks) { role_external_id: 'Example987', role_session_name: 'my-session', role_tags: { Project: 'Automation', foo: 'bar' }, + secrets_hmac: null, }, version: 1, }; diff --git a/ui/admin/app/controllers/scopes/scope/storage-buckets.js b/ui/admin/app/controllers/scopes/scope/storage-buckets.js index cf14667ad..249913c40 100644 --- a/ui/admin/app/controllers/scopes/scope/storage-buckets.js +++ b/ui/admin/app/controllers/scopes/scope/storage-buckets.js @@ -10,7 +10,6 @@ export default class ScopesScopeStorageBucketsController extends Controller { // =services @service intl; - queryParams = ['credentialType']; // = attributes /** diff --git a/ui/admin/app/routes/scopes/scope/storage-buckets.js b/ui/admin/app/routes/scopes/scope/storage-buckets.js index 3d0b4cd66..819c00831 100644 --- a/ui/admin/app/routes/scopes/scope/storage-buckets.js +++ b/ui/admin/app/routes/scopes/scope/storage-buckets.js @@ -20,11 +20,6 @@ export default class ScopesScopeStorageBucketsRoute extends Route { // =attributes - queryParams = { - credentialType: { - refreshModel: true, - }, - }; // =methods /** @@ -101,7 +96,7 @@ export default class ScopesScopeStorageBucketsRoute extends Route { * @param {string} credentialType */ @action - changeType(credentialType) { - this.router.replaceWith({ queryParams: { credentialType } }); + changeType(storageBucket, credentialType) { + storageBucket.credentialType = credentialType; } } diff --git a/ui/admin/app/routes/scopes/scope/storage-buckets/new.js b/ui/admin/app/routes/scopes/scope/storage-buckets/new.js index fe8df1d22..94cf7b315 100644 --- a/ui/admin/app/routes/scopes/scope/storage-buckets/new.js +++ b/ui/admin/app/routes/scopes/scope/storage-buckets/new.js @@ -25,10 +25,23 @@ export default class ScopesScopeStorageBucketsNewRoute extends Route { */ model() { const scopeModel = this.store.peekRecord('scope', 'global'); - let name, description, scope, bucket_name, bucket_prefix, region; + let name, + description, + scope, + bucket_name, + bucket_prefix, + region, + worker_filter; if (this.currentModel?.isNew) { - ({ name, description, scope, bucket_name, bucket_prefix, region } = - this.currentModel); + ({ + name, + description, + scope, + bucket_name, + bucket_prefix, + region, + worker_filter, + } = this.currentModel); this.currentModel.rollbackAttributes(); } const record = this.store.createRecord('storage-bucket', { @@ -40,6 +53,7 @@ export default class ScopesScopeStorageBucketsNewRoute extends Route { bucket_name, bucket_prefix, region, + worker_filter, }); record.scopeModel = scopeModel; return record; diff --git a/ui/admin/app/templates/scopes/scope/storage-buckets/new.hbs b/ui/admin/app/templates/scopes/scope/storage-buckets/new.hbs index 8b2e3d110..1c38ac244 100644 --- a/ui/admin/app/templates/scopes/scope/storage-buckets/new.hbs +++ b/ui/admin/app/templates/scopes/scope/storage-buckets/new.hbs @@ -23,7 +23,7 @@ @submit={{route-action 'save' @model}} @cancel={{route-action 'cancel' @model}} @scopes={{this.scopes}} - @changeType={{route-action 'changeType'}} + @changeType={{route-action 'changeType' @model}} /> diff --git a/ui/admin/app/templates/scopes/scope/storage-buckets/storage-bucket/index.hbs b/ui/admin/app/templates/scopes/scope/storage-buckets/storage-bucket/index.hbs index 115fb83a7..bf58d6bd1 100644 --- a/ui/admin/app/templates/scopes/scope/storage-buckets/storage-bucket/index.hbs +++ b/ui/admin/app/templates/scopes/scope/storage-buckets/storage-bucket/index.hbs @@ -36,7 +36,7 @@ @submit={{route-action 'save' @model}} @cancel={{route-action 'cancel' @model}} @scopes={{this.scopes}} - @changeType={{route-action 'changeType'}} + @changeType={{route-action 'changeType' @model}} /> \ No newline at end of file