From c8f18956a24ef1d7c67a1b2b8d58b711fcd4d210 Mon Sep 17 00:00:00 2001
From: ale <ale@incal.net>
Date: Sun, 15 Dec 2019 21:48:24 +0000
Subject: [PATCH] Improve error message when the 2FA constraints are not met

---
 server/login/login.go | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/server/login/login.go b/server/login/login.go
index 17872c3..d936618 100644
--- a/server/login/login.go
+++ b/server/login/login.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/gob"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log"
 	"net/http"
@@ -68,9 +69,25 @@ func (l *loginSession) Reset() {
 	// Keep Redir.
 }
 
-func (l *loginSession) Can2FA(method auth.TFAMethod) bool {
-	return (l.Username != "" && l.Password != "" && l.AuthResponse != nil &&
-		l.AuthResponse.Has2FAMethod(method))
+// This method is needlessly detailed, but the error message is useful in debugging.
+//
+// A boolean version could simply be:
+//
+//    return (l.Username != "" && l.Password != "" && l.AuthResponse != nil &&
+//            l.AuthResponse.Has2FAMethod(method))
+//
+func (l *loginSession) Can2FA(method auth.TFAMethod) error {
+	switch {
+	case l.Username == "":
+		return errors.New("empty username")
+	case l.Password == "":
+		return errors.New("empty password")
+	case l.AuthResponse == nil:
+		return errors.New("empty auth response")
+	case !l.AuthResponse.Has2FAMethod(method):
+		return errors.New("unsupported 2fa method")
+	}
+	return nil
 }
 
 func init() {
@@ -301,8 +318,8 @@ func (l *Login) handleLoginOTP(w http.ResponseWriter, req *http.Request, sess *l
 	}
 
 	// First verify that we are ready to do 2FA.
-	if !sess.Can2FA(auth.TFAMethodOTP) {
-		log.Printf("got invalid 2FA request")
+	if err := sess.Can2FA(auth.TFAMethodOTP); err != nil {
+		log.Printf("got invalid 2FA request (%v)", err)
 		http.Redirect(w, req, l.urlFor("/login"), http.StatusFound)
 		return
 	}
@@ -326,7 +343,7 @@ func (l *Login) handleLoginOTP(w http.ResponseWriter, req *http.Request, sess *l
 		}
 		env["Error"] = true
 		sess.Failures++
-		if sess.Failures > maxFailures {
+		if sess.Failures >= maxFailures {
 			log.Printf("too many login failures for %s, starting over", sess.Username)
 			http.Redirect(w, req, l.urlFor("/login"), http.StatusFound)
 			return
@@ -343,8 +360,8 @@ func (l *Login) handleLoginU2F(w http.ResponseWriter, req *http.Request, sess *l
 	}
 
 	// First verify that we are ready to do 2FA.
-	if !sess.Can2FA(auth.TFAMethodU2F) {
-		log.Printf("got invalid 2FA request")
+	if err := sess.Can2FA(auth.TFAMethodU2F); err != nil {
+		log.Printf("got invalid 2FA request (%v)", err)
 		http.Redirect(w, req, l.urlFor("/login"), http.StatusFound)
 		return
 	}
@@ -376,7 +393,7 @@ func (l *Login) handleLoginU2F(w http.ResponseWriter, req *http.Request, sess *l
 		}
 		env["Error"] = true
 		sess.Failures++
-		if sess.Failures > maxFailures {
+		if sess.Failures >= maxFailures {
 			log.Printf("too many login failures for %s, starting over", sess.Username)
 			http.Redirect(w, req, l.urlFor("/login"), http.StatusFound)
 			return
-- 
GitLab