Merge pull request #45 from willemschots/4-error-handling

4: Improve error handling
willemschots · May 1, 2024 · 605a1f0 · 605a1f0
2 parents 53bd8ac + 4b4caac
commit 605a1f0
Show file tree

Hide file tree

Showing 26 changed files with 677 additions and 333 deletions.
diff --git a/assets/templates/activate-user.html b/assets/templates/activate-user.html
@@ -14,10 +14,12 @@ <h1 class="text-2xl">Activate your acount</h1>
 
     {{ template "flash-messages" . }}
 
+    {{ template "input-errors" . }}
+
     <form action="/user-activations" id="activate-user" method="POST" class="mt-4">
       {{ template "csrf-input" . }}
-      <input type="hidden" name="id" value="{{ .View.ID }}">
-      <input type="hidden" name="token" value="{{ .View.Token }}">
+      <input type="hidden" name="id" value="{{ .Data.ID }}">
+      <input type="hidden" name="token" value="{{ .Data.Token }}">
       <input type="submit" class="btn btn-blue" value="Activate Account">
     </form>
   </div>

diff --git a/assets/templates/base.html b/assets/templates/base.html
@@ -3,12 +3,12 @@
 <head>
   <title>{{ template "title" .}}</title>
 
-  <link rel="stylesheet" href="/static/bundle.css?v={{ .Global.Version }}">
+  <link rel="stylesheet" href="/static/bundle.css?v={{ .Version }}">
 </head>
 <body>
 
   {{ template "body" . }}
 
-  <script defer src="/static/bundle.js?v={{ .Global.Version }}"></script>
+  <script defer src="/static/bundle.js?v={{ .Version }}"></script>
 </body>
 </html>
diff --git a/assets/templates/error.html b/assets/templates/error.html
@@ -0,0 +1,15 @@
+{{ define "title" }}Something went wrong{{end}}
+
+{{define "body"}}
+
+<div class="w-full h-full bg-slate-100 flex flex-wrap justify-center items-start">
+  <div class="w-full">
+    {{ template "header" . }}
+  </div>
+
+  <div class="max-w-[480px] bg-slate-50 rounded-md shadow-md p-8">
+    <h1 class="text-2xl">Something went wrong</h1>
+  </div>
+</div>
+
+{{end}}
diff --git a/assets/templates/forgot-password.html b/assets/templates/forgot-password.html
@@ -14,6 +14,8 @@ <h1 class="text-2xl">Request new password</h1>
 
     {{ template "flash-messages" . }}
 
+    {{ template "input-errors" . }}
+
     <form action="/forgot-password" id="forgot-password" method="POST" class="mt-4">
       {{ template "csrf-input" . }}
       <input type="email" name="email" placeholder="Email" required class="text-input">

diff --git a/assets/templates/login-user.html b/assets/templates/login-user.html
@@ -8,11 +8,13 @@
   </div>
 
   <div class="max-w-[480px] bg-slate-50 rounded-md shadow-md p-8">
-    <h1 class="text-2xl">Login</h1>
+    <h1 class="text-2xl mb-2">Login</h1>
 
     {{ template "flash-messages" . }}
 
-    <form action="/login" id="login-user" method="POST" class="mt-4">
+    {{ template "input-errors" . }}
+
+    <form action="/login" id="login-user" method="POST" class="mt-2">
       {{ template "csrf-input" . }}
 
       <input type="email" name="email" placeholder="Email" required class="text-input">

diff --git a/assets/templates/partials/csrf-input.html b/assets/templates/partials/csrf-input.html
@@ -1,3 +1,3 @@
 {{ define "csrf-input" }}
-<input type="hidden" name="csrfToken" value="{{ .Global.CSRFToken }}"/>
+<input type="hidden" name="csrfToken" value="{{ .CSRFToken }}"/>
 {{ end }}
diff --git a/assets/templates/partials/flash-messages.html b/assets/templates/partials/flash-messages.html
@@ -1,6 +1,6 @@
 {{ define "flash-messages" }}
-  {{ if gt (len .Global.Flashes) 0 }}
-    {{ range .Global.Flashes }}
+  {{ if gt (len .Flashes) 0 }}
+    {{ range .Flashes }}
       {{ . }}
     {{ end }}
   {{ end }}

