From 58ff7ab797fcde081b3a0a802487f13a4ab4945a Mon Sep 17 00:00:00 2001
From: adasauce <60991921+adasauce@users.noreply.github.com>
Date: Fri, 27 Sep 2024 16:21:34 -0300
Subject: [PATCH] [management] improve zitadel idp error response detail by
 decoding errors (#2634)

* [management] improve zitadel idp error response detail by decoding errors

* [management] extend readZitadelError to be used for requestJWTToken

more generically parse the error returned by zitadel.

* fix lint

---------

Co-authored-by: bcmmbaga <bethuelmbaga12@gmail.com>
---
 management/server/idp/zitadel.go      | 49 +++++++++++++++++++++++++--
 management/server/idp/zitadel_test.go | 10 +++---
 2 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/management/server/idp/zitadel.go b/management/server/idp/zitadel.go
index 729b49733..9d7626844 100644
--- a/management/server/idp/zitadel.go
+++ b/management/server/idp/zitadel.go
@@ -2,10 +2,12 @@ package idp
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
 	"net/url"
+	"slices"
 	"strings"
 	"sync"
 	"time"
@@ -97,6 +99,42 @@ type zitadelUserResponse struct {
 	PasswordlessRegistration zitadelPasswordlessRegistration `json:"passwordlessRegistration"`
 }
 
+// readZitadelError parses errors returned by the zitadel APIs from a response.
+func readZitadelError(body io.ReadCloser) error {
+	bodyBytes, err := io.ReadAll(body)
+	if err != nil {
+		return fmt.Errorf("failed to read response body: %w", err)
+	}
+
+	helper := JsonParser{}
+	var target map[string]interface{}
+	err = helper.Unmarshal(bodyBytes, &target)
+	if err != nil {
+		return fmt.Errorf("error unparsable body: %s", string(bodyBytes))
+	}
+
+	// ensure keys are ordered for consistent logging behaviour.
+	errorKeys := make([]string, 0, len(target))
+	for k := range target {
+		errorKeys = append(errorKeys, k)
+	}
+	slices.Sort(errorKeys)
+
+	var errsOut []string
+	for _, k := range errorKeys {
+		if _, isEmbedded := target[k].(map[string]interface{}); isEmbedded {
+			continue
+		}
+		errsOut = append(errsOut, fmt.Sprintf("%s: %v", k, target[k]))
+	}
+
+	if len(errsOut) == 0 {
+		return errors.New("unknown error")
+	}
+
+	return errors.New(strings.Join(errsOut, " "))
+}
+
 // NewZitadelManager creates a new instance of the ZitadelManager.
 func NewZitadelManager(config ZitadelClientConfig, appMetrics telemetry.AppMetrics) (*ZitadelManager, error) {
 	httpTransport := http.DefaultTransport.(*http.Transport).Clone()
@@ -176,7 +214,8 @@ func (zc *ZitadelCredentials) requestJWTToken(ctx context.Context) (*http.Respon
 	}
 
 	if resp.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("unable to get zitadel token, statusCode %d", resp.StatusCode)
+		zErr := readZitadelError(resp.Body)
+		return nil, fmt.Errorf("unable to get zitadel token, statusCode %d, zitadel: %w", resp.StatusCode, zErr)
 	}
 
 	return resp, nil
@@ -489,7 +528,9 @@ func (zm *ZitadelManager) post(ctx context.Context, resource string, body string
 			zm.appMetrics.IDPMetrics().CountRequestStatusError()
 		}
 
-		return nil, fmt.Errorf("unable to post %s, statusCode %d", reqURL, resp.StatusCode)
+		zErr := readZitadelError(resp.Body)
+
+		return nil, fmt.Errorf("unable to post %s, statusCode %d, zitadel: %w", reqURL, resp.StatusCode, zErr)
 	}
 
 	return io.ReadAll(resp.Body)
@@ -561,7 +602,9 @@ func (zm *ZitadelManager) get(ctx context.Context, resource string, q url.Values
 			zm.appMetrics.IDPMetrics().CountRequestStatusError()
 		}
 
-		return nil, fmt.Errorf("unable to get %s, statusCode %d", reqURL, resp.StatusCode)
+		zErr := readZitadelError(resp.Body)
+
+		return nil, fmt.Errorf("unable to get %s, statusCode %d, zitadel: %w", reqURL, resp.StatusCode, zErr)
 	}
 
 	return io.ReadAll(resp.Body)
diff --git a/management/server/idp/zitadel_test.go b/management/server/idp/zitadel_test.go
index 6bc612e78..722f94fe0 100644
--- a/management/server/idp/zitadel_test.go
+++ b/management/server/idp/zitadel_test.go
@@ -66,7 +66,6 @@ func TestNewZitadelManager(t *testing.T) {
 }
 
 func TestZitadelRequestJWTToken(t *testing.T) {
-
 	type requestJWTTokenTest struct {
 		name                    string
 		inputCode               int
@@ -88,15 +87,14 @@ func TestZitadelRequestJWTToken(t *testing.T) {
 	requestJWTTokenTestCase2 := requestJWTTokenTest{
 		name:                    "Request Bad Status Code",
 		inputCode:               400,
-		inputRespBody:           "{}",
+		inputRespBody:           "{\"error\": \"invalid_scope\", \"error_description\":\"openid missing\"}",
 		helper:                  JsonParser{},
-		expectedFuncExitErrDiff: fmt.Errorf("unable to get zitadel token, statusCode 400"),
+		expectedFuncExitErrDiff: fmt.Errorf("unable to get zitadel token, statusCode 400, zitadel: error: invalid_scope error_description: openid missing"),
 		expectedToken:           "",
 	}
 
 	for _, testCase := range []requestJWTTokenTest{requestJWTTokenTesttCase1, requestJWTTokenTestCase2} {
 		t.Run(testCase.name, func(t *testing.T) {
-
 			jwtReqClient := mockHTTPClient{
 				resBody: testCase.inputRespBody,
 				code:    testCase.inputCode,
@@ -156,7 +154,7 @@ func TestZitadelParseRequestJWTResponse(t *testing.T) {
 	}
 	parseRequestJWTResponseTestCase2 := parseRequestJWTResponseTest{
 		name:                 "Parse Bad json JWT Body",
-		inputRespBody:        "",
+		inputRespBody:        "{}",
 		helper:               JsonParser{},
 		expectedToken:        "",
 		expectedExpiresIn:    0,
@@ -254,7 +252,7 @@ func TestZitadelAuthenticate(t *testing.T) {
 		inputCode:               400,
 		inputResBody:            "{}",
 		helper:                  JsonParser{},
-		expectedFuncExitErrDiff: fmt.Errorf("unable to get zitadel token, statusCode 400"),
+		expectedFuncExitErrDiff: fmt.Errorf("unable to get zitadel token, statusCode 400, zitadel: unknown error"),
 		expectedCode:            200,
 		expectedToken:           "",
 	}