mikebridge opened a new pull request, #42708:
URL: https://github.com/apache/superset/pull/42708

   ### SUMMARY
   
   The server's `is_editor` (`superset/security/manager.py`) unions the 
explicit editor list with subjects resolved by a deployment's 
`EXTRA_EDITORS_RESOLVER`, and the dashboard GET attaches those ids as 
`extra_editors` after the response schema is dumped 
(`superset/dashboards/api.py:614`), so they survive the `columns` projection. 
The frontend predicate `canUserEditDashboard` checked `editors` alone.
   
   Result: anyone whose editorship comes only through the resolver sees no Edit 
button on a dashboard the API would happily let them write to. This bites 
exactly — and only — in deployments that configure the resolver.
   
   This PR widens the shared predicate in `permissionUtils.ts` to `editors ∪ 
extra_editors`, matching the server, and types the field on `Dashboard`. Since 
`dash_edit_perm` is derived from this function at hydrate time, every consumer 
of that flag is corrected together. Behaviour is unchanged for deployments 
without the resolver.
   
   Split out of #41551, where the gap was found during review (thanks 
@kgabryje) — it is a platform-wide fix independent of version history and 
shouldn't ride inside a feature-flagged PR.
   
   Disclosure: this change was developed with AI assistance (Claude), on behalf 
of and reviewed by @mikebridge.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A — no visual change except the Edit button appearing for users the API 
already authorizes.
   
   ### TESTING INSTRUCTIONS
   
   1. Configure `EXTRA_EDITORS_RESOLVER` in `superset_config.py` to return a 
subject id for a non-admin test user on some dashboard, e.g. a resolver 
granting a specific role's subject.
   2. As that user (no explicit editor entry on the dashboard, no Admin role), 
open the dashboard.
   3. Before: no Edit button, though `PUT /api/v1/dashboard/{id}` succeeds for 
this user. After: Edit button shows and editing works.
   4. Unit coverage: `npm run test -- permissionUtils.test.ts` — includes cases 
for editorship granted through `extra_editors` and for `extra_editors` 
belonging to other subjects.
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [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
   


-- 
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]

Reply via email to