diff --git a/pkg/connectors/keycloak_client.go b/pkg/connectors/keycloak_client.go index 5d6aeed..81712b4 100644 --- a/pkg/connectors/keycloak_client.go +++ b/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 } diff --git a/pkg/connectors/keycloak_client_test.go b/pkg/connectors/keycloak_client_test.go new file mode 100644 index 0000000..d0a8391 --- /dev/null +++ b/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("not json")) + })) + 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()) + } +} diff --git a/pkg/connectors/microcks_client.go b/pkg/connectors/microcks_client.go index 8ecd420..b3a3eac 100644 --- a/pkg/connectors/microcks_client.go +++ b/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. diff --git a/pkg/connectors/microcks_client_error_test.go b/pkg/connectors/microcks_client_error_test.go new file mode 100644 index 0000000..2cba706 --- /dev/null +++ b/pkg/connectors/microcks_client_error_test.go @@ -0,0 +1,180 @@ +package connectors + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/microcks/microcks-cli/pkg/config" +) + +func TestGetKeycloakURLReturnsErrorOnNon2xx(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte("internal error")) + })) + defer server.Close() + + client := NewMicrocksClient(server.URL) + _, err := client.GetKeycloakURL() + if err == nil { + t.Fatal("expected error for non-2xx response, got nil") + } + if !strings.Contains(err.Error(), "HTTP 500") { + t.Fatalf("expected error to mention HTTP 500, got: %s", err.Error()) + } +} + +func TestGetKeycloakURLReturnsErrorOnMalformedJSON(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte("not json")) + })) + defer server.Close() + + client := NewMicrocksClient(server.URL) + _, err := client.GetKeycloakURL() + 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 TestGetKeycloakURLReturnsErrorOnMissingFields(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"enabled": true}`)) + })) + defer server.Close() + + client := NewMicrocksClient(server.URL) + _, err := client.GetKeycloakURL() + if err == nil { + t.Fatal("expected error for missing fields, got nil") + } + if !strings.Contains(err.Error(), "missing required field") { + t.Fatalf("expected missing field error, got: %s", err.Error()) + } +} + +func TestGetKeycloakURLReturnsNullWhenKeycloakDisabled(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"enabled": false}`)) + })) + defer server.Close() + + client := NewMicrocksClient(server.URL) + keycloakURL, err := client.GetKeycloakURL() + if err != nil { + t.Fatalf("expected no error for disabled Keycloak config, got: %s", err.Error()) + } + if keycloakURL != "null" { + t.Fatalf("expected disabled Keycloak URL to be null, got: %s", keycloakURL) + } +} + +func TestRedeemRefreshTokenReturnsKeycloakConfigError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte("config unavailable")) + })) + defer server.Close() + + client := NewMicrocksClient(server.URL).(*microcksClient) + _, _, err := client.redeemRefreshToken(config.Auth{}) + if err == nil { + t.Fatal("expected error for Keycloak config failure, got nil") + } + if !strings.Contains(err.Error(), "HTTP 500") { + t.Fatalf("expected error to mention HTTP 500, got: %s", err.Error()) + } +} + +func TestCreateTestResultReturnsErrorOnNon2xx(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/keycloak/config" { + json.NewEncoder(w).Encode(map[string]interface{}{ + "enabled": false, + "auth-server-url": "", + "realm": "", + }) + return + } + w.WriteHeader(http.StatusUnauthorized) + w.Write([]byte("unauthorized")) + })) + defer server.Close() + + client := NewMicrocksClient(server.URL) + client.SetOAuthToken("expired-token") + + _, err := client.CreateTestResult("svc:1.0", "http://endpoint", "HTTP", "", 5000, "", "", "") + if err == nil { + t.Fatal("expected error for non-2xx response, got nil") + } + if !strings.Contains(err.Error(), "HTTP 401") { + t.Fatalf("expected error to mention HTTP 401, got: %s", err.Error()) + } +} + +func TestCreateTestResultReturnsErrorOnMissingID(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"status": "created"}`)) + })) + defer server.Close() + + client := NewMicrocksClient(server.URL) + client.SetOAuthToken("test-token") + + _, err := client.CreateTestResult("svc:1.0", "http://endpoint", "HTTP", "", 5000, "", "", "") + if err == nil { + t.Fatal("expected error for missing id field, got nil") + } + if !strings.Contains(err.Error(), "missing required field") { + t.Fatalf("expected missing field error, got: %s", err.Error()) + } +} + +func TestGetTestResultReturnsErrorOnNon2xx(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + w.Write([]byte("forbidden")) + })) + defer server.Close() + + client := NewMicrocksClient(server.URL) + client.SetOAuthToken("test-token") + + _, err := client.GetTestResult("abc-123") + if err == nil { + t.Fatal("expected error for non-2xx response, got nil") + } + if !strings.Contains(err.Error(), "HTTP 403") { + t.Fatalf("expected error to mention HTTP 403, got: %s", err.Error()) + } +} + +func TestDownloadArtifactReturnsErrorOnNon2xx(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() + + client := NewMicrocksClient(server.URL) + client.SetOAuthToken("test-token") + + _, err := client.DownloadArtifact("https://example.com/spec.yaml", true, "") + if err == nil { + t.Fatal("expected error for non-2xx response, got nil") + } + if !strings.Contains(err.Error(), "not found") { + t.Fatalf("expected error body in message, got: %s", err.Error()) + } +}