From 5807d61ec7b95ab0d55c3098b0c8a23325b77547 Mon Sep 17 00:00:00 2001 From: Vault Automation Date: Wed, 1 Oct 2025 12:58:16 -0400 Subject: [PATCH] [UI] Ember Data Migration - Control Group Error Handling (#9483) (#9548) * adds error handling for control groups to api service as post request middleware * adds waitFor to async middleware in api service to attempt to fix race conditions in tests Co-authored-by: Jordan Reimer --- ui/app/routes/application.js | 3 +- ui/app/services/api.ts | 37 ++++++++++++++++++---- ui/app/services/control-group.js | 16 +++++++--- ui/tests/helpers/api/error-response.ts | 9 ++++-- ui/tests/unit/services/api-test.js | 19 +++++++++-- ui/types/vault/services/control-group.d.ts | 7 ++-- 6 files changed, 70 insertions(+), 21 deletions(-) diff --git a/ui/app/routes/application.js b/ui/app/routes/application.js index a3f84e88da..32746fc8b6 100644 --- a/ui/app/routes/application.js +++ b/ui/app/routes/application.js @@ -25,7 +25,8 @@ export default class ApplicationRoute extends Route { @action error(error, transition) { const controlGroup = this.controlGroup; - if (error instanceof ControlGroupError) { + // support both types of control group errors until Ember Data is removed + if (error instanceof ControlGroupError || error.isControlGroupError) { return controlGroup.handleError(error); } if (error.path === '/v1/sys/wrapping/unwrap') { diff --git a/ui/app/services/api.ts b/ui/app/services/api.ts index 4b3bedd0f0..12318a4450 100644 --- a/ui/app/services/api.ts +++ b/ui/app/services/api.ts @@ -18,7 +18,7 @@ import { ResponseError, } from '@hashicorp/vault-client-typescript'; import config from 'vault/config/environment'; -import { waitForPromise } from '@ember/test-waiters'; +import { waitForPromise, waitFor } from '@ember/test-waiters'; import type AuthService from 'vault/services/auth'; import type NamespaceService from 'vault/services/namespace'; @@ -84,7 +84,7 @@ export default class ApiService extends Service { }; // -- Post Request Middleware -- - showWarnings = async (context: ResponseContext) => { + showWarnings = waitFor(async (context: ResponseContext) => { const response = context.response.clone(); // if the response is empty, don't try to parse it if (response.headers.get('Content-Length')) { @@ -96,15 +96,35 @@ export default class ApiService extends Service { }); } } - }; + }); - deleteControlGroupToken = async (context: ResponseContext) => { + checkControlGroup = waitFor(async (context: ResponseContext) => { const { url } = context; + const response = context.response.clone(); + const { headers } = response; + const controlGroupToken = this.controlGroup.tokenForUrl(url); if (controlGroupToken) { this.controlGroup.deleteControlGroupToken(controlGroupToken.accessor); } - }; + // if the requested path is locked by a control group we need to create a new error response + if (headers.get('Content-Length')) { + const json = await response.json(); + const wrapTtl = headers.get('X-Vault-Wrap-TTL'); + const isLockedByControlGroup = this.controlGroup.isRequestedPathLocked(json, wrapTtl); + + if (isLockedByControlGroup) { + const error = { + message: 'Control Group encountered', + isControlGroupError: true, + ...json.wrap_info, + }; + return new Response(JSON.stringify(error), { headers, status: 403, statusText: 'Forbidden' }); + } + } + + return; + }); // --- End Middleware --- configuration = new Configuration({ @@ -114,7 +134,7 @@ export default class ApiService extends Service { { pre: this.getControlGroupToken }, { pre: this.setHeaders }, { post: this.showWarnings }, - { post: this.deleteControlGroupToken }, + { post: this.checkControlGroup }, ], fetchApi: (...args: [Request]) => { return waitForPromise(window.fetch(...args)); @@ -165,7 +185,10 @@ export default class ApiService extends Service { async parseError(e: unknown, fallbackMessage = 'An error occurred, please try again') { if (e instanceof ResponseError) { const { status, url } = e.response; - const error = await e.response.json(); + // instances where an error is thrown multiple times could result in the body already being read + // this will result in a readable stream failure and we can't parse the body + // to avoid this, clone the response so we can access the body consistently + const error = await e.response.clone().json(); // typically the Vault API error response looks like { errors: ['some error message'] } // but sometimes (eg RespondWithStatusCode) it's { data: { error: 'some error message' } } const errors = error.data?.error && !error.errors ? [error.data.error] : error.errors; diff --git a/ui/app/services/control-group.js b/ui/app/services/control-group.js index f0649e3aa2..c355c0608c 100644 --- a/ui/app/services/control-group.js +++ b/ui/app/services/control-group.js @@ -86,7 +86,7 @@ export default Service.extend({ return null; }, - checkForControlGroup(callbackArgs, response, wasWrapTTLRequested) { + isRequestedPathLocked(response, wasWrapTTLRequested) { const creationPath = response && response?.wrap_info?.creation_path; if ( this.version.isCommunity || @@ -95,10 +95,18 @@ export default Service.extend({ (creationPath && WRAPPED_RESPONSE_PATHS.includes(creationPath)) || !response.wrap_info ) { - return RSVP.resolve(...callbackArgs); + return false; } - const error = new ControlGroupError(response.wrap_info); - return RSVP.reject(error); + return true; + }, + + checkForControlGroup(callbackArgs, response, wasWrapTTLRequested) { + const isLocked = this.isRequestedPathLocked(response, wasWrapTTLRequested); + if (isLocked) { + const error = new ControlGroupError(response.wrap_info); + return RSVP.reject(error); + } + return RSVP.resolve(...callbackArgs); }, handleError(error) { diff --git a/ui/tests/helpers/api/error-response.ts b/ui/tests/helpers/api/error-response.ts index e75de330ed..045b4136c2 100644 --- a/ui/tests/helpers/api/error-response.ts +++ b/ui/tests/helpers/api/error-response.ts @@ -11,9 +11,14 @@ export const getErrorResponse = (error?: T, status?: number) => { message: 'there were some errors', }; // url is readonly on Response so mock it and cast to Response type - return new ResponseError({ + const response = { status: status || 404, url: `${document.location.origin}/v1/test/error/parsing`, json: () => Promise.resolve(e), - } as Response); + } as Response; + + return new ResponseError({ + ...response, + clone: () => response, + }); }; diff --git a/ui/tests/unit/services/api-test.js b/ui/tests/unit/services/api-test.js index be3ebeaf9f..a3a0d1fc12 100644 --- a/ui/tests/unit/services/api-test.js +++ b/ui/tests/unit/services/api-test.js @@ -26,6 +26,7 @@ module('Unit | Service | api', function (hooks) { this.wrapInfo = { token: 'ctrl-group', accessor: '84tfdfd5pQ5vOOEMxC2o3Ymt' }; this.tokenForUrl = sinon.stub(controlGroupService, 'tokenForUrl').returns(this.wrapInfo); this.deleteControlGroupToken = sinon.spy(controlGroupService, 'deleteControlGroupToken'); + this.isRequestedPathLocked = sinon.stub(controlGroupService, 'isRequestedPathLocked').returns(true); const flashMessageService = this.owner.lookup('service:flash-messages'); this.info = sinon.spy(flashMessageService, 'info'); @@ -128,14 +129,28 @@ module('Unit | Service | api', function (hooks) { assert.true(this.info.notCalled, 'No warning messages are shown'); }); - test('it should delete control group token', async function (assert) { - await this.apiService.deleteControlGroupToken({ url: this.url }); + test('it should check for control group', async function (assert) { + const headers = new Headers({ 'Content-Length': '100', 'X-Vault-Wrap-TTL': 1800 }); + const body = { data: null, wrap_info: this.wrapInfo }; + const apiResponse = new Response(JSON.stringify(body), { headers }); + + const response = await this.apiService.checkControlGroup({ url: this.url, response: apiResponse }); assert.true(this.tokenForUrl.calledWith(this.url), 'Url is passed to tokenForUrl method'); assert.true( this.deleteControlGroupToken.calledWith(this.wrapInfo.accessor), 'Control group token is deleted' ); + assert.true(this.isRequestedPathLocked.calledWith(body, '1800'), 'isRequestedPathLocked called'); + + assert.strictEqual(response.status, 403, 'Response status is updated to 403 for control group error'); + const ctrlError = await response.json(); + const expectedError = { + message: 'Control Group encountered', + isControlGroupError: true, + ...this.wrapInfo, + }; + assert.deepEqual(ctrlError, expectedError, 'Control group error is returned in response body'); }); test('it should build headers', async function (assert) { diff --git a/ui/types/vault/services/control-group.d.ts b/ui/types/vault/services/control-group.d.ts index 06ca7990cc..8a4b1c7d3b 100644 --- a/ui/types/vault/services/control-group.d.ts +++ b/ui/types/vault/services/control-group.d.ts @@ -30,11 +30,8 @@ export default class ControlGroupService extends Service { markTokenForUnwrap(accessor: string): void; unmarkTokenForUnwrap(): void; tokenForUrl(url: string): { token: string; accessor: string; creationTime: string } | null; - checkForControlGroup( - callbackArgs: unknown, - response: ApiResponse, - wasWrapTTLRequested: boolean - ): Promise; + isRequestedPathLocked(response: ApiResponse, wrapTtl?: string | null): boolean; + checkForControlGroup(callbackArgs: unknown, response: ApiResponse, wrapTtl?: string): Promise; saveTokenFromError(error: WrapInfo): void; logFromError(error: WrapInfo): ControlGroupErrorLog; }