diff --git a/assets/templates/partials/header.html b/assets/templates/partials/header.html
@@ -3,7 +3,7 @@
 <div class="bg-slate-50 flex justify-between py-2 px-4 shadow-sm">
   <a href="/" class="text-blue-600 font-bold uppercase tracking-wide">Househunt</a>
   <div>
-  {{ if .Global.IsLoggedIn }}
+  {{ if .IsLoggedIn }}
     <form action="/logout" id="logout-user" method="POST">
       {{ template "csrf-input" . }}
       <input type="submit" class="btn btn-text-only" value="Logout">

diff --git a/assets/templates/partials/input-errors.html b/assets/templates/partials/input-errors.html
@@ -0,0 +1,12 @@
+{{ define "input-errors" }}
+  {{ if gt (len .InputErrors) 0 }}
+  <div class="bg-red-100 px-2 py-1 text-red-700 rounded-md">
+    <p class="text-sm">Errors:</p>
+    <ul class="pl-4 rounded-md list-disc">
+    {{ range .InputErrors }}
+      <li>{{ . }}</li>
+    {{ end }}
+    </ul>
+  </div>
+  {{ end }}
+{{ end }}
diff --git a/assets/templates/register-user.html b/assets/templates/register-user.html
@@ -12,6 +12,8 @@ <h1 class="text-2xl">Register an account</h1>
 
     {{ template "flash-messages" . }}
 
+    {{ template "input-errors" . }}
+
     <form action="/register" id="register-user" method="POST" class="mt-4">
       {{ template "csrf-input" . }}
       <input type="email" name="email" placeholder="Email" required class="text-input">

diff --git a/assets/templates/reset-password.html b/assets/templates/reset-password.html
@@ -12,10 +12,12 @@ <h1 class="text-2xl">Reset your password</h1>
 
     {{ template "flash-messages" . }}
 
+    {{ template "input-errors" . }}
+
     <form action="/password-resets" id="reset-password" method="POST" class="mt-4">
       {{ template "csrf-input" . }}
-      <input type="hidden" name="rawtoken.id" value="{{ .View.ID }}">
-      <input type="hidden" name="rawtoken.token" value="{{ .View.Token }}">
+      <input type="hidden" name="rawtoken.id" value="{{ .Data.ID }}">
+      <input type="hidden" name="rawtoken.token" value="{{ .Data.Token }}">
       <input type="password" name="password" required class="text-input">
       <input type="submit" class="btn btn-blue mt-4" value="Reset password">
     </form>

