fix(connectors): replace panics with error returns and add HTTP status code validation

pkg/connectors/keycloak_client.go

@@ -97,26 +97,32 @@ func (c *keycloakClient) ConnectAndGetToken() (string, error) {
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		panic(err.Error())
+		return "", fmt.Errorf("failed to read token response: %w", err)
+	}
+
+	if resp.StatusCode/100 != 2 {
+		return "", fmt.Errorf("token request returned HTTP %d: %s", resp.StatusCode, string(body))
 	}
 
 	var openIDResp map[string]interface{}
 	if err := json.Unmarshal(body, &openIDResp); err != nil {
-		panic(err)
+		return "", fmt.Errorf("failed to parse token response: %w", err)
 	}
 
-	accessToken := openIDResp["access_token"].(string)
-	return accessToken, err
+	accessToken, ok := openIDResp["access_token"].(string)
+	if !ok {
+		return "", fmt.Errorf("token response missing required field \"access_token\"")
+	}
+	return accessToken, nil
 }
 
 func (c *keycloakClient) GetOIDCConfig() (*oauth2.Config, error) {
 	rel := &url.URL{Path: ".well-known/openid-configuration"}
 	u := c.BaseURL.ResolveReference(rel)
 
-	// Create HTTP request
 	req, err := http.NewRequest("GET", u.String(), nil)
 	if err != nil {
-		fmt.Println("Error creating request:", err)
+		return nil, fmt.Errorf("failed to create OIDC config request: %w", err)
 	}
 
 	resp, err := c.httpClient.Do(req)
@@ -127,16 +133,26 @@ func (c *keycloakClient) GetOIDCConfig() (*oauth2.Config, error) {
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		panic(err.Error())
+		return nil, fmt.Errorf("failed to read OIDC config response: %w", err)
+	}
+
+	if resp.StatusCode/100 != 2 {
+		return nil, fmt.Errorf("OIDC config request returned HTTP %d: %s", resp.StatusCode, string(body))
 	}
 
 	var openIDResp map[string]interface{}
 	if err := json.Unmarshal(body, &openIDResp); err != nil {
-		panic(err)
+		return nil, fmt.Errorf("failed to parse OIDC config response: %w", err)
 	}
 
-	authURL := openIDResp["authorization_endpoint"].(string)
-	tokenURL := openIDResp["token_endpoint"].(string)
+	authURL, ok := openIDResp["authorization_endpoint"].(string)
+	if !ok {
+		return nil, fmt.Errorf("OIDC config response missing required field \"authorization_endpoint\"")
+	}
+	tokenURL, ok := openIDResp["token_endpoint"].(string)
+	if !ok {
+		return nil, fmt.Errorf("OIDC config response missing required field \"token_endpoint\"")
+	}
 
 	return &oauth2.Config{
 		Endpoint: oauth2.Endpoint{
@@ -157,13 +173,11 @@ func (c *keycloakClient) ConnectAndGetTokenAndRefreshToken(username, password st
 	data.Set("username", username)
 	data.Set("password", password)
 	data.Set("grant_type", "password")
-	// Create HTTP request
 	req, err := http.NewRequest("POST", u.String(), bytes.NewBufferString(data.Encode()))
 	if err != nil {
-		fmt.Println("Error creating request:", err)
+		return "", "", fmt.Errorf("failed to create password token request: %w", err)
 	}
 
-	// Set headers
 	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 
 	resp, err := c.httpClient.Do(req)
@@ -174,16 +188,26 @@ func (c *keycloakClient) ConnectAndGetTokenAndRefreshToken(username, password st
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		panic(err.Error())
+		return "", "", fmt.Errorf("failed to read password token response: %w", err)
+	}
+
+	if resp.StatusCode/100 != 2 {
+		return "", "", fmt.Errorf("password token request returned HTTP %d: %s", resp.StatusCode, string(body))
 	}
 
 	var openIDResp map[string]interface{}
 	if err := json.Unmarshal(body, &openIDResp); err != nil {
-		panic(err)
+		return "", "", fmt.Errorf("failed to parse password token response: %w", err)
 	}
 
-	authToken := openIDResp["access_token"].(string)
-	refreshToken := openIDResp["refresh_token"].(string)
+	authToken, ok := openIDResp["access_token"].(string)
+	if !ok {
+		return "", "", fmt.Errorf("password token response missing required field \"access_token\"")
+	}
+	refreshToken, ok := openIDResp["refresh_token"].(string)
+	if !ok {
+		return "", "", fmt.Errorf("password token response missing required field \"refresh_token\"")
+	}
 
 	return authToken, refreshToken, nil
 }

pkg/connectors/keycloak_client_test.go

@@ -0,0 +1,127 @@
+package connectors
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+func TestConnectAndGetTokenReturnsErrorOnNon2xx(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusUnauthorized)
+		w.Write([]byte(`{"error":"unauthorized_client"}`))
+	}))
+	defer server.Close()
+
+	kc := NewKeycloakClient(server.URL+"/", "bad-id", "bad-secret")
+	_, err := kc.ConnectAndGetToken()
+	if err == nil {
+		t.Fatal("expected error for 401 response, got nil")
+	}
+	if !strings.Contains(err.Error(), "HTTP 401") {
+		t.Fatalf("expected error to mention HTTP 401, got: %s", err.Error())
+	}
+}
+
+func TestConnectAndGetTokenReturnsErrorOnMalformedJSON(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte("<html>not json</html>"))
+	}))
+	defer server.Close()
+
+	kc := NewKeycloakClient(server.URL+"/", "id", "secret")
+	_, err := kc.ConnectAndGetToken()
+	if err == nil {
+		t.Fatal("expected error for malformed JSON, got nil")
+	}
+	if !strings.Contains(err.Error(), "failed to parse") {
+		t.Fatalf("expected parse error, got: %s", err.Error())
+	}
+}
+
+func TestConnectAndGetTokenReturnsErrorOnMissingAccessToken(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`{"token_type":"bearer"}`))
+	}))
+	defer server.Close()
+
+	kc := NewKeycloakClient(server.URL+"/", "id", "secret")
+	_, err := kc.ConnectAndGetToken()
+	if err == nil {
+		t.Fatal("expected error for missing access_token, got nil")
+	}
+	if !strings.Contains(err.Error(), "missing required field") {
+		t.Fatalf("expected missing field error, got: %s", err.Error())
+	}
+}
+
+func TestGetOIDCConfigReturnsErrorOnNon2xx(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+		w.Write([]byte("not found"))
+	}))
+	defer server.Close()
+
+	kc := NewKeycloakClient(server.URL+"/", "", "")
+	_, err := kc.GetOIDCConfig()
+	if err == nil {
+		t.Fatal("expected error for 404 response, got nil")
+	}
+	if !strings.Contains(err.Error(), "HTTP 404") {
+		t.Fatalf("expected error to mention HTTP 404, got: %s", err.Error())
+	}
+}
+
+func TestGetOIDCConfigReturnsErrorOnMissingFields(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`{"issuer":"https://auth.example.com"}`))
+	}))
+	defer server.Close()
+
+	kc := NewKeycloakClient(server.URL+"/", "", "")
+	_, err := kc.GetOIDCConfig()
+	if err == nil {
+		t.Fatal("expected error for missing OIDC fields, got nil")
+	}
+	if !strings.Contains(err.Error(), "missing required field") {
+		t.Fatalf("expected missing field error, got: %s", err.Error())
+	}
+}
+
+func TestConnectAndGetTokenAndRefreshTokenReturnsErrorOnNon2xx(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusBadRequest)
+		w.Write([]byte(`{"error":"invalid_grant"}`))
+	}))
+	defer server.Close()
+
+	kc := NewKeycloakClient(server.URL+"/", "client-id", "client-secret")
+	_, _, err := kc.ConnectAndGetTokenAndRefreshToken("user", "wrong-pass")
+	if err == nil {
+		t.Fatal("expected error for 400 response, got nil")
+	}
+	if !strings.Contains(err.Error(), "HTTP 400") {
+		t.Fatalf("expected error to mention HTTP 400, got: %s", err.Error())
+	}
+}
+
+func TestConnectAndGetTokenAndRefreshTokenReturnsErrorOnMissingFields(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`{"access_token":"tok123"}`))
+	}))
+	defer server.Close()
+
+	kc := NewKeycloakClient(server.URL+"/", "client-id", "client-secret")
+	_, _, err := kc.ConnectAndGetTokenAndRefreshToken("user", "pass")
+	if err == nil {
+		t.Fatal("expected error for missing refresh_token field, got nil")
+	}
+	if !strings.Contains(err.Error(), "refresh_token") {
+		t.Fatalf("expected error to mention refresh_token, got: %s", err.Error())
+	}
+}

