hughhhh opened a new pull request, #45869:
URL: https://github.com/apache/superset/pull/45869
### SUMMARY
Stacked on #45068 (base branch `hughhhh/embedded-oauth2-fallback` mirrors
its head, `bdf7917`). Fixes three ways an admin or the API could save an
embedded guest credential that is unsafe, or that does nothing:
1. **Turning impersonation off through the API was accepted while a
credential was stored** (sc-130268). The save check returned early unless
Secure Extra was in the payload, so `PUT {"impersonate_user": false}` went
through and embedded viewers then ran as the connection's base account. Saves
are now checked against the stored row with the payload applied. That update is
refused with a message saying to clear the credential first, and the connection
is left unchanged.
2. **With `EMBEDDED_CREDENTIAL_FALLBACK` off, any credential was stored
unchecked** (sc-130270), and it went live when the flag was turned on. With
either flag off, POST/PUT now refuse a new or changed credential. A stored
credential is left alone so it works again when the flag returns. Re-saving it
unchanged, including the dialog's masked round-trip, clearing it, unrelated
edits, connection tests and imports all keep working.
3. **An OAuth2 client with a blank id and secret counted as a configured
client** (sc-127897). That let a credential save on a connection with no real
OAuth2 client, served guests through the fallback account, and logged the wrong
refusal reason. A new shared rule, `oauth2_client_info_configured()`, requires
a non-blank id and secret (a masked secret counts). The save check and
`Database._has_embedded_oauth2_client` both use it, and the form's
`connectionUsesOAuth2` mirrors it. Guests on such a connection now get the "no
OAuth2 client information" reason.
One commit per issue. `UPDATING.md` and the embedding docs are updated.
### TESTING INSTRUCTIONS
Unit tests cover each case in `tests/unit_tests/databases/schema_tests.py`,
`tests/unit_tests/databases/api_test.py`,
`tests/unit_tests/models/core_test.py` and `EmbeddedCredentialsField.test.tsx`.
Each commit's new tests fail on the code before it.
```
pytest tests/unit_tests/databases/ tests/unit_tests/models/core_test.py
tests/unit_tests/utils/oauth2_tests.py
npm run test -- EmbeddedCredentialsField.test.tsx
```
Manual, with `EMBEDDED_SUPERSET` and `EMBEDDED_CREDENTIAL_FALLBACK` on and a
Snowflake OAuth2 connection that stores an embedded credential:
- `PUT /api/v1/database/<id>` with `{"impersonate_user": false}` returns
400, and the connection is unchanged.
- Save an OAuth2 client with `id: ""` and `secret: ""`, then try to save a
credential: 400. An embedded viewer is refused with the "no OAuth2 client
information" reason.
- Turn `EMBEDDED_CREDENTIAL_FALLBACK` off. PUT a new `embedded_credentials`
and get 400. Open and re-save the existing connection unchanged and get 200
with the credential kept. Turn the flag back on and the credential applies.
Known gaps, not addressed here:
- `messages.pot` was already behind source on #45068 and needs a
regeneration (this PR adds two msgids).
- A logged-in user on a blank-client connection still gets an authorization
link with an empty client id (`get_oauth2_config` is unchanged).
- `test_transient_schema_listing_failure_is_recovered_by_retry` also fails
on #45068's head.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: `EMBEDDED_SUPERSET`,
`EMBEDDED_CREDENTIAL_FALLBACK`
- [x] 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
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]
