Skip to content

feature: Added FGTs support in github integration #648

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

Closed
wants to merge 4 commits into from
Closed

feature: Added FGTs support in github integration #648

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f313cd
Added FGTs support in github integration
sahitya-chandra Apr 22, 2025
c3b92e6
ran pre-commit
sahitya-chandra Apr 22, 2025
2997bf1
linting done
sahitya-chandra Apr 22, 2025
7e19386
linting done
sahitya-chandra Apr 23, 2025
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
85 changes: 34 additions & 51 deletions web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { useDispatch } from '@/store';
import {
checkGitHubValidity,
linkProvider,
getMissingPATScopes
getMissingTokenScopes
} from '@/utils/auth';
import { depFn } from '@/utils/fn';

Expand All @@ -27,7 +27,6 @@ export const ConfigureGithubModalBody: FC<{
const { enqueueSnackbar } = useSnackbar();
const dispatch = useDispatch();
const isLoading = useBoolState();

const showError = useEasyState<string>('');

const setError = useCallback(
Expand All @@ -49,49 +48,35 @@ export const ConfigureGithubModalBody: FC<{
return;
}
depFn(isLoading.true);
checkGitHubValidity(token.value)
.then(async (isValid) => {
if (!isValid) throw new Error('Invalid token');
})
.then(async () => {
try {
const res = await getMissingPATScopes(token.value);
if (res.length) {
throw new Error(`Token is missing scopes: ${res.join(', ')}`);
}
} catch (e) {
// @ts-ignore
throw new Error(e?.message, e);
}
})
.then(async () => {
try {
return await linkProvider(token.value, orgId, Integration.GITHUB);
} catch (e: any) {
throw new Error(
`Failed to link Github${e?.message ? `: ${e?.message}` : ''}`,
e
);
}
})
.then(() => {
dispatch(fetchCurrentOrg());
dispatch(
fetchTeams({
org_id: orgId
})
try {
const { isValid, tokenType } = await checkGitHubValidity(token.value);
if (!isValid) {
throw new Error('Invalid token');
}

const missingScopes = await getMissingTokenScopes(token.value, tokenType);
if (missingScopes.length) {
throw new Error(
`Token is missing ${tokenType === 'PAT' ? 'scopes' : 'permissions'}: ${missingScopes.join(', ')}`
);
enqueueSnackbar('Github linked successfully', {
variant: 'success',
autoHideDuration: 2000
});
onClose();
})
.catch((e) => {
setError(e.message);
console.error(`Error while linking token: ${e.message}`, e);
})
.finally(isLoading.false);
}

await linkProvider(token.value, orgId, Integration.GITHUB, { tokenType });
dispatch(fetchCurrentOrg());
dispatch(fetchTeams({ org_id: orgId }));
enqueueSnackbar(`GitHub linked successfully with ${tokenType}`, {
variant: 'success',
autoHideDuration: 2000
});
onClose();
} catch (e: any) {
setError(
`Failed to link GitHub${e?.message ? `: ${e?.message}` : ''}`
);
console.error(`Error while linking token: ${e.message}`, e);
} finally {
depFn(isLoading.false);
}
}, [
dispatch,
enqueueSnackbar,
Expand All @@ -106,7 +91,7 @@ export const ConfigureGithubModalBody: FC<{
return (
<FlexBox gap2>
<FlexBox gap={2} minWidth={'400px'} col>
<FlexBox>Enter you Github token below.</FlexBox>
<FlexBox>Enter your GitHub Personal Access Token (PAT) or Fine-Grained Token (FGT) below.</FlexBox>
<FlexBox fullWidth minHeight={'80px'} col>
<TextField
onKeyDown={(e) => {
Expand All @@ -124,7 +109,7 @@ export const ConfigureGithubModalBody: FC<{
onChange={(e) => {
handleChange(e.currentTarget.value);
}}
label="Github Personal Access Token"
label="GitHub Token (PAT or FGT)"
type="password"
/>
<Line error tiny mt={1}>
Expand All @@ -143,7 +128,7 @@ export const ConfigureGithubModalBody: FC<{
textUnderlineOffset: '2px'
}}
>
Generate new classic token
Generate new token
</Line>
</Link>
<Line ml={'5px'}>{' ->'}</Line>
Expand All @@ -153,9 +138,9 @@ export const ConfigureGithubModalBody: FC<{

<FlexBox justifyBetween alignCenter mt={'auto'}>
<FlexBox col sx={{ opacity: 0.8 }}>
<Line>Learn more about Github</Line>
<Line>Learn more about GitHub</Line>
<Line>
Personal Access Token (PAT)
Personal Access Tokens and Fine-Grained Tokens
<Link
ml={1 / 2}
href="https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens"
Expand Down Expand Up @@ -209,12 +194,10 @@ const TokenPermissions = () => {
},
{
height: '120px',

top: '378px'
},
{
height: '120px',

top: '806px'
}
].map((item) => ({ ...item, ...baseStyles }));
Expand Down
41 changes: 34 additions & 7 deletions web-server/src/utils/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const linkProvider = async (
reject(isNil, {
provider,
the_good_stuff: stuff,
meta_data: meta
meta_data: { ...meta, tokenType: meta?.tokenType || 'PAT' }
})
);
};
Expand All @@ -29,21 +29,49 @@ export const linkProvider = async (

export async function checkGitHubValidity(
good_stuff: string
): Promise<boolean> {
): Promise<{ isValid: boolean; tokenType: 'PAT' | 'FGT' }> {
try {
await axios.get('https://api.github.com/user', {
const response = await axios.get('https://api.github.com/user', {
headers: {
Authorization: `token ${good_stuff}`
}
});
return true;

const scopesString = response.headers['x-oauth-scopes'] as string;
// PATs have scopes in x-oauth-scopes, FGTs do not
const tokenType = scopesString ? 'PAT' : 'FGT';
return { isValid: true, tokenType };
} catch (error) {
return false;
return { isValid: false, tokenType: 'PAT' }; // Default to PAT for error handling
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

❓ Verification inconclusive

Invalid‑token path always labels the token as a PAT
Inside the catch block we return { isValid: false, tokenType: 'PAT' }.
If the call fails because the user supplied an FGT with no user scope, we’ll still tell the UI it’s a PAT. The UI then shows “PAT” in its error toast, which can mislead users.

Diff suggestion:

-    return { isValid: false, tokenType: 'PAT' }; // Default to PAT for error handling
+    // We can’t know the type here – surface the uncertainty
+    return { isValid: false, tokenType: 'unknown' as never };

You can branch on 'unknown' in the caller to render a neutral error message.

#!/bin/bash
set -e

echo "=== checkGitHubValidity implementation ==="
rg -n "export async function checkGitHubValidity" -A10 -B3 web-server/src/utils/auth.ts || true

echo
echo "=== All references to checkGitHubValidity ==="
rg -n "checkGitHubValidity" -n || true

Revise catch handling to avoid mis‑labeling tokens

The catch block today always returns { isValid: false, tokenType: 'PAT' }, which misleads callers when the failure was for an FGT (no scopes). Instead, we should surface “unknown” and update the function’s signature and callers accordingly.

Changes required:

  • Expand the return type to include an "UNKNOWN" variant.
  • In the catch, return { isValid: false, tokenType: 'UNKNOWN' }.
  • Update all call sites of checkGitHubValidity to handle the new "UNKNOWN" branch and render a neutral error.

Diff in web-server/src/utils/auth.ts:

-export async function checkGitHubValidity(
-  githubToken: string
-): Promise<{ isValid: boolean; tokenType: 'PAT' | 'FGT' }> {
+export async function checkGitHubValidity(
+  githubToken: string
+): Promise<{ isValid: boolean; tokenType: 'PAT' | 'FGT' | 'UNKNOWN' }> {
   try {
     const response = await axios.get('https://api.github.com/user', {
       headers: { Authorization: `token ${githubToken}` }
     });
     const scopesString = response.headers['x-oauth-scopes'] as string;
     const tokenType = scopesString ? 'PAT' : 'FGT';
     return { isValid: true, tokenType };
   } catch (error) {
-    return { isValid: false, tokenType: 'PAT' }; // Default to PAT for error handling
+    // Couldn’t verify scopes – type is unknown
+    return { isValid: false, tokenType: 'UNKNOWN' };
   }
 }

And in each caller:

  • Add a case for tokenType === 'UNKNOWN' and show a neutral “couldn’t verify token” message.

Committable suggestion skipped: line range outside the PR's diff.

}

const PAT_SCOPES = ['read:org', 'read:user', 'repo', 'workflow'];
export const getMissingPATScopes = async (pat: string) => {

export const getMissingTokenScopes = async (pat: string, tokenType: 'PAT' | 'FGT') => {
if (tokenType === 'FGT') {
try {
// For FGTs, check repository permissions (example endpoint)
const response = await axios.get('https://api.github.com/user/repos', {
headers: {
Authorization: `token ${pat}`
},
params: { per_page: 1 } // Fetch one repo to check permissions
});

// FGTs don't return scopes in headers, so we infer permissions from API access
// Example: Check if user has access to repos (simplified)
if (!response.data.length) {
return ['repository_access']; // Custom error for missing repo access
}
// Note: For precise FGT permission checking, use /repos/{owner}/{repo} or specific endpoints
return []; // Assume permissions are sufficient for this example
} catch (error) {
throw new Error('Failed to verify FGT permissions', error);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

getMissingTokenScopes re‑throws with an invalid Error signature
new Error(message, error) is not valid JS/TS and will throw another error,
masking the real cause.

-      throw new Error('Failed to verify FGT permissions', error);
+      // Node ≥ 16 supports `cause`; fall back to message concat otherwise.
+      throw new Error('Failed to verify FGT permissions', { cause: error as any });

The same pattern exists for the PAT branch (line 88 – unchanged). Fixing both prevents double‑faults in the modal.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const getMissingTokenScopes = async (pat: string, tokenType: 'PAT' | 'FGT') => {
if (tokenType === 'FGT') {
try {
// For FGTs, check repository permissions (example endpoint)
const response = await axios.get('https://api.github.com/user/repos', {
headers: {
Authorization: `token ${pat}`
},
params: { per_page: 1 } // Fetch one repo to check permissions
});
// FGTs don't return scopes in headers, so we infer permissions from API access
// Example: Check if user has access to repos (simplified)
if (!response.data.length) {
return ['repository_access']; // Custom error for missing repo access
}
// Note: For precise FGT permission checking, use /repos/{owner}/{repo} or specific endpoints
return []; // Assume permissions are sufficient for this example
} catch (error) {
throw new Error('Failed to verify FGT permissions', error);
}
}
} catch (error) {
// Node ≥ 16 supports `cause`; fall back to message concat otherwise.
throw new Error('Failed to verify FGT permissions', { cause: error as any });
}


// Existing PAT logic
try {
const response = await axios.get('https://api.github.com', {
headers: {
Expand All @@ -62,7 +90,6 @@ export const getMissingPATScopes = async (pat: string) => {
};

// Gitlab functions

export const checkGitLabValidity = async (
accessToken: string,
customDomain?: string
Expand Down
Loading