diff --git a/integrations/access/jira/client.go b/integrations/access/jira/client.go index 6159211767574..839482d249d0e 100644 --- a/integrations/access/jira/client.go +++ b/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: diff --git a/integrations/access/jira/types.go b/integrations/access/jira/types.go index 5cfc9c1ede315..62cd167029cff 100644 --- a/integrations/access/jira/types.go +++ b/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 { diff --git a/integrations/access/jira/types_test.go b/integrations/access/jira/types_test.go new file mode 100644 index 0000000000000..2e414c33dd11f --- /dev/null +++ b/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 . + */ + +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) + }) + } +}