sadpandajoe commented on code in PR #42404:
URL: https://github.com/apache/superset/pull/42404#discussion_r3993334673
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -139,13 +160,46 @@ const ThemeModal: FunctionComponent<ThemeModalProps> = ({
const supersetTheme = useTheme();
const { setTemporaryTheme } = useThemeContext();
const [disableSave, setDisableSave] = useState<boolean>(true);
- const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
- const [initialTheme, setInitialTheme] = useState<ThemeObject | null>(null);
+ const [currentTheme, setCurrentTheme] = useState<ThemeModalObject | null>(
+ null,
+ );
+ const [initialTheme, setInitialTheme] = useState<ThemeModalObject | null>(
+ null,
+ );
const [isHidden, setIsHidden] = useState<boolean>(true);
const [showConfirmAlert, setShowConfirmAlert] = useState<boolean>(false);
const isEditMode = theme !== null;
const isSystemTheme = currentTheme?.is_system === true;
- const isReadOnly = isSystemTheme;
+
+ const currentUser = useSelector<any, UserWithPermissionsAndRoles>(
+ state => state.user,
+ );
+ const currentUserSubjectId = getBootstrapData()?.common?.user_subject_id;
+
+ // theme fetch logic
+ const {
+ state: { loading, resource },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<ThemeObject, ThemeSavePayload>(
+ 'theme',
+ t('theme'),
+ addDangerToast,
+ );
+
+ // In edit mode a non-editor (and non-admin) may only view the theme. The
+ // editorship check runs against the persisted editors (and any editorship
+ // granted indirectly via EXTRA_EDITORS_RESOLVER) from the fetched
+ // resource, not the in-progress picker selection.
+ const canEditTheme =
+ !isEditMode ||
+ isUserEditorOrAdmin(
+ currentUser,
+ (resource?.editors as Subject[]) || [],
+ resource?.extra_editors,
+ );
+ const isReadOnly = isSystemTheme || !canEditTheme;
Review Comment:
A non-admin editor of an active default/dark theme is shown a View action in
the list, but this read-only check becomes false after the resource loads
because it ignores `is_system_default`/`is_system_dark`; the modal then enables
Save and every PUT ends in the server's 403. Should the modal apply the same
active-slot/admin guard as `UpdateThemeCommand`?
--
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]
