feat: add localStorage persistence for agent config and auth#136
feat: add localStorage persistence for agent config and auth#136rubencagnie wants to merge 1 commit intoa2aproject:mainfrom
Conversation
Implements automatic saving and loading of agent configuration to improve user experience by eliminating the need to re-enter settings on each visit. Features added: - Agent card URL persists across sessions - Authentication type and credentials (Bearer, API Key, Basic Auth) are saved - Custom headers automatically save and restore - Authentication & Headers section now expanded by default - All settings save on input change and when connecting Technical changes: - Added localStorage.getItem/setItem for agent URL, auth data, and headers - Implemented saveAuthInputValues() and loadAuthInputValues() functions - Added event listeners to save on input change and header removal - Auto-formatted code with gts fix (prettier/eslint) All tests pass (74/74) and code follows project linting standards. Co-Authored-By: Claude <noreply@anthropic.com>
Summary of ChangesHello @rubencagnie, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant user experience improvements by implementing client-side persistence for agent configuration and authentication details. By leveraging localStorage, the application now automatically saves and loads critical user settings such as the agent URL, chosen authentication method, associated credentials, and custom headers. This enhancement streamlines the workflow by eliminating repetitive data entry, ensuring a more seamless and efficient interaction with the agent interface across browser sessions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a great usability improvement by persisting agent configuration and authentication details to localStorage. The implementation is straightforward and covers agent URL, authentication data, and custom headers.
My review includes a few suggestions:
- A high-level security consideration regarding the use of
localStoragefor credentials. - Refactoring suggestions for the data loading logic to improve robustness and readability by removing unnecessary
setTimeoutcalls and reducing code duplication. - A small fix to remove another unnecessary
setTimeout.
| function saveAuthInputValues() { | ||
| const authType = authTypeSelect.value; | ||
| const authData: Record<string, string> = {}; | ||
|
|
||
| switch (authType) { | ||
| case 'bearer': | ||
| authData.token = getInputValue('bearer-token'); | ||
| break; | ||
| case 'api-key': | ||
| authData.header = getInputValue('api-key-header'); | ||
| authData.value = getInputValue('api-key-value'); | ||
| break; | ||
| case 'basic': | ||
| authData.username = getInputValue('basic-username'); | ||
| authData.password = getInputValue('basic-password'); | ||
| break; | ||
| } | ||
|
|
||
| localStorage.setItem(`auth_${authType}`, JSON.stringify(authData)); | ||
| } |
There was a problem hiding this comment.
Storing authentication credentials like tokens and especially username/password in localStorage poses a security risk. Data in localStorage is accessible via JavaScript, making it vulnerable to Cross-Site Scripting (XSS) attacks. An attacker who manages to inject a script could steal these credentials. For a developer tool this might be an acceptable risk, but it's important to be aware of it. Consider adding a warning to the user in the UI about the risks of saving sensitive credentials in the browser.
| function loadAuthInputValues(authType: string) { | ||
| const savedData = localStorage.getItem(`auth_${authType}`); | ||
| if (!savedData) return; | ||
|
|
||
| try { | ||
| const authData = JSON.parse(savedData); | ||
|
|
||
| // Wait for inputs to be rendered | ||
| setTimeout(() => { | ||
| switch (authType) { | ||
| case 'bearer': { | ||
| const tokenInput = document.getElementById( | ||
| 'bearer-token', | ||
| ) as HTMLInputElement; | ||
| if (tokenInput && authData.token) tokenInput.value = authData.token; | ||
| break; | ||
| } | ||
| case 'api-key': { | ||
| const headerInput = document.getElementById( | ||
| 'api-key-header', | ||
| ) as HTMLInputElement; | ||
| const valueInput = document.getElementById( | ||
| 'api-key-value', | ||
| ) as HTMLInputElement; | ||
| if (headerInput && authData.header) | ||
| headerInput.value = authData.header; | ||
| if (valueInput && authData.value) valueInput.value = authData.value; | ||
| break; | ||
| } | ||
| case 'basic': { | ||
| const usernameInput = document.getElementById( | ||
| 'basic-username', | ||
| ) as HTMLInputElement; | ||
| const passwordInput = document.getElementById( | ||
| 'basic-password', | ||
| ) as HTMLInputElement; | ||
| if (usernameInput && authData.username) | ||
| usernameInput.value = authData.username; | ||
| if (passwordInput && authData.password) | ||
| passwordInput.value = authData.password; | ||
| break; | ||
| } | ||
| } | ||
| }, 0); | ||
| } catch (e) { | ||
| console.error('Failed to load auth data:', e); | ||
| } | ||
| } |
There was a problem hiding this comment.
This function can be simplified. The setTimeout is not necessary because renderAuthInputs is a synchronous function, so the DOM elements are available immediately after it runs. Additionally, the logic for setting input values is repetitive and can be extracted into a helper function to make the switch statement cleaner and more maintainable. The proposed change also fixes a bug where empty saved values (e.g., an empty token) would not be loaded back into the input field.
function loadAuthInputValues(authType: string) {
const savedData = localStorage.getItem(`auth_${authType}`);
if (!savedData) return;
try {
const authData = JSON.parse(savedData);
const setInputValue = (id: string, value?: string) => {
// Only set value if it's defined in the saved data.
// This allows clearing fields.
if (value !== undefined) {
const input = document.getElementById(id) as HTMLInputElement | null;
if (input) {
input.value = value;
}
}
};
switch (authType) {
case 'bearer':
setInputValue('bearer-token', authData.token);
break;
case 'api-key':
setInputValue('api-key-header', authData.header);
setInputValue('api-key-value', authData.value);
break;
case 'basic':
setInputValue('basic-username', authData.username);
setInputValue('basic-password', authData.password);
break;
}
} catch (e) {
console.error('Failed to load auth data:', e);
}
}| // Wait for the DOM to update | ||
| setTimeout(saveHeadersToLocalStorage, 0); |
There was a problem hiding this comment.
The setTimeout here is unnecessary. The remove() operation on a DOM element is synchronous. Another event listener for 'click' is already registered on headersList via setupRemoveItemListener, and since it was registered first, it will execute before this one, removing the element from the DOM. Therefore, you can call saveHeadersToLocalStorage() directly without a timeout.
saveHeadersToLocalStorage();
Implements automatic saving and loading of agent configuration to improve user experience by eliminating the need to re-enter settings on each visit.
Features added:
Technical changes:
All tests pass (74/74) and code follows project linting standards.
Description
Thank you for opening a Pull Request!
Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
CONTRIBUTINGGuide.fix:which represents bug fixes, and correlates to a SemVer patch.feat:represents a new feature, and correlates to a SemVer minor.feat!:, orfix!:,refactor!:, etc., which represent a breaking change (indicated by the!) and will result in a SemVer major.bash scripts/format.shfrom the repository root to format)