fix: add information about target response into error message if inte… (#8281)

# Which Problems Are Solved

Execution responses with HTTP StatusCode not equal to 200 interrupt the
client request silently.

# How the Problems Are Solved

Adds information about the recieved StatusCode and Body into the error
if StatusCode not 200.

# Additional Context

Closes #8177

---------

Co-authored-by: Elio Bischof <elio@zitadel.com>
Co-authored-by: Livio Spring <livio.a@gmail.com>
This commit is contained in:
Stefan Benz
2024-08-16 11:26:15 +02:00
committed by GitHub
parent 11d01b9b35
commit 83c78a470c
21 changed files with 400 additions and 6 deletions

View File

@@ -3,12 +3,14 @@ package execution
import (
"bytes"
"context"
"encoding/json"
"io"
"net/http"
"time"
"github.com/zitadel/logging"
zhttp "github.com/zitadel/zitadel/internal/api/http"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/telemetry/tracing"
"github.com/zitadel/zitadel/internal/zerrors"
@@ -114,9 +116,35 @@ func Call(ctx context.Context, url string, timeout time.Duration, body []byte) (
}
defer resp.Body.Close()
return HandleResponse(resp)
}
func HandleResponse(resp *http.Response) ([]byte, error) {
data, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
}
// Check for success between 200 and 299, redirect 300 to 399 is handled by the client, return error with statusCode >= 400
if resp.StatusCode >= 200 && resp.StatusCode <= 299 {
return io.ReadAll(resp.Body)
var errorBody ErrorBody
if err := json.Unmarshal(data, &errorBody); err != nil {
// if json unmarshal fails, body has no ErrorBody information, so will be taken as successful response
return data, nil
}
if errorBody.ForwardedStatusCode != 0 || errorBody.ForwardedErrorMessage != "" {
if errorBody.ForwardedStatusCode >= 400 && errorBody.ForwardedStatusCode < 500 {
return nil, zhttp.HTTPStatusCodeToZitadelError(nil, errorBody.ForwardedStatusCode, "EXEC-reUaUZCzCp", errorBody.ForwardedErrorMessage)
}
return nil, zerrors.ThrowPreconditionFailed(nil, "EXEC-bmhNhpcqpF", errorBody.ForwardedErrorMessage)
}
// no ErrorBody filled in response, so will be taken as successful response
return data, nil
}
return nil, zerrors.ThrowUnknown(nil, "EXEC-dra6yamk98", "Errors.Execution.Failed")
return nil, zerrors.ThrowPreconditionFailed(nil, "EXEC-dra6yamk98", "Errors.Execution.Failed")
}
type ErrorBody struct {
ForwardedStatusCode int `json:"forwardedStatusCode,omitempty"`
ForwardedErrorMessage string `json:"forwardedErrorMessage,omitempty"`
}

View File

@@ -1,7 +1,10 @@
package execution_test
import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
@@ -15,6 +18,7 @@ import (
"github.com/zitadel/zitadel/internal/api/grpc/server/middleware"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/execution"
"github.com/zitadel/zitadel/internal/zerrors"
)
func Test_Call(t *testing.T) {
@@ -513,3 +517,152 @@ var requestContextInfoBody2 = []byte("{\"request\":{\"request\":\"content2\"}}")
type request struct {
Request string `json:"request"`
}
func testErrorBody(code int, message string) []byte {
body := &execution.ErrorBody{ForwardedStatusCode: code, ForwardedErrorMessage: message}
data, _ := json.Marshal(body)
return data
}
func Test_handleResponse(t *testing.T) {
type args struct {
resp *http.Response
}
type res struct {
data []byte
wantErr func(error) bool
}
tests := []struct {
name string
args args
res res
}{
{
"response, statuscode unknown and body",
args{
resp: &http.Response{
StatusCode: 1000,
Body: io.NopCloser(bytes.NewReader([]byte(""))),
},
},
res{
wantErr: func(err error) bool {
return errors.Is(err, zerrors.ThrowPreconditionFailed(nil, "EXEC-dra6yamk98", "Errors.Execution.Failed"))
},
},
},
{
"response, statuscode >= 400 and no body",
args{
resp: &http.Response{
StatusCode: http.StatusForbidden,
Body: io.NopCloser(bytes.NewReader([]byte(""))),
},
},
res{
wantErr: func(err error) bool {
return errors.Is(err, zerrors.ThrowPreconditionFailed(nil, "EXEC-dra6yamk98", "Errors.Execution.Failed"))
},
},
},
{
"response, statuscode >= 400 and body",
args{
resp: &http.Response{
StatusCode: http.StatusForbidden,
Body: io.NopCloser(bytes.NewReader([]byte("body"))),
},
},
res{
wantErr: func(err error) bool {
return errors.Is(err, zerrors.ThrowPreconditionFailed(nil, "EXEC-dra6yamk98", "Errors.Execution.Failed"))
}},
},
{
"response, statuscode = 200 and body",
args{
resp: &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(bytes.NewReader([]byte("body"))),
},
},
res{
data: []byte("body"),
wantErr: nil,
},
},
{
"response, statuscode = 200 no body",
args{
resp: &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(bytes.NewReader([]byte(""))),
},
},
res{
data: []byte(""),
wantErr: nil,
},
},
{
"response, statuscode = 200, error body >= 400 < 500",
args{
resp: &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(bytes.NewReader(testErrorBody(http.StatusForbidden, "forbidden"))),
},
},
res{
wantErr: func(err error) bool {
return errors.Is(err, zerrors.ThrowPermissionDenied(nil, "EXEC-reUaUZCzCp", "forbidden"))
},
},
},
{
"response, statuscode = 200, error body >= 500",
args{
resp: &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(bytes.NewReader(testErrorBody(http.StatusInternalServerError, "internal"))),
},
},
res{
wantErr: func(err error) bool {
return errors.Is(err, zerrors.ThrowPreconditionFailed(nil, "EXEC-bmhNhpcqpF", "internal"))
},
},
},
{
"response, statuscode = 308, no body, should not happen",
args{
resp: &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(bytes.NewReader(testErrorBody(http.StatusPermanentRedirect, "redirect"))),
},
},
res{
wantErr: func(err error) bool {
return errors.Is(err, zerrors.ThrowPreconditionFailed(nil, "EXEC-bmhNhpcqpF", "redirect"))
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
respBody, err := execution.HandleResponse(
tt.args.resp,
)
if tt.res.wantErr == nil {
if !assert.NoError(t, err) {
t.FailNow()
}
} else if !tt.res.wantErr(err) {
t.Errorf("got wrong err: %v", err)
return
}
assert.Equal(t, tt.res.data, respBody)
})
}
}