-
Notifications
You must be signed in to change notification settings - Fork 99
Add "bacalhau auth sso token" command #4905
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
Conversation
Remove the option from login method Linear: https://linear.app/expanso/issue/ENG-711/show-auth-token-should-be-its-own-command
WalkthroughThis pull request refactors the SSO authentication commands. In the SSO login command, the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant T as SSOTokenCmd
participant F as Token File Manager
U->>T: Execute "bacalhau auth sso token" command
T->>F: Request token file
alt Token file exists
F-->>T: Return token
alt Decode flag enabled
T->>T: Decode JWT token
end
T-->>U: Output token information
else Token file missing
F-->>T: Return error/empty response
T-->>U: Output error message
end
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.64.8){"Issues":[{"FromLinter":"typecheck","Text":"pattern build/: no matching files found","Severity":"","SourceLines":["//go:embed build/"],"Pos":{"Filename":"webui/webui.go","Offset":0,"Line":23,"Column":12},"ExpectNoLint":false,"ExpectedNoLintLinter":""}],"Report":{"Linters":[{"Name":"asasalint"},{"Name":"asciicheck"},{"Name":"bidichk"},{"Name":"bodyclose","Enabled":true},{"Name":"canonicalheader"},{"Name":"containedctx"},{"Name":"contextcheck"},{"Name":"copyloopvar","Enabled":true},{"Name":"cyclop"},{"Name":"decorder"},{"Name":"deadcode"},{"Name":"depguard","Enabled":true},{"Name":"dogsled","Enabled":true},{"Name":"dupl"},{"Name":"dupword"},{"Name":"durationcheck"},{"Name":"errcheck","Enabled":true,"EnabledByDefault":true},{"Name":"errchkjson"},{"Name":"errname"},{"Name":"errorlint"},{"Name":"execinquery"},{"Name":"exhaustive"},{"Name":"exhaustivestruct"},{"Name":"exhaustruct"},{"Name":"exportloopref"},{"Name":"exptostd"},{"Name":"forbidigo","Enabled":true},{"Name":"forcetypeassert"},{"Name":"fatcontext"},{"Name":"funlen","Enabled":true},{"Name":"gci"},{"Name":"ginkgolinter"},{"Name":"gocheckcompilerdirectives"},{"Name":"gochecknoglobals"},{"Name":"gochecknoinits","Enabled":true},{"Name":"gochecksumtype"},{"Name":"gocognit"},{"Name":"goconst","Enabled":true},{"Name":"gocritic"},{"Name":"gocyclo","Enabled":true},{"Name":"godot"},{"Name":"godox"},{"Name":"err113"},{"Name":"gofmt","Enabled":true},{"Name":"gofumpt"},{"Name":"goheader"},{"Name":"goimports","Enabled":true},{"Name":"golint"},{"Name":"mnd","Enabled":true},{"Name":"gomnd"},{"Name":"gomoddirectives"},{"Name":"gomodguard"},{"Name":"goprintffuncname","Enabled":true},{"Name":"gosec","Enabled":true},{"Name":"gosimple","Enabled":true,"EnabledByDefault":true},{"Name":"gosmopolitan"},{"Name":"govet","Enabled":true,"EnabledByDefault":true},{"Name":"grouper"},{"Name":"ifshort"},{"Name":"iface"},{"Name":"importas"},{"Name":"inamedparam"},{"Name":"ineffassign","Enabled":true,"EnabledByDefault":true},{"Name":"interfacebloat"},{"Name":"interfacer"},{"Name":"intrange"},{"Name":"ireturn"},{"Name":"lll","Enabled":true},{"Name":"loggercheck"},{"Name":"maintidx"},{"Name":"makezero"},{"Name":"maligned"},{"Name":"mirror"},{"Name":"misspell"},{"Name":"musttag"},{"Name":"nakedret","Enabled":true},{"Name":"nestif"},{"Name":"nilerr"},{"Name":"nilnesserr"},{"Name":"nilnil"},{"Name":"nlreturn"},{"Name":"noctx","Enabled":true},{"Name":"nonamedreturns"},{"Name":"nosnakecase"},{"Name":"nosprintfhostport"},{"Name":"paralleltest"},{"Name":"perfsprint"},{"Name":"prealloc"},{"Name":"predeclared"},{"Name":"promlinter"},{"Name":"protogetter"},{"Name":"reassign"},{"Name":"recvcheck"},{"Name":"revive"},{"Name":"rowserrcheck"},{"Name":"sloglint"},{"Name":"scopelint"},{"Name":"sqlclosecheck"},{"Name":"spancheck"},{"Name":"staticcheck","Enabled":true,"EnabledByDefault":true},{"Name":"structcheck"},{"Name":"stylecheck","Enabled":true},{"Name":"tagalign"},{"Name":"tagliatelle"},{"Name":"tenv"},{"Name":"testableexamples"},{"Name":"testifylint"},{"Name":"testpackage"},{"Name":"thelper"},{"Name":"tparallel"},{"Name":"typecheck","Enabled":true,"EnabledByDefault":true},{"Name":"unconvert","Enabled":true},{"Name":"unparam"},{"Name":"unused","Enabled":true,"EnabledByDefault":true},{"Name":"usestdlibvars"},{"Name":"usetesting"},{"Name":"varcheck"},{"Name":"varnamelen"},{"Name":"wastedassign"},{"Name":"whitespace","Enabled":true},{"Name":"wrapcheck"},{"Name":"wsl"},{"Name":"zerologlint"},{"Name":"nolintlint","Enabled":true}]}} ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cmd/cli/auth/sso/token.go (2)
69-76
: Consider improving JWT parsing error handling.The current error handling relies on string matching to filter out the expected error that occurs during JWT parsing without verification. This approach is somewhat fragile as error messages might change in different versions of the JWT library.
Consider using a more robust approach:
- if err != nil && !strings.Contains(err.Error(), "key is of invalid type") { + // We expect a validation error since we're not providing a key for verification + // Look specifically for errors other than validation errors + if err != nil { + var validationError *jwt.ValidationError + if !errors.As(err, &validationError) || validationError.Errors != jwt.ValidationErrorSignatureInvalid { + return bacerrors.New("failed to parse token: %s", err) + } + }
64-67
: Add documenting comment for early return.This early return handles the case when no token is found. Adding a comment would improve code clarity and maintainability.
if existingCred == nil { + // No token found, inform the user and exit gracefully fmt.Fprintln(cmd.OutOrStdout(), "No authentication token found") return nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmd/cli/auth/sso/login.go
(2 hunks)cmd/cli/auth/sso/root.go
(1 hunks)cmd/cli/auth/sso/token.go
(1 hunks)cmd/cli/auth/sso/token_test.go
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
cmd/cli/auth/sso/root.go (1)
cmd/cli/auth/sso/token.go (1) (1)
NewSSOTokenCmd
(28-48)
cmd/cli/auth/sso/token.go (1)
cmd/cli/auth/sso/login.go (1) (1)
o
(60-130)
cmd/cli/auth/sso/login.go (2)
cmd/cli/auth/sso/token.go (1) (1)
o
(51-98)cmd/cli/auth/sso/logout.go (1) (1)
o
(48-77)
🪛 Gitleaks (8.21.2)
cmd/cli/auth/sso/token_test.go
41-41: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
63-63: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🔇 Additional comments (12)
cmd/cli/auth/sso/root.go (1)
15-15
: Addition of the token command is appropriate and well integrated.The addition of
NewSSOTokenCmd()
to the SSO root command structure logically extends the existing command set by adding token display functionality alongside login and logout commands.cmd/cli/auth/sso/token.go (2)
16-19
: Simple and focused options struct.The struct design correctly follows the single responsibility principle by keeping the options focused solely on token display functionality.
28-48
: Command structure follows best practices.The command implementation properly follows the Cobra command pattern with appropriate descriptions, argument validation, and error handling. The decode flag provides useful functionality for inspecting token contents.
cmd/cli/auth/sso/login.go (2)
24-24
: Good simplification of the options struct.The simplification of
SSOLoginOptions
to an empty struct is appropriate as part of the refactoring to move token display functionality to a separate command.
33-33
: Improved variable naming for clarity.Renaming from
nodeCmd
tologinCmd
better reflects the command's purpose and improves code readability.cmd/cli/auth/sso/token_test.go (7)
15-36
: Well-designed test helper functions.The helper functions properly set up the test environment and manage test resources. The cleanup function ensures proper teardown.
38-57
: Comprehensive test for raw token display.This test effectively verifies that the command correctly retrieves and displays the raw JWT token.
🧰 Tools
🪛 Gitleaks (8.21.2)
41-41: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
59-83
: Thorough validation of token decoding functionality.The test comprehensively validates both header and claim decoding of a JWT token, ensuring the command's core functionality works correctly.
🧰 Tools
🪛 Gitleaks (8.21.2)
63-63: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
85-104
: Good error path testing.This test properly verifies the behavior when no token is found, ensuring appropriate user feedback.
106-126
: Appropriate error handling validation.This test correctly verifies that the command handles invalid tokens appropriately by returning a meaningful error.
128-140
: Command structure verification.This test ensures the command is properly configured with the expected name, description, and flags.
41-41
: Test JWT tokens are appropriate for testing purposes.The sample JWT tokens are being used appropriately for testing purposes. These are standard test tokens and don't pose a security risk.
Also applies to: 63-63
🧰 Tools
🪛 Gitleaks (8.21.2)
41-41: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
Add
bacalhau auth sso token
commandDescription
Added a new CLI command
bacalhau auth sso token
that allows users to view and decode their current SSO JWT token. This command helps users inspect their authentication state and debug SSO-related issues.Features
--decode/-d
flag to decode and pretty-print the token's contents:Usage Examples
Testing
Added test suite covering:
Linear: https://linear.app/expanso/issue/ENG-711/show-auth-token-should-be-its-own-command
Summary by CodeRabbit