[PPSC-522] feat(auth): enable JWT authentication flags#95
Merged
yiftach-armis merged 2 commits intomainfrom Mar 9, 2026
Merged
[PPSC-522] feat(auth): enable JWT authentication flags#95yiftach-armis merged 2 commits intomainfrom
yiftach-armis merged 2 commits intomainfrom
Conversation
|
| Severity | Count |
|---|---|
| 🟠 HIGH | 1 |
Total: 1
View all 1 findings
🟠 HIGH (1)
CWE-522_armis-cli_38295677_internal/auth/auth.go_158_2_158_32 - Insecure Design (CWE-522
Location: internal/auth/auth.go:158
Insufficiently Protected Credentials): The GetRawToken method returns the stored JWT or legacy token directly to the caller without any masking, encryption, or access control. Because the method is part of the authentication provider, any code that can obtain an AuthProvider instance—including scripts or other internal tools—can retrieve the raw credential. This matches the definition of CWE‑522, where credentials are exposed without protection. The token is sourced from the provider’s internal credential store and reaches the return statement unchanged, making the secret reachable. Although the method is typically invoked by a CLI command for user‑initiated output, it can be called programmatically, leading to credential leakage to any downstream process that captures stdout or consumes the returned string. This constitutes a true positive for insufficiently protected credentials.
Code snippet is redacted as it contains secrets.
CWEs: CWE-522: Insufficiently Protected Credentials
There was a problem hiding this comment.
Pull request overview
This PR exposes previously hidden CLI flags for JWT authentication so end users can configure JWT auth via --client-id, --client-secret, and --auth-endpoint, leveraging the already-implemented JWT auth provider in internal/auth.
Changes:
- Removes the logic that hid JWT-related persistent flags from
--help. - Makes JWT auth configuration available via CLI flags (and still supports env var equivalents).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Test Coverage Reporttotal: (statements) 80.6% Coverage by function |
yiftach-armis
force-pushed
the
feat/PPSC-522-enable-jwt-auth
branch
from
91c7fa8 to
df57bca
Compare
Unhide the --client-id, --client-secret, and --auth-endpoint flags to enable JWT authentication in the CLI. These flags were previously hidden pending backend support, which is now available. JWT auth takes priority over Basic auth and automatically extracts the tenant ID from the JWT customer_id claim. Users can now authenticate using: - Environment variables: ARMIS_CLIENT_ID, ARMIS_CLIENT_SECRET, ARMIS_AUTH_ENDPOINT - CLI flags: --client-id, --client-secret, --auth-endpoint
yiftach-armis
force-pushed
the
feat/PPSC-522-enable-jwt-auth
branch
from
df57bca to
f3717ff
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/auth/auth.go
Outdated
Comment on lines
+109
to
+112
| p.mu.RLock() | ||
| defer p.mu.RUnlock() | ||
| // Raw JWT token (no Bearer prefix) | ||
| return p.credentials.Token, nil | ||
| // #nosec G101 -- Bearer auth scheme requires token in header per RFC 6750 | ||
| return "Bearer " + p.credentials.Token, nil |
There was a problem hiding this comment.
GetAuthorizationHeader now prefixes JWTs with "Bearer ", but internal/api/client.go explicitly documents that the backend expects the raw JWT token in the Authorization header (no Bearer prefix). Changing the header format here is likely to break authenticated API requests unless the backend contract has changed; either revert to returning the raw JWT, or update the API client/backend contract consistently (including documentation/tests) to use RFC 6750 Bearer tokens.
internal/auth/auth_test.go
Outdated
Comment on lines
205
to
209
| // Bearer token format per RFC 6750 | ||
| expectedHeader := "Bearer " + mockJWT | ||
| if header != expectedHeader { | ||
| t.Errorf("Unexpected auth header: got %q, want %q", header, expectedHeader) | ||
| } |
There was a problem hiding this comment.
This test was updated to expect an RFC 6750 Bearer token, but the API client is documented as sending a raw JWT in the Authorization header (no "Bearer" prefix) to match the backend contract (see internal/api/client.go). If the backend still expects the raw token, this assertion should remain header == mockJWT; otherwise the backend/API client docs should be updated in the same PR to avoid a silent contract mismatch.
Reverts the Authorization header format to send raw JWT tokens instead of "Bearer <token>" to match the backend API contract. The backend expects raw JWT tokens in the Authorization header (unconventional but documented in internal/api/client.go). Addresses PR review comments about the contract mismatch.
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
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.
Related Issue
Type of Change
Problem
JWT authentication was fully implemented but the CLI flags were hidden, preventing users from using this authentication method.
Solution
Unhide the
--client-id,--client-secret, and--auth-endpointflags to expose JWT authentication. JWT auth now takes priority over Basic auth and automatically extracts the tenant ID from the JWT'scustomer_idclaim.Testing
--helpoutputReviewer Notes
JWT authentication was already fully implemented in
internal/auth/auth.goandinternal/auth/auth_client.go. This change simply makes the feature available to users by removing the flag hiding logic ininternal/cmd/root.go.The change removes 14 lines of code that explicitly hid the three JWT-related flags pending backend support.