Merge pull request #42 from cpu/cpu-differentiate-errors

API: Differentiate bad TXT update error.
joohoi · Mar 13, 2018 · d542ee0 · d542ee0
2 parents 978ac5d + 92f8cc2
commit d542ee0
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 5 deletions.
diff --git 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
@@ -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)