From efdd560ee4ebf68801f12484019cb86f35f8abe4 Mon Sep 17 00:00:00 2001
From: Daniel <daniel@binaryparadox.net>
Date: Tue, 27 Feb 2018 19:20:35 -0500
Subject: [PATCH] API: Differentiate bad TXT update error.

Previous to this commit, if the update message had a valid subdomain but
an invalid TXT the error returned was for a bad subdomain. This can
confuse developers who were POSTing junk TXT records to test acme-dns
:-)

This commit adjusts the `webUpdatePost` error handling such that
`!validSubdomain(input)` and `!validTXT(input)` give distinct errors.

The `!validSubdomain` case should never happen in `webUpdatePost`
because `auth.go`'s `Auth` function already vets the post data
subdomain but I retained the error handling code just in case.

Unit tests for an update with an invalid subdomain and an update with an
invalid TXT are included.
---
 api.go      | 17 ++++++++++-----
 api_test.go | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/api.go b/api.go
index ef2b02f..d151c82 100644
--- a/api.go
+++ b/api.go
@@ -66,7 +66,18 @@ func webUpdatePost(w http.ResponseWriter, r *http.Request, _ httprouter.Params)
 	if !ok {
 		log.WithFields(log.Fields{"error": "context"}).Error("Context error")
 	}
-	if validSubdomain(a.Subdomain) && validTXT(a.Value) {
+	// NOTE: An invalid subdomain should not happen - the auth handler should
+	// reject POSTs with an invalid subdomain before this handler. Reject any
+	// invalid subdomains anyway as a matter of caution.
+	if !validSubdomain(a.Subdomain) {
+		log.WithFields(log.Fields{"error": "subdomain", "subdomain": a.Subdomain, "txt": a.Value}).Debug("Bad update data")
+		updStatus = http.StatusBadRequest
+		upd = jsonError("bad_subdomain")
+	} else if !validTXT(a.Value) {
+		log.WithFields(log.Fields{"error": "txt", "subdomain": a.Subdomain, "txt": a.Value}).Debug("Bad update data")
+		updStatus = http.StatusBadRequest
+		upd = jsonError("bad_txt")
+	} else if validSubdomain(a.Subdomain) && validTXT(a.Value) {
 		err := DB.Update(a)
 		if err != nil {
 			log.WithFields(log.Fields{"error": err.Error()}).Debug("Error while trying to update record")
@@ -77,10 +88,6 @@ func webUpdatePost(w http.ResponseWriter, r *http.Request, _ httprouter.Params)
 			updStatus = http.StatusOK
 			upd = []byte("{\"txt\": \"" + a.Value + "\"}")
 		}
-	} else {
-		log.WithFields(log.Fields{"error": "subdomain", "subdomain": a.Subdomain, "txt": a.Value}).Debug("Bad update data")
-		updStatus = http.StatusBadRequest
-		upd = jsonError("bad_subdomain")
 	}
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(updStatus)
diff --git a/api_test.go b/api_test.go
index b546966..3338447 100644
--- a/api_test.go
+++ b/api_test.go
@@ -159,6 +159,66 @@ func TestApiRegisterWithMockDB(t *testing.T) {
 	DB.SetBackend(oldDb)
 }
 
+func TestApiUpdateWithInvalidSubdomain(t *testing.T) {
+	validTxtData := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+
+	updateJSON := map[string]interface{}{
+		"subdomain": "",
+		"txt":       ""}
+
+	router := setupRouter(false, false)
+	server := httptest.NewServer(router)
+	defer server.Close()
+	e := getExpect(t, server)
+	newUser, err := DB.Register(cidrslice{})
+	if err != nil {
+		t.Errorf("Could not create new user, got error [%v]", err)
+	}
+	// Invalid subdomain data
+	updateJSON["subdomain"] = "example.com"
+	updateJSON["txt"] = validTxtData
+	e.POST("/update").
+		WithJSON(updateJSON).
+		WithHeader("X-Api-User", newUser.Username.String()).
+		WithHeader("X-Api-Key", newUser.Password).
+		Expect().
+		Status(http.StatusUnauthorized).
+		JSON().Object().
+		ContainsKey("error").
+		NotContainsKey("txt").
+		ValueEqual("error", "forbidden")
+}
+
+func TestApiUpdateWithInvalidTxt(t *testing.T) {
+	invalidTXTData := "idk m8 bbl lmao"
+
+	updateJSON := map[string]interface{}{
+		"subdomain": "",
+		"txt":       ""}
+
+	router := setupRouter(false, false)
+	server := httptest.NewServer(router)
+	defer server.Close()
+	e := getExpect(t, server)
+	newUser, err := DB.Register(cidrslice{})
+	if err != nil {
+		t.Errorf("Could not create new user, got error [%v]", err)
+	}
+	updateJSON["subdomain"] = newUser.Subdomain
+	// Invalid txt data
+	updateJSON["txt"] = invalidTXTData
+	e.POST("/update").
+		WithJSON(updateJSON).
+		WithHeader("X-Api-User", newUser.Username.String()).
+		WithHeader("X-Api-Key", newUser.Password).
+		Expect().
+		Status(http.StatusBadRequest).
+		JSON().Object().
+		ContainsKey("error").
+		NotContainsKey("txt").
+		ValueEqual("error", "bad_txt")
+}
+
 func TestApiUpdateWithoutCredentials(t *testing.T) {
 	router := setupRouter(false, false)
 	server := httptest.NewServer(router)