diff --git a/utils/config/tokenrefresh.go b/utils/config/tokenrefresh.go index 1cdd97f4b..fac6c58c9 100644 --- a/utils/config/tokenrefresh.go +++ b/utils/config/tokenrefresh.go @@ -2,13 +2,11 @@ package config import ( "errors" - "fmt" "sync" "time" "github.com/jfrog/jfrog-client-go/access" accessservices "github.com/jfrog/jfrog-client-go/access/services" - clientutils "github.com/jfrog/jfrog-client-go/utils" "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" @@ -185,56 +183,23 @@ func writeNewTokens(serverConfiguration *ServerDetails, serverId, accessToken, r } func createTokensForConfig(serverDetails *ServerDetails, expirySeconds int) (auth.CreateTokenResponseData, error) { - expiresIn := uint(max(expirySeconds, 0)) // #nosec G115 -- expirySeconds is validated positive by callers - createTokenParams := accessservices.CreateTokenParams{ - CommonTokenParams: auth.CommonTokenParams{ - Scope: "applied-permissions/user", - ExpiresIn: &expiresIn, - Refreshable: clientutils.Pointer(true), - }, - Username: serverDetails.User, - } - - // First, try with the original credentials (basic auth: user + password). - servicesManager, err := createAccessTokensServiceManager(serverDetails) + servicesManager, err := createArtifactoryTokensServiceManager(serverDetails) if err != nil { return auth.CreateTokenResponseData{}, err } - newToken, err := servicesManager.CreateAccessToken(createTokenParams) - if err == nil { - return newToken, nil - } - // If basic auth failed and a password is available, retry using it as a Bearer token. - // This handles reference tokens, which the Access service can resolve server-side. - if serverDetails.Password != "" { - bearerDetails := serverDetailsForBearerAuth(serverDetails, serverDetails.Password) - servicesManager, err = createAccessTokensServiceManager(bearerDetails) - if err != nil { - return auth.CreateTokenResponseData{}, err - } - newToken, err = servicesManager.CreateAccessToken(createTokenParams) - if err == nil { - return newToken, nil - } - log.Debug("Access token creation with Bearer auth failed: " + err.Error()) - } + createTokenParams := services.NewCreateTokenParams() + createTokenParams.Username = serverDetails.User + createTokenParams.ExpiresIn = expirySeconds + // User-scoped token + createTokenParams.Scope = "member-of-groups:*" + createTokenParams.Refreshable = true - return auth.CreateTokenResponseData{}, fmt.Errorf( - "automatic token creation via the Access API failed: %s. "+ - "If your JFrog Platform version does not support the Access API, please upgrade. "+ - "Alternatively, use the --basic-auth-only flag to skip automatic token creation", - err.Error()) -} - -// serverDetailsForBearerAuth returns a ServerDetails configured to authenticate using the -// given token as a Bearer token, with user/password credentials cleared. -func serverDetailsForBearerAuth(original *ServerDetails, token string) *ServerDetails { - details := *original - details.AccessToken = token - details.User = "" - details.Password = "" - return &details + newToken, err := servicesManager.CreateToken(createTokenParams) + if err != nil { + return auth.CreateTokenResponseData{}, err + } + return newToken, nil } func CreateInitialRefreshableTokensIfNeeded(serverDetails *ServerDetails) (err error) { @@ -256,10 +221,9 @@ func CreateInitialRefreshableTokensIfNeeded(serverDetails *ServerDetails) (err e return } - newToken, tokenErr := createTokensForConfig(serverDetails, serverDetails.ArtifactoryTokenRefreshInterval*60) - if tokenErr != nil { - serverDetails.ArtifactoryTokenRefreshInterval = 0 - return nil + newToken, err := createTokensForConfig(serverDetails, serverDetails.ArtifactoryTokenRefreshInterval*60) + if err != nil { + return } // Remove initializing value. serverDetails.ArtifactoryTokenRefreshInterval = 0 diff --git a/utils/config/tokenrefresh_test.go b/utils/config/tokenrefresh_test.go index 76418cca6..38ffc6542 100644 --- a/utils/config/tokenrefresh_test.go +++ b/utils/config/tokenrefresh_test.go @@ -1,16 +1,10 @@ package config import ( - "encoding/json" - "net/http" - "net/http/httptest" - "strings" - "sync" "testing" configtests "github.com/jfrog/jfrog-cli-core/v2/utils/config/tests" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestCreateInitialRefreshableTokensIfNeededEarlyReturns(t *testing.T) { @@ -202,11 +196,21 @@ func TestCreateInitialRefreshableTokensIfNeededValidInputs(t *testing.T) { err = CreateInitialRefreshableTokensIfNeeded(serverDetailsCopy) - // With the Access API migration, token creation fails gracefully when no server is reachable: - // the function returns nil error and resets the interval to 0 (disabling token refresh). - assert.NoError(t, err, "Should not return error — graceful fallback to basic auth") - assert.Equal(t, 0, serverDetailsCopy.ArtifactoryTokenRefreshInterval, - "ArtifactoryTokenRefreshInterval should be reset to 0 after token creation attempt") + if tt.expectError { + assert.Error(t, err, "Expected an error but got none") + } else if err == nil { + // Note: This will fail if createTokensForConfig requires actual Artifactory connection + // In that case, this test would need to be an integration test with a mock server + assert.Equal(t, tt.expectedIntervalAfterCall, serverDetailsCopy.ArtifactoryTokenRefreshInterval, + "ArtifactoryTokenRefreshInterval should be reset to 0 after successful token creation") + // Verify tokens were set (if no error occurred) + // Note: This assumes createTokensForConfig succeeded + // In a real scenario, you'd need to mock the Artifactory service + if tt.shouldCreateTokens { + assert.NotEmpty(t, serverDetailsCopy.AccessToken, "AccessToken should be set after successful creation") + assert.NotEmpty(t, serverDetailsCopy.ArtifactoryRefreshToken, "ArtifactoryRefreshToken should be set after successful creation") + } + } }) } } @@ -302,11 +306,20 @@ func TestCreateInitialRefreshableTokensIfNeededInputValidation(t *testing.T) { // 1. Return early (if conditions are met) // 2. Attempt to create tokens and potentially fail due to invalid input // We validate that the function handles the input gracefully - // With graceful fallback, token creation failures return nil error - assert.NoError(t, err, "Should not return error — graceful fallback") - if initialInterval > 0 { - assert.Equal(t, 0, serverDetailsCopy.ArtifactoryTokenRefreshInterval, - "Interval should be reset to 0 after token creation attempt") + if err != nil { + // If there's an error, it should be due to invalid configuration + // The interval should be reset to 0 if token creation was attempted + if serverDetailsCopy.ArtifactoryTokenRefreshInterval == 0 { + // Token creation was attempted but failed + assert.Error(t, err, tt.description) + } + } else { + // If no error, either early return occurred or tokens were created successfully + if initialInterval > 0 && serverDetailsCopy.ArtifactoryTokenRefreshInterval == 0 { + // Tokens were created successfully + assert.NotEmpty(t, serverDetailsCopy.AccessToken, "AccessToken should be set after successful creation") + assert.NotEmpty(t, serverDetailsCopy.ArtifactoryRefreshToken, "ArtifactoryRefreshToken should be set after successful creation") + } } }) } @@ -557,156 +570,3 @@ func TestCreateInitialRefreshableTokensIfNeededBranchCoverage(t *testing.T) { assert.NotNil(t, serverDetails) }) } - -func TestAccessAPITokenCreation_MockServer(t *testing.T) { - cleanUpTempEnv := configtests.CreateTempEnv(t, false) - defer cleanUpTempEnv() - - var mu sync.Mutex - var accessAPIHit bool - var authMethod string - - mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/access/api/v1/tokens" && r.Method == http.MethodPost { - mu.Lock() - accessAPIHit = true - if _, _, ok := r.BasicAuth(); ok { - authMethod = "basic" - } else if strings.HasPrefix(r.Header.Get("Authorization"), "Bearer ") { - authMethod = "bearer" - } - mu.Unlock() - w.Header().Set("Content-Type", "application/json") - // #nosec G101 -- mock test credentials, not real tokens - resp := map[string]interface{}{ - "access_token": "mock-jwt-token", // #nosec G101 - "refresh_token": "mock-refresh-token", // #nosec G101 - "expires_in": 3600, - "scope": "applied-permissions/user", - "token_type": "Bearer", - } - _ = json.NewEncoder(w).Encode(resp) - return - } - w.WriteHeader(http.StatusNotFound) - })) - defer mockServer.Close() - - platformURL := mockServer.URL + "/" - - serverDetails := &ServerDetails{ - ServerId: "test-access-api", - ArtifactoryTokenRefreshInterval: 60, - ArtifactoryRefreshToken: "", - AccessToken: "", - ArtifactoryUrl: mockServer.URL + "/artifactory/", - Url: platformURL, - User: "testuser", - Password: "my-regular-password", - } - - err := SaveServersConf([]*ServerDetails{serverDetails}) - require.NoError(t, err) - - _ = CreateInitialRefreshableTokensIfNeeded(serverDetails) - - mu.Lock() - assert.True(t, accessAPIHit, - "Token creation should use the Access API endpoint") - assert.Equal(t, "basic", authMethod, - "Regular password should use basic auth") - mu.Unlock() -} - -func TestBasicAuthFailsFallsBackToBearer_MockServer(t *testing.T) { - cleanUpTempEnv := configtests.CreateTempEnv(t, false) - defer cleanUpTempEnv() - - var mu sync.Mutex - var attempts []string - - // #nosec G101 -- mock reference token, not a real credential - fakeRefToken := "cmVmdGtuOjAxOjE3NzczNTczOTI6ZmFrZVRva2VuRm9yVGVzdGluZ09ubHk=" - - mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/access/api/v1/tokens" && r.Method == http.MethodPost { - mu.Lock() - if _, _, ok := r.BasicAuth(); ok { - attempts = append(attempts, "basic") - mu.Unlock() - w.WriteHeader(http.StatusUnauthorized) - return - } else if strings.HasPrefix(r.Header.Get("Authorization"), "Bearer ") { - attempts = append(attempts, "bearer") - mu.Unlock() - w.Header().Set("Content-Type", "application/json") - // #nosec G101 -- mock test credentials, not real tokens - resp := map[string]interface{}{ - "access_token": "mock-scoped-jwt", // #nosec G101 - "refresh_token": "mock-refresh-token", // #nosec G101 - "expires_in": 3600, - "scope": "applied-permissions/user", - "token_type": "Bearer", - } - _ = json.NewEncoder(w).Encode(resp) - return - } - mu.Unlock() - } - w.WriteHeader(http.StatusNotFound) - })) - defer mockServer.Close() - - serverDetails := &ServerDetails{ - ServerId: "test-retry-bearer", - ArtifactoryTokenRefreshInterval: 60, - ArtifactoryRefreshToken: "", - AccessToken: "", - ArtifactoryUrl: mockServer.URL + "/artifactory/", - Url: mockServer.URL + "/", - User: "testuser", - Password: fakeRefToken, - } - - err := SaveServersConf([]*ServerDetails{serverDetails}) - require.NoError(t, err) - - err = CreateInitialRefreshableTokensIfNeeded(serverDetails) - assert.NoError(t, err) - - mu.Lock() - assert.Equal(t, []string{"basic", "bearer"}, attempts, - "Should first try basic auth, then fall back to bearer when basic fails") - mu.Unlock() -} - -func TestAccessAPITokenCreationFails_MockServer(t *testing.T) { - cleanUpTempEnv := configtests.CreateTempEnv(t, false) - defer cleanUpTempEnv() - - mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusUnauthorized) - })) - defer mockServer.Close() - - serverDetails := &ServerDetails{ - ServerId: "test-api-fail", - ArtifactoryTokenRefreshInterval: 60, - ArtifactoryRefreshToken: "", - AccessToken: "", - ArtifactoryUrl: mockServer.URL + "/", - Url: mockServer.URL + "/", - User: "testuser", - Password: "my-regular-password", - } - - err := SaveServersConf([]*ServerDetails{serverDetails}) - require.NoError(t, err) - - err = CreateInitialRefreshableTokensIfNeeded(serverDetails) - assert.NoError(t, err, "Should not return error when Access API fails — gracefully falls back to basic auth") - assert.Equal(t, 0, serverDetails.ArtifactoryTokenRefreshInterval, - "Token refresh should be disabled after Access API failure") - assert.Empty(t, serverDetails.AccessToken, - "No access token should be set when Access API fails") -} \ No newline at end of file