feature: Added FGTs support in github integration #648
Conversation
|
""" WalkthroughThe changes introduce support for GitHub Fine-Grained Tokens (FGT) in addition to Personal Access Tokens (PAT) for integration within the application. The token validation, permission checking, and linking logic have been refactored to handle both token types. The user interface and instructional text have been updated to reflect support for FGTs. Internal utility functions now distinguish between PAT and FGT, adapting permission checks accordingly, and the linking process records the token type. No changes were made to exported component signatures or GitLab-related logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConfigureGithubModalBody
participant authUtils as Auth Utils (auth.ts)
participant GitHubAPI
User->>ConfigureGithubModalBody: Submit GitHub token
ConfigureGithubModalBody->>authUtils: checkGitHubValidity(token)
authUtils->>GitHubAPI: Validate token
GitHubAPI-->>authUtils: Response with headers
authUtils-->>ConfigureGithubModalBody: { isValid, tokenType }
alt Token invalid
ConfigureGithubModalBody-->>User: Show error
else Token valid
ConfigureGithubModalBody->>authUtils: getMissingTokenScopes(token, tokenType)
alt Missing scopes/permissions
ConfigureGithubModalBody-->>User: Show missing scopes/permissions error
else All required permissions present
ConfigureGithubModalBody->>authUtils: linkProvider(token, orgId, 'github', { tokenType })
authUtils->>GitHubAPI: Link provider
GitHubAPI-->>authUtils: Success
authUtils-->>ConfigureGithubModalBody: Success
ConfigureGithubModalBody-->>User: Show success message with token type
end
end
Assessment against linked issues
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
web-server/src/utils/auth.ts (3)
23-24: Prefer descriptive metadata field names
meta_datais a new top‑level property sent to the backend. We’ve been gradually standardising onmetadata(no underscore) in other endpoints – using a single naming style prevents confusion server‑side and in analytics queries. Consider aligning here before the field becomes entrenched.
40-44: Header access might be case‑sensitive in some environments
Axios lower‑cases all header keys (response.headers['x-oauth-scopes']).
If another HTTP layer is swapped in later (e.g.,fetch), header names will
need normalisation. Small defensive tweak:-const scopesString = response.headers['x-oauth-scopes'] as string; +const scopesString = (response.headers['x-oauth-scopes'] || + response.headers['X-OAuth-Scopes']) as string | undefined;
51-52: Parameter name still references “pat”
Now that the function handles both PATs and FGTs, the parameter name
patis misleading.-export const getMissingTokenScopes = async (pat: string, tokenType: 'PAT' | 'FGT') => { +export const getMissingTokenScopes = async (token: string, tokenType: 'PAT' | 'FGT') => {Remember to rename occurrences inside the function.
web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx (3)
52-61: Error message conflates scopes & permissions
For PATs we speak of “scopes”, for FGTs of “permissions”; nice touch.
However, the message currently lumps the whole array into a single string which can overflow the snackbar.Consider:
-`Token is missing ${tokenType === 'PAT' ? 'scopes' : 'permissions'}: ${missingScopes.join(', ')}` +`Token is missing ${tokenType === 'PAT' ? 'scopes' : 'permissions'}:\n• ${missingScopes.join('\n• ')}`This formats the list vertically and keeps each message readable.
64-70: Race‑condition guard for rapid double‑click / Enter presses
isLoading.true()is set after the early‑return validations, but a very quick
double‑click (or the user pressing Enter twice) can still firehandleSubmissionagain
before the promise chain toggles the flag back. A simple debounce or an
if (isLoading.value) returncheck at the top prevents accidental duplicate
requests.
95-113: Label and helper text mention PAT/FGT – screenshot still says PAT only
/assets/PAT_permissions.pngshows the classic token permission screen.
Users testing FGTs might be confused. Either update the asset or dynamically
switch images based ontokenTypeonce detected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx(6 hunks)web-server/src/utils/auth.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
web-server/src/utils/auth.ts (1)
web-server/http-server.js (1)
axios(5-5)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: All file linting
web-server/src/utils/auth.ts
Outdated
| 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); | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
| 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 }); | |
| } |
web-server/src/utils/auth.ts
Outdated
| 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 | ||
| } |
There was a problem hiding this comment.
💡 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
checkGitHubValidityto 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.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
web-server/src/utils/auth.ts (2)
45-45:⚠️ Potential issueConsider returning 'UNKNOWN' for token type during validation failures
When validation fails, returning a default of 'PAT' could be misleading if the error occurred while validating an FGT. This might cause incorrect error messaging in the UI.
- return { isValid: false, tokenType: 'PAT' }; + return { isValid: false, tokenType: 'UNKNOWN' as never };You'll also need to update the function's return type signature and handle the 'UNKNOWN' case in callers.
70-70:⚠️ Potential issueFix invalid Error constructor usage
The current Error constructor usage is not valid JavaScript/TypeScript. The second parameter should be an options object with a
causeproperty for modern Node.js, or you need another approach for older versions.- throw new Error('Failed to verify FGT permissions', error); + // For Node.js ≥16.9.0 + throw new Error('Failed to verify FGT permissions', { cause: error }); + // For older Node.js versions + // const e = new Error('Failed to verify FGT permissions'); + // e.cause = error; + // throw e;The same issue exists in the PAT error handling at line 87, which should also be fixed.
🧹 Nitpick comments (2)
web-server/src/utils/auth.ts (2)
51-54: Rename parameter for clarityThe parameter name
patis now misleading since it could be either a PAT or an FGT. Consider using a more neutral name liketoken.export const getMissingTokenScopes = async ( - pat: string, + token: string, tokenType: 'PAT' | 'FGT' ) => {Remember to update all references to this parameter inside the function body.
57-62: Consider adding timeout to API requestNetwork requests can hang indefinitely without proper timeout settings. It's a good practice to add a timeout to API requests, especially for authentication-related functionality.
const response = await axios.get('https://api.github.com/user/repos', { headers: { Authorization: `token ${pat}` }, params: { per_page: 1 }, + timeout: 10000 // 10 seconds });This would also be beneficial for other API requests in this file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx(6 hunks)web-server/src/utils/auth.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
web-server/src/utils/auth.ts (1)
web-server/http-server.js (1)
axios(5-5)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: All file linting
🔇 Additional comments (2)
web-server/src/utils/auth.ts (2)
23-23: Good addition of default token typeThe addition of a default token type ensures backward compatibility with existing code while supporting the new FGT functionality.
40-43: Effective token type detection mechanismUsing the presence of scopes in the response headers to differentiate between PAT and FGT is an elegant solution. The comment explains the logic clearly.
| if (!response.data.length) { | ||
| return ['repository_access']; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Clarify repository access requirement logic
The current logic assumes that if no repositories are returned, it means the token lacks repository access. However, this could also happen if the user genuinely has no repositories. Consider adding a more robust check.
🌐 Web query:
How to verify GitHub Fine-Grained Token permissions for repository access?
💡 Result:
Verifying GitHub Fine-Grained Token Permissions for Repository Access
Overview
GitHub Fine-Grained Personal Access Tokens (PATs) allow you to specify exactly which repositories and permissions a token has, providing more control and security compared to classic tokens[7]. However, verifying what a fine-grained token can access after it's been created is less straightforward than with classic tokens.
How to Check Permissions of a Fine-Grained Token
1. Review Token Settings in the GitHub UI
- Navigate to Settings > Developer settings > Personal access tokens > Fine-grained tokens in your GitHub account.
- Here, you can see each token, its associated repositories, and the granularity of its permissions (read, write, admin, etc.)[2][9].
- This section presents the most complete view of the token's capabilities after creation.
2. Understand Permissions via the API
- When you use a fine-grained token with the REST API, GitHub responds with a special HTTP header:
X-Accepted-GitHub-Permissions - This header indicates the permissions your token provided during the request and what permissions are required for the endpoint. By making test API requests (e.g., to a repository endpoint), you can see if your token is authorized for the action you desire[1][4].
Note: There is currently no single API endpoint that returns a list of all permissions granted to a fine-grained token. Unlike classic tokens, simply requesting the API root does not reveal detailed permissions for fine-grained tokens[3].
3. Testing Permissions
- Attempt actions (via CLI or API) that require specific permissions (e.g., listing issues, pushing commits). If your token lacks the required permission, the request will fail with an error indicating insufficient permissions.
- The API documentation for each endpoint specifies which fine-grained permissions are required[1][4][10].
4. Reference Documentation
- GitHub REST API docs list the fine-grained permissions required for each endpoint, helping you cross-check your token's settings with the needs of your automation or integration[1][4][10].
Key Points to Remember
- Fine-grained PATs are limited to only those repositories you specify at creation, and only with the precise permissions granted.
- To verify access, consult the token management page in GitHub or use the REST API and inspect the
X-Accepted-GitHub-Permissionsheader in API responses[1][2][4]. - If your token needs access to additional repositories or permissions, you must generate a new token or edit the existing one, as permissions cannot be expanded by the token itself after creation.
- Classic methods for checking token scopes (e.g., requesting the API root) do not work with fine-grained PATs[3].
Summary Table: Fine-Grained Token Verification Methods
| Method | What You Learn | Where to Use |
|---|---|---|
| GitHub UI (Token Settings) | See repositories/permissions assigned | Personal access tokens management page |
REST API + X-Accepted-GitHub-Permissions |
See if a specific API call will succeed | Any API request using the fine-grained PAT |
| Test Actions (CLI/API) | Confirm permission by success/failure | Command line or custom scripts |
| API Docs | Understand required permissions | GitHub REST API documentation |
Conclusion
To verify a fine-grained PAT's permissions for repository access, review the token's configuration in GitHub account settings and test API or CLI actions for required functionality. For specific API permission details, inspect the X-Accepted-GitHub-Permissions header in API responses. There is no direct API endpoint to enumerate all permissions granted to a fine-grained token at this time[1][2][3][4].
Citations:
- 1: https://docs.github.com/en/rest/authentication/permissions-required-for-fine-grained-personal-access-tokens
- 2: https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens
- 3: https://github.com/orgs/community/discussions/55914
- 4: https://docs.github.com/en/enterprise-server@3.16/rest/overview/permissions-required-for-fine-grained-personal-access-tokens
- 5: https://gitlab.com/gitlab-org/gitlab/-/issues/439110
- 6: https://docs.github.com/actions/security-guides/automatic-token-authentication
- 7: https://github.blog/security/application-security/introducing-fine-grained-personal-access-tokens-for-github/
- 8: https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/about-authentication-to-github
- 9: https://www.youtube.com/watch?v=sbKyTis2hnI
- 10: https://docs.github.com/en/rest/authentication/authenticating-to-the-rest-api
Clarify repository‐access check vs. “no repos” case
Right now, an empty response.data always signals missing repo permissions—even if the user simply has no repos. Instead, inspect the X-Accepted-GitHub-Permissions header to distinguish “no repos because you’re not allowed” from “no repos because there aren’t any.” For example:
- if (!response.data.length) {
- return ['repository_access'];
- }
+ const perms = response.headers['x-accepted-github-permissions'] as string;
+ if (!response.data.length) {
+ // If the token wasn’t granted repository read access, flag it
+ if (!perms?.includes('contents:read')) {
+ return ['repository_access'];
+ }
+ // Otherwise it’s just an empty repo list
+ return [];
+ }• File: web-server/src/utils/auth.ts (around lines 65–67)
📝 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.
| if (!response.data.length) { | |
| return ['repository_access']; | |
| } | |
| const perms = response.headers['x-accepted-github-permissions'] as string; | |
| if (!response.data.length) { | |
| // If the token wasn’t granted repository read access, flag it | |
| if (!perms?.includes('contents:read')) { | |
| return ['repository_access']; | |
| } | |
| // Otherwise it’s just an empty repo list | |
| return []; | |
| } |
Linked Issue(s)
fixed #643
Acceptance Criteria fulfillment
Added FGT support for GIthub Integration while retaining the PAT usage.
Proposed changes (including videos or screenshots)
Integration using FGT
Screencast.from.2025-04-22.18-48-15.webm
Integration using PAT
Screencast.from.2025-04-22.18-49-07.webm
Further comments
Renamed
getMissingPATScopestogetMissingTokenScopesfor handling FGTs and PATs both.Updated the UI text to mention both PATs and FGTs and modified
handleSubmissionfunction.Summary by CodeRabbit