BasicAuth middleware returns 500 InternalServerError on invalid base64 strings

[stevenwhitehead]

Issue Description

When using the provided BasicAuth middleware handler, it returns 500 InternalServerError when the authorization string is not base64. This is just bad input, so I expected to get a 401 instead of an unexpected 500.

Didn't do a deep dive testing, but I assume this is because the middleware code just returns the base64 decode error. I can do a PR if we want to just change it to a 401.

Checklist

• Dependencies installed
• No typos
• Searched existing issues and docs

Expected behaviour

I expected 401 Not Authorized

Actual behaviour

500 InternalServerError

Steps to reproduce

Compile the code below, run the executable, and then execute the following commands. Notice the second curl has a slightly modified auth string.

$ echo "username:password" | base64
dXNlcm5hbWU6cGFzc3dvcmQK
$ curl localhost:1234 -H "Authorization: Basic dXNlcm5hbWU6cGFzc3dvcmQK"
all good%
$ curl localhost:1234 -H "Authorization: Basic dXNlcm5hbWU6cGFzc3dvcmQ"
{"message":"Internal Server Error"}

Working code to debug

package main

import (
	"net/http"

	echo "github.com/labstack/echo/v4"
	"github.com/labstack/echo/v4/middleware"
)

func main() {
	e := echo.New()
	e.Use(
		middleware.BasicAuth(
			func(username, password string, ctx echo.Context) (bool, error) {
				return true, nil
			},
		),
	)
	e.GET("/", func(ctx echo.Context) error {
		return ctx.String(http.StatusOK, "all good")
	})
	e.Start("localhost:1234")
}

Version/commit

github.com/labstack/echo/v4 v4.7.2
go version go1.18.1 darwin/amd64

[wellingguzman]

I bumped into the same issue, I created a PR and see if it gets merged. #2191

I think we shouldn't treat the invalid string as error, but invalid client input.

[aldas]

I think returning 400 is probably better than 401.

if err != nil {
	return echo.NewHTTPError(http.StatusBadRequest).SetInternal(err)
}

[wellingguzman]

@aldas you are right, it actually makes more sense, if we want it to treat it as a client invalid input, 400 is a better choice.

[stevenwhitehead]

@aldas @wellingguzman thanks everyone! I'm good with either status code.