sa: NewOrderAndAuthzs checks that nonzero authzs were provided (#8464)

Also document the relevant fields.

Author: jsha

sa/proto/sa.pb.go

@@ -213,6 +213,8 @@ message NewOrderRequest {
   google.protobuf.Timestamp expires = 5;
   reserved 3; // Previously dnsNames
   repeated core.Identifier identifiers = 9;
+  // A list of already-existing authorization IDs that should be associated with
+  // the new Order object. This is for authorization reuse.
   repeated int64 v2Authorizations = 4;
   string certificateProfileName = 7;
   // Replaces is the ARI certificate Id that this order replaces.
@@ -223,9 +225,7 @@ message NewOrderRequest {
 
 }
 
-// NewAuthzRequest starts with all the same fields as corepb.Authorization,
-// because it is replacing that type in NewOrderAndAuthzsRequest, and then
-// improves from there.
+// NewAuthzRequest represents a request to create an authorization.
 message NewAuthzRequest {
   // Next unused field number: 13
   reserved 1; // previously id
@@ -244,6 +244,11 @@ message NewAuthzRequest {
 
 message NewOrderAndAuthzsRequest {
   NewOrderRequest newOrder = 1;
+  // Authorizations to be newly created alongside the order, and associated with it.
+  // These will be combined with any reused authorizations (newOrder.v2Authorizations)
+  // to make the overall set of authorizations for the order. This field and
+  // newOrder.v2Authorizations may both be present, or only one of the two may be
+  // present, but they may not both be absent.
   repeated NewAuthzRequest newAuthzs = 2;
 }
 

sa/proto/sa.proto

@@ -452,6 +452,9 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb
 	if req.NewOrder == nil {
 		return nil, errIncompleteRequest
 	}
+	if len(req.NewAuthzs) == 0 && len(req.NewOrder.V2Authorizations) == 0 {
+		return nil, errIncompleteRequest
+	}
 
 	for _, authz := range req.NewAuthzs {
 		if authz.RegistrationID != req.NewOrder.RegistrationID {

sa/sa.go

@@ -1007,6 +1007,110 @@ func TestNewOrderAndAuthzs(t *testing.T) {
 	test.AssertDeepEquals(t, authzIDs, []int64{1, 2, 3, 4})
 }
 
+func TestNewOrderAndAuthzs_ReuseOnly(t *testing.T) {
+	sa, fc, cleanup := initSA(t)
+	defer cleanup()
+
+	reg := createWorkingRegistration(t, sa)
+	expires := fc.Now().Add(2 * time.Hour)
+
+	// Insert two pre-existing authorizations to reference
+	idA := createPendingAuthorization(t, sa, identifier.NewDNS("a.com"), sa.clk.Now().Add(time.Hour))
+	idB := createPendingAuthorization(t, sa, identifier.NewDNS("b.com"), sa.clk.Now().Add(time.Hour))
+	test.AssertEquals(t, idA, int64(1))
+	test.AssertEquals(t, idB, int64(2))
+	req := &sapb.NewOrderAndAuthzsRequest{
+		// Insert an order for four names, two of which already have authzs
+		NewOrder: &sapb.NewOrderRequest{
+			RegistrationID: reg.Id,
+			Expires:        timestamppb.New(expires),
+			Identifiers: []*corepb.Identifier{
+				identifier.NewDNS("a.com").ToProto(),
+				identifier.NewDNS("b.com").ToProto(),
+			},
+			V2Authorizations: []int64{1, 2},
+		},
+	}
+	order, err := sa.NewOrderAndAuthzs(context.Background(), req)
+	if err != nil {
+		t.Fatal("sa.NewOrderAndAuthzs:", err)
+	}
+	if !reflect.DeepEqual(order.V2Authorizations, []int64{1, 2}) {
+		t.Errorf("sa.NewOrderAndAuthzs().V2Authorizations: want [1, 2], got %v", order.V2Authorizations)
+	}
+}
+
+func TestNewOrderAndAuthzs_CreateOnly(t *testing.T) {
+	sa, fc, cleanup := initSA(t)
+	defer cleanup()
+
+	reg := createWorkingRegistration(t, sa)
+	expires := fc.Now().Add(2 * time.Hour)
+
+	// Insert two pre-existing authorizations to reference
+	idA := createPendingAuthorization(t, sa, identifier.NewDNS("a.com"), sa.clk.Now().Add(time.Hour))
+	idB := createPendingAuthorization(t, sa, identifier.NewDNS("b.com"), sa.clk.Now().Add(time.Hour))
+	test.AssertEquals(t, idA, int64(1))
+	test.AssertEquals(t, idB, int64(2))
+	req := &sapb.NewOrderAndAuthzsRequest{
+		// Insert an order for four names, two of which already have authzs
+		NewOrder: &sapb.NewOrderRequest{
+			RegistrationID: reg.Id,
+			Expires:        timestamppb.New(expires),
+			Identifiers: []*corepb.Identifier{
+				identifier.NewDNS("a.com").ToProto(),
+				identifier.NewDNS("b.com").ToProto(),
+			},
+		},
+		NewAuthzs: []*sapb.NewAuthzRequest{
+			{
+				Identifier:     &corepb.Identifier{Type: "dns", Value: "a.com"},
+				RegistrationID: reg.Id,
+				Expires:        timestamppb.New(expires),
+				ChallengeTypes: []string{string(core.ChallengeTypeDNS01)},
+				Token:          core.NewToken(),
+			},
+		},
+	}
+	order, err := sa.NewOrderAndAuthzs(context.Background(), req)
+	if err != nil {
+		t.Fatal("sa.NewOrderAndAuthzs:", err)
+	}
+	if len(order.V2Authorizations) != 1 {
+		t.Fatalf("len(sa.NewOrderAndAuthzs().V2Authorizations): want 1, got %v", len(order.V2Authorizations))
+	}
+	gotAuthz, err := sa.GetAuthorization2(context.Background(), &sapb.AuthorizationID2{Id: order.V2Authorizations[0]})
+	if err != nil {
+		t.Fatalf("retrieving inserted authz: %s", err)
+	}
+	if gotAuthz.Identifier.Value != "a.com" {
+		t.Errorf("New order authz identifier = %v, want %v", gotAuthz.Identifier.Value, "a.com")
+	}
+}
+
+func TestNewOrderAndAuthzs_NoAuthzsError(t *testing.T) {
+	sa, fc, cleanup := initSA(t)
+	defer cleanup()
+
+	reg := createWorkingRegistration(t, sa)
+	expires := fc.Now().Add(2 * time.Hour)
+
+	// Insert two pre-existing authorizations to reference
+	req := &sapb.NewOrderAndAuthzsRequest{
+		// Insert an order for four names, two of which already have authzs
+		NewOrder: &sapb.NewOrderRequest{
+			RegistrationID: reg.Id,
+			Expires:        timestamppb.New(expires),
+			Identifiers:    nil,
+		},
+		NewAuthzs: nil,
+	}
+	_, err := sa.NewOrderAndAuthzs(context.Background(), req)
+	if err != errIncompleteRequest {
+		t.Errorf("sa.NewOrderAndAuthzs with no authzs: want %v, got %v", errIncompleteRequest, err)
+	}
+}
+
 // TestNewOrderAndAuthzs_NonNilInnerOrder verifies that a nil
 // sapb.NewOrderAndAuthzsRequest NewOrder object returns an error.
 func TestNewOrderAndAuthzs_NonNilInnerOrder(t *testing.T) {