[MISC] Fail with an error message when X-Forwarded-* headers are missing (#631)

* Fail with an error message when X-Forwarded-* headers are missing.

* Remove useless comments.

internal/handlers/errors.go

@@ -9,5 +9,5 @@ const InternalError = "Internal error."
 // UnauthorizedError is the error message sent when the user is not authorized.
 const UnauthorizedError = "You're not authorized."
 
-var errMissingXForwardedHost = errors.New("Missing header X-Fowarded-Host used to detect target URL")
-var errMissingXForwardedProto = errors.New("Missing header X-Fowarded-Proto used to detect target URL")
+var errMissingXForwardedHost = errors.New("Missing header X-Fowarded-Host")
+var errMissingXForwardedProto = errors.New("Missing header X-Fowarded-Proto")

internal/handlers/handler_register_u2f_step1.go

@@ -24,6 +24,16 @@ var SecondFactorU2FIdentityStart = middlewares.IdentityVerificationStart(middlew
 })
 
 func secondFactorU2FIdentityFinish(ctx *middlewares.AutheliaCtx, username string) {
+	if ctx.XForwardedProto() == nil {
+		ctx.Error(errMissingXForwardedProto, operationFailedMessage)
+		return
+	}
+
+	if ctx.XForwardedHost() == nil {
+		ctx.Error(errMissingXForwardedHost, operationFailedMessage)
+		return
+	}
+
 	appID := fmt.Sprintf("%s://%s", ctx.XForwardedProto(), ctx.XForwardedHost())
 	ctx.Logger.Tracef("U2F appID is %s", appID)
 	var trustedFacets = []string{appID}

internal/handlers/handler_register_u2f_step1_test.go

@@ -0,0 +1,96 @@
+package handlers
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/authelia/authelia/internal/middlewares"
+	"github.com/authelia/authelia/internal/mocks"
+	"github.com/dgrijalva/jwt-go"
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/suite"
+)
+
+type HandlerRegisterU2FStep1Suite struct {
+	suite.Suite
+
+	mock *mocks.MockAutheliaCtx
+}
+
+func (s *HandlerRegisterU2FStep1Suite) SetupTest() {
+	s.mock = mocks.NewMockAutheliaCtx(s.T())
+
+	userSession := s.mock.Ctx.GetSession()
+	userSession.Username = "john"
+	s.mock.Ctx.SaveSession(userSession)
+}
+
+func (s *HandlerRegisterU2FStep1Suite) TearDownTest() {
+	s.mock.Close()
+}
+
+func createToken(secret string, username string, action string, expiresAt time.Time) string {
+	claims := &middlewares.IdentityVerificationClaim{
+		StandardClaims: jwt.StandardClaims{
+			ExpiresAt: expiresAt.Unix(),
+			Issuer:    "Authelia",
+		},
+		Action:   action,
+		Username: username,
+	}
+	token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
+	ss, _ := token.SignedString([]byte(secret))
+	return ss
+}
+
+func newFinishArgs() middlewares.IdentityVerificationFinishArgs {
+	return middlewares.IdentityVerificationFinishArgs{
+		ActionClaim:          U2FRegistrationAction,
+		IsTokenUserValidFunc: func(ctx *middlewares.AutheliaCtx, username string) bool { return true },
+	}
+}
+
+func (s *HandlerRegisterU2FStep1Suite) TestShouldRaiseWhenXForwardedProtoIsMissing() {
+	token := createToken(s.mock.Ctx.Configuration.JWTSecret, "john", U2FRegistrationAction,
+		time.Now().Add(1*time.Minute))
+	s.mock.Ctx.Request.SetBodyString(fmt.Sprintf("{\"token\":\"%s\"}", token))
+
+	s.mock.StorageProviderMock.EXPECT().
+		FindIdentityVerificationToken(gomock.Eq(token)).
+		Return(true, nil)
+
+	s.mock.StorageProviderMock.EXPECT().
+		RemoveIdentityVerificationToken(gomock.Eq(token)).
+		Return(nil)
+
+	SecondFactorU2FIdentityFinish(s.mock.Ctx)
+
+	assert.Equal(s.T(), 200, s.mock.Ctx.Response.StatusCode())
+	assert.Equal(s.T(), "Missing header X-Fowarded-Proto", s.mock.Hook.LastEntry().Message)
+}
+
+func (s *HandlerRegisterU2FStep1Suite) TestShouldRaiseWhenXForwardedHostIsMissing() {
+	s.mock.Ctx.Request.Header.Add("X-Forwarded-Proto", "http")
+	token := createToken(s.mock.Ctx.Configuration.JWTSecret, "john", U2FRegistrationAction,
+		time.Now().Add(1*time.Minute))
+	s.mock.Ctx.Request.SetBodyString(fmt.Sprintf("{\"token\":\"%s\"}", token))
+
+	s.mock.StorageProviderMock.EXPECT().
+		FindIdentityVerificationToken(gomock.Eq(token)).
+		Return(true, nil)
+
+	s.mock.StorageProviderMock.EXPECT().
+		RemoveIdentityVerificationToken(gomock.Eq(token)).
+		Return(nil)
+
+	SecondFactorU2FIdentityFinish(s.mock.Ctx)
+
+	assert.Equal(s.T(), 200, s.mock.Ctx.Response.StatusCode())
+	assert.Equal(s.T(), "Missing header X-Fowarded-Host", s.mock.Hook.LastEntry().Message)
+}
+
+func TestShouldRunHandlerRegisterU2FStep1Suite(t *testing.T) {
+	suite.Run(t, new(HandlerRegisterU2FStep1Suite))
+}

