Skip to content

Conversation

@ianchen0119
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings December 24, 2025 06:14
@ianchen0119 ianchen0119 merged commit ad83813 into main Dec 24, 2025
3 checks passed
@ianchen0119 ianchen0119 deleted the feat/disable-jwt-token branch December 24, 2025 06:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 AuthEnabled boolean field to the APIConfig struct
  • Updated JWT client initialization to accept and store the authEnabled parameter
  • Modified MakeAuthenticatedRequest to 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.

Comment on lines +46 to 59
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,
}
}
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines 30 to +36
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,
)
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants