[BUG] OSD returns HTTP 500 internal server error messages when the wrapping key is changed

[zhongnansu]

OSD returns HTTP 500 internal server error messages when the wrapping key is changed and the service is unable to decrypt the credentials. When the user does not update the credentials and attempts to create a new index pattern, the calls to the target endpoint fails and return HTTP 500 errors. A generic error message is returned in response.

The recommendation will be to return an error message or at least a suggestion to the user asking to verify if the credentials have been updated post changes to the wrapping key.

[noCharger]

More context -

When we use random keyring to decrypt the message, an exception will throw with error message unencryptedDataKey has not been set. We have a test case to cover this https://github.com/opensearch-project/OpenSearch-Dashboards/blob/feature/multi-datasource/src/plugins/data_source/server/cryptography/cryptography_client.test.ts#L97-L115

This exception is caught in the described situation.

[noCharger]

Issue - Internal server error after expection throw

Procedure to reproduce -

1. Create a new set of credentials
2. Create a data source and configure the data source to use the newly created credentials
3. Navigate to Stack Management -> Index Patterns -> Create index pattern
4. Select the data source created in Step 2 above
5. Observe that the service is able to retrieve the available indices
6. From the opensearch_dashboards.yml file change one of the values in
   data_source.encryption.wrappingKey
7. From the OpenSearch dashboard, navigate to Stack Management -> Index Patterns -> Create index
   pattern
8. Search for an index pattern, observe that the OSD fails to load any indices
9. In the browser developer console, under Networks observe that the API requests fail and return HTTP 500
   Internal Server Error responses

[noCharger]

Proposed solution -

Add error handling logic and toast/other alert UX on everywhere integrated with new data source client.

For example -
https://github.com/opensearch-project/OpenSearch-Dashboards/blob/feature/multi-datasource/src/plugins/index_pattern_management/public/components/create_index_pattern_wizard/lib/get_indices.ts#L105-L131

and

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/feature/multi-datasource/src/plugins/index_pattern_management/server/routes/resolve_index.ts#L62-L70

[zhongnansu]

@noCharger Thanks for the investigation

Since the correct 400 error is thrown by SavedObjectsErrorHelpers.createBadRequestError(error.message); in the crypto manager side. The issue is more with the opensearch/internal/search API route handler side. It failed to interpret the error object accordingly in the response handling. Because the error is a hapi/boom object.

• The hapi/boom 400 error object looks like this

isBoom: true,
  isServer: false,
  data: null,
  output: {
    statusCode: 400,
    payload: {
      message: 'unencryptedDataKey has not been set: Bad Request',
      statusCode: 400,
      error: 'Bad Request'
    },
    headers: {}
  },
  [Symbol(SavedObjectsClientErrorCode)]: 'SavedObjectsClient/badRequest'

• Since the hapi/boom error object doesn't have a direct error.statusCode field, this search API will just return a 500. See
  

  OpenSearch-Dashboards/src/plugins/data/server/search/routes/search.ts

  Lines 81 to 86 in 730a75a

  	return res.customError({
  	statusCode: err.statusCode || 500,
  	body: {
  	message: err.message,
  	attributes: {
  	error: err.body?.error || err.message,

There are 2 approaches we can solve this.

1. we don't use SavedObjectsErrorHelpers.createBadRequestError(error.message); to create error object, instead we build our own Error object that has the statusCode field and error message, that search API route handler can understand
2. We add a wrapper to help search API understand the hapi/boom error object, I also found a code reference
   

   OpenSearch-Dashboards/src/core/server/http/router/error_wrapper.ts

   Lines 31 to 46 in 730a75a

   	import Boom from '@hapi/boom';
   	import { RequestHandlerWrapper } from './router';
   	export const wrapErrors: RequestHandlerWrapper = (handler) => {
   	return async (context, request, response) => {
   	try {
   	return await handler(context, request, response);
   	} catch (e) {
   	if (Boom.isBoom(e)) {
   	return response.customError({
   	body: e.output.payload,
   	statusCode: e.output.statusCode,
   	headers: e.output.headers as { [key: string]: string },
   	});
   	}
   	throw e;

As for the UI/UX side change, not sure if for 400 it already has some UI popup there, you can look into it.