hughhhh opened a new pull request, #45889:
URL: https://github.com/apache/superset/pull/45889
### SUMMARY
An embedded guest credential that uses a key pair could be saved broken.
With a wrong passphrase, or a private key that isn't PEM, the connection saved
with a 200. Every embedded chart on it then failed with HTTP 500 "Issue 1011 –
unexpected error". The server log showed a raw `ValueError: Incorrect password,
could not decrypt key` traceback instead of the "Embedded fallback credential
rejected" line an admin looks for. A wrong password already fails cleanly, with
a 400 and a readable log line.
The key is decoded in `apply_embedded_credentials`, before the engine and
its `handle_error` listener exist, so the error never reached the existing
rejection handling. Save time only checked that the fields were filled in.
**At save:** a new engine-spec hook,
`BaseEngineSpec.get_embedded_credentials_error`, says why a complete credential
can't be used (it does nothing by default). Snowflake loads the key there. The
database API schema and the connection dialog's validate-parameters command
both call it, so a bad key is refused with:
> The embedded guest private key could not be read. Provide a PEM private
key and its correct passphrase (leave the passphrase empty for an unencrypted
key).
The message never quotes the key or passphrase. On an edit, unchanged
secrets arrive masked, so they are restored from the stored connection before
the check. An untouched credential still saves, and a new passphrase is checked
against the stored key.
**At query time:** if a key still fails (for example, one saved before this
change), `_get_sqla_engine` logs `Embedded fallback credential rejected for
database X (id=N): ValueError`, with the exception class only. It then raises a
400 `CONNECTION_ACCESS_DENIED_ERROR`. Guest redaction turns that into the same
"An error occurred while fetching the data." a rejected password gives.
I kept the key check out of `validate_embedded_credentials` on purpose. That
method is also the query-time gate, so making it reject a bad key would quietly
drop the credential, show "no credential configured" and skip the admin log
line.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (backend only).
### TESTING INSTRUCTIONS
1. Turn on `EMBEDDED_SUPERSET` and `EMBEDDED_CREDENTIAL_FALLBACK`. Create a
Snowflake connection with an OAuth2 client and "Impersonate logged in user" on.
2. Set Key pair embedded guest credentials with an encrypted PEM key and the
wrong passphrase, then save. Before: saved. After: refused with the message
above, on the embedded credential field.
3. Repeat with a body that isn't a PEM key: refused the same way. With the
right passphrase: saves.
4. Edit the connection without touching the credential: it still saves.
5. To check query time, store a broken key directly (or from before this
change) and open an embedded dashboard as a guest. Before: HTTP 500 "Issue
1011" and a raw `ValueError` traceback. After: HTTP 400 with the generic
message, and one "Embedded fallback credential rejected … ValueError" warning
in the log.
Unit tests, all using real generated keys:
```
pytest tests/unit_tests/db_engine_specs/test_snowflake.py \
tests/unit_tests/databases/schema_tests.py \
tests/unit_tests/commands/databases/validate_test.py \
tests/unit_tests/models/core_test.py
```
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: `EMBEDDED_SUPERSET`,
`EMBEDDED_CREDENTIAL_FALLBACK`
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
Stacked on apache/superset#45068: merging this lands the fix on that PR's
branch.
Created with Claude Opus 5.5 in Claude Code.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
