chore: refactor the auth signup #33
Merged
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
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the authentication signup flow by updating command structures, credential management, and token generation.
- Changed the LoginOptions.ToAquaCreds function to return a pointer instead of a value.
- Introduced new subcommands (login, logout, status, token) with refined behavior and descriptions.
- Consolidated credentials storage into a unified JSON structure in the keyring and enhanced JWT token handling.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/flag/options.go | Updated ToAquaCreds signature to return a pointer to credentials. |
| pkg/commands/auth.go | Refactored auth command structure and introduced multiple subcommands; note an incorrect long description in the status command. |
| internal/creds/verify.go | Added GenerateToken and improved JWT error handling. |
| internal/creds/aqua_test.go | Added tests ensuring proper saving, loading, and clearing of credentials. |
| internal/creds/aqua.go | Consolidated credential storage to a single key using JSON encoding. |
Comments suppressed due to low confidence (2)
pkg/flag/options.go:23
- Changing ToAquaCreds to return a pointer instead of a value is a breaking change; ensure that all consumers of this function are updated accordingly.
func (o *LoginOptions) ToAquaCreds() *creds.AquaCreds {
internal/creds/aqua.go:37
- Consolidating credential storage into the 'aqua-creds' key simplifies management but consider a migration strategy or clear documentation for users with existing stored credentials.
if err := keyring.Delete("trivy-mcp-aqua", "aqua-creds"); err != nil && err != keyring.ErrNotFound {
Signed-off-by: Owen Rumney <owen.rumney@aquasec.com>
3b08e3c to
02e62a9
Compare
Signed-off-by: Owen Rumney <owen.rumney@aquasec.com>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the authentication flow for the Aqua signup by converting credential handling to a JSON-based keyring store, restructuring the CLI commands, and introducing token generation.
- Changed
ToAquaCredsto return a pointer and updated callers - Split the
authcommand intologin,logout,status, andtokensubcommands with interactive input - Switched keyring storage to a single JSON blob, added
GenerateToken, and improved JWT decoding - Added unit tests for credential save, load, and clear
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/flag/options.go | Updated ToAquaCreds signature to return *creds.AquaCreds |
| pkg/commands/auth.go | Broke out subcommands, new input helpers, and keyring integration |
| internal/creds/verify.go | Added GenerateToken, enhanced JWT decoding and error wrapping |
| internal/creds/aqua.go | Merged credential fields into JSON, added Token/ExpiresAt |
| internal/creds/aqua_test.go | Tests for Save, Load, and Clear credential operations |
Comments suppressed due to low confidence (4)
internal/creds/verify.go:34
- The new
GenerateTokenmethod and JWT decoding logic are not covered by existing tests. Consider adding unit tests forGenerateToken,Verify, anddecodeJWTto ensure correct behavior.
func (c *AquaCreds) GenerateToken() (string, error) {
pkg/commands/auth.go:161
- The code uses fmt.Print but the fmt package is not imported, which will cause a compile error. Please add
import "fmt".
fmt.Print(title)
pkg/commands/auth.go:164
- The code references syscall.Stdin but the syscall package is not imported. Please add
import "syscall".
readIn, err := term.ReadPassword(int(syscall.Stdin))
pkg/commands/auth.go:72
- The code calls
creds.Clear()and othercredsmethods but the creds package is not imported. Please add the appropriate import (e.g.,github.com/aquasecurity/trivy-mcp/internal/creds).
if err := creds.Clear(); err != nil {
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.
Signed-off-by: Owen Rumney owen.rumney@aquasec.com