Preserve encrypted credentials when editing data sources

[revijay]

Description

When editing an existing data source, the UI intentionally does not send stored credentials back to the server. The scoped Saved Objects client strips encrypted credentials, which prevents the data source client from being reconstructed for operations such as Test Connection and Fetch Metadata when credentials are omitted during edit.

This change introduces an internal Saved Objects repository that is used only after access has been authorized through the scoped Saved Objects client. The internal repository is then used to retrieve the encrypted credentials so the data source client can be created successfully.

Issues Resolved

Closes #12385

Screenshot

N/A (No UI changes)

Testing the changes

• Ran yarn test:jest src/plugins/data_source.
• Created a new data source using Username/Password authentication.
• Verified Test Connection succeeds during data source creation.
• Saved the data source successfully.
• Edited the existing data source without re-entering the password.
• Verified Test Connection succeeds while editing the data source.
• Verified the data source can be saved successfully after editing.

Check List

• All tests pass
  • yarn test:jest
  • yarn test:jest_integration
• New functionality includes testing.
• New functionality has been documented.
• Commits are signed per the DCO using --signoff

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 Security concerns Privilege escalation risk: The routes now pass an internal (unscoped) saved objects repository to getDataSourceClient. If getDataSourceClient does not first authorize the data source read via the scoped savedObjects client before using internalSavedObjects to fetch encrypted credentials, an authenticated user could invoke test-connection/fetch-metadata against data sources they lack permission to read, effectively leveraging stored credentials they are not authorized to use.

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: Pass internal saved objects to test_connection route Relevant files: src/plugins/data_source/server/plugin.ts src/plugins/data_source/server/routes/test_connection.ts Sub-PR theme: Pass internal saved objects to fetch_data_source_metadata route Relevant files: src/plugins/data_source/server/plugin.ts src/plugins/data_source/server/routes/fetch_data_source_metadata.ts

⚡ Recommended focus areas for review Uninitialized Internal Repository The routes are registered during setup() but this.internalSavedObjects appears to be assigned later (likely in start()). The getter () => this.internalSavedObjects will return undefined if a request arrives before start() completes. Additionally, if the internal repository is never assigned (e.g., wiring missing in start()), the fix silently falls back to the scoped client and the original bug persists. Verify initialization ordering and consider logging/erroring when the internal repository is unavailable at request time. config.endpointAllowlistedSuffixes, () => this.internalSavedObjects ); registerFetchDataSourceMetaDataRoute( router, dataSourceService, cryptographyServiceSetup, authRegistryPromise, customApiSchemaRegistryPromise, this.logger.get('fetch-data-source-metadata'), endpointDeniedIPs, config.endpointAllowlistedSuffixes, () => this.internalSavedObjects Privilege Escalation Risk Passing an internal (unscoped) saved objects repository into getDataSourceClient bypasses the user's scoped ACL for reading the data source object. The PR description states access is authorized first via the scoped client, but this must be strictly enforced in getDataSourceClient — otherwise a user could trigger test-connection/fetch-metadata against a data source they cannot access, using its stored encrypted credentials. Confirm the scoped client is used to authorize the read before the internal repository is used to fetch credentials. savedObjects: context.core.savedObjects.client, internalSavedObjects: getInternalSavedObjects?.(), Privilege Escalation Risk Same concern as in test_connection.ts: the internal saved objects repository is passed to getDataSourceClient, which must ensure the caller has been authorized through the scoped client before credentials are decrypted and used. Otherwise this could expose data sources the user is not permitted to access. savedObjects: context.core.savedObjects.client, internalSavedObjects: getInternalSavedObjects?.(),

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Verify internal saved objects getter initialization The internalSavedObjects property is referenced via a getter callback, which suggests it is assigned asynchronously (likely in start()). Ensure this property is declared on the class and initialized before routes are invoked; otherwise the callback will always return undefined at request time if it is only ever set on a local variable. Also confirm the same getter is passed consistently to both route registrations (it is here) to avoid drift. src/plugins/data_source/server/plugin.ts [152-153] +config.endpointAllowlistedSuffixes, +() => this.internalSavedObjects - Suggestion importance[1-10]: 4 __ Why: The suggestion only asks to verify that internalSavedObjects is initialized before use, without proposing an actual code change (existing_code equals improved_code). It raises a valid concern but offers no concrete improvement.	Low
	Warn when internal repository is missing If getInternalSavedObjects is not provided or returns undefined, the credential preservation logic downstream will silently fail during test-connection for existing data sources. Consider logging a warning when the repository is unavailable but a dataSourceId is present, so misconfigurations are diagnosable rather than causing opaque auth failures. src/plugins/data_source/server/routes/test_connection.ts [113-114] savedObjects: context.core.savedObjects.client, -internalSavedObjects: getInternalSavedObjects?.(), +internalSavedObjects: (() => { + const repo = getInternalSavedObjects?.(); + if (!repo && dataSourceId) { + logger.warn('Internal saved objects repository unavailable; encrypted credentials may not be preserved for existing data source.'); + } + return repo; +})(), cryptography, Suggestion importance[1-10]: 3 __ Why: Adding a warning log for diagnostics is a minor improvement for observability, but it's speculative and may add noise. The impact is low and it doesn't fix a functional issue.	Low