From 95a5eb125fcc440a88b7d8d39dda6c6228b7cea4 Mon Sep 17 00:00:00 2001 From: appleboy Date: Sat, 27 Dec 2025 11:13:06 +0800 Subject: [PATCH] feat: refactor authentication logic and broaden test coverage - Improve authentication checks to only require username and token when both are provided - Update validation logic to allow either (username and token) or remote-token for authentication - Enhance test coverage for various authentication scenarios - Refine error messages to indicate a generic authentication requirement instead of specifying missing username or token Signed-off-by: appleboy --- jenkins.go | 14 +++++++++----- plugin.go | 24 +++++++++++++++--------- plugin_test.go | 41 ++++++++++++++++++++++++++++++++++------- 3 files changed, 58 insertions(+), 21 deletions(-) diff --git a/jenkins.go b/jenkins.go index f0bfc6c..512b42f 100644 --- a/jenkins.go +++ b/jenkins.go @@ -224,7 +224,7 @@ func (jenkins *Jenkins) sendRequest( req *http.Request, crumb *CrumbResponse, ) (*http.Response, error) { - if jenkins.Auth != nil { + if jenkins.Auth != nil && jenkins.Auth.Username != "" && jenkins.Auth.Token != "" { req.SetBasicAuth(jenkins.Auth.Username, jenkins.Auth.Token) } @@ -277,10 +277,14 @@ func (jenkins *Jenkins) postAndGetLocation( path string, params url.Values, ) (int, error) { - // Fetch CSRF crumb before POST request - crumb, err := jenkins.getCrumb(ctx) - if err != nil { - return 0, fmt.Errorf("failed to get crumb: %w", err) + // Fetch CSRF crumb before POST request (only if authenticated) + var crumb *CrumbResponse + if jenkins.Auth != nil && jenkins.Auth.Username != "" && jenkins.Auth.Token != "" { + var err error + crumb, err = jenkins.getCrumb(ctx) + if err != nil { + return 0, fmt.Errorf("failed to get crumb: %w", err) + } } requestURL := jenkins.buildURL(path, params) diff --git a/plugin.go b/plugin.go index 60a7ae7..c552769 100644 --- a/plugin.go +++ b/plugin.go @@ -88,12 +88,15 @@ func (p Plugin) validateConfig() error { if p.BaseURL == "" { return errors.New("jenkins base URL is required") } - if p.Username == "" { - return errors.New("jenkins username is required") - } - if p.Token == "" { - return errors.New("jenkins API token is required") + + // Validate authentication: either (user + token) or remote-token must be provided + hasUserAuth := p.Username != "" && p.Token != "" + hasRemoteToken := p.RemoteToken != "" + + if !hasUserAuth && !hasRemoteToken { + return errors.New("authentication required: provide either (user + token) or remote-token") } + return nil } @@ -113,10 +116,13 @@ func (p Plugin) Exec(ctx context.Context) error { return errors.New("at least one Jenkins job name is required") } - // Set up authentication - auth := &Auth{ - Username: p.Username, - Token: p.Token, + // Set up authentication (only if username and token are provided) + var auth *Auth + if p.Username != "" && p.Token != "" { + auth = &Auth{ + Username: p.Username, + Token: p.Token, + } } // Initialize Jenkins client diff --git a/plugin_test.go b/plugin_test.go index 095c719..06d2d59 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -32,24 +32,33 @@ func TestValidateConfig(t *testing.T) { errorMsg: "jenkins base URL is required", }, { - name: "missing username and token", + name: "missing authentication", plugin: Plugin{ BaseURL: "http://example.com", }, wantError: true, - errorMsg: "jenkins username is required", + errorMsg: "authentication required", }, { - name: "missing token", + name: "missing token (only username)", plugin: Plugin{ BaseURL: "http://example.com", Username: "foo", }, wantError: true, - errorMsg: "jenkins API token is required", + errorMsg: "authentication required", }, { - name: "all required config present", + name: "missing username (only token)", + plugin: Plugin{ + BaseURL: "http://example.com", + Token: "bar", + }, + wantError: true, + errorMsg: "authentication required", + }, + { + name: "user and token auth", plugin: Plugin{ BaseURL: "http://example.com", Username: "foo", @@ -57,6 +66,24 @@ func TestValidateConfig(t *testing.T) { }, wantError: false, }, + { + name: "remote token auth", + plugin: Plugin{ + BaseURL: "http://example.com", + RemoteToken: "remote-token-123", + }, + wantError: false, + }, + { + name: "both auth methods", + plugin: Plugin{ + BaseURL: "http://example.com", + Username: "foo", + Token: "bar", + RemoteToken: "remote-token-123", + }, + wantError: false, + }, } for _, tt := range tests { @@ -232,7 +259,7 @@ func TestExecMissingJenkinsUsername(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "configuration error") - assert.Contains(t, err.Error(), "jenkins username is required") + assert.Contains(t, err.Error(), "authentication required") } // TestExecMissingJenkinsToken tests Exec with missing token @@ -246,7 +273,7 @@ func TestExecMissingJenkinsToken(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "configuration error") - assert.Contains(t, err.Error(), "jenkins API token is required") + assert.Contains(t, err.Error(), "authentication required") } // TestExecMissingJenkinsJob tests Exec with missing or empty job list