[hughhhh]

SUMMARY

Stacked on #45068. The diff includes all of #45068; review only the top two commits (5107dbad69, 4c72545c17). Merge after #45068.

With EMBEDDED_CREDENTIAL_FALLBACK on, an embedded dashboard on a Snowflake connection that has "Impersonate logged in user" on but no OAuth2 client stops loading for outside viewers. Refusing those viewers is intended, but nobody could tell why. The viewer saw only "Data error: An error occurred while fetching the data.", and the server log said the connection's settings "may have been lost", which is rarely the cause. On Snowflake, impersonation without OAuth2 never changed the login, so these connections were usually just never set up for OAuth2. The docs also left out several limits an admin needs to know before turning the flag on.

Refusal message (5107dbad69)

• Guest errors are redacted to the generic message unless Superset wrote the error type. Both guest refusals (no OAuth2 client, no embedded credential) now raise DATABASE_SECURITY_ACCESS_ERROR, which survives that redaction, with a fixed message that names nothing about the connection: "This chart can't be shown here because its database connection isn't set up for embedded dashboards. Please contact the dashboard's owner."
• A server-log warning names the connection and says how to fix it. With no OAuth2 client: turn off "Impersonate logged in user", or configure an OAuth2 client and embedded guest credentials. With no credential: add embedded guest credentials.

Docs (4c72545c17): UPDATING.md, embedding.mdx and the SIP now cover:

• which embedded dashboards stop loading when the flag is turned on, and the two ways to fix them;
• with GLOBAL_ASYNC_QUERIES on, logged-in users' charts on a Snowflake OAuth2 connection run as the base account (this predates the flag);
• with "Per user caching" off, logged-in users on an OAuth2 connection can be shown each other's cached results;
• connections created in SQLAlchemy URI mode show neither the OAuth2 nor the embedded credential section, so set those values in Secure extra instead;
• turning off "Impersonate logged in user" moves embedded viewers, and logged-in users, onto the base account.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not captured. The embedded viewer's banner changes from "Data error: An error occurred while fetching the data." to the message above.

TESTING INSTRUCTIONS

1. Enable EMBEDDED_SUPERSET and EMBEDDED_CREDENTIAL_FALLBACK.
2. Create a Snowflake connection with a username and password, turn on "Impersonate logged in user", and configure no OAuth2 client.
3. Embed a dashboard with a chart on it and open it with a guest token. The chart shows the new message, and the server log has a Refused an embedded viewer's query on database connection ... warning naming the connection and both fixes.
4. Turn off "Impersonate logged in user" and reload. The chart loads under the base account.

Unit tests: pytest tests/unit_tests/models/core_test.py

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags: EMBEDDED_SUPERSET, EMBEDDED_CREDENTIAL_FALLBACK
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

Created with Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 95.76613% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.96%. Comparing base (15e4aca) to head (32d5a84).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/commands/database/validate.py	80.95%	3 Missing and 1 partial ⚠️
superset/commands/dataset/export.py	73.33%	3 Missing and 1 partial ⚠️
superset/databases/utils.py	90.24%	2 Missing and 2 partials ⚠️
superset/db_engine_specs/snowflake.py	90.62%	2 Missing and 1 partial ⚠️
...rset-frontend/src/components/ImportModal/index.tsx	66.66%	2 Missing ⚠️
superset/db_engine_specs/base.py	95.45%	1 Missing and 1 partial ⚠️
superset/databases/types.py	85.71%	0 Missing and 1 partial ⚠️
superset/models/core.py	98.76%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #45890      +/-   ##
==========================================
+ Coverage   84.91%   84.96%   +0.04%     
==========================================
  Files        3075     3077       +2     
  Lines      207804   208214     +410     
  Branches    48986    49103     +117     
==========================================
+ Hits       176452   176901     +449     
+ Misses      27992    27947      -45     
- Partials     3360     3366       +6     

Flag	Coverage Δ
hive	34.22% <25.58%> (-0.03%)	⬇️
javascript	81.27% <98.71%> (+0.03%)	⬆️
mysql	52.22% <38.52%> (-0.05%)	⬇️
postgres	52.23% <39.11%> (-0.05%)	⬇️
presto	35.93% <25.88%> (-0.03%)	⬇️
python	87.92% <94.41%> (+0.05%)	⬆️
sqlite	51.98% <38.52%> (-0.05%)	⬇️
unit	82.80% <94.41%> (+0.11%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sadpandajoe]

