mirror of
https://github.com/appleboy/drone-jenkins.git
synced 2026-06-04 10:15:02 +08:00
feat: refactor authentication logic and broaden test coverage (#50)
* 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 <appleboy.tw@gmail.com> * style: streamline authentication error handling in config validation - Simplify authentication error message in config validation Signed-off-by: appleboy <appleboy.tw@gmail.com> --------- Signed-off-by: appleboy <appleboy.tw@gmail.com>
This commit is contained in:
+9
-5
@@ -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)
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
|
||||
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
|
||||
|
||||
+34
-7
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user