pkg/connectors/microcks_client.go

@@ -35,7 +35,6 @@ import (
 	"github.com/coreos/go-oidc/v3/oidc"
 	"github.com/golang-jwt/jwt/v4"
 	"github.com/microcks/microcks-cli/pkg/config"
-	"github.com/microcks/microcks-cli/pkg/errors"
 	"golang.org/x/oauth2"
 )
 
@@ -239,24 +238,37 @@ func (c *microcksClient) GetKeycloakURL() (string, error) {
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		panic(err.Error())
+		return "", fmt.Errorf("failed to read Keycloak config response: %w", err)
+	}
+
+	if resp.StatusCode/100 != 2 {
+		return "", fmt.Errorf("Keycloak config request returned HTTP %d: %s", resp.StatusCode, string(body))
 	}
 
 	var configResp map[string]interface{}
 	if err := json.Unmarshal(body, &configResp); err != nil {
-		panic(err)
+		return "", fmt.Errorf("failed to parse Keycloak config response: %w", err)
 	}
 
 	// Retrieve auth server url and realm name.
-	enabled := configResp["enabled"].(bool)
-	authServerURL := configResp["auth-server-url"].(string)
-	realmName := configResp["realm"].(string)
+	enabled, ok := configResp["enabled"].(bool)
+	if !ok {
+		return "", fmt.Errorf("Keycloak config response missing required field \"enabled\"")
+	}
+	if !enabled {
+		return "null", nil
+	}
 
