-
Notifications
You must be signed in to change notification settings - Fork 0
Add CodeCatalyst user information command for AWS Toolkit #56
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.
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
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: cobbdan <[email protected]>
|
/q review |
|
⏳ Code review in progress. Analyzing for code quality issues and best practices. Detailed findings will be posted upon completion. Using Amazon Q Developer for GitHubAmazon Q Developer1 is an AI-powered assistant that integrates directly into your GitHub workflow, enhancing your development process with intelligent features for code development, review, and transformation. Slash Commands
FeaturesAgentic Chat Code Review CustomizationYou can create project-specific rules for Amazon Q Developer to follow:
Example rule: FeedbackTo provide feedback on Amazon Q Developer, create an issue in the Amazon Q Developer public repository. For more detailed information, visit the Amazon Q for GitHub documentation. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
This PR successfully implements a new CodeCatalyst user information command that addresses the user's request for "Atlassian user info" by providing AWS CodeCatalyst user details instead. The implementation follows established patterns in the codebase and provides good integration with VS Code's command system.
Key Strengths
- Clear Intent: The PR addresses user confusion by providing the correct AWS CodeCatalyst user information instead of non-existent Atlassian functionality
- Consistent Architecture: Follows existing patterns for CodeCatalyst commands with proper authentication flow and error handling
- Good Integration: Properly integrates with VS Code's command palette and context menus
- Test Coverage: Includes unit tests for both success and error scenarios
Critical Issues to Address
- Security Concern: User information (User ID, email) is displayed in full without any masking, which could lead to information exposure
- Missing Localization: Several error message strings are not included in the localization file
- Test Quality: Mock client creation uses incorrect typing that could cause runtime issues
Recommendations
- Implement data masking for sensitive user information in the display
- Add missing localization strings for all error messages
- Fix the test mock setup to use proper TypeScript types
- Consider more specific error handling for different authentication states
- Add a newline at the end of the test file to follow coding standards
The core functionality is solid, but the security and localization issues should be addressed before merging.
| const userInfoMessage = [ | ||
| `**User Information**`, | ||
| ``, | ||
| `**User ID:** ${userDetails.userId}`, | ||
| `**Username:** ${userDetails.userName}`, | ||
| `**Display Name:** ${userDetails.displayName}`, | ||
| `**Email:** ${userDetails.primaryEmail}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛑 Security Vulnerability: The user information displayed in the modal dialog may contain sensitive data that could be exposed to unauthorized users. The userDetails.userId and userDetails.primaryEmail fields should be handled with care to prevent information disclosure.
| const userInfoMessage = [ | |
| `**User Information**`, | |
| ``, | |
| `**User ID:** ${userDetails.userId}`, | |
| `**Username:** ${userDetails.userName}`, | |
| `**Display Name:** ${userDetails.displayName}`, | |
| `**Email:** ${userDetails.primaryEmail}`, | |
| const userInfoMessage = [ | |
| `**User Information**`, | |
| ``, | |
| `**User ID:** ${userDetails.userId.substring(0, 8)}...`, | |
| `**Username:** ${userDetails.userName}`, | |
| `**Display Name:** ${userDetails.displayName}`, | |
| `**Email:** ${userDetails.primaryEmail.replace(/(.{2})(.*)(@.*)/, '$1***$3')}`, | |
| ].join('\n') |
| if (error instanceof ToolkitError) { | ||
| await vscode.window.showErrorMessage( | ||
| localize('aws.codecatalyst.userInfo.error', 'Failed to get user information: {0}', error.message) | ||
| ) | ||
| } else { | ||
| await vscode.window.showErrorMessage( | ||
| localize('aws.codecatalyst.userInfo.errorGeneric', 'Failed to get user information. Please ensure you are authenticated with CodeCatalyst.') | ||
| ) | ||
| } | ||
| throw error | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handling could be improved by providing more specific error messages based on the error type. Consider checking for authentication-specific errors and providing clearer guidance to users.
| if (error instanceof ToolkitError) { | |
| await vscode.window.showErrorMessage( | |
| localize('aws.codecatalyst.userInfo.error', 'Failed to get user information: {0}', error.message) | |
| ) | |
| } else { | |
| await vscode.window.showErrorMessage( | |
| localize('aws.codecatalyst.userInfo.errorGeneric', 'Failed to get user information. Please ensure you are authenticated with CodeCatalyst.') | |
| ) | |
| } | |
| throw error | |
| } | |
| } catch (error) { | |
| if (error instanceof ToolkitError) { | |
| if (error.code === 'NoConnectionBadState') { | |
| await vscode.window.showErrorMessage( | |
| localize('aws.codecatalyst.userInfo.notConnected', 'Not connected to CodeCatalyst. Please connect first.') | |
| ) | |
| } else { | |
| await vscode.window.showErrorMessage( | |
| localize('aws.codecatalyst.userInfo.error', 'Failed to get user information: {0}', error.message) | |
| ) | |
| } | |
| } else { | |
| await vscode.window.showErrorMessage( | |
| localize('aws.codecatalyst.userInfo.errorGeneric', 'Failed to get user information. Please ensure you are authenticated with CodeCatalyst.') | |
| ) | |
| } | |
| throw error | |
| } |
|
|
||
| beforeEach(function () { | ||
| sandbox = sinon.createSandbox() | ||
| mockClient = sandbox.createStubInstance(Object as any) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mock client creation is incorrect. Object as any doesn't provide the proper type structure for CodeCatalystClient. This could lead to runtime errors in tests.
| } | ||
| }) | ||
| }) | ||
| }) No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test file is missing a newline at the end, which violates common coding standards and may cause issues with some tools.
| "AWS.command.codecatalyst.login": "Connect to CodeCatalyst", | ||
| "AWS.command.codecatalyst.logout": "Sign out of CodeCatalyst", | ||
| "AWS.command.codecatalyst.signout": "Sign Out", | ||
| "AWS.command.codecatalyst.showUserInfo": "Show User Information", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The localization string is missing additional entries for the error messages used in the showUserInfo function. This could cause localization issues.
Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com>
Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com>
This PR addresses a user request for "Atlassian user info" by implementing a new VS Code command that displays AWS CodeCatalyst user information. The user appears to have been confused about which service they were using, as this is the AWS Toolkit repository, not an Atlassian product.
What's Added
New Command:
aws.codecatalyst.showUserInfoImplementation Details
The command leverages existing AWS Toolkit infrastructure:
client.verifySession()Example Usage
Ctrl+Shift+P→ "AWS: Show User Information"The command displays user information in a modal dialog like:
Files Changed
packages/core/src/codecatalyst/commands.ts- Added showUserInfo function and command methodpackages/toolkit/package.json- Added VS Code command contributions and context menu entrypackages/core/package.nls.json- Added localization string for command titlepackages/core/src/test/codecatalyst/commands.test.ts- Added unit tests for success and error scenariosUser Experience
This change transforms a confusing situation where users ask about "Atlassian user info" in an AWS context into a clear path to see their actual AWS/CodeCatalyst user information. The command follows established AWS Toolkit patterns and provides proper error guidance for authentication issues.
Fixes #55.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.