[v15] fix jira error parsing

integrations/access/jira/client.go

@@ -130,7 +130,7 @@ func NewJiraClient(conf JiraConfig, clusterName, teleportProxyAddr string, statu
 				if resp.IsError() {
 					switch result := resp.Error().(type) {
 					case *ErrorResult:
-						return trace.Errorf("http error code=%v, errors=[%v]", resp.StatusCode(), strings.Join(result.ErrorMessages, ", "))
+						return trace.Errorf("http error code=%v, errors=[%s]", resp.StatusCode(), result)
 					case nil:
 						return nil
 					default:

integrations/access/jira/types.go

@@ -18,11 +18,86 @@
 
 package jira
 
+import (
+	"encoding/json"
+	"fmt"
+	"strings"
+
+	"github.com/gravitational/trace"
+)
+
 // Jira REST API resources
 
+// ErrorResult is used to parse the errors from Jira.
+// The JSON Schema is specified here:
+// https://docs.atlassian.com/software/jira/docs/api/REST/1000.1223.0/#error-responses
+// However JIRA does not consistently respect the schema (especially for old instances).
+// We need to support legacy errors as well (array of strings).
 type ErrorResult struct {
-	ErrorMessages []string `url:"errorMessages"`
-	Errors        []string `url:"errors"`
+	ErrorMessages []string     `url:"errorMessages" json:"errorMessages"`
+	Details       ErrorDetails `url:"errors" json:"errors"`
+}
+
+// Error implements the error interface and returns a string describing the
+// error returned by Jira.
+func (e ErrorResult) Error() string {
+	sb := strings.Builder{}
+	if len(e.ErrorMessages) > 0 {
+		sb.WriteString(fmt.Sprintf("error messages: %s ", strings.Join(e.ErrorMessages, ";")))
+	}
+	if details := e.Details.String(); details != "" {
+		sb.WriteString(fmt.Sprintf("error details: %s", details))
+	}
+	result := sb.String()
+	if result == "" {
+		return "Unknown Jira error"
+	}
+	return result
+}
+
+// ErrorDetails are used to unmarshall inconsistently formatted Jira errors
+// details.
+type ErrorDetails struct {
+	// Errors contain object-formatted Jira Errors. Usually Jira returns
+	// errors in an object where keys are single word representing what is
+	// broken, and values containing text description of the issue.
+	// This is the official return format, according to Jira's docs.
+	Errors map[string]string
+	// LegacyErrors ensures backward compatibility with Jira errors returned as
+	// a list. It's unclear which Jira version and which part of Jira can return
+	// such errors, but they existed at some point, and we might still get them.
+	LegacyErrors []string
+}
+
+func (e *ErrorDetails) UnmarshalJSON(data []byte) error {
+	// Try to parse as a new error
+	var errors map[string]string
+	if err := json.Unmarshal(data, &errors); err == nil {
+		e.Errors = errors
+		return nil
+	}
+
+	// Try to parse as a legacy error
+	var legacyErrors []string
+	if err := json.Unmarshal(data, &legacyErrors); err == nil {
+		e.LegacyErrors = legacyErrors
+		return nil
+	}
+
+	// Everything failed, we return an unrmarshalling error that contains the data.
+	// This way, even if everything failed, the user still has the original response in the logs.
+	return trace.Errorf("Failed to unmarshal Jira error: %q", string(data))
+}
+
+func (e ErrorDetails) String() string {
+	switch {
+	case len(e.Errors) > 0:
+		return fmt.Sprintf("%s", e.Errors)
+	case len(e.LegacyErrors) > 0:
+		return fmt.Sprintf("%s", e.LegacyErrors)
+	default:
+		return ""
+	}
 }
 
 type GetMyPermissionsQueryOptions struct {

integrations/access/jira/types_test.go

@@ -0,0 +1,83 @@
+/*
+ * Teleport
+ * Copyright (C) 2023  Gravitational, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package jira
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/stretchr/testify/require"
+)
+
+func TestErrorResultUnmarshal(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name           string
+		input          string
+		expectedOutput ErrorResult
+		assertErr      require.ErrorAssertionFunc
+	}{
+		{
+			name:  "new error",
+			input: `{"errorMessages":[], "errors": {"project": "project is required"}}`,
+			expectedOutput: ErrorResult{
+				Details: ErrorDetails{Errors: map[string]string{"project": "project is required"}},
+			},
+			assertErr: require.NoError,
+		},
+		{
+			name:  "legacy error",
+			input: `{"errorMessages":["foo"],"errors":["bar", "baz"]}`,
+			expectedOutput: ErrorResult{
+				ErrorMessages: []string{"foo"},
+				Details:       ErrorDetails{LegacyErrors: []string{"bar", "baz"}},
+			},
+			assertErr: require.NoError,
+		},
+		{
+			name:  "empty error",
+			input: `{"errorMessages":["There was an error parsing JSON. Check that your request body is valid."]}`,
+			expectedOutput: ErrorResult{
+				ErrorMessages: []string{"There was an error parsing JSON. Check that your request body is valid."},
+			},
+			assertErr: require.NoError,
+		},
+		{
+			name:           "malformed error",
+			input:          `{"errorMessages":["Foo"],"errors":"This is a single string"}`,
+			expectedOutput: ErrorResult{ErrorMessages: []string{"Foo"}},
+			assertErr: func(t require.TestingT, err error, i ...interface{}) {
+				require.ErrorContains(t, err, "This is a single string")
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tt := tt
+			t.Parallel()
+			data := []byte(tt.input)
+			var result ErrorResult
+			tt.assertErr(t, json.Unmarshal(data, &result))
+			diff := cmp.Diff(tt.expectedOutput, result, cmpopts.EquateEmpty())
+			require.Empty(t, diff)
+		})
+	}
+}