From ef6dbed39d0d54527f20bbdf9a264d4950b11fc1 Mon Sep 17 00:00:00 2001
From: Daniel McCarney <daniel@binaryparadox.net>
Date: Thu, 1 Mar 2018 13:26:46 -0500
Subject: [PATCH] Treat orders with expired authzs as invalid. (#3500)

This commit updates `sa.getAllOrderAuthorizations` to not exclude
expired authorizations. The expired authorizations are used in
`sa.statusForOrder` to set the overall order status to invalid when one
or more authorizations are expired.

Fixes #3499
---
 sa/sa.go      | 12 +++++++++---
 sa/sa_test.go | 19 +++++++++++++++++++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/sa/sa.go b/sa/sa.go
index 8261ade3d..722f06fd7 100644
--- a/sa/sa.go
+++ b/sa/sa.go
@@ -1654,6 +1654,7 @@ func (ssa *SQLStorageAuthority) GetOrder(ctx context.Context, req *sapb.OrderReq
 // determine what the overall status of the order should be. In summary:
 //   * If the order has an error, the order is invalid
 //   * If any of the order's authorizations are invalid, the order is invalid.
+//   * If any of the order's authorizations are expired, the order is invalid.
 //   * If any of the order's authorizations are deactivated, the order is deactivated.
 //   * If any of the order's authorizations are pending, the order is pending.
 //   * If all of the order's authorizations are valid, and there is
@@ -1688,6 +1689,7 @@ func (ssa *SQLStorageAuthority) statusForOrder(ctx context.Context, order *corep
 
 	// Keep a count of the authorizations seen
 	invalidAuthzs := 0
+	expiredAuthzs := 0
 	deactivatedAuthzs := 0
 	pendingAuthzs := 0
 	validAuthzs := 0
@@ -1708,12 +1710,19 @@ func (ssa *SQLStorageAuthority) statusForOrder(ctx context.Context, order *corep
 				"Order is in an invalid state. Authz %s has invalid status %q",
 				authz.ID, authz.Status)
 		}
+		if authz.Expires.Before(ssa.clk.Now()) {
+			expiredAuthzs++
+		}
 	}
 
 	// An order is invalid if **any** of its authzs are invalid
 	if invalidAuthzs > 0 {
 		return string(core.StatusInvalid), nil
 	}
+	// An order is invalid if **any** of its authzs are expired
+	if expiredAuthzs > 0 {
+		return string(core.StatusInvalid), nil
+	}
 	// An order is deactivated if **any** of its authzs are deactivated
 	if deactivatedAuthzs > 0 {
 		return string(core.StatusDeactivated), nil
@@ -1763,7 +1772,6 @@ func (ssa *SQLStorageAuthority) statusForOrder(ctx context.Context, order *corep
 func (ssa *SQLStorageAuthority) getAllOrderAuthorizations(
 	ctx context.Context,
 	orderID, acctID int64) (map[string]*core.Authorization, error) {
-	now := ssa.clk.Now()
 	var allAuthzs []*core.Authorization
 
 	for _, table := range authorizationTables {
@@ -1774,10 +1782,8 @@ func (ssa *SQLStorageAuthority) getAllOrderAuthorizations(
 		INNER JOIN orderToAuthz
 		ON authz.ID = orderToAuthz.authzID
 		WHERE authz.registrationID = ? AND
-		authz.expires > ? AND
 		orderToAuthz.orderID = ?`, authzFields, table),
 			acctID,
-			now,
 			orderID)
 		if err != nil {
 			return nil, err
diff --git a/sa/sa_test.go b/sa/sa_test.go
index c56e4a645..81a9625b5 100644
--- a/sa/sa_test.go
+++ b/sa/sa_test.go
@@ -2048,6 +2048,7 @@ func TestStatusForOrder(t *testing.T) {
 	ctx := context.Background()
 	expires := fc.Now().Add(time.Hour)
 	expiresNano := expires.UnixNano()
+	alreadyExpired := expires.Add(-2 * time.Hour)
 
 	// Create a registration to work with
 	reg := satest.CreateWorkingRegistration(t, sa)
@@ -2062,8 +2063,19 @@ func TestStatusForOrder(t *testing.T) {
 	pendingAuthz, err := sa.NewPendingAuthorization(ctx, newAuthz)
 	test.AssertNotError(t, err, "Couldn't create new pending authorization")
 
+	// Create an expired authz
+	newExpiredAuthz := core.Authorization{
+		RegistrationID: newAuthz.RegistrationID,
+		Expires:        &alreadyExpired,
+		Status:         newAuthz.Status,
+		Identifier:     core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "expired.your.order.is.up"},
+	}
+	expiredAuthz, err := sa.NewPendingAuthorization(ctx, newExpiredAuthz)
+	test.AssertNotError(t, err, "Couldn't create new expired pending authorization")
+
 	// Create an invalid authz
 	invalidAuthz, err := sa.NewPendingAuthorization(ctx, newAuthz)
+	test.AssertNotError(t, err, "Couldn't create new pending authorization")
 	invalidAuthz.Status = core.StatusInvalid
 	invalidAuthz.Identifier.Value = "invalid.your.order.is.up"
 	err = sa.FinalizeAuthorization(ctx, invalidAuthz)
@@ -2071,6 +2083,7 @@ func TestStatusForOrder(t *testing.T) {
 
 	// Create a deactivate authz
 	deactivatedAuthz, err := sa.NewPendingAuthorization(ctx, newAuthz)
+	test.AssertNotError(t, err, "Couldn't create new pending authorization")
 	deactivatedAuthz.Status = core.StatusDeactivated
 	deactivatedAuthz.Identifier.Value = "deactivated.your.order.is.up"
 	err = sa.FinalizeAuthorization(ctx, deactivatedAuthz)
@@ -2097,6 +2110,12 @@ func TestStatusForOrder(t *testing.T) {
 			AuthorizationIDs: []string{pendingAuthz.ID, invalidAuthz.ID, deactivatedAuthz.ID, validAuthz.ID},
 			ExpectedStatus:   string(core.StatusInvalid),
 		},
+		{
+			Name:             "Order with an expired authz",
+			OrderNames:       []string{"pending.your.order.is.up", "expired.your.order.is.up", "deactivated.your.order.is.up", "valid.your.order.is.up"},
+			AuthorizationIDs: []string{pendingAuthz.ID, expiredAuthz.ID, deactivatedAuthz.ID, validAuthz.ID},
+			ExpectedStatus:   string(core.StatusInvalid),
+		},
 		{
 			Name:             "Order with a deactivated authz",
 			OrderNames:       []string{"pending.your.order.is.up", "deactivated.your.order.is.up", "valid.your.order.is.up"},