With both flags set, this makes the database schema ask for any OAuth2 or embedded secret the stored connection can't reveal. But import_database() only overwrites when the caller also has can_write on Database (superset/commands/database/importers/v1/utils.py); without it, it returns the existing connection untouched. A role that can import dashboards but not edit connections, importing with overwrite=true&overwrite_all=true onto an existing database whose Secure Extra lacks the exported blocks, now fails validation asking for secrets that would never be applied, where that import used to succeed. Should the schema flag also account for the Database write permission, so validation and execution share one overwrite decision?

[geido]

+1, same as my note on #45068 (#45068 (comment)), and it still reproduces on this head. As Alpha, an "overwrite all" dashboard import asks for a secret that then gets dropped, and /api/v1/assets/import/ 500s with "unknown reason"; both work on master. Checking can_write on Database next to overwrites_existing in validate_masked_encrypted_extra (schemas.py:1280) should cover both paths.

[sadpandajoe]

This partitions the chart's own connection, but a chart-backed annotation layer reads through another dataset's connection, and _annotation_cache_context in query_context_processor.py binds only that source's RLS and the user id. Guests share one user id, so after a guest warms chart A, an admin who swaps the annotation source's embedded credential for a less-privileged one (or clears it) still gets A's cached annotation_data replayed, without reaching the refusal. Should the annotation source connection's embedded discriminator go into the parent key too?

[sadpandajoe]

This says nothing changes for an existing deployment and that export/import round-trips unchanged with the flag off. But superset/commands/dataset/export.py now writes the carried Secure Extra blocks whatever the flag, and a destination on an earlier release replaces its whole Secure Extra on an overwrite import, which UPDATING.md already warns about. An operator who reads only this section could upgrade the export source first and lose the destination's unrelated Secure Extra. Could this section match UPDATING.md? Also, the interface list above names validated_embedded_credentials() as a new Database method, but it is a module-level function in superset/databases/utils.py.

[sadpandajoe]

Nothing in DatabaseModal/index.test.tsx exercises this renderer; the new textarea assertions cover ImportModal. If renderAsTextArea were dropped here, every existing test would still pass while a pasted multi-line key is flattened by a single-line input and the stored key can't be loaded. Could you add a test that imports a database asking for $.embedded_credentials.privatekey_body and its passphrase, pastes a multi-line PEM, and asserts a textarea for the key, a password input for the passphrase, and unchanged newlines in the submitted encrypted_extra_secrets?

[sadpandajoe]

A key pair with a wrong passphrase or a malformed PEM raises a plain ValueError from _load_private_key here, while the engine URL is still being built. That is before create_engine's error mapping and before _log_rejected_embedded_credential attaches its listener, and save-time validation only checks that the fields are present. The embedded viewer gets an unmapped server error instead of the refusal message, and the admin gets a bare traceback instead of the rejected-credential warning this change adds. Should a load failure here be turned into the same refusal and warning as a rejected sign-in?

[geido]

Went through this together with #45870 and #45068, since they land as one. Nothing in this PR's own changes worries me; the open bits are all in the shared import code:

• can_write: +1'd @sadpandajoe's thread here, it still reproduces on this head.
• Stored null secrets: +1'd @sadpandajoe's json.py:395 note on #45068 (#45068 (comment)), same story.
• One new note inline: "overwrite all" can pair one instance's OAuth2 client id with another's secret.

For landing, since this and #45870 both carry all of #45068 and we squash:

1. Fix the UPDATING.md conflict (keep both entries) and push; that re-runs lint-frontend, which failed on the usual tsc build race.
2. Land this one first under a feat title that names #45068, then close #45068.
3. Rebuild #45870 on master. What's left is the import fixes (9 files, no core.py).
4. Rebuild #45889 and #45869 on master too. QA's d74a648 build had their early commits, so what QA signed off on isn't complete without them. When rebuilding #45889, routing its refusal through _refuse_embedded_guest would give viewers the proper message.

Happy to take another look once the threads are settled.

[geido]

Something I don't think anyone has flagged yet: if staging and prod share a connection (same UUID and URI) but each has its own OAuth2 client, an "overwrite all" import of a staging dashboard leaves prod with staging's client id and prod's secret, with no prompt, so OAuth2 breaks for everyone on that connection. Embedded credentials mix the same way (staging's username, prod's password). Master's database import already does this, but dashboard bundles never carried these blocks before. Could the secret count as unrevealable when the bundle's oauth2_client_info.id differs from the stored one, and the embedded password when the username differs?