Skip to content

Commit

Permalink
Handle promise-proxy deprecation (#28563)
Browse files Browse the repository at this point in the history
* fix promise issues on transformation-edit

* fix one test and the transition problem

* cannot call capabilities service directly inside template because its an unresolved promise

* address transit capabilities issues

* remove deprecations line for promise-proxies

* handle hot mess of delete permissions and such

* blah

* update flash message language. It will now show a flash message for each role whose transformationw as not removed.

* small wording change

* one small change to the default flash message

* Update ui/app/components/transformation-edit.js

Co-authored-by: claire bontempo <[email protected]>

* Update ui/app/components/transformation-edit.js

Co-authored-by: claire bontempo <[email protected]>

* Update ui/app/components/transformation-edit.js

Co-authored-by: claire bontempo <[email protected]>

* fix policy flow

* fix linting and can't define let outside if block

* fix flashmessage things

* make show and edit use same param

---------

Co-authored-by: claire bontempo <[email protected]>
  • Loading branch information
Monkeychip and hellobontempo authored Oct 3, 2024
1 parent 1eaca82 commit c006568
Show file tree
Hide file tree
Showing 14 changed files with 68 additions and 61 deletions.
4 changes: 2 additions & 2 deletions ui/app/components/alphabet-edit.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
{{#if (eq this.mode "show")}}
<Toolbar>
<ToolbarActions>
{{#if this.capabilities.canDelete}}
{{#if this.model.updatePath.canDelete}}
<Hds::Button
@text="Delete alphabet"
@color="secondary"
Expand All @@ -40,7 +40,7 @@
/>
<div class="toolbar-separator"></div>
{{/if}}
{{#if this.capabilities.canUpdate}}
{{#if this.model.updatePath.canUpdate}}
<ToolbarSecretLink
@secret={{concat this.model.idPrefix this.model.id}}
@mode="edit"
Expand Down
74 changes: 36 additions & 38 deletions ui/app/components/transformation-edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,12 @@ export default TransformBase.extend({
this.set('initialRoles', this.model.allowed_roles);
},

updateOrCreateRole(role, transformationId, backend) {
return this.store
async updateOrCreateRole(role, transformationId, backend) {
const roleRecord = await this.store
.queryRecord('transform/role', {
backend,
id: role.id,
})
.then((roleStore) => {
let transformations = roleStore.transformations;
if (role.action === 'ADD') {
transformations = addToList(transformations, transformationId);
} else if (role.action === 'REMOVE') {
transformations = removeFromList(transformations, transformationId);
}
roleStore.setProperties({
backend,
transformations,
});
return roleStore.save().catch((e) => {
return {
errorStatus: e.httpStatus,
...role,
};
});
})
.catch((e) => {
if (e.httpStatus !== 403 && role.action === 'ADD') {
// If role doesn't yet exist, create it with this transformation attached
Expand All @@ -64,29 +46,45 @@ export default TransformBase.extend({
errorStatus: e.httpStatus,
};
});
// if an error occurs while querying the role, exit function and return the error
if (roleRecord.errorStatus) return roleRecord;
// otherwise update the role with the transformation and save
let transformations = roleRecord.transformations;
if (role.action === 'ADD') {
transformations = addToList(transformations, transformationId);
} else if (role.action === 'REMOVE') {
transformations = removeFromList(transformations, transformationId);
}
roleRecord.setProperties({
backend,
transformations,
});
return roleRecord.save().catch((e) => {
return {
errorStatus: e.httpStatus,
...role,
};
});
},

handleUpdateRoles(updateRoles, transformationId) {
if (!updateRoles) return;
const backend = this.model.backend;
const promises = updateRoles.map((r) => this.updateOrCreateRole(r, transformationId, backend));

Promise.all(promises).then((results) => {
const hasError = results.find((role) => !!role.errorStatus);

if (hasError) {
let message =
'The edits to this transformation were successful, but transformations for its roles was not edited due to a lack of permissions.';
if (results.find((e) => !!e.errorStatus && e.errorStatus !== 403)) {
// if the errors weren't all due to permissions show generic message
// eg. trying to update a role with empty array as transformations
message = `You've edited the allowed_roles for this transformation. However, the corresponding edits to some roles' transformations were not made`;
}
this.flashMessages.info(message, {
sticky: true,
priority: 300,
});
const { backend } = this.model;
updateRoles.forEach(async (record) => {
// For each role that needs to be updated, update the role with the transformation.
const updateOrCreateResponse = await this.updateOrCreateRole(record, transformationId, backend);
// If an error was returned, check error type and show a message.
const errorStatus = updateOrCreateResponse?.errorStatus;
let message;
if (errorStatus == 403) {
message = `The edits to this transformation were successful, but transformations for the role ${record.id} were not edited due to a lack of permissions.`;
} else if (errorStatus) {
message = `You've edited the allowed_roles for this transformation. However, there was a problem updating the role: ${record.id}.`;
}
this.flashMessages.info(message, {
sticky: true,
priority: 300,
});
});
},

Expand Down
16 changes: 11 additions & 5 deletions ui/app/models/transit-key.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,6 @@ export default class TransitKeyModel extends Model {
});
}

get canDelete() {
const deleteAttrChanged = Boolean(this.changedAttributes().deletionAllowed);
return this.deletionAllowed && deleteAttrChanged === false;
}

get keyVersions() {
let maxVersion = Math.max(...this.validKeyVersions);
const versions = [];
Expand Down Expand Up @@ -181,6 +176,17 @@ export default class TransitKeyModel extends Model {
get canRead() {
return this.secretPath.get('canUpdate') !== false;
}
get canUpdate() {
return this.secretPath.get('canUpdate') !== false;
}
get canDelete() {
// there's more to just a permissions check here.
// must also check if there's a property on the key called deletionAllowed that is set to true
const deleteAttrChanged = Boolean(this.changedAttributes().deletionAllowed);
const keyAllowedDeletion = this.deletionAllowed && deleteAttrChanged === false;
return this.secretPath.get('canDelete') !== false && keyAllowedDeletion;
}

get canEdit() {
return this.secretPath.get('canUpdate') !== false;
}
Expand Down
1 change: 0 additions & 1 deletion ui/app/routes/vault/cluster/policy/show.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export default Route.extend(UnloadModelRoute, {
const type = this.policyType();
return hash({
policy: this.store.findRecord(`policy/${type}`, params.policy_name),
capabilities: this.store.findRecord('capabilities', `sys/policies/${type}/${params.policy_name}`),
});
},

Expand Down
4 changes: 2 additions & 2 deletions ui/app/templates/components/transform-role-edit.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
{{#if (eq this.mode "show")}}
<Toolbar>
<ToolbarActions>
{{#if this.capabilities.canDelete}}
{{#if this.model.updatePath.canDelete}}
<ConfirmAction
@buttonText="Delete role"
class="toolbar-button"
Expand All @@ -41,7 +41,7 @@
/>
<div class="toolbar-separator"></div>
{{/if}}
{{#if this.capabilities.canUpdate}}
{{#if this.model.updatePath.canUpdate}}
<ToolbarSecretLink
@secret={{concat this.model.idPrefix this.model.id}}
@mode="edit"
Expand Down
4 changes: 2 additions & 2 deletions ui/app/templates/components/transform-template-edit.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
{{#if (eq this.mode "show")}}
<Toolbar>
<ToolbarActions>
{{#if this.capabilities.canDelete}}
{{#if this.model.updatePath.canDelete}}
<Hds::Button
@text="Delete template"
@color="secondary"
Expand All @@ -40,7 +40,7 @@
/>
<div class="toolbar-separator"></div>
{{/if}}
{{#if this.capabilities.canUpdate}}
{{#if this.model.updatePath.canUpdate}}
<ToolbarSecretLink
@secret={{concat this.model.idPrefix this.model.id}}
@mode="edit"
Expand Down
4 changes: 2 additions & 2 deletions ui/app/templates/components/transformation-edit.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
{{#if (eq this.mode "show")}}
<Toolbar>
<ToolbarActions>
{{#if this.capabilities.canDelete}}
{{#if this.model.updatePath.canDelete}}
{{#if (gt this.model.allowed_roles.length 0)}}
<ToolTip @verticalPosition="above" @horizontalPosition="center" as |T|>
<T.Trigger @tabindex="-1">
Expand Down Expand Up @@ -58,7 +58,7 @@
{{/if}}
<div class="toolbar-separator"></div>
{{/if}}
{{#if this.capabilities.canUpdate}}
{{#if this.model.updatePath.canUpdate}}
{{#if (gt this.model.allowed_roles.length 0)}}
<Hds::Button
@text="Edit transformation"
Expand Down
3 changes: 1 addition & 2 deletions ui/app/templates/components/transit-edit.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,13 @@
@deleteKey={{action "deleteKey"}}
@key={{this.key}}
@requestInFlight={{this.requestInFlight}}
@capabilities={{this.capabilities}}
@model={{this.model}}
/>
{{else if (eq this.mode "show")}}
<TransitFormShow
@refresh={{action "refresh"}}
@tab={{this.tab}}
@key={{this.key}}
@capabilities={{this.capabilities}}
@mode={{this.mode}}
@model={{this.model}}
@backend={{this.backend}}
Expand Down
4 changes: 2 additions & 2 deletions ui/app/templates/components/transit-form-edit.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
</div>
<div class="field is-grouped is-grouped-split box is-fullwidth is-bottomless">
<div class="field is-grouped">
{{#if @capabilities.canUpdate}}
{{#if @model.canUpdate}}
<div class="control">
<Hds::Button
@text="Update transit key"
Expand All @@ -109,7 +109,7 @@
/>
</div>
</div>
{{#if (and @key.canDelete @capabilities.canDelete)}}
{{#if @model.canDelete}}
<ConfirmAction @buttonText="Delete transit key" @onConfirmAction={{@deleteKey}} />
{{/if}}
</div>
Expand Down
2 changes: 1 addition & 1 deletion ui/app/templates/components/transit-form-show.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
/>
{{/if}}
{{#if (eq @mode "show")}}
{{#if (or @capabilities.canUpdate @capabilities.canDelete)}}
{{#if (or @model.canUpdate @model.canDelete)}}
<ToolbarSecretLink @secret={{@key.id}} @backend={{@key.backend}} @mode="edit" replace={{true}}>
Edit key
</ToolbarSecretLink>
Expand Down
4 changes: 2 additions & 2 deletions ui/app/templates/vault/cluster/policy/edit.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
</h1>
</p.levelLeft>
</PageHeader>
{{#if (and (not-eq this.model.id "root") (or this.capabilities.canUpdate this.capabilities.canDelete))}}
{{#if (and (not-eq this.model.id "root") (or this.model.canUpdate this.model.canDelete))}}
<Toolbar>
<ToolbarActions>
{{#if (and (not-eq this.model.id "default") this.capabilities.canDelete)}}
{{#if (and (not-eq this.model.id "default") this.model.canDelete)}}
<ConfirmAction
@buttonText="Delete policy"
class="toolbar-button"
Expand Down
2 changes: 1 addition & 1 deletion ui/app/templates/vault/cluster/policy/show.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
@extension={{if (eq this.policyType "acl") this.model.format "sentinel"}}
@text="Download policy"
/>
{{#if (and (not-eq this.model.id "root") (or this.capabilities.canUpdate this.capabilities.canDelete))}}
{{#if (and (not-eq this.model.id "root") (or this.model.canUpdate this.model.canDelete))}}
<ToolbarLink
@route="vault.cluster.policy.edit"
@models={{array this.policyType this.model.id}}
Expand Down
1 change: 0 additions & 1 deletion ui/config/deprecation-workflow.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ self.deprecationWorkflow.config = {
workflow: [
{ handler: 'silence', matchId: 'ember-engines.deprecation-router-service-from-host' },
// ember-data
{ handler: 'silence', matchId: 'ember-data:deprecate-promise-proxies' }, // Transform secrets
{ handler: 'silence', matchId: 'ember-data:no-a-with-array-like' }, // MFA
{ handler: 'silence', matchId: 'ember-data:deprecate-promise-many-array-behaviors' }, // MFA
],
Expand Down
6 changes: 6 additions & 0 deletions ui/tests/integration/components/transit-edit-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { module, test } from 'qunit';
import { setupRenderingTest } from 'vault/tests/helpers';
import { click, fillIn, render } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';
import { setupMirage } from 'ember-cli-mirage/test-support';
import { capabilitiesStub } from 'vault/tests/helpers/stubs';

const SELECTORS = {
createForm: '[data-test-transit-create-form]',
Expand All @@ -16,9 +18,13 @@ const SELECTORS = {
};
module('Integration | Component | transit-edit', function (hooks) {
setupRenderingTest(hooks);
setupMirage(hooks);

hooks.beforeEach(function () {
this.store = this.owner.lookup('service:store');
this.server.post('/sys/capabilities-self', () =>
capabilitiesStub('transit-backend/keys/some-key', ['sudo'])
);
this.model = this.store.createRecord('transit-key', { backend: 'transit-backend', id: 'some-key' });
this.backendCrumb = {
label: 'transit',
Expand Down

0 comments on commit c006568

Please sign in to comment.