Added gitlab token #215
Open
Added gitlab token #215
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR adds support for GitLab personal access tokens in the extension settings, persists them to storage, and updates both the CLI and UI layers to make authenticated GitLab requests via GraphQL (with automatic REST fallback). It also adjusts caching logic to account for token usage and conditionally shows GitHub/GitLab configuration sections. Sequence diagram for authenticated GitLab data fetching with token and fallbacksequenceDiagram
actor User
participant Popup
participant ChromeStorage
participant GitLabAPI
User->>Popup: Selects GitLab and enters token
Popup->>ChromeStorage: Save gitlabToken
User->>Popup: Requests data fetch (e.g., generate report)
Popup->>ChromeStorage: Retrieve gitlabToken
Popup->>GitLabAPI: Fetch data using GraphQL with token
alt GraphQL fails
Popup->>GitLabAPI: Fallback to REST API with token
end
GitLabAPI-->>Popup: Return data
Popup-->>User: Display report
Class diagram for platform-specific settings and UI logicclassDiagram
class Popup {
+updatePlatformUI(platform)
+platformSelect
+githubTokenSection
+gitlabTokenSection
}
class PlatformSelect {
+value
+onChange()
}
Popup --> PlatformSelect : listens for changes
Popup : +show/hide GitHub and GitLab token sections
Popup : +update username label and org section
PlatformSelect : +switches between GitHub and GitLab
Class diagram for cache key and data fetching enhancementsclassDiagram
class CacheManager {
+repoCache
+gitlabCache
+cacheKey (includes token and fetch type)
+timestamp
}
class GitLabHelper {
+fetchData(token)
+fetchGraphQL(token)
+fetchREST(token)
}
CacheManager <.. GitLabHelper : uses for caching
GitLabHelper : +uses Authorization header if token present
CacheManager : +distinguishes cache by token and fetch type
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Preeti9764 - I've reviewed your changes - here's some feedback:
Blocking issues:
- User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
scrumReport.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link)
General comments:
- The GraphQL fetch method currently returns early to the REST fallback, leaving the subsequent caching and response-handling code unreachable—consider removing or refactoring that dead code to clean up the flow.
- There's a lot of duplicated logic for building headers, handling tokens, and logging between the REST and GraphQL flows—extract common utilities or helper functions to DRY up the implementation.
- You’ve added very verbose console logging throughout; consider gating these behind a debug flag or trimming down to essential messages to avoid cluttering the console in production.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The GraphQL fetch method currently returns early to the REST fallback, leaving the subsequent caching and response-handling code unreachable—consider removing or refactoring that dead code to clean up the flow.
- There's a lot of duplicated logic for building headers, handling tokens, and logging between the REST and GraphQL flows—extract common utilities or helper functions to DRY up the implementation.
- You’ve added very verbose console logging throughout; consider gating these behind a debug flag or trimming down to essential messages to avoid cluttering the console in production.
## Individual Comments
### Comment 1
<location> `src/scripts/gitlabHelper.js:158` </location>
<code_context>
+ return this.fetchGitLabDataREST(username, startDate, endDate, token);
+ }
+
+ // Process GraphQL response
+ const user = graphqlData.data?.user;
+ if (!user) {
</code_context>
<issue_to_address>
Unreachable code after return in GraphQL branch.
The lines after the return statement won't execute. Please remove or restructure them for clarity and maintainability.
</issue_to_address>
### Comment 2
<location> `src/scripts/scrumHelper.js:288` </location>
<code_context>
- const mappedMRs = (data.mergeRequests || data.mrs || []).map(mr => mapGitLabItem(mr, data.projects, 'mr'));
+
+ // Ensure data is not null and has required properties
+ if (!data) {
+ throw new Error('GitLab data is null or undefined');
+ }
+
</code_context>
<issue_to_address>
Error thrown for null data may not be user-friendly.
Consider providing a more descriptive error message or handling null data more gracefully in the UI to improve user experience.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
// Ensure data is not null and has required properties
+ if (!data) {
+ throw new Error('GitLab data is null or undefined');
+ }
=======
// Ensure data is not null and has required properties
+ if (!data) {
+ // Show a user-friendly message in the UI
+ alert('Unable to load GitLab data. Please check your connection or authentication and try again.');
+ // Optionally, update a status element in the DOM if available
+ const statusElem = document.getElementById('gitlab-status');
+ if (statusElem) {
+ statusElem.textContent = 'Failed to load GitLab data. Please refresh or check your credentials.';
+ }
+ return; // Stop further processing
+ }
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `src/scripts/scrumHelper.js:312` </location>
<code_context>
if (scrumReport) {
- scrumReport.innerHTML = `<div class=\"error-message\" style=\"color: #dc2626; font-weight: bold; padding: 10px;\">${err.message || 'An error occurred while fetching GitLab data.'}</div>`;
+ const errorMessage = err.message || 'An error occurred while fetching GitLab data.';
+ console.error('GitLab error details:', {
+ message: err.message,
+ stack: err.stack,
+ name: err.name
+ });
+ scrumReport.innerHTML = `<div class=\"error-message\" style=\"color: #dc2626; font-weight: bold; padding: 10px;\">${errorMessage}</div>`;
</code_context>
<issue_to_address>
Console error logging may expose sensitive information.
Restrict detailed error logs to development mode or sanitize output to avoid leaking sensitive data in production.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
console.error('GitLab error details:', {
message: err.message,
stack: err.stack,
name: err.name
});
=======
if (window.NODE_ENV === 'development') {
console.error('GitLab error details:', {
message: err.message,
stack: err.stack,
name: err.name
});
} else {
console.error('An error occurred while fetching GitLab data.');
}
>>>>>>> REPLACE
</suggested_fix>
## Security Issues
### Issue 1
<location> `src/scripts/scrumHelper.js:317` </location>
<issue_to_address>
**security (javascript.browser.security.insecure-document-method):** User controlled data in methods like `innerHTML`, `outerHTML` or `document.write` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Issue 2
<location> `src/scripts/scrumHelper.js:317` </location>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `scrumReport.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| stack: err.stack, | ||
| name: err.name | ||
| }); | ||
| scrumReport.innerHTML = `<div class=\"error-message\" style=\"color: #dc2626; font-weight: bold; padding: 10px;\">${errorMessage}</div>`; |
There was a problem hiding this comment.
security (javascript.browser.security.insecure-innerhtml): User controlled data in a scrumReport.innerHTML is an anti-pattern that can lead to XSS vulnerabilities
Source: opengrep
github-actions
bot
added
javascript
|
@hpdang @vedansh-5 Please review the gitlab token support is working good .Thanks! |
There was a problem hiding this comment.
Hey @Preeti9764 - I've reviewed your changes - here's some feedback:
Blocking issues:
- User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
scrumReport.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link)
General comments:
- The GitLabHelper class has grown very large and mixes GraphQL, REST, caching, and fallback logic—consider extracting each strategy into smaller modules or methods to simplify its responsibilities.
- The new GitLab token UI in popup.js largely duplicates the GitHub token section—refactor into a reusable token‐input component or helper function to reduce repetition and ensure consistency.
- There are many scattered console.log calls across the code; consider wrapping them in a debug flag or using a logging utility so you can toggle verbose output in production.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The GitLabHelper class has grown very large and mixes GraphQL, REST, caching, and fallback logic—consider extracting each strategy into smaller modules or methods to simplify its responsibilities.
- The new GitLab token UI in popup.js largely duplicates the GitHub token section—refactor into a reusable token‐input component or helper function to reduce repetition and ensure consistency.
- There are many scattered console.log calls across the code; consider wrapping them in a debug flag or using a logging utility so you can toggle verbose output in production.
## Security Issues
### Issue 1
<location> `src/scripts/scrumHelper.js:319` </location>
<issue_to_address>
**security (javascript.browser.security.insecure-document-method):** User controlled data in methods like `innerHTML`, `outerHTML` or `document.write` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Issue 2
<location> `src/scripts/scrumHelper.js:319` </location>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `scrumReport.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@Preeti9764 did you manage to check out the sourcery comments? |
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
|
@hpdang I have checked them , now this gitlab token functionality is working fine and ready for merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #219
Summary of changes:
Added gitlab token in the setting for better support of gitlab and authenticated request made for gitlab for token users.
Summary by Sourcery
Add support for GitLab personal access tokens and enhance GitLab data fetching to use authenticated requests with GraphQL fallback.
New Features:
Enhancements:
Summary by Sourcery
Add support for GitLab personal access tokens and integrate authenticated GitLab data fetching with GraphQL fallback.
New Features:
Enhancements: