acme: make WaitAuthorization return authorization errors consistently

Fixes golang/go#37340

Change-Id: I19c4f150b8607ad4a1613cf97ad3362f4b779d7c
GitHub-Last-Rev: 4215964b4a680b135301695ccd56cff88a8ffb26
GitHub-Pull-Request: golang/crypto#121
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/220343
Reviewed-by: Filippo Valsorda <filippo@golang.org>

acme/acme_test.go

@@ -569,6 +569,45 @@ func TestWaitAuthorization(t *testing.T) {
 			t.Errorf("err is %v (%T); want non-nil *AuthorizationError", err, err)
 		}
 	})
+	t.Run("invalid status with error returns the authorization error", func(t *testing.T) {
+		_, err := runWaitAuthorization(context.Background(), t, func(w http.ResponseWriter, r *http.Request) {
+			fmt.Fprintf(w, `{
+				"type": "dns-01",
+				"status": "invalid",
+				"error": {
+				  "type": "urn:ietf:params:acme:error:caa",
+				  "detail": "CAA record for <domain> prevents issuance",
+				  "status": 403
+				},
+				"url": "https://acme-v02.api.letsencrypt.org/acme/chall-v3/xxx/xxx",
+				"token": "xxx",
+				"validationRecord": [
+				  {
+					"hostname": "<domain>"
+				  }
+				]
+			  }`)
+		})
+
+		want := &AuthorizationError{
+			Errors: []error{
+				(&wireError{
+					Status: 403,
+					Type:   "urn:ietf:params:acme:error:caa",
+					Detail: "CAA record for <domain> prevents issuance",
+				}).error(nil),
+			},
+		}
+
+		_, ok := err.(*AuthorizationError)
+		if !ok {
+			t.Errorf("err is %T; want non-nil *AuthorizationError", err)
+		}
+
+		if err.Error() != want.Error() {
+			t.Errorf("err is %v; want %v", err, want)
+		}
+	})
 	t.Run("non-retriable error", func(t *testing.T) {
 		const code = http.StatusBadRequest
 		_, err := runWaitAuthorization(context.Background(), t, func(w http.ResponseWriter, r *http.Request) {

acme/types.go

@@ -102,7 +102,12 @@ func (a *AuthorizationError) Error() string {
 	for i, err := range a.Errors {
 		e[i] = err.Error()
 	}
-	return fmt.Sprintf("acme: authorization error for %s: %s", a.Identifier, strings.Join(e, "; "))
+
+	if a.Identifier != "" {
+		return fmt.Sprintf("acme: authorization error for %s: %s", a.Identifier, strings.Join(e, "; "))
+	}
+
+	return fmt.Sprintf("acme: authorization error: %s", strings.Join(e, "; "))
 }
 
 // OrderError is returned from Client's order related methods.
@@ -407,6 +412,7 @@ type wireAuthz struct {
 	Wildcard     bool
 	Challenges   []wireChallenge
 	Combinations [][]int
+	Error        *wireError
 }
 
 func (z *wireAuthz) authorization(uri string) *Authorization {
@@ -430,11 +436,17 @@ func (z *wireAuthz) error(uri string) *AuthorizationError {
 		URI:        uri,
 		Identifier: z.Identifier.Value,
 	}
+
+	if z.Error != nil {
+		err.Errors = append(err.Errors, z.Error.error(nil))
+	}
+
 	for _, raw := range z.Challenges {
 		if raw.Error != nil {
 			err.Errors = append(err.Errors, raw.Error.error(nil))
 		}
 	}
+
 	return err
 }
 

acme/types_test.go

@@ -61,3 +61,46 @@ func TestRateLimit(t *testing.T) {
 		}
 	}
 }
+
+func TestAuthorizationError(t *testing.T) {
+	tests := []struct {
+		desc string
+		err  *AuthorizationError
+		msg  string
+	}{
+		{
+			desc: "when auth error identifier is set",
+			err: &AuthorizationError{
+				Identifier: "domain.com",
+				Errors: []error{
+					(&wireError{
+						Status: 403,
+						Type:   "urn:ietf:params:acme:error:caa",
+						Detail: "CAA record for domain.com prevents issuance",
+					}).error(nil),
+				},
+			},
+			msg: "acme: authorization error for domain.com: 403 urn:ietf:params:acme:error:caa: CAA record for domain.com prevents issuance",
+		},
+
+		{
+			desc: "when auth error identifier is unset",
+			err: &AuthorizationError{
+				Errors: []error{
+					(&wireError{
+						Status: 403,
+						Type:   "urn:ietf:params:acme:error:caa",
+						Detail: "CAA record for domain.com prevents issuance",
+					}).error(nil),
+				},
+			},
+			msg: "acme: authorization error: 403 urn:ietf:params:acme:error:caa: CAA record for domain.com prevents issuance",
+		},
+	}
+
+	for _, tt := range tests {
+		if tt.err.Error() != tt.msg {
+			t.Errorf("got: %s\nwant: %s", tt.err, tt.msg)
+		}
+	}
+}