Repository navigation
Conversation
An embedded viewer authenticates with a guest token rather than a Superset account, so there is no per-user OAuth2 token to resolve for them and no route by which they could authorize one. `start_oauth2_dance` nonetheless reads `g.user.id` to build its state, and `GuestUser` defines no `id` -- so a guest who hits an auth failure on any OAuth2-enabled connection (Snowflake, Databricks, Trino, GSheets) got an `AttributeError` and an HTTP 500 instead of the driver's own error. `needs_oauth2` now refuses the dance for guests, which covers the nine call sites that gate on it. `check_for_oauth2` needs the check repeated because its `OAuth2TokenRefreshError` arm reaches the dance without consulting `needs_oauth2`. `start_oauth2_dance` itself now raises `OAuth2Error` rather than `AttributeError` if a future caller skips both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Snowflake's OAuth2 support is complete on the backend -- `supports_oauth2`, `is_oauth2_enabled`, `get_oauth2_config`, an OAuth2-aware `impersonate_user`, and `$.oauth2_client_info.secret` already masked -- but none of it is reachable from the UI. `SnowflakeParametersSchema` never declared `oauth2_client_info`, and `parameters_json_schema` never attached `x-encrypted-extra`, which is the annotation the connection dialog uses both to render the field and to move its value into `masked_encrypted_extra`. The only way to configure Snowflake OAuth2 was to hand-edit the Secure extra JSON blob. Adds the field and the annotation, mirroring gsheets and bigquery. The endpoints are deliberately left without defaults: they live on the customer's own Snowflake account, so no URI would be right for every deployment. No frontend change is needed -- `oauth2_client_info` is already in `FormFieldOrder` and already mapped to `OAuth2ClientField`. The typing gap that existing tests cast around is closed, and the leak-prevention properties of `build_sqlalchemy_uri` and `update_params_from_encrypted_extra` are pinned by test rather than left to inspection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Per-user database OAuth2 and embedded dashboards do not currently compose. An embedded viewer signs in with a guest token rather than a Superset account, so there is no per-user OAuth2 token to resolve for them -- and an external viewer often has no identity on the analytical database at all. Embedded dashboards on a per-user OAuth2 connection therefore cannot run queries. This lets one connection carry a username and password used *only* when the requesting principal is an embedded guest, while logged-in users continue to authenticate individually via OAuth2 with no route to the stored credential. Gated behind a new `EMBEDDED_CREDENTIAL_FALLBACK` feature flag, default False, so nothing changes for any existing deployment. The credential lives per-connection in the existing `encrypted_extra` column -- no migration -- and is configured through the connection dialog; the password masks on read while the username stays visible so an admin can see who embedded queries run as. `Database.get_embedded_fallback_credentials` is the single gate, requiring all of: the deployment flag, an engine opt-in (Snowflake only), a complete stored credential, an embedded guest principal with EMBEDDED_SUPERSET on, and a connection that actually uses OAuth2. The fallback path deliberately skips impersonation rather than extending it. That is load-bearing twice: an opted-in engine sets its OAuth2 authenticator during impersonation whether or not a token exists, and `effective_username` resolves to a guest token's own `username` claim, which whoever mints the token controls. The credential is set on the URL rather than in `connect_args` so the engine cache key, which has no user component, separates the two principals. Accompanied by a SIP draft in docs/sip/ to seed the formal discussion; the security model and rejected alternatives are set out there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
feat(db): support Snowflake embedded fallback key pairs
…spec
Splits the embedded-only fallback credential along one line: `Database` owns
*when* a credential may be used, and the DB engine spec owns *whether* the
engine supports one, *what shape* it has, and *how* it reaches the driver.
Before this, `Database._get_sqla_engine` inlined a `URL.create` that assumed
every credential maps onto URL `username`/`password`, and the shape rule --
`auth_method in {password, keypair}`, `privatekey_body`, `privatekey_pass` --
lived as a static method on the model. Snowflake's PEM key could not be
expressed that way, so `SnowflakeEngineSpec.update_params_from_encrypted_extra`
reached back into `Database.get_embedded_fallback_credentials()` to fetch the
credential a second time. That back-reference meant the engine spec could not be
read, imported or unit-tested against a core without the feature, and an engine
whose driver wants the credential anywhere else had nowhere to say so.
Three new members on `BaseEngineSpec`:
- `apply_embedded_credentials(database, credentials, url, engine_kwargs)` --
called instead of `impersonate_user`. Default replaces the identity on the URL
with `URL.create` (not `URL.set`, which ignores `password=None` and would
leave a key pair carrying the connection's own password). Snowflake overrides
it to add `connect_args["private_key"]`, and its
`update_params_from_encrypted_extra` is now only a guard that asks
`embedded_fallback_applies()`.
- `validate_embedded_credentials(credentials)` -- the canonical shape rule,
moved out of the model to `superset.databases.utils` so the marshmallow
validators can reach it without an engine in hand, and so `databases/utils.py`
no longer imports `Database`.
- `get_embedded_credential_cache_key(credentials)` -- what `cache_keys.py`
hardcoded.
`parameters_json_schema` was a near-verbatim copy on four engine specs; the
branch had made Snowflake's copy the only place that registered
`encrypted_field_properties` and gated the credential field on
`EMBEDDED_CREDENTIAL_FALLBACK`, so opting a second engine in meant copying all
of it. It is now `build_parameters_json_schema()` in `base.py`, called by
`BasicParametersMixin` and by a four-line delegation on Snowflake, which cannot
mix the mixin in because its `build_sqlalchemy_uri` takes a narrower parameters
type. `encrypted_field_properties` moved to `superset/databases/types.py`, which
exists so it can be imported outside an app context, and is re-exported from
`schemas.py`; this also drops a heavy import edge from `snowflake.py`.
The `encrypted_extra` key is now `constants.EMBEDDED_CREDENTIALS_KEY` rather
than a literal repeated in six places.
No behaviour change: the five-condition gate, the refusal when a guest has no
applicable credential, and the engine-cache separation are untouched. New tests
pin the extension points -- a spec subclass changing how the credential is
applied, one changing what counts as a credential, one changing the cache key,
and a second engine opting in with a schema field and a class attribute alone --
plus a regression test that the Snowflake spec no longer fetches the credential
in `update_params_from_encrypted_extra`.
This commit also carries uncommitted working-tree work that predates it and was
not authored in this session: the save-time completeness validation
(`databases/utils.py`, `databases/schemas.py`, `commands/database/validate.py`),
the `embedded_fallback_applies()` split and refusal error in `models/core.py`,
the data-cache key separation in `utils/cache_keys.py`, the key-pair UI in
`EmbeddedCredentialsField.tsx`, and the accompanying docs, tests and catalog.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings the branch up to b411454, 170 commits on from the base it was cut at. Five files conflicted; all five were mechanical. - `tests/unit_tests/db_engine_specs/test_base.py` and `test_snowflake.py`: both sides append at EOF. Kept both, including master's `test_expand_nested_columns_default_returns_input` and `test_extended_aggregation_func_median_uses_native_snowflake_syntax`. - `superset/db_engine_specs/snowflake.py`: master added `import sqlalchemy as sa` (used by its new `_extended_aggregations`) immediately above the `apispec` imports this branch stopped using when it dropped the file's `parameters_json_schema` override. Kept `sa`, dropped the apispec imports. - `UPDATING.md`: both sides insert at the top of `## Next`. Kept both blocks. - `superset/translations/messages.pot`: took master's `% calculation` format-flag correction and its four new `Alerts & Reports` entries, keeping this branch's nine new msgids. The resolved template differs from master's only by the creation stamp and those nine strings -- no reordering, no dropped entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…client Dashboard and chart bundles do not carry encrypted_extra, so a connection promoted by export/import keeps "Impersonate logged in user" but loses its OAuth2 client and embedded viewer credential. The fallback gate then no longer applies, and guests go through impersonation on whatever identity the engine derives from the URL -- for Snowflake, the base account, with no error or warning. An embedded guest on an opted-in, impersonating connection with no usable OAuth2 client (global or stored) is now refused with a message telling the admin the OAuth2 and embedded viewer settings are missing, for example after an import. The cache key gets the same `embedded_credentials_missing` marker so a warm logged-in entry cannot bypass the refusal. Logged-in users, service-account connections, and deployments without the flag or engine opt-in are unaffected. Also pins that the base update_params_from_encrypted_extra drops the embedded credential from engine kwargs, and stubs the fallback lookups on the query-context and serialization tests' mocked databases, which otherwise serialize a MagicMock into the cache key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes found validating the feature against QA's secret scenarios (sc-124914) and the epic's bugs: - Masking: every engine masks the embedded credential's password, private key and passphrase, as it already does the OAuth2 client secret, so an engine that overrides encrypted_extra_sensitive_fields can't expose it. - Import over an existing connection never stores the mask as the secret. The import schema asks for any masked value the existing connection has no stored value for (e.g. a copy promoted by a dashboard bundle, or a repointed connection), and import_database refuses to store one that is still masked (QA OEF-SECRET-04). - Empty secrets are no longer masked, so a removed credential round-trips and later saves aren't rejected as half-filled (QA OEF-SECRET-07). - A non-blank credential is refused unless the connection has an OAuth2 client (stored or deployment-wide) and impersonation on; elsewhere it saved and silently did nothing (sc-126624). - A create no longer fails when only the post-create permission sync needs the admin's OAuth2 token, which reported 422 after the row was committed (sc-126623). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n import Dashboard, chart and dataset bundles wrote each database YAML without encrypted_extra, so promoting a dashboard dropped the connection's OAuth2 client and embedded viewer credential with no warning, and the import had no way to put them back. - Export: the dataset exporter (which writes the database YAML for chart, dashboard and dataset bundles) carries Secure Extra like the database export does -- masked when it holds secrets. - Import API: the dashboard, chart, dataset and saved-query import endpoints accept encrypted_extra_secrets, as the database import does. - Import schema: those commands reuse an existing database rather than overwrite it, so ImportV1DatabaseSchema(overwrites_existing=False) skips asking for secrets that would never be applied; a new database still asks for every masked secret, and an overwrite that needs one is refused by import_database. - Frontend: the dashboard, chart, dataset and saved-query lists pass the import modal its encrypted-extra field state, so the prompts render, and the import help text no longer says Secure Extra is missing from export files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…role The connection form always writes a role into the URI, and the embedded login kept it, so a least-privilege fallback user -- or a programmatic access token restricted to its own role -- was refused by Snowflake (390186) and every embedded chart failed. Drop the connection's role on the embedded login so Snowflake uses the fallback user's default role. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A guest on a per-user OAuth2 connection with no embedded credential, or on one that lost its OAuth2 client, was refused with a 500. Before the fallback, the same viewer got a 400 from the failed OAuth2 query. This is a configuration problem an administrator fixes, not a server fault, so return a 400 and keep it out of server-error monitoring. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ogin A connection URI naming authenticator=SNOWFLAKE_JWT kept it on the guest login, so the driver expected a key pair from a password or token guest and failed before contacting Snowflake. Drop it as role already is; the driver picks key-pair sign-in itself when the embedded credential supplies private_key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nnect - A database error raised while a chart query is being built -- a virtual dataset's default-schema lookup connects -- escaped as an HTML 500. Report it as a failed query (a 400) like any other. - Log when the database rejects a guest's fallback credential at connect time, naming the connection and the exception class only. - Never pass oauth2_client_info or embedded_credentials to the driver. Engines that copy all of encrypted_extra into the engine kwargs failed every connect after a switch away from Snowflake, a 500 on that save. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
QA review of the embedded guest credential dialog: - Keep the section visible but disabled, with the reason, unless the connection has an OAuth2 client and impersonation is on -- the only case where the backend accepts a credential. - Put the help text under the section title, use the base password input (with show/hide) for the secrets, add placeholders, turn off autofill, and show a Configured tag when a credential is stored. - Mirror edits into Secure Extra so its JSON view stays current, and add an explicit Remove that deletes the block instead of leaving it empty. - The impersonation tooltip mentions the Snowflake requirement. - Docs: OAuth2 client information is on the Basic tab, setup is numbered steps, and a requirements and limitations list covers flags, OAuth2 and impersonation, the default role, warehouse access, supported credential types, async queries and removal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The "OAuth2 connections and guests" note said such a viewer gets the database driver's own authentication error, contradicting the EMBEDDED_CREDENTIAL_FALLBACK section. With the flag on, Superset refuses the query before connecting; with it off, the driver's error is redacted for guests. Either way the viewer sees the generic data error and the reason is in the server logs. Fixes sc-127527 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…on path Chart data on a physical table connects through get_raw_connection(). On SQLAlchemy 2, a failed raw_connection() doesn't dispatch handle_error, so the "Embedded fallback credential rejected" line never fired for the most common chart; only the virtual-dataset path (engine.connect()) logged it. get_raw_connection now logs the rejection for engines that carry the fallback credential, through one helper shared with the engine event that logs each failure once and names the driver's exception class either way. Fixes sc-127710 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rry only two blocks From review of the 6.0 backport: - The guest login dropped only role and authenticator, so a user, password, key or token in the connection's URI query or connect_args still reached the Snowflake driver and won over the embedded account. Drop every sign-in argument from both. - Dashboard, chart and dataset bundles wrote the connection's whole Secure Extra, raw whenever masking left it unchanged, exposing settings an engine doesn't register as sensitive (a Postgres client key) to anyone who can export a dashboard. Carry only the masked oauth2_client_info and embedded_credentials blocks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An overwrite import whose embedded guest credential leaves out a secret the stored one has (e.g. a key-pair credential without privatekey_body) answered OK and replaced the stored credential with one no embedded viewer can log in with: only masked values are revealed, an absent one is neither revealed nor asked for (sc-127530). With EMBEDDED_SUPERSET and EMBEDDED_CREDENTIAL_FALLBACK on, the import runs the completeness check the dialog and the API already run, and refuses with an ImportFailedError naming the missing field, so the stored credential is kept. A new connection with a half-filled credential is refused the same way. With the fallback off nothing changes, matching the API. The assets import re-raises an ImportFailedError as is, so the refusal (and the existing "Must provide value" refusals) reach the caller instead of "Import failed for an unknown reason". The new-connection path looks up its engine spec via Database.get_db_engine_spec, which copes with a missing driver. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hughhhh
requested review from
geido,
michael-s-molina,
msyavuz,
rusackas,
sadpandajoe and
villebro
as code owners
October 30, 2026 00:42
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
sadpandajoe
reviewed
Oct 30, 2026
github-actions
Bot
added
the
requires:rebase
…uth2-fallback Conflicts: UPDATING.md (kept both Next entries), messages.pot (master's template plus this branch's msgids, re-normalized with msgcat). Semantic conflict: master's apache#45579 moved the import routes to parse_json_form_field and dropped their json import, so this branch's json.loads(encrypted_extra_secrets) in the chart, dashboard, dataset and saved-query import routes raised NameError. Those reads use parse_json_form_field too, so a malformed value is a 400 like the other form fields. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The import routes' encrypted_extra_secrets form field was missing from openapi.json, and messages.pot had drifted from the branch's later string edits; both fail their CI drift checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EncryptedExtraInputChange stores embedded_credentials as an object, but the action shared a payload type whose value is a string, so tsc rejected the reducer test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An engine that masks Secure Extra with `$.*` (Databricks, the base spec) turns `oauth2_client_info` into a bare mask string. Carrying it made a fresh import ask for `$.oauth2_client_info` and store the answer as a string, so every query then failed with "Invalid input type". Carry a block only when it is still a dict, which keeps master's behavior for those engines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Earlier releases stored the mask when an overwrite import had nothing to reveal. `unrevealable_masked_fields` counted that stored mask as a value, so the schema asked for nothing and `import_database`'s backstop failed the import with a 500. A stored mask now counts as missing. The same 500 happened when the import moves the connection: `import_database` never reveals stored secrets onto a new destination, but the schema still treated them as revealable. The schema now shares the importer's identity check (moved to `commands/database/utils.py`) and asks for every masked value in that case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ecrets With `overwrite` and `overwrite_all` set, the dashboard importer overwrites related databases, but validated them with a schema that assumed reuse and skipped the missing-secret check. A secret the stored connection could not reveal then reached `import_database`'s backstop as a 500 instead of the structured prompt. The database schema now follows the effective overwrite setting. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uth2-fallback Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # tests/unit_tests/importexport/api_test.py
# Conflicts: # UPDATING.md
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #45870 +/- ##
==========================================
+ Coverage 84.91% 84.95% +0.04%
==========================================
Files 3075 3077 +2
Lines 207804 208242 +438
Branches 48986 49113 +127
==========================================
+ Hits 176452 176912 +460
+ Misses 27992 27956 -36
- Partials 3360 3374 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The import refused a half-filled embedded guest credential with the right message but as HTTP 500, since ImportFailedError is a server fault (sc-127530). Raise it as ImportRefusedError, an ImportFailedError with status 400. UPDATING.md no longer says asset import is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
removed
the
requires:rebase
geido
reviewed
Nov 2, 2026
- Treat a whitespace-only private key as missing. - Refuse a masked embedded secret the connection has nothing stored for, which would otherwise be saved as the credential. - Check imported masks and credentials with the incoming URI's engine, so an overwrite that changes engine is not judged by the old engine's paths. - Count any stored value except None, an empty string or a mask as revealable. - Cover an overwrite that supplies only encrypted_extra. - Correct the SIP on import validation and the flag text on key pairs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Test connection no longer refuses the masked embedded secret the dialog sends back for a saved connection. It saves nothing, and the command unmasks. - Importing a dashboard, chart or dataset bundle keeps Secure Extra settings the bundle does not carry. A database export still replaces the stored value. - A guest on a connection with an incomplete OAuth2 client gets the refusal rather than the ValidationError from get_oauth2_config(). - Saving refuses a credential sent as a JSON string, which query time rejects. - Test the chart data database error path through execute() as well as run(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Partial bundles now say they are partial. The importer used to infer it from the keys present, so a database export holding only oauth2_client_info kept a revoked embedded credential. - A Secure Extra edit retires the typed embedded credential, which onSave would otherwise copy back over the edit. - Import asks for private keys in a multiline input, so PEM newlines survive. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Export a bundle and overwrite-import it, so dropping the marker fails a test. - The dialog refuses a save with impersonation off while a credential is set, and a PUT sending only impersonate_user is accepted. The SIP still described an inline warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Replace the partial bundle marker with an argument from the dashboard importer, the only import that overwrites from a partial bundle. The marker made an earlier release reject the file. - Key column value caching on the embedded and per-user identity, so a guest and a logged-in user do not share native filter values. - Clear the typed OAuth2 client on a Secure Extra edit, as well as the credential. - Use a multiline input for private keys in database file imports too. - Note in UPDATING what exports now carry, whatever the flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # superset/commands/database/validate.py # tests/unit_tests/commands/databases/validate_test.py # tests/unit_tests/databases/api_test.py # tests/unit_tests/databases/schema_tests.py
Other tests leave an id on the shared import fixtures, which the dashboard schema rejects, so the test failed in some orderings of the full suite. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sadpandajoe
reviewed
Nov 3, 2026
| return | ||
|
|
||
| credentials = encrypted_extra.get(EMBEDDED_CREDENTIALS_KEY) | ||
| if not embedded_credentials_incomplete( |
Member
There was a problem hiding this comment.
A bundle whose embedded_credentials is a JSON string, e.g. "{\"username\":\"svc\",\"password\":\"pw\"}", passes this check because accept_json_string defaults to True, but the importer stores the string as-is. Query time then rejects a non-dict value, so embedded viewers get the missing-credential refusal and an overwrite replaces a working credential with one that can't be used. The save schema passes accept_json_string=False for this reason. Should the import path do the same?
|
|
||
| # Never store a mask as if it were the secret: the schema asks for any | ||
| # value that cannot be revealed, so one left here is a bug, not input. | ||
| if masked_encrypted_extra and ( |
Member
There was a problem hiding this comment.
This still-masked check only runs when the bundle carried masked_encrypted_extra. A file using the plain encrypted_extra key skips it (the import schema has no rename like the API schemas), and _refuse_incomplete_embedded_credentials counts PASSWORD_MASK as a present password. Database export writes plain encrypted_extra when the stored value already equals its masked form, so exporting such a connection and importing it with both flags on would store XXXXXXXXXX as the guest password. Should the mask check also run for the plain encrypted_extra path?
| // `onSave` copies these typed blocks over Secure Extra, which would undo | ||
| // this later edit of them. | ||
| const { | ||
| embedded_credentials: _typedCredentials, |
Member
There was a problem hiding this comment.
Dropping oauth2_client_info from parameters on any Secure Extra edit loses OAuth2 values that only live there. OAuth2ClientField.handleChange only calls onParametersChange, so a typed client ID never reaches masked_encrypted_extra. Changing the ID to B in the OAuth2 sub-form and then typing in the Secure Extra editor resets the inputs to the stored A, and Save keeps A. Should only embedded_credentials be stripped here, or should the OAuth2 field also copy into Secure Extra?
github-actions
Bot
added
the
requires:rebase
1 of 9 tasks
geido
reviewed
Nov 3, 2026
geido
left a comment
geido
left a comment
Member
There was a problem hiding this comment.
Left my notes on the shared import code over on #45890 (#45890 (review)), since the code is the same here. The plan there is to land #45890 first and rebuild this one on master, which leaves just the import fixes.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
Importing a connection file onto an existing per-user OAuth2 Snowflake connection could quietly break every embedded dashboard on it. If the file's embedded guest credential had the username, the Key pair method and the passphrase but no private key, the overwrite import answered "OK" and asked for nothing. The stored private key was gone afterwards, and every embedded viewer got a generic "An error occurred while fetching the data."
The connection dialog and the database API already refuse that half-filled credential ("Embedded guest credentials are invalid or incomplete…"). Import never ran that check. It restores secrets that arrive masked, but one the file leaves out entirely is neither restored nor asked for.
This PR runs the same completeness check on import when
EMBEDDED_SUPERSETandEMBEDDED_CREDENTIAL_FALLBACKare both on, as the API does. A half-filled credential is refused with a message that names what's missing, and the stored credential is left alone:/api/v1/assets/import/used to turn every error into "Import failed for an unknown reason". It now passes anImportFailedErrorthrough unchanged, as/api/v1/database/import/already does, so this refusal and the existing "Must provide value…" refusals reach the caller.Database.get_db_engine_spec, which handles a missing driver instead of crashing.The other option was to keep the stored secret when the file omits it. I didn't take it: it would keep a value the file deliberately left out, and it still wouldn't stop a half-filled credential from being saved, which the dialog and API already enforce.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (backend only).
TESTING INSTRUCTIONS
EMBEDDED_SUPERSETandEMBEDDED_CREDENTIAL_FALLBACK. Create a Snowflake connection with an OAuth2 client, "Impersonate logged in user" on, and Key pair embedded guest credentials (username, private key, passphrase).privatekey_bodyfromembedded_credentialsinmasked_encrypted_extra.200 OKand the private key is gone. After: the import is refused with a message naming$.embedded_credentials.privatekey_body, and the Edit dialog still shows the stored key.Unit tests:
These cover an overwrite with the key missing (refused, key kept), a new connection missing its password (refused), a complete credential (imports, key restored), and the flag off (imports as before). The API cases send a real bundle through
/api/v1/assets/import/.ADDITIONAL INFORMATION
EMBEDDED_SUPERSET,EMBEDDED_CREDENTIAL_FALLBACKStacked on #45068, which it fixes: the diff against
masterincludes that PR's commits. Review only the top commit,dca0eb342a("fix(embedded): refuse a half-filled embedded credential on import"). This should merge after #45068, or be folded into it.Created with Claude Opus 5.5 in Claude Code.
🤖 Generated with Claude Code