Skip to content

Commit

Permalink
API: Differentiate bad TXT update error.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cpu committed Feb 28, 2018
1 parent 830cceb commit efdd560
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 5 deletions.
17 changes: 12 additions & 5 deletions api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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)
Expand Down
60 changes: 60 additions & 0 deletions api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit efdd560

Please sign in to comment.