Skip to content

Tighten token validation handling #20

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

Merged
jonmartin721 merged 3 commits into from
Mar 9, 2026
Merged

Tighten token validation handling #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8087982
Tighten token validation handling
jonmartin721 Mar 9, 2026
ad03f05
Preserve stored tokens during token edits
jonmartin721 Mar 9, 2026
4dbc0cb
Expand token coverage
jonmartin721 Mar 9, 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
26 changes: 24 additions & 2 deletions options/controllers/token-controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const notifications = NotificationManager.getInstance();

export async function clearToken() {
if (!confirm('Are you sure you want to clear your GitHub token?')) {
return;
return false;
}

document.getElementById('githubToken').value = '';
Expand All @@ -25,10 +25,12 @@ export async function clearToken() {
await clearStoredToken();

notifications.info('GitHub token cleared successfully');
return true;
}

export async function validateToken(token, toastManager) {
export async function validateToken(token, toastManager, options = {}) {
Copy link

@gemini-code-assist gemini-code-assist bot Mar 9, 2026

Choose a reason for hiding this comment

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

medium

The validateToken function has a lot of repeated logic, specifically the if (!shouldApplyResult()) { return ... } check in every if/else if/else/catch block. This makes the function harder to read and maintain.

You can refactor this to first determine the validation result, and then, in a single place, decide whether to apply the UI changes based on shouldApplyResult(). This would separate the concerns of validation and UI updates, leading to cleaner and more maintainable code.

Here's an example of how it could be refactored:

export async function validateToken(token, toastManager, options = {}) {
  const statusEl = document.getElementById('tokenStatus');
  const shouldApplyResult = options.shouldApplyResult ?? (() => true);

  let validationResult;

  try {
    const response = await fetch('https://api.github.com/user', {
      headers: createHeaders(token),
    });

    if (response.ok) {
      const user = await response.json();
      validationResult = { isValid: true, user: user.login };
    } else if (response.status === 401) {
      validationResult = { isValid: false, reason: 'invalid' };
    } else {
      validationResult = { isValid: false, reason: 'http', status: response.status };
    }
  } catch (_error) {
    validationResult = { isValid: false, reason: 'network' };
  }

  if (!shouldApplyResult()) {
    return validationResult;
  }

  // Apply UI changes based on the result
  if (validationResult.isValid) {
    statusEl.textContent = `✓ Valid (${validationResult.user})`;
    statusEl.className = 'token-status valid';
    // ... other success UI updates from the original function
    if (!toastManager.lastValidToken || toastManager.lastValidToken !== token) {
      if (toastManager.isManualTokenEntry) {
        notifications.success(`GitHub token validated successfully for user: ${validationResult.user}`);
      }
      toastManager.lastValidToken = token;
    }
    toastManager.isManualTokenEntry = false;
  } else {
    // Handle different failure reasons
    switch (validationResult.reason) {
      case 'invalid':
        statusEl.textContent = '✗ Invalid token';
        // ... invalid token UI updates
        if (!toastManager.lastInvalidToken || toastManager.lastInvalidToken !== token) {
          notifications.error('Invalid GitHub token. Please check your token and try again.');
          toastManager.lastInvalidToken = token;
        }
        break;
      case 'http':
        statusEl.textContent = `✗ Error (${validationResult.status})`;
        // ... http error UI updates
        if (!toastManager.lastApiError || toastManager.lastApiError !== validationResult.status) {
          notifications.error(`GitHub API error (${validationResult.status}). Please try again later.`);
          toastManager.lastApiError = validationResult.status;
        }
        break;
      case 'network':
        statusEl.textContent = '✗ Network error';
        // ... network error UI updates
        notifications.error('Network error while validating token. Please check your connection and try again.');
        break;
    }
    statusEl.className = 'token-status invalid';
    document.getElementById('clearTokenBtn').style.display = 'none';
    // ... other common failure UI updates from the original function
  }

  return validationResult;
}

Copy link
Owner Author

@jonmartin721 jonmartin721 Mar 9, 2026

Choose a reason for hiding this comment

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

Kept the current structure for now. I agreed with the behavioral issues and fixed those in ad03f05, but I left the broader refactor as follow-up cleanup so this PR stays focused on the persistence and stale-response bugs.

const statusEl = document.getElementById('tokenStatus');
const shouldApplyResult = options.shouldApplyResult ?? (() => true);

try {
const response = await fetch('https://api.github.com/user', {
Expand All @@ -37,6 +39,10 @@ export async function validateToken(token, toastManager) {

if (response.ok) {
const user = await response.json();
if (!shouldApplyResult()) {
return { isValid: true, user: user.login };
}

statusEl.textContent = `✓ Valid (${user.login})`;
statusEl.className = 'token-status valid';
document.getElementById('clearTokenBtn').style.display = 'block';
Expand All @@ -59,7 +65,12 @@ export async function validateToken(token, toastManager) {
}

toastManager.isManualTokenEntry = false;
return { isValid: true, user: user.login };
} else if (response.status === 401) {
if (!shouldApplyResult()) {
return { isValid: false, reason: 'invalid' };
}

statusEl.textContent = '✗ Invalid token';
statusEl.className = 'token-status invalid';
document.getElementById('clearTokenBtn').style.display = 'none';
Expand All @@ -78,7 +89,12 @@ export async function validateToken(token, toastManager) {
notifications.error('Invalid GitHub token. Please check your token and try again.');
toastManager.lastInvalidToken = token;
}
return { isValid: false, reason: 'invalid' };
} else {
if (!shouldApplyResult()) {
return { isValid: false, reason: 'http', status: response.status };
}

statusEl.textContent = `✗ Error (${response.status})`;
statusEl.className = 'token-status invalid';
document.getElementById('clearTokenBtn').style.display = 'none';
Expand All @@ -97,8 +113,13 @@ export async function validateToken(token, toastManager) {
notifications.error(`GitHub API error (${response.status}). Please try again later.`);
toastManager.lastApiError = response.status;
}
return { isValid: false, reason: 'http', status: response.status };
}
} catch (_error) {
if (!shouldApplyResult()) {
return { isValid: false, reason: 'network' };
}

statusEl.textContent = '✗ Network error';
statusEl.className = 'token-status invalid';
document.getElementById('clearTokenBtn').style.display = 'none';
Expand All @@ -112,5 +133,6 @@ export async function validateToken(token, toastManager) {
document.getElementById('importReposSection').style.display = 'none';

notifications.error('Network error while validating token. Please check your connection and try again.');
return { isValid: false, reason: 'network' };
}
}
2 changes: 1 addition & 1 deletion options/options.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ <h3>Create a GitHub Token</h3>
</div>

<p id="token-help" class="help-text">
<a href="https://github.com/settings/tokens/new?scopes=repo,read:user&description=GitHub%20Devwatch%20Extension&expiration=90" target="_blank" aria-label="Create GitHub token with recommended permissions" class="help-link-primary">→ Create a token with recommended permissions</a> or
<a href="https://github.com/settings/tokens/new?scopes=repo,read:user&description=GitHub%20Devwatch%20Extension&expiration=90" target="_blank" rel="noopener noreferrer" aria-label="Create GitHub token with recommended permissions" class="help-link-primary">→ Create a token with recommended permissions</a> or
manually create one with <code>repo</code> and <code>read:user</code> scopes.
</p>

Expand Down
86 changes: 68 additions & 18 deletions options/options.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { applyTheme, formatDateVerbose } from '../shared/utils.js';
import { getToken, setToken, getLocalItems, setLocalItem } from '../shared/storage-helpers.js';
import { getToken, setToken, clearToken as clearStoredToken, getLocalItems, setLocalItem } from '../shared/storage-helpers.js';
import { createHeaders } from '../shared/github-api.js';
import { STORAGE_CONFIG, VALIDATION_PATTERNS } from '../shared/config.js';
import { validateRepository } from '../shared/repository-validator.js';
Expand Down Expand Up @@ -29,6 +29,7 @@ const state = {
searchQuery: '',
hidePinnedRepos: false
};
let persistedToken = null;

if (typeof document !== 'undefined') {
document.addEventListener('DOMContentLoaded', async () => {
Expand Down Expand Up @@ -121,12 +122,41 @@ function handleUrlParameters() {

}

function syncTokenUiWithStoredCredential(hasStoredToken) {
const clearTokenBtn = document.getElementById('clearTokenBtn');
const repoInput = document.getElementById('repoInput');
const addRepoBtn = document.getElementById('addRepoBtn');
const repoHelpText = document.getElementById('repoHelpText');
const importSection = document.getElementById('importReposSection');

clearTokenBtn.style.display = hasStoredToken ? 'block' : 'none';
repoInput.disabled = !hasStoredToken;
repoInput.placeholder = hasStoredToken
? 'e.g., react, facebook/react, or GitHub URL'
: 'Enter a valid GitHub token to add repositories';
addRepoBtn.disabled = !hasStoredToken;
repoHelpText.textContent = hasStoredToken
? 'Add repositories to monitor (npm package, owner/repo, or GitHub URL)'
: 'Add a valid GitHub token above to start adding repositories';
importSection.classList.toggle('hidden', !hasStoredToken);
importSection.style.display = hasStoredToken ? 'block' : 'none';
}

function shouldClearStoredToken(validationResult) {
return !validationResult.isValid && validationResult.reason === 'invalid';
}

function setupEventListeners() {
// Tab navigation
setupTabNavigation();

document.getElementById('addRepoBtn').addEventListener('click', addRepo);
document.getElementById('clearTokenBtn').addEventListener('click', clearToken);
document.getElementById('clearTokenBtn').addEventListener('click', async () => {
const tokenCleared = await clearToken();
if (tokenCleared) {
persistedToken = null;
}
});

// Action button toggles
const hidePinnedToggleBtn = document.getElementById('hidePinnedToggleBtn2');
Expand Down Expand Up @@ -200,16 +230,19 @@ function setupEventListeners() {
}
});

// Validate and auto-save token on input
// Validate and persist token only after the current input has been confirmed valid.
let tokenValidationTimeout;
document.getElementById('githubToken').addEventListener('input', (e) => {
let tokenValidationRequestId = 0;
document.getElementById('githubToken').addEventListener('input', async (e) => {
clearTimeout(tokenValidationTimeout);
const token = e.target.value.trim();
tokenValidationRequestId++;
const validationId = tokenValidationRequestId;

if (!token) {
document.getElementById('tokenStatus').textContent = '';
document.getElementById('tokenStatus').className = 'token-status';
document.getElementById('clearTokenBtn').style.display = 'none';
syncTokenUiWithStoredCredential(Boolean(persistedToken));
return;
}

Expand All @@ -220,10 +253,26 @@ function setupEventListeners() {
toastManager.isManualTokenEntry = true;

tokenValidationTimeout = setTimeout(async () => {
await validateToken(token, toastManager);
// Auto-save token after validation
if (token) {
const validationResult = await validateToken(token, toastManager, {
shouldApplyResult: () =>
validationId === tokenValidationRequestId &&
document.getElementById('githubToken')?.value.trim() === token
});

if (validationId !== tokenValidationRequestId) {
return;
}

if (validationResult.isValid) {
await setToken(token);
persistedToken = token;
} else if (shouldClearStoredToken(validationResult)) {
await clearStoredToken();
persistedToken = null;
}

if (!validationResult.isValid && persistedToken) {
syncTokenUiWithStoredCredential(true);
}
}, 500);
});
Expand Down Expand Up @@ -516,20 +565,17 @@ async function loadSettings() {
applyTheme(theme);

if (githubToken) {
persistedToken = githubToken;
document.getElementById('githubToken').value = githubToken;
document.getElementById('clearTokenBtn').style.display = 'block';
// Validate existing token
validateToken(githubToken, toastManager);
const validationResult = await validateToken(githubToken, toastManager);
if (shouldClearStoredToken(validationResult)) {
await clearStoredToken();
persistedToken = null;
}
} else {
// No token - set appropriate placeholder and help text
const repoInput = document.getElementById('repoInput');
repoInput.disabled = true;
repoInput.placeholder = 'Enter a valid GitHub token to add repositories';
document.getElementById('addRepoBtn').disabled = true;
document.getElementById('repoHelpText').textContent = 'Add a valid GitHub token above to start adding repositories';
const importSection = document.getElementById('importReposSection');
importSection.classList.add('hidden');
importSection.style.display = 'none';
syncTokenUiWithStoredCredential(false);
}

state.watchedRepos = settings.watchedRepos || [];
Expand Down Expand Up @@ -1148,12 +1194,16 @@ setInterval(async () => {
export {
state,
validateToken,
loadSettings,
setupEventListeners,
addRepo,
validateRepo,
removeRepo,
cleanupRepoNotifications,
getFilteredRepos,
renderRepoList,
shouldClearStoredToken,
syncTokenUiWithStoredCredential,
formatNumber,
formatDateVerbose as formatDate, // Export verbose formatter for tests
exportSettings,
Expand Down
2 changes: 1 addition & 1 deletion popup/views/onboarding-view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ async function renderTokenStep() {
<div class="token-instructions">
<h3>Quick setup:</h3>
<ol>
<li><a href="${tokenUrl}" target="_blank" class="token-link">Create a GitHub token</a></li>
<li><a href="${tokenUrl}" target="_blank" rel="noopener noreferrer" class="token-link">Create a GitHub token</a></li>
<li>Copy the generated token</li>
<li>Paste it below</li>
</ol>
Expand Down
Loading