Skip to content

fix(AuthVisible): move hooks above early return to satisfy Rules of H… #5615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kashish00208 wants to merge 9 commits into
base: main
Choose a base branch
from
+36 −22
Open

fix(AuthVisible): move hooks above early return to satisfy Rules of H… #5615

Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b088c3f
frontend: AuthVisible: Move hooks before early return
kashish00208 May 13, 2026
d61e271
frontend: AuthVisible: Prevent auth checks for invalid verbs
kashish00208 May 14, 2026
922d2ff
frontend: AuthVisible: Re-add justified exhaustive-deps ESLint disable
kashish00208 May 14, 2026
940a10a
frontend: AuthVisible: Prevent auth checks for invalid verbs
kashish00208 May 15, 2026
a2fdb7f
frontend: AuthVisible: Remove duplicate auth handling logic
kashish00208 May 15, 2026
a7d1346
frontend: AuthVisible: Reuse auth verb validation result
kashish00208 May 15, 2026
0733a84
frontend: AuthVisible: Handle null itemClass in queryKey
kashish00208 May 16, 2026
55f6fbc
frontend: AuthVisible: Fix multi-cluster auth cache reuse
kashish00208 May 17, 2026
78e5a56
frontend: AuthVisible: Use active cluster for auth cache keys
kashish00208 May 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions frontend/src/components/common/Resource/AuthVisible.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,13 @@ export interface AuthVisibleProps extends React.PropsWithChildren<{}> {
export default function AuthVisible(props: AuthVisibleProps) {
const { item, authVerb, subresource, namespace, onError, onAuthResult, children } = props;

if (!VALID_AUTH_VERBS.includes(authVerb)) {
console.warn(`Invalid authVerb provided: "${authVerb}". Skipping authorization check.`);
return null;
}
const isValidVerb = VALID_AUTH_VERBS.includes(authVerb);

Comment thread
kashish00208 marked this conversation as resolved.
const itemClass: KubeObjectClass | null = (item as KubeObject)?._class?.() ?? item;
const itemName = (item as KubeObject)?.getName?.();

// eslint-disable-next-line react-hooks/rules-of-hooks
const { data } = useQuery<any>({
enabled: !!item,
enabled: !!item && isValidVerb,
queryKey: [
Comment thread
kashish00208 marked this conversation as resolved.
'authVisible',
itemName,
Expand All @@ -90,20 +86,27 @@ export default function AuthVisible(props: AuthVisibleProps) {
},
});

const visible = data?.status?.allowed ?? false;
const visible = isValidVerb && (data?.status?.allowed ?? false);

// eslint-disable-next-line react-hooks/rules-of-hooks
useEffect(() => {
if (data) {
onAuthResult?.({
allowed: visible,
reason: data.status?.reason ?? '',
});
if (!isValidVerb || !data) {
return;
}
onAuthResult?.({
allowed: visible,
reason: data.status?.reason ?? '',
});

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [data]);
Comment thread
kashish00208 marked this conversation as resolved.

if (!visible) {
useEffect(() => {
if (!isValidVerb) {
console.warn(`Invalid authVerb provided: "${authVerb}". Skipping authorization check.`);
}
}, [isValidVerb, authVerb]);

if (!isValidVerb || !visible) {
return null;
}

Expand Down
Loading