fix tests and be more consistent with the returned status code

This commit is contained in:
Tim Möhlmann 2024-01-17 11:15:35 +02:00
parent c22649b20b
commit cf5a6e94c1
3 changed files with 31 additions and 14 deletions

View file

@ -157,16 +157,29 @@ func (e StatusError) Is(err error) bool {
e.statusCode == target.statusCode e.statusCode == target.statusCode
} }
// WriteError asserts for a StatusError containing an [oidc.Error]. // WriteError asserts for a [StatusError] containing an [oidc.Error].
// If no StatusError is found, the status code will default to [http.StatusBadRequest]. // If no `StatusError` is found, the status code will default to [http.StatusBadRequest].
// If no [oidc.Error] was found in the parent, the error type defaults to [oidc.ServerError]. // If no `oidc.Error` was found in the parent, the error type defaults to [oidc.ServerError].
// When the final oidc Error is a server error, the status code is adjusted to [http.StatusInternalServerError]. // When there was no `StatusError` and the `oidc.Error` is of type `oidc.ServerError`,
// the status code will be set to [http.StatusInternalServerError]
func WriteError(w http.ResponseWriter, r *http.Request, err error, logger *slog.Logger) { func WriteError(w http.ResponseWriter, r *http.Request, err error, logger *slog.Logger) {
statusError := AsStatusError(err, http.StatusBadRequest) var statusError StatusError
e := oidc.DefaultToServerError(statusError.parent, statusError.parent.Error()) if errors.As(err, &statusError) {
if e.ErrorType == oidc.ServerError { writeError(w, r,
statusError.statusCode = http.StatusInternalServerError oidc.DefaultToServerError(statusError.parent, statusError.parent.Error()),
statusError.statusCode, logger,
)
return
} }
logger.Log(r.Context(), e.LogLevel(), "request error", "oidc_error", e, "status_code", statusError.statusCode) statusCode := http.StatusBadRequest
httphelper.MarshalJSONWithStatus(w, e, statusError.statusCode) e := oidc.DefaultToServerError(err, err.Error())
if e.ErrorType == oidc.ServerError {
statusCode = http.StatusInternalServerError
}
writeError(w, r, e, statusCode, logger)
}
func writeError(w http.ResponseWriter, r *http.Request, err *oidc.Error, statusCode int, logger *slog.Logger) {
logger.Log(r.Context(), err.LogLevel(), "request error", "oidc_error", err, "status_code", statusCode)
httphelper.MarshalJSONWithStatus(w, err, statusCode)
} }

View file

@ -579,7 +579,7 @@ func TestWriteError(t *testing.T) {
{ {
name: "not a status or oidc error", name: "not a status or oidc error",
err: io.ErrClosedPipe, err: io.ErrClosedPipe,
wantStatus: http.StatusBadRequest, wantStatus: http.StatusInternalServerError,
wantBody: `{ wantBody: `{
"error":"server_error", "error":"server_error",
"error_description":"io: read/write on closed pipe" "error_description":"io: read/write on closed pipe"
@ -592,6 +592,7 @@ func TestWriteError(t *testing.T) {
"parent":"io: read/write on closed pipe", "parent":"io: read/write on closed pipe",
"type":"server_error" "type":"server_error"
}, },
"status_code":500,
"time":"not" "time":"not"
}`, }`,
}, },
@ -611,6 +612,7 @@ func TestWriteError(t *testing.T) {
"parent":"io: read/write on closed pipe", "parent":"io: read/write on closed pipe",
"type":"server_error" "type":"server_error"
}, },
"status_code":500,
"time":"not" "time":"not"
}`, }`,
}, },
@ -629,6 +631,7 @@ func TestWriteError(t *testing.T) {
"description":"oops", "description":"oops",
"type":"invalid_request" "type":"invalid_request"
}, },
"status_code":400,
"time":"not" "time":"not"
}`, }`,
}, },
@ -650,6 +653,7 @@ func TestWriteError(t *testing.T) {
"description":"oops", "description":"oops",
"type":"unauthorized_client" "type":"unauthorized_client"
}, },
"status_code":401,
"time":"not" "time":"not"
}`, }`,
}, },

View file

@ -365,14 +365,14 @@ func Test_webServer_authorizeHandler(t *testing.T) {
}, },
}, },
{ {
name: "authorize error", name: "server error",
fields: fields{ fields: fields{
server: &requestVerifier{}, server: &requestVerifier{},
decoder: testDecoder, decoder: testDecoder,
}, },
r: httptest.NewRequest(http.MethodPost, "/authorize", strings.NewReader("foo=bar")), r: httptest.NewRequest(http.MethodPost, "/authorize", strings.NewReader("foo=bar")),
want: webServerResult{ want: webServerResult{
wantStatus: http.StatusBadRequest, wantStatus: http.StatusInternalServerError,
wantBody: `{"error":"server_error"}`, wantBody: `{"error":"server_error"}`,
}, },
}, },
@ -1237,7 +1237,7 @@ func Test_webServer_simpleHandler(t *testing.T) {
}, },
r: httptest.NewRequest(http.MethodGet, "/", bytes.NewReader(make([]byte, 11<<20))), r: httptest.NewRequest(http.MethodGet, "/", bytes.NewReader(make([]byte, 11<<20))),
want: webServerResult{ want: webServerResult{
wantStatus: http.StatusBadRequest, wantStatus: http.StatusInternalServerError,
wantBody: `{"error":"server_error", "error_description":"io: read/write on closed pipe"}`, wantBody: `{"error":"server_error", "error_description":"io: read/write on closed pipe"}`,
}, },
}, },