amaannawab923 commented on code in PR #45522:
URL: https://github.com/apache/superset/pull/45522#discussion_r4401289758
##########
superset-frontend/src/extensions/ExtensionsLoader.ts:
##########
@@ -211,12 +226,25 @@ class ExtensionsLoader {
// receives the same pre-bound instance. Parallel loading is safe because
each
// container gets its own scope with its own resolved module instance.
const context = createExtensionContext(extension);
+ const declaredPages = extension.contributions?.pages ?? [];
Review Comment:
Confirmed as described -- `declaredPages` is captured once when the
extension loads -- but the Security tag overstates where it lands.
Two things bound it. The server authorises per request against the
requirement recorded when the rule was registered, and every endpoint on an
extension's own API is `@protect()`ed, so the worst outcome is a menu entry
that should not be offered and a 403 on the click, not access to anything.
And this is how the rest of Superset already behaves: roles arrive in
bootstrap data read at page load, and the SPA does not re-fetch them after a
role change either. A host screen a user has just lost access to stays in their
menu until they reload. Re-fetching only for contributed pages would make
extension pages the one surface in the app that behaves differently, for no
security gain.
So I would close this unless re-fetching bootstrap data on a role change is
something we want host-wide, which is a larger change than this PR.
--
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]