diff --git a/cmd/server/main.go b/cmd/server/main.go
@@ -15,7 +15,7 @@ import (
 	"time"
 
 	"github.com/google/uuid"
-	"github.com/gorilla/sessions"
+	gorillaSess "github.com/gorilla/sessions"
 	"github.com/willemschots/househunt/assets"
 	"github.com/willemschots/househunt/internal"
 	"github.com/willemschots/househunt/internal/auth"
@@ -27,6 +27,7 @@ import (
 	emailview "github.com/willemschots/househunt/internal/email/view"
 	"github.com/willemschots/househunt/internal/krypto"
 	"github.com/willemschots/househunt/internal/web"
+	"github.com/willemschots/househunt/internal/web/sessions"
 	"github.com/willemschots/househunt/internal/web/view"
 	"github.com/willemschots/househunt/migrations"
 	"golang.org/x/sync/errgroup"
@@ -136,7 +137,7 @@ func run(ctx context.Context, w io.Writer) int {
 	for i, key := range cfg.http.cookieKeys {
 		keysAsBytes[i] = key.SecretValue()
 	}
-	sessionStore := sessions.NewCookieStore(keysAsBytes...)
+	sessionStore := gorillaSess.NewCookieStore(keysAsBytes...)
 	sessionStore.Options.Secure = cfg.http.server.SecureCookie
 	sessionStore.Options.HttpOnly = true
 	sessionStore.MaxAge(7 * 24 * 60 * 60) // 1 week
@@ -161,7 +162,7 @@ func run(ctx context.Context, w io.Writer) int {
 		Logger:       logger,
 		ViewRenderer: viewRenderer,
 		AuthService:  authSvc,
-		SessionStore: sessionStore,
+		SessionStore: sessions.NewStore(sessionStore),
 		DistFS:       http.FS(assets.DistFS),
 	}
 

diff --git a/cmd/server/user_stories_test.go b/cmd/server/user_stories_test.go
@@ -25,6 +25,19 @@ func Test_UserStories(t *testing.T) {
 
 		c := newClient(t)
 
+		t.Run("prevent mistakes when registering a new account", func(t *testing.T) {
+			// first view the form.
+			body := c.mustGetBody(t, "/register", assertStatusCode(t, http.StatusOK))
+
+			form := parseHTMLFormWithID(t, strings.NewReader(body), "register-user")
+
+			// then submit it with invalid values.
+			form.values.Set("email", "") // empty email
+			form.values.Set("password", "reallyStrongPassword1")
+
+			c.mustSubmitForm(t, form, assertStatusCode(t, http.StatusBadRequest))
+		})
+
 		t.Run("register a new account", func(t *testing.T) {
 			// first view the form.
 			body := c.mustGetBody(t, "/register", assertStatusCode(t, http.StatusOK))
@@ -68,6 +81,19 @@ func Test_UserStories(t *testing.T) {
 			c.mustGetBody(t, "/dashboard", assertStatusCode(t, http.StatusNotFound))
 		})
 
+		t.Run("prevent mistakes when logging into my account", func(t *testing.T) {
+			// first view the login form.
+			body := c.mustGetBody(t, "/login", assertStatusCode(t, http.StatusOK))
+
+			form := parseHTMLFormWithID(t, strings.NewReader(body), "login-user")
+
+			// then submit it with invalid values.
+			form.values.Set("email", "") // empty email.
+			form.values.Set("password", "reallyStrongPassword1")
+
+			c.mustSubmitForm(t, form, assertStatusCode(t, http.StatusBadRequest))
+		})
+
 		t.Run("login to my now active account", func(t *testing.T) {
 			// first view the login form.
 			body := c.mustGetBody(t, "/login", assertStatusCode(t, http.StatusOK))
@@ -111,6 +137,18 @@ func Test_UserStories(t *testing.T) {
 			c.mustGetBody(t, "/dashboard", assertStatusCode(t, http.StatusNotFound))
 		})
 
+		t.Run("prevent mistakes when requesting a new password", func(t *testing.T) {
+			// first view the form.
+			body := c.mustGetBody(t, "/forgot-password", assertStatusCode(t, http.StatusOK))
+
+			form := parseHTMLFormWithID(t, strings.NewReader(body), "forgot-password")
+
+			// then submit it with invalid values.
+			form.values.Set("email", "") // empty email.
+
+			c.mustSubmitForm(t, form, assertStatusCode(t, http.StatusBadRequest))
+		})
+
 		t.Run("request a new password because I forgot my old one", func(t *testing.T) {
 			// first view the form.
 			body := c.mustGetBody(t, "/forgot-password", assertStatusCode(t, http.StatusOK))
@@ -135,6 +173,18 @@ func Test_UserStories(t *testing.T) {
 			passwordResetURL = waitAndCaptureURL(t, logs, "agent@example.com", "/password-resets")
 		})
 
+		t.Run("prevent mistakes when resetting my password", func(t *testing.T) {
+			// first view the form
+			body := c.mustGetBody(t, passwordResetURL.String(), assertStatusCode(t, http.StatusOK))
+
+			form := parseHTMLFormWithID(t, strings.NewReader(body), "reset-password")
+
+			// then submit it with invalid values.
+			form.values.Set("password", "") // empty password.
+
+			c.mustSubmitForm(t, form, assertStatusCode(t, http.StatusBadRequest))
+		})
+
 		t.Run("reset my password", func(t *testing.T) {
 			// first view the form
 			body := c.mustGetBody(t, passwordResetURL.String(), assertStatusCode(t, http.StatusOK))

diff --git a/internal/auth/service.go b/internal/auth/service.go
@@ -268,12 +268,12 @@ func (s *Service) Authenticate(ctx context.Context, c Credentials) (User, error)
 		// Even if no user is found we compare to a hash to prevent timing differences
 		// that could result in user enumeration attacks.
 		_ = c.Password.Match(s.comparisonHash)
-		return User{}, ErrInvalidCredentials
+		return User{}, errorz.InvalidInput{ErrInvalidCredentials}
 	}
 
 	match := c.Password.Match(users[0].PasswordHash)
 	if !match {
-		return User{}, ErrInvalidCredentials
+		return User{}, errorz.InvalidInput{ErrInvalidCredentials}
 	}
 
 	return users[0], nil

diff --git a/internal/auth/service_test.go b/internal/auth/service_test.go
@@ -317,8 +317,14 @@ func Test_Service_Authenticate(t *testing.T) {
 		credentials.Password = must(auth.ParsePassword("wrongPassword"))
 
 		_, err := st.svc.Authenticate(context.Background(), credentials)
-		if !errors.Is(err, auth.ErrInvalidCredentials) {
-			t.Fatalf("expected error %v, got %v (via errors.Is)", auth.ErrInvalidCredentials, err)
+
+		var invalidInput errorz.InvalidInput
+		if !errors.As(err, &invalidInput) {
+			t.Fatalf("expected error to be of type %T, got %T (via errors.As)", invalidInput, err)
+		}
+
+		if !errors.Is(invalidInput, auth.ErrInvalidCredentials) {
+			t.Fatalf("expected error %v, got %v (via errors.Is)", auth.ErrInvalidCredentials, invalidInput)
 		}
 
 		// assert no async errors were reported.
@@ -334,8 +340,14 @@ func Test_Service_Authenticate(t *testing.T) {
 		credentials.Email = must(email.ParseAddress("jacob@example.com"))
 
 		_, err := st.svc.Authenticate(context.Background(), credentials)
-		if !errors.Is(err, auth.ErrInvalidCredentials) {
-			t.Fatalf("expected error %v, got %v (via errors.Is)", auth.ErrInvalidCredentials, err)
+
+		var invalidInput errorz.InvalidInput
+		if !errors.As(err, &invalidInput) {
+			t.Fatalf("expected error to be of type %T, got %T (via errors.As)", invalidInput, err)
+		}
+
+		if !errors.Is(invalidInput, auth.ErrInvalidCredentials) {
+			t.Fatalf("expected error %v, got %v (via errors.Is)", auth.ErrInvalidCredentials, invalidInput)
 		}
 
 		// assert no async errors were reported.
@@ -348,8 +360,14 @@ func Test_Service_Authenticate(t *testing.T) {
 		credentials, _ := st.registerUser()
 
 		_, err := st.svc.Authenticate(context.Background(), credentials)
-		if !errors.Is(err, auth.ErrInvalidCredentials) {
-			t.Fatalf("expected error %v, got %v (via errors.Is)", auth.ErrInvalidCredentials, err)
+
+		var invalidInput errorz.InvalidInput
+		if !errors.As(err, &invalidInput) {
+			t.Fatalf("expected error to be of type %T, got %T (via errors.As)", invalidInput, err)
+		}
+
+		if !errors.Is(invalidInput, auth.ErrInvalidCredentials) {
+			t.Fatalf("expected error %v, got %v (via errors.Is)", auth.ErrInvalidCredentials, invalidInput)
 		}
 
 		// assert no async errors were reported.

diff --git a/internal/email/address.go b/internal/email/address.go
@@ -33,3 +33,14 @@ func ParseAddress(raw string) (Address, error) {
 
 	return Address(addr.Address), nil
 }
+
+func (a *Address) UnmarshalText(text []byte) error {
+	addr, err := ParseAddress(string(text))
+	if err != nil {
+		return err
+	}
+
+	*a = addr
+
+	return nil
+}
diff --git a/internal/errorz/invalid_input.go b/internal/errorz/invalid_input.go
@@ -0,0 +1,20 @@
+package errorz
+
+import "strings"
+
+// InvalidInput signals that a provided input is invalid due to the wrapped errors.
+type InvalidInput []error
+
+func (e InvalidInput) Error() string {
+	var b strings.Builder
+	b.WriteString("invalid input:\n")
+	for _, err := range e {
+		b.WriteString(err.Error())
+		b.WriteString("\n")
+	}
+	return b.String()
+}
+
+func (e InvalidInput) Unwrap() []error {
+	return e
+}
diff --git a/internal/errorz/keyed.go b/internal/errorz/keyed.go
@@ -0,0 +1,14 @@
+package errorz
+
+type Keyed struct {
+	Key string
+	Err error
+}
+
+func (k Keyed) Error() string {
+	return k.Key + ": " + k.Err.Error()
+}
+
+func (k Keyed) Unwrap() error {
+	return k.Err
+}