internal/handlers/handler_sign_u2f_step1.go

@@ -12,7 +12,15 @@ import (
 
 // SecondFactorU2FSignGet handler for initiating a signing request.
 func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) {
-	userSession := ctx.GetSession()
+	if ctx.XForwardedProto() == nil {
+		ctx.Error(errMissingXForwardedProto, operationFailedMessage)
+		return
+	}
+
+	if ctx.XForwardedHost() == nil {
+		ctx.Error(errMissingXForwardedHost, operationFailedMessage)
+		return
+	}
 
 	appID := fmt.Sprintf("%s://%s", ctx.XForwardedProto(), ctx.XForwardedHost())
 	var trustedFacets = []string{appID}
@@ -23,6 +31,7 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) {
 		return
 	}
 
+	userSession := ctx.GetSession()
 	keyHandleBytes, publicKeyBytes, err := ctx.Providers.StorageProvider.LoadU2FDeviceHandle(userSession.Username)
 
 	if err != nil {

internal/handlers/handler_sign_u2f_step1_test.go

@@ -0,0 +1,42 @@
+package handlers
+
+import (
+	"testing"
+
+	"github.com/authelia/authelia/internal/mocks"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/suite"
+)
+
+type HandlerSignU2FStep1Suite struct {
+	suite.Suite
+
+	mock *mocks.MockAutheliaCtx
+}
+
+func (s *HandlerSignU2FStep1Suite) SetupTest() {
+	s.mock = mocks.NewMockAutheliaCtx(s.T())
+}
+
+func (s *HandlerSignU2FStep1Suite) TearDownTest() {
+	s.mock.Close()
+}
+
+func (s *HandlerSignU2FStep1Suite) TestShouldRaiseWhenXForwardedProtoIsMissing() {
+	SecondFactorU2FSignGet(s.mock.Ctx)
+
+	assert.Equal(s.T(), 200, s.mock.Ctx.Response.StatusCode())
+	assert.Equal(s.T(), "Missing header X-Fowarded-Proto", s.mock.Hook.LastEntry().Message)
+}
+
+func (s *HandlerSignU2FStep1Suite) TestShouldRaiseWhenXForwardedHostIsMissing() {
+	s.mock.Ctx.Request.Header.Add("X-Forwarded-Proto", "http")
+	SecondFactorU2FSignGet(s.mock.Ctx)
+
+	assert.Equal(s.T(), 200, s.mock.Ctx.Response.StatusCode())
+	assert.Equal(s.T(), "Missing header X-Fowarded-Host", s.mock.Hook.LastEntry().Message)
+}
+
+func TestShouldRunHandlerSignU2FStep1Suite(t *testing.T) {
+	suite.Run(t, new(HandlerSignU2FStep1Suite))
+}

internal/handlers/handler_verify_test.go

@@ -72,7 +72,7 @@ func TestShouldRaiseWhenNoHeaderProvidedToDetectTargetURL(t *testing.T) {
 	defer mock.Close()
 	_, err := getOriginalURL(mock.Ctx)
 	assert.Error(t, err)
-	assert.Equal(t, "Missing header X-Fowarded-Proto used to detect target URL", err.Error())
+	assert.Equal(t, "Missing header X-Fowarded-Proto", err.Error())
 }
 
 func TestShouldRaiseWhenNoXForwardedHostHeaderProvidedToDetectTargetURL(t *testing.T) {
@@ -82,7 +82,7 @@ func TestShouldRaiseWhenNoXForwardedHostHeaderProvidedToDetectTargetURL(t *testi
 	mock.Ctx.Request.Header.Set("X-Forwarded-Proto", "https")
 	_, err := getOriginalURL(mock.Ctx)
 	assert.Error(t, err)
-	assert.Equal(t, "Missing header X-Fowarded-Host used to detect target URL", err.Error())
+	assert.Equal(t, "Missing header X-Fowarded-Host", err.Error())
 }
 
 func TestShouldRaiseWhenXForwardedProtoIsNotParseable(t *testing.T) {

internal/middlewares/errors.go

@@ -1,8 +1,13 @@
 package middlewares
 
+import "errors"
+
 // InternalError is the error message sent when there was an internal error but it should
 // be hidden to the end user. In that case the error should be in the server logs.
 const InternalError = "Internal error."
 
 // UnauthorizedError is the error message sent when the user is not authorized.
 const UnauthorizedError = "You're not authorized."
+
+var errMissingXForwardedHost = errors.New("Missing header X-Fowarded-Host")
+var errMissingXForwardedProto = errors.New("Missing header X-Fowarded-Proto")

internal/middlewares/identity_verification.go

@@ -49,6 +49,16 @@ func IdentityVerificationStart(args IdentityVerificationStartArgs) RequestHandle
 			return
 		}
 
+		if ctx.XForwardedProto() == nil {
+			ctx.Error(errMissingXForwardedProto, operationFailedMessage)
+			return
+		}
+
+		if ctx.XForwardedHost() == nil {
+			ctx.Error(errMissingXForwardedHost, operationFailedMessage)
+			return
+		}
+
 		link := fmt.Sprintf("%s://%s%s?token=%s", ctx.XForwardedProto(),
 			ctx.XForwardedHost(), args.TargetEndpoint, ss)
 

internal/middlewares/identity_verification_test.go

@@ -42,7 +42,6 @@ func TestShouldFailStartingProcessIfUserHasNoEmailAddress(t *testing.T) {
 
 	middlewares.IdentityVerificationStart(newArgs(retriever))(mock.Ctx)
 
-	// Return 200 KO
 	assert.Equal(t, 200, mock.Ctx.Response.StatusCode())
 	assert.Equal(t, "User does not have any email", mock.Hook.LastEntry().Message)
 }
@@ -60,7 +59,6 @@ func TestShouldFailIfJWTCannotBeSaved(t *testing.T) {
 	args := newArgs(defaultRetriever)
 	middlewares.IdentityVerificationStart(args)(mock.Ctx)
 
-	// Return 200 KO
 	assert.Equal(t, 200, mock.Ctx.Response.StatusCode())
 	assert.Equal(t, "cannot save", mock.Hook.LastEntry().Message)
 }
@@ -70,6 +68,8 @@ func TestShouldFailSendingAnEmail(t *testing.T) {
 	defer mock.Close()
 
 	mock.Ctx.Configuration.JWTSecret = "abc"
+	mock.Ctx.Request.Header.Add("X-Forwarded-Proto", "http")
+	mock.Ctx.Request.Header.Add("X-Forwarded-Host", "host")
 
 	mock.StorageProviderMock.EXPECT().
 		SaveIdentityVerificationToken(gomock.Any()).
@@ -82,16 +82,53 @@ func TestShouldFailSendingAnEmail(t *testing.T) {
 	args := newArgs(defaultRetriever)
 	middlewares.IdentityVerificationStart(args)(mock.Ctx)
 
-	// Return 200 KO
 	assert.Equal(t, 200, mock.Ctx.Response.StatusCode())
 	assert.Equal(t, "no notif", mock.Hook.LastEntry().Message)
 }
 
+func TestShouldFailWhenXForwardedProtoHeaderIsMissing(t *testing.T) {
+	mock := mocks.NewMockAutheliaCtx(t)
+	defer mock.Close()
+
+	mock.Ctx.Configuration.JWTSecret = "abc"
+	mock.Ctx.Request.Header.Add("X-Forwarded-Host", "host")
+
+	mock.StorageProviderMock.EXPECT().
+		SaveIdentityVerificationToken(gomock.Any()).
+		Return(nil)
+
+	args := newArgs(defaultRetriever)
+	middlewares.IdentityVerificationStart(args)(mock.Ctx)
+
+	assert.Equal(t, 200, mock.Ctx.Response.StatusCode())
+	assert.Equal(t, "Missing header X-Fowarded-Proto", mock.Hook.LastEntry().Message)
+}
+
+func TestShouldFailWhenXForwardedHostHeaderIsMissing(t *testing.T) {
+	mock := mocks.NewMockAutheliaCtx(t)
+	defer mock.Close()
+
+	mock.Ctx.Configuration.JWTSecret = "abc"
+	mock.Ctx.Request.Header.Add("X-Forwarded-Proto", "http")
+
+	mock.StorageProviderMock.EXPECT().
+		SaveIdentityVerificationToken(gomock.Any()).
+		Return(nil)
+
+	args := newArgs(defaultRetriever)
+	middlewares.IdentityVerificationStart(args)(mock.Ctx)
+
+	assert.Equal(t, 200, mock.Ctx.Response.StatusCode())
+	assert.Equal(t, "Missing header X-Fowarded-Host", mock.Hook.LastEntry().Message)
+}
+
 func TestShouldSucceedIdentityVerificationStartProcess(t *testing.T) {
 	mock := mocks.NewMockAutheliaCtx(t)
 	defer mock.Close()
 
 	mock.Ctx.Configuration.JWTSecret = "abc"
+	mock.Ctx.Request.Header.Add("X-Forwarded-Proto", "http")
+	mock.Ctx.Request.Header.Add("X-Forwarded-Host", "host")
 
 	mock.StorageProviderMock.EXPECT().
 		SaveIdentityVerificationToken(gomock.Any()).