-	// Return a proper URL or 'null' if Keycloak is disables.
-	if enabled {
-		return authServerURL + "/realms/" + realmName + "/", nil
+	authServerURL, ok := configResp["auth-server-url"].(string)
+	if !ok {
+		return "", fmt.Errorf("Keycloak config response missing required field \"auth-server-url\"")
 	}
-	return "null", nil
+	realmName, ok := configResp["realm"].(string)
+	if !ok {
+		return "", fmt.Errorf("Keycloak config response missing required field \"realm\"")
+	}
+
+	return authServerURL + "/realms/" + realmName + "/", nil
 }
 
 func (c *microcksClient) refreshAuthToken(localCfg *config.LocalConfig, ctxName, configPath string) error {
@@ -304,10 +316,15 @@ func (c *microcksClient) refreshAuthToken(localCfg *config.LocalConfig, ctxName,
 
 func (c *microcksClient) redeemRefreshToken(auth config.Auth) (string, string, error) {
 	keyCloakUrl, err := c.GetKeycloakURL()
-	errors.CheckError(err)
+	if err != nil {
+		return "", "", err
+	}
+
 	kc := NewKeycloakClient(keyCloakUrl, "", "")
 	oauth2Conf, err := kc.GetOIDCConfig()
-	errors.CheckError(err)
+	if err != nil {
+		return "", "", err
+	}
 	oauth2Conf.ClientID = auth.ClientId
 	oauth2Conf.ClientSecret = auth.ClientSecret
 
@@ -381,16 +398,23 @@ func (c *microcksClient) CreateTestResult(serviceID string, testEndpoint string,
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		panic(err.Error())
+		return "", fmt.Errorf("failed to read create test response: %w", err)
+	}
+
+	if resp.StatusCode/100 != 2 {
+		return "", fmt.Errorf("create test request returned HTTP %d: %s", resp.StatusCode, string(body))
 	}
 
 	var createTestResp map[string]interface{}
 	if err := json.Unmarshal(body, &createTestResp); err != nil {
-		panic(err)
+		return "", fmt.Errorf("failed to parse create test response: %w", err)
 	}
 
-	testID := createTestResp["id"].(string)
-	return testID, err
+	testID, ok := createTestResp["id"].(string)
+	if !ok {
+		return "", fmt.Errorf("create test response missing required field \"id\"")
+	}
+	return testID, nil
 }
 
 func (c *microcksClient) GetTestResult(testResultID string) (*TestResultSummary, error) {
@@ -420,7 +444,11 @@ func (c *microcksClient) GetTestResult(testResultID string) (*TestResultSummary,
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		panic(err.Error())
+		return nil, fmt.Errorf("failed to read test result response: %w", err)
+	}
+
+	if resp.StatusCode/100 != 2 {
+		return nil, fmt.Errorf("get test result request returned HTTP %d: %s", resp.StatusCode, string(body))
 	}
 
 	result := TestResultSummary{}
@@ -553,7 +581,7 @@ func (c *microcksClient) DownloadArtifact(artifactURL string, mainArtifact bool,
 
 	respBody, err := io.ReadAll(resp.Body)
 	if err != nil {
-		panic(err.Error())
+		return "", fmt.Errorf("failed to read download artifact response: %w", err)
 	}
 
 	// Raise exception if not created.