Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds the ability to disable JWT token authentication through a new auth_enabled configuration flag. This allows the plugin to communicate with API endpoints without requiring JWT authentication when disabled.
Key changes:
- Added
AuthEnabledboolean field to theAPIConfigstruct - Updated JWT client initialization to accept and store the
authEnabledparameter - Modified
MakeAuthenticatedRequestto conditionally add JWT authentication based on the flag
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| plugin/internal/registry/registry.go | Added AuthEnabled field to APIConfig struct for configuration support |
| plugin/gthulhu/gthulhu.go | Updated InitJWTClient signature to accept and pass authEnabled parameter from config |
| plugin/gthulhu/auth.go | Modified NewJWTClient constructor and MakeAuthenticatedRequest to support conditional authentication |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func NewJWTClient( | ||
| publicKeyPath, | ||
| apiBaseURL string, | ||
| authEnabled bool, | ||
| ) *JWTClient { | ||
| return &JWTClient{ | ||
| publicKeyPath: publicKeyPath, | ||
| apiBaseURL: strings.TrimSuffix(apiBaseURL, "/"), | ||
| httpClient: &http.Client{ | ||
| Timeout: 30 * time.Second, | ||
| }, | ||
| authEnabled: authEnabled, | ||
| } | ||
| } |
There was a problem hiding this comment.
The GetAuthenticatedClient method does not respect the authEnabled flag. While MakeAuthenticatedRequest was updated to conditionally add authentication based on authEnabled, GetAuthenticatedClient still always calls ensureValidToken and creates an authenticatedTransport that adds the Authorization header. This means any code using GetAuthenticatedClient will still attempt JWT authentication even when authEnabled is false. The method should check the authEnabled flag and either return a plain HTTP client or skip token validation when authentication is disabled.
| if config.APIConfig.Enabled && | ||
| config.APIConfig.PublicKeyPath != "" && config.APIConfig.BaseURL != "" { | ||
| err := gthulhuPlugin.InitJWTClient(config.APIConfig.PublicKeyPath, config.APIConfig.BaseURL) | ||
| err := gthulhuPlugin.InitJWTClient( | ||
| config.APIConfig.PublicKeyPath, | ||
| config.APIConfig.BaseURL, | ||
| config.APIConfig.AuthEnabled, | ||
| ) |
There was a problem hiding this comment.
When authEnabled is false, the JWT client initialization still requires valid PublicKeyPath and BaseURL values (checked at lines 30-31). However, when authentication is disabled, the public key path is unnecessary since no JWT token will be requested. Consider either relaxing this validation when authEnabled is false, or updating the initialization condition to check authEnabled alongside the other parameters.
No description provided.