Fix ACME policy comments

acme/api/account_test.go

@@ -65,6 +65,15 @@ func newACMEProv(t *testing.T) *provisioner.ACME {
 	return a
 }
 
+func newACMEProvWithOptions(t *testing.T, options *provisioner.Options) *provisioner.ACME {
+	p := newProvWithOptions(options)
+	a, ok := p.(*provisioner.ACME)
+	if !ok {
+		t.Fatal("not a valid ACME provisioner")
+	}
+	return a
+}
+
 func createEABJWS(jwk *jose.JSONWebKey, hmacKey []byte, keyID, u string) (*jose.JSONWebSignature, error) {
 	signer, err := jose.NewSigner(
 		jose.SigningKey{

acme/api/order.go

@@ -103,15 +103,23 @@ func (h *Handler) NewOrder(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// TODO(hs): gather all errors, so that we can build one response with subproblems; include the nor.Validate()
+	// TODO(hs): gather all errors, so that we can build one response with ACME subproblems
-	// error here too, like in example?
+	// include the nor.Validate() error here too, like in the example in the ACME RFC?
 
-	eak, err := h.db.GetExternalAccountKeyByAccountID(ctx, prov.GetID(), acc.ID)
+	acmeProv, err := acmeProvisionerFromContext(ctx)
 	if err != nil {
-		render.Error(w, acme.WrapErrorISE(err, "error retrieving external account binding key"))
+		render.Error(w, err)
 		return
 	}
 
+	var eak *acme.ExternalAccountKey
+	if acmeProv.RequireEAB {
+		if eak, err = h.db.GetExternalAccountKeyByAccountID(ctx, prov.GetID(), acc.ID); err != nil {
+			render.Error(w, acme.WrapErrorISE(err, "error retrieving external account binding key"))
+			return
+		}
+	}
+
 	acmePolicy, err := newACMEPolicyEngine(eak)
 	if err != nil {
 		render.Error(w, acme.WrapErrorISE(err, "error creating ACME policy engine"))

acme/api/order_test.go

@@ -761,7 +761,7 @@ func TestHandler_NewOrder(t *testing.T) {
 				err:        acme.NewError(acme.ErrorMalformedType, "identifiers list cannot be empty"),
 			}
 		},
-		"fail/db.GetExternalAccountKeyByAccountID-error": func(t *testing.T) test {
+		"fail/acmeProvisionerFromContext-error": func(t *testing.T) test {
 			acc := &acme.Account{ID: "accID"}
 			fr := &NewOrderRequest{
 				Identifiers: []acme.Identifier{
@@ -770,7 +770,35 @@ func TestHandler_NewOrder(t *testing.T) {
 			}
 			b, err := json.Marshal(fr)
 			assert.FatalError(t, err)
-			ctx := context.WithValue(context.Background(), provisionerContextKey, prov)
+			ctx := context.WithValue(context.Background(), provisionerContextKey, &acme.MockProvisioner{})
+			ctx = context.WithValue(ctx, accContextKey, acc)
+			ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: b})
+			return test{
+				ctx:        ctx,
+				statusCode: 500,
+				ca:         &mockCA{},
+				db: &acme.MockDB{
+					MockGetExternalAccountKeyByAccountID: func(ctx context.Context, provisionerID, accountID string) (*acme.ExternalAccountKey, error) {
+						assert.Equals(t, prov.GetID(), provisionerID)
+						assert.Equals(t, "accID", accountID)
+						return nil, errors.New("force")
+					},
+				},
+				err: acme.NewErrorISE("error retrieving external account binding key: force"),
+			}
+		},
+		"fail/db.GetExternalAccountKeyByAccountID-error": func(t *testing.T) test {
+			acmeProv := newACMEProv(t)
+			acmeProv.RequireEAB = true
+			acc := &acme.Account{ID: "accID"}
+			fr := &NewOrderRequest{
+				Identifiers: []acme.Identifier{
+					{Type: "dns", Value: "zap.internal"},
+				},
+			}
+			b, err := json.Marshal(fr)
+			assert.FatalError(t, err)
+			ctx := context.WithValue(context.Background(), provisionerContextKey, acmeProv)
 			ctx = context.WithValue(ctx, accContextKey, acc)
 			ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: b})
 			return test{
@@ -788,6 +816,8 @@ func TestHandler_NewOrder(t *testing.T) {
 			}
 		},
 		"fail/newACMEPolicyEngine-error": func(t *testing.T) test {
+			acmeProv := newACMEProv(t)
+			acmeProv.RequireEAB = true
 			acc := &acme.Account{ID: "accID"}
 			fr := &NewOrderRequest{
 				Identifiers: []acme.Identifier{
@@ -796,7 +826,7 @@ func TestHandler_NewOrder(t *testing.T) {
 			}
 			b, err := json.Marshal(fr)
 			assert.FatalError(t, err)
-			ctx := context.WithValue(context.Background(), provisionerContextKey, prov)
+			ctx := context.WithValue(context.Background(), provisionerContextKey, acmeProv)
 			ctx = context.WithValue(ctx, accContextKey, acc)
 			ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: b})
 			return test{
@@ -822,6 +852,8 @@ func TestHandler_NewOrder(t *testing.T) {
 			}
 		},
 		"fail/isIdentifierAllowed-error": func(t *testing.T) test {
+			acmeProv := newACMEProv(t)
+			acmeProv.RequireEAB = true
 			acc := &acme.Account{ID: "accID"}
 			fr := &NewOrderRequest{
 				Identifiers: []acme.Identifier{
@@ -830,7 +862,7 @@ func TestHandler_NewOrder(t *testing.T) {
 			}
 			b, err := json.Marshal(fr)
 			assert.FatalError(t, err)
-			ctx := context.WithValue(context.Background(), provisionerContextKey, prov)
+			ctx := context.WithValue(context.Background(), provisionerContextKey, acmeProv)
 			ctx = context.WithValue(ctx, accContextKey, acc)
 			ctx = context.WithValue(ctx, payloadContextKey, &payloadInfo{value: b})
 			return test{
@@ -863,7 +895,8 @@ func TestHandler_NewOrder(t *testing.T) {
 					},
 				},
 			}
-			provWithPolicy := newProvWithOptions(options)
+			provWithPolicy := newACMEProvWithOptions(t, options)
+			provWithPolicy.RequireEAB = true
 			acc := &acme.Account{ID: "accID"}
 			fr := &NewOrderRequest{
 				Identifiers: []acme.Identifier{
@@ -905,7 +938,8 @@ func TestHandler_NewOrder(t *testing.T) {
 					},
 				},
 			}
-			provWithPolicy := newProvWithOptions(options)
+			provWithPolicy := newACMEProvWithOptions(t, options)
+			provWithPolicy.RequireEAB = true
 			acc := &acme.Account{ID: "accID"}
 			fr := &NewOrderRequest{
 				Identifiers: []acme.Identifier{
@@ -1567,7 +1601,8 @@ func TestHandler_NewOrder(t *testing.T) {
 					},
 				},
 			}
-			provWithPolicy := newProvWithOptions(options)
+			provWithPolicy := newACMEProvWithOptions(t, options)
+			provWithPolicy.RequireEAB = true
 			acc := &acme.Account{ID: "accID"}
 			nor := &NewOrderRequest{
 				Identifiers: []acme.Identifier{

authority/admin/api/acme.go

@@ -38,13 +38,7 @@ func (h *Handler) requireEABEnabled(next http.HandlerFunc) http.HandlerFunc {
 		ctx := r.Context()
 		prov := linkedca.ProvisionerFromContext(ctx)
 
-		details := prov.GetDetails()
+		acmeProvisioner := prov.GetDetails().GetACME()
-		if details == nil {
-			render.Error(w, admin.NewErrorISE("error getting details for provisioner '%s'", prov.GetName()))
-			return
-		}
-
-		acmeProvisioner := details.GetACME()
 		if acmeProvisioner == nil {
 			render.Error(w, admin.NewErrorISE("error getting ACME details for provisioner '%s'", prov.GetName()))
 			return

authority/admin/api/acme_test.go

@@ -13,13 +13,15 @@ import (
 	"time"
 
 	"github.com/go-chi/chi"
-	"github.com/smallstep/assert"
-	"github.com/smallstep/certificates/acme"
-	"github.com/smallstep/certificates/authority/admin"
-	"go.step.sm/linkedca"
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/timestamppb"
+
+	"go.step.sm/linkedca"
+
+	"github.com/smallstep/assert"
+	"github.com/smallstep/certificates/acme"
+	"github.com/smallstep/certificates/authority/admin"
 )
 
 func readProtoJSON(r io.ReadCloser, m proto.Message) error {
@@ -45,15 +47,15 @@ func TestHandler_requireEABEnabled(t *testing.T) {
 				Name: "provName",
 			}
 			ctx := linkedca.NewContextWithProvisioner(context.Background(), prov)
-			err := admin.NewErrorISE("error getting details for provisioner 'provName'")
+			err := admin.NewErrorISE("error getting ACME details for provisioner 'provName'")
-			err.Message = "error getting details for provisioner 'provName'"
+			err.Message = "error getting ACME details for provisioner 'provName'"
 			return test{
 				ctx:        ctx,
 				err:        err,
 				statusCode: 500,
 			}
 		},
-		"fail/details.GetACME": func(t *testing.T) test {
+		"fail/prov.GetDetails.GetACME": func(t *testing.T) test {
 			prov := &linkedca.Provisioner{
 				Id:      "provID",
 				Name:    "provName",