From 10db7e9e32dfdf942be0c2460053d10ce555148d Mon Sep 17 00:00:00 2001 From: Daniel Date: Wed, 17 Jan 2018 12:43:58 -0500 Subject: [PATCH 01/19] Only reuse pending order, invalidate order on invalid authz. Prior to this commit the combination of: * Order reuse * Authz reuse within orders * Orders not being set invalid on a failed challenge/authz Meant that if a user created an order, failed (or deactivated) one of the associated authzs, and then tried to create another order for the same names they would get back the original order with the failed/deactivated authzs. Oops! This commit addresses this problem in three main ways: 1. Pending authz reuse within orders is removed. There's no advantage to reusing one pending authz across multiple orders when we have reuse at the order level and it complicates invalidating an order when a pending authz is finalized to invalid. 2. When an authz is finalized to an invalid state the SA now tries to find an associated order (if any) using the `orderToAuthz` join table. If an order is associated with the authz being finalized as invalid then the order is also updated to be invalid. 3. When NewOrder looks for an order to reuse it only reuses the order if the status is pending. The combination of the above means that calling NewOrder, failing (or deactivating an authz) and then calling NewOrder again will not result in the same "stuck" order being returned for the second call. The associated integration test was written ahead of the fixes and verifies that the change has the intended affect. Prior to the fixes it failed in the same manner as reported by users of the V2 staging API. --- ra/ra.go | 9 +++++ ra/ra_test.go | 66 +++++++++++++++++++++++++++++- sa/sa.go | 81 +++++++++++++++++++++++++++++++++++-- sa/sa_test.go | 71 ++++++++++++++++++++++++++++++++ test/integration-test-v2.py | 57 +++++++++++++++++++++++--- 5 files changed, 275 insertions(+), 9 deletions(-) diff --git a/ra/ra.go b/ra/ra.go index d5b400df24c..2e792ad52d0 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -1655,6 +1655,15 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New continue } authz := nameToExistingAuthz[name] + // If the existing authz isn't valid, note that its missing and continue. + // Since we reuse whole pending orders there isn't any benefit to reusing + // individual pending authorizations between orders. Worse, it makes the + // process of invalidating an order when an authorization is invalidated + // confusing because multiple orders would be invalidated simultaneously. + if *authz.Status != string(core.StatusValid) { + missingAuthzNames = append(missingAuthzNames, name) + continue + } // If the identifier is a wildcard and the existing authz only has one // DNS-01 type challenge we can reuse it. In theory we will // never get back an authorization for a domain with a wildcard prefix diff --git a/ra/ra_test.go b/ra/ra_test.go index 045e4fe5b5c..635dc311a23 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -13,6 +13,7 @@ import ( "net" "net/url" "os" + "reflect" "sort" "strings" "sync" @@ -2166,7 +2167,14 @@ func TestNewOrder(t *testing.T) { // Abuse the order of the queries used to extract the reused authorizations existing := orderC.Authorizations[:3] sort.Strings(existing) - test.AssertDeepEquals(t, existing, orderA.Authorizations) + + // We expect the pending authorizations were not reused between separate + // orders + // NOTE(@cpu): There's no test.AssertNotDeepEquals to use here so we call + // reflect.DeepEqual ourselves. + if reflect.DeepEqual(existing, orderA.Authorizations) { + t.Fatal("existing authorizations matched orderA authorizations") + } _, err = ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ RegistrationID: &id, @@ -2284,6 +2292,62 @@ func TestNewOrderReuse(t *testing.T) { } } +func TestNewOrderReuseInvalidAuthz(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + + _, _, ra, _, cleanUp := initAuthorities(t) + defer cleanUp() + + ctx := context.Background() + regA := int64(1) + names := []string{"zombo.com"} + + // Create an initial request with regA and names + orderReq := &rapb.NewOrderRequest{ + RegistrationID: ®A, + Names: names, + } + + // First, add an order with `names` for regA + order, err := ra.NewOrder(ctx, orderReq) + // It shouldn't fail + test.AssertNotError(t, err, "Adding an initial order for regA failed") + // It should have an ID + test.AssertNotNil(t, order.Id, "Initial order had a nil ID") + // It should have one authorization + test.AssertEquals(t, len(order.Authorizations), 1) + + // Fetch the full authz by the ID + authzID := order.Authorizations[0] + authz, err := ra.SA.GetAuthorization(ctx, authzID) + test.AssertNotError(t, err, "Error getting order authorization") + + // Finalize the authz to an invalid status + authz.Status = core.StatusInvalid + err = ra.SA.FinalizeAuthorization(ctx, authz) + test.AssertNotError(t, err, fmt.Sprintf("Could not finalize authorization %q", authzID)) + + // The order associated with the authz should now be invalid + updatedOrder, err := ra.SA.GetOrder(ctx, &sapb.OrderRequest{Id: order.Id}) + test.AssertNotError(t, err, "Error getting order to check status") + test.AssertEquals(t, *updatedOrder.Status, "invalid") + + // Create a second order for the same names/regID + secondOrder, err := ra.NewOrder(ctx, orderReq) + // It shouldn't fail + test.AssertNotError(t, err, "Adding an initial order for regA failed") + // It should have a different ID than the first now-invalid order + test.AssertNotEquals(t, *secondOrder.Id, *order.Id) + // It should be status pending + test.AssertEquals(t, *secondOrder.Status, "pending") + // It should have one authorization + test.AssertEquals(t, len(secondOrder.Authorizations), 1) + // It should have a different authorization than the first order's now-invalid authorization + test.AssertNotEquals(t, secondOrder.Authorizations[0], order.Authorizations[0]) +} + func TestNewOrderWildcard(t *testing.T) { // Only run under test/config-next config where 20170731115209_AddOrders.sql // has been applied diff --git a/sa/sa.go b/sa/sa.go index cf2f9398646..ffb91d8fc9e 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -791,7 +791,7 @@ func (ssa *SQLStorageAuthority) UpdatePendingAuthorization(ctx context.Context, // FinalizeAuthorization converts a Pending Authorization to a final one. If the // Authorization is not found a berrors.NotFound result is returned. If the -// Authorization is not valid a berrors.InternalServer error is returned. +// Authorization is status pending an berrors.InternalServer error is returned. func (ssa *SQLStorageAuthority) FinalizeAuthorization(ctx context.Context, authz core.Authorization) error { tx, err := ssa.dbMap.Begin() if err != nil { @@ -832,6 +832,30 @@ func (ssa *SQLStorageAuthority) FinalizeAuthorization(ctx context.Context, authz return Rollback(tx, err) } + // When an authorization is being finalized to an invalid state we need to see + // if there is an order associated with this authorization that itself should + // now become invalid as a result of the authz being invalid. + if authz.Status == core.StatusInvalid { + // Try to find an order associated with this authz ID. There may not be one if + // this is a legacy V1 authorization from the new-authz endpoint. + orderID, err := ssa.orderIDForAuthz(tx, authz.ID) + // If there was an error, and it wasn't a no-result error, then we encountered + // something unexpected and must rollback + if err != nil && err != sql.ErrNoRows { + return Rollback(tx, err) + } + // If the err was nil then orderID is an associated order for this authz + if err == nil { + // Set the order to invalid + err := ssa.setOrderInvalid(ctx, tx, &corepb.Order{ + Id: &orderID, + }) + if err != nil { + return Rollback(tx, err) + } + } + } + return tx.Commit() } @@ -1456,6 +1480,29 @@ func (ssa *SQLStorageAuthority) SetOrderProcessing(ctx context.Context, req *cor return tx.Commit() } +// setOrderInvalid updates a provided *corepb.Order in pending status to be in +// invalid status by updating the status field of the corresponding row in the +// DB. +func (ssa *SQLStorageAuthority) setOrderInvalid(ctx context.Context, tx *gorp.Transaction, req *corepb.Order) error { + result, err := tx.Exec(` + UPDATE orders + SET status = ? + WHERE id = ? + AND status = ?`, + string(core.StatusInvalid), + *req.Id, + string(core.StatusPending)) + if err != nil { + return err + } + + n, err := result.RowsAffected() + if err != nil || n == 0 { + return berrors.InternalServerError("no order updated to invalid status") + } + return nil +} + // FinalizeOrder finalizes a provided *corepb.Order by persisting the // CertificateSerial and a valid status to the database. No fields other than // CertificateSerial and the order ID on the provided order are processed (e.g. @@ -1504,6 +1551,26 @@ func (ssa *SQLStorageAuthority) authzForOrder(orderID int64) ([]string, error) { return ids, nil } +// orderForAuthz finds an order ID associated with a given authz (If any). +func (ssa *SQLStorageAuthority) orderIDForAuthz(tx *gorp.Transaction, authzID string) (int64, error) { + var orderID int64 + + err := tx.SelectOne(&orderID, ` + SELECT orderID + FROM orderToAuthz + WHERE authzID = ?`, + authzID) + + // NOTE(@cpu): orderIDForAuthz does not handle the sql.ErrNoRows that could be + // returned by `SelectOne`. The caller must do so. + if err != nil { + // If there is an err, return it as-is. + return 0, err + } + + return orderID, nil +} + // namesForOrder finds all of the requested names associated with an order. The // names are returned in their reversed form (see `sa.ReverseName`). func (ssa *SQLStorageAuthority) namesForOrder(orderID int64) ([]string, error) { @@ -1631,8 +1698,16 @@ func (ssa *SQLStorageAuthority) GetOrderForNames( return nil, err } - // Get & return the order - return ssa.GetOrder(ctx, &sapb.OrderRequest{Id: &orderID}) + // Get the order + order, err := ssa.GetOrder(ctx, &sapb.OrderRequest{Id: &orderID}) + if err != nil { + return nil, err + } + // Only return a pending order + if *order.Status != string(core.StatusPending) { + return nil, berrors.NotFoundError("no order matching request found") + } + return order, nil } func (ssa *SQLStorageAuthority) getAuthorizations(ctx context.Context, table string, status string, diff --git a/sa/sa_test.go b/sa/sa_test.go index 8a43f1fb25e..7790af97e58 100644 --- a/sa/sa_test.go +++ b/sa/sa_test.go @@ -1864,3 +1864,74 @@ func TestGetOrderForNames(t *testing.T) { // already test.Assert(t, result == nil, "sa.GetOrderForNames returned non-nil result for finalized order case") } + +func TestUpdatePendingAuthorizationInvalidOrder(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + + sa, fc, cleanUp := initSA(t) + defer cleanUp() + + expires := fc.Now().Add(time.Hour) + ctx := context.Background() + + // Create a registration to work with + reg := satest.CreateWorkingRegistration(t, sa) + + // Create a pending authz, not associated with any orders + authz := core.Authorization{ + RegistrationID: reg.ID, + Expires: &expires, + Status: core.StatusPending, + } + pendingAuthz, err := sa.NewPendingAuthorization(ctx, authz) + test.AssertNotError(t, err, "Couldn't create new pending authorization") + + // Update the pending authz to be invalid. This shouldn't error. + pendingAuthz.Status = core.StatusInvalid + err = sa.FinalizeAuthorization(ctx, pendingAuthz) + test.AssertNotError(t, err, "Couldn't finalize legacy pending authz to invalid") + + // Create a pending authz that will be associated with an order + authz = core.Authorization{ + RegistrationID: reg.ID, + Expires: &expires, + Status: core.StatusPending, + } + pendingAuthz, err = sa.NewPendingAuthorization(ctx, authz) + test.AssertNotError(t, err, "Couldn't create new pending authorization") + + // Add a new order that references the above pending authz + status := string(core.StatusPending) + expiresNano := expires.UnixNano() + order, err := sa.NewOrder(ctx, &corepb.Order{ + RegistrationID: ®.ID, + Expires: &expiresNano, + Status: &status, + Authorizations: []string{pendingAuthz.ID}, + Names: []string{"your.order.is.up"}, + }) + // It shouldn't error + test.AssertNotError(t, err, "sa.NewOrder failed") + // The order ID shouldn't be nil + test.AssertNotNil(t, *order.Id, "NewOrder returned with a nil Id") + // The order should be pending + test.AssertEquals(t, *order.Status, string(core.StatusPending)) + // The order should have one authz with the correct ID + test.AssertEquals(t, len(order.Authorizations), 1) + test.AssertEquals(t, order.Authorizations[0], pendingAuthz.ID) + + // Now finalize the authz to an invalid status. + pendingAuthz.Status = core.StatusInvalid + err = sa.FinalizeAuthorization(ctx, pendingAuthz) + test.AssertNotError(t, err, "Couldn't finalize pending authz associated with order to invalid") + + // Fetch the order to get its updated status + updatedOrder, err := sa.GetOrder( + context.Background(), + &sapb.OrderRequest{Id: order.Id}) + test.AssertNotError(t, err, "GetOrder failed") + // We expect the updated order status to be invalid + test.AssertEquals(t, *updatedOrder.Status, string(core.StatusInvalid)) +} diff --git a/test/integration-test-v2.py b/test/integration-test-v2.py index 0ed5b75c45f..7c47f7a8ffa 100644 --- a/test/integration-test-v2.py +++ b/test/integration-test-v2.py @@ -29,11 +29,12 @@ def main(): if not startservers.start(race_detection=True): raise Exception("startservers failed") - test_multidomain() - test_wildcardmultidomain() - test_overlapping_wildcard() - test_wildcard_exactblacklist() - test_wildcard_authz_reuse() + #test_multidomain() + #test_wildcardmultidomain() + #test_overlapping_wildcard() + #test_wildcard_exactblacklist() + #test_wildcard_authz_reuse() + test_order_reuse_failed_authz() if not startservers.check(): raise Exception("startservers.check failed") @@ -118,6 +119,52 @@ def test_wildcard_authz_reuse(): raise Exception("order for %s included a non-pending authorization (status: %s) from a previous HTTP-01 order" % ((domains), str(authz.body.status))) +def test_order_reuse_failed_authz(): + """ + Test that creating an order for a domain name, failing an authorization in + that order, and submitting another new order request for the same name + doesn't reuse a failed authorizaton in the new order. + """ + + client = make_client(None) + domains = [ random_domain() ] + csr_pem = make_csr(domains) + + order = client.new_order(csr_pem) + firstOrderURI = order.uri + + # Pick the first authz's first challenge, doesn't matter what type it is + chall_body = order.authorizations[0].body.challenges[0] + # Answer it, but with nothing set up to solve the challenge request + client.answer_challenge(chall_body, chall_body.response(client.key)) + + # Wait for validation to occur + # TODO(@cpu): This should be handled better but it involves Python + # concurrency and I'm going to punt on that while working on other parts of + # this PR + import time + time.sleep(10) + + # Make another order with the same domains + order = client.new_order(csr_pem) + + # It should not be the same order as before + if order.uri == firstOrderURI: + raise Exception("new-order for %s returned a , now-invalid, order" % domains) + + # We expect all of the returned authorizations to be pending status + for authz in order.authorizations: + if authz.body.status != Status("pending"): + raise Exception("order for %s included a non-pending authorization (status: %s) from a previous order" % + ((domains), str(authz.body.status))) + + # We expect the new order can be fulfilled + cleanup = do_http_challenges(client, order.authorizations) + try: + order = client.poll_order_and_request_issuance(order) + finally: + cleanup() + if __name__ == "__main__": try: main() From e6b3207a6fd9fc3bb5367ac5b5397290c5810c2a Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 19 Jan 2018 15:47:15 -0500 Subject: [PATCH 02/19] Reenable other integration tests --- test/integration-test-v2.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration-test-v2.py b/test/integration-test-v2.py index 7c47f7a8ffa..9d47b984da6 100644 --- a/test/integration-test-v2.py +++ b/test/integration-test-v2.py @@ -29,11 +29,11 @@ def main(): if not startservers.start(race_detection=True): raise Exception("startservers failed") - #test_multidomain() - #test_wildcardmultidomain() - #test_overlapping_wildcard() - #test_wildcard_exactblacklist() - #test_wildcard_authz_reuse() + test_multidomain() + test_wildcardmultidomain() + test_overlapping_wildcard() + test_wildcard_exactblacklist() + test_wildcard_authz_reuse() test_order_reuse_failed_authz() if not startservers.check(): From de55acd6e0a0f447fd170bd735c5f49544cb8bd8 Mon Sep 17 00:00:00 2001 From: Daniel Date: Mon, 22 Jan 2018 12:27:18 -0500 Subject: [PATCH 03/19] Remove hackish unconditional sleep --- test/integration-test-v2.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/test/integration-test-v2.py b/test/integration-test-v2.py index 9d47b984da6..3c7f01fedeb 100644 --- a/test/integration-test-v2.py +++ b/test/integration-test-v2.py @@ -10,6 +10,9 @@ import shutil import subprocess import tempfile +import requests +import datetime +import time import startservers @@ -138,12 +141,19 @@ def test_order_reuse_failed_authz(): # Answer it, but with nothing set up to solve the challenge request client.answer_challenge(chall_body, chall_body.response(client.key)) - # Wait for validation to occur - # TODO(@cpu): This should be handled better but it involves Python - # concurrency and I'm going to punt on that while working on other parts of - # this PR - import time - time.sleep(10) + # Poll for a fixed amount of time checking for the order to become invalid + # from the authorization attempt initiated above failing + deadline = datetime.datetime.now() + datetime.timedelta(seconds=60) + while datetime.datetime.now() < deadline: + time.sleep(1) + updatedOrder = requests.get(firstOrderURI).json() + if updatedOrder['status'] == "invalid": + break + + # If the loop ended and the status isn't invalid then we reached the + # deadline waiting for the order to become invalid, fail the test + if updatedOrder['status'] != "invalid": + raise Exception("timed out waiting for order %s to become invalid" % firstOrderURI) # Make another order with the same domains order = client.new_order(csr_pem) From 4920290af7df9f6e63301bd9337310cd3f194491 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 26 Jan 2018 17:15:08 -0500 Subject: [PATCH 04/19] Calculate order status by assoc. authz status. This commit removes the stored Order status field and replaces it with a value that is calculated on-the-fly by the SA when fetching an order, based on the order's associated authorizations. Credit to Jacob for the idea. In summary: * If any of the order's authorizations are invalid, 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 a certificate serial, the order is valid. * If all of the order's authorizations are valid, and we have began processing, but there is no certificate serial, the order is processing. * If all of the order's authorizations are valid, and we haven't processing, then the order is pending waiting a finalization request. This avoids having to explicitly update the order status when an associated authorization changes status. --- core/proto/core.pb.go | 108 +++--- core/proto/core.proto | 1 + grpc/pb-marshalling.go | 15 +- grpc/pb-marshalling_test.go | 27 +- ra/ra.go | 10 +- ra/ra_test.go | 5 + .../20180124141109_DropOrderStatusField.sql | 11 + sa/model.go | 5 +- sa/sa.go | 292 ++++++++++++----- sa/sa_test.go | 310 +++++++++++++++--- 10 files changed, 575 insertions(+), 209 deletions(-) create mode 100644 sa/_db-next/migrations/20180124141109_DropOrderStatusField.sql diff --git a/core/proto/core.pb.go b/core/proto/core.pb.go index 79d3ac03e68..94b99a9cea2 100644 --- a/core/proto/core.pb.go +++ b/core/proto/core.pb.go @@ -467,6 +467,7 @@ type Order struct { Authorizations []string `protobuf:"bytes,6,rep,name=authorizations" json:"authorizations,omitempty"` Status *string `protobuf:"bytes,7,opt,name=status" json:"status,omitempty"` Names []string `protobuf:"bytes,8,rep,name=names" json:"names,omitempty"` + BeganProcessing *bool `protobuf:"varint,9,opt,name=beganProcessing" json:"beganProcessing,omitempty"` XXX_unrecognized []byte `json:"-"` } @@ -531,6 +532,13 @@ func (m *Order) GetNames() []string { return nil } +func (m *Order) GetBeganProcessing() bool { + if m != nil && m.BeganProcessing != nil { + return *m.BeganProcessing + } + return false +} + type Empty struct { XXX_unrecognized []byte `json:"-"` } @@ -557,53 +565,55 @@ func init() { func init() { proto1.RegisterFile("core/proto/core.proto", fileDescriptor0) } var fileDescriptor0 = []byte{ - // 764 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x74, 0x55, 0xc1, 0x4e, 0xf3, 0x46, - 0x10, 0x96, 0xe3, 0x98, 0xe0, 0x89, 0x1b, 0xc2, 0x2a, 0xa5, 0x56, 0x55, 0x21, 0xcb, 0x07, 0x64, - 0xa1, 0x16, 0x54, 0xde, 0x80, 0x86, 0x22, 0x21, 0x21, 0x15, 0x6d, 0xa0, 0x87, 0xde, 0x8c, 0x3d, - 0x24, 0x5b, 0x1c, 0x6f, 0xb4, 0xbb, 0x41, 0x4d, 0xdf, 0xa1, 0x0f, 0xd2, 0x17, 0xeb, 0xb1, 0x97, - 0xbe, 0x40, 0xb5, 0xbb, 0x76, 0xb2, 0x4e, 0xf8, 0x6f, 0xf3, 0x7d, 0x3b, 0xde, 0x9d, 0x99, 0x6f, - 0x66, 0x0c, 0x5f, 0x17, 0x5c, 0xe0, 0xf5, 0x4a, 0x70, 0xc5, 0xaf, 0xb5, 0x79, 0x65, 0x4c, 0xd2, - 0xd7, 0x76, 0xfa, 0x57, 0x0f, 0xc2, 0xe9, 0x22, 0xaf, 0x2a, 0xac, 0xe7, 0x48, 0x46, 0xd0, 0x63, - 0x65, 0xec, 0x25, 0x5e, 0xe6, 0xd3, 0x1e, 0x2b, 0x09, 0x81, 0xbe, 0xda, 0xac, 0x30, 0xee, 0x25, - 0x5e, 0x16, 0x52, 0x63, 0x93, 0x33, 0x38, 0x92, 0x2a, 0x57, 0x6b, 0x19, 0x1f, 0x19, 0xb6, 0x41, - 0x64, 0x0c, 0xfe, 0x5a, 0xb0, 0x38, 0x34, 0xa4, 0x36, 0xc9, 0x04, 0x02, 0xc5, 0xdf, 0xb1, 0x8e, - 0x7d, 0xc3, 0x59, 0x40, 0x2e, 0x61, 0xfc, 0x8e, 0x9b, 0xdb, 0xb5, 0x5a, 0x70, 0xc1, 0xfe, 0xcc, - 0x15, 0xe3, 0x75, 0x1c, 0x18, 0x87, 0x03, 0x9e, 0xdc, 0xc1, 0xe9, 0x47, 0x5e, 0xb1, 0xd2, 0x20, - 0x81, 0x05, 0x17, 0xa5, 0x8c, 0x21, 0xf1, 0xb3, 0xe1, 0xcd, 0xd9, 0x95, 0xc9, 0xe5, 0xd7, 0xed, - 0x31, 0x35, 0xc7, 0xf4, 0xf0, 0x03, 0x72, 0x09, 0x01, 0x0a, 0xc1, 0x45, 0x3c, 0x48, 0xbc, 0x6c, - 0x78, 0x33, 0xb1, 0x5f, 0x3e, 0x09, 0xfe, 0x5a, 0xe1, 0xf2, 0x0e, 0x55, 0xce, 0x2a, 0x49, 0xad, - 0x4b, 0xfa, 0xaf, 0x07, 0xe3, 0xfd, 0x3b, 0xc9, 0xb7, 0x70, 0xbc, 0xe0, 0x52, 0xd5, 0xf9, 0x12, - 0x4d, 0x71, 0x42, 0xba, 0xc5, 0xba, 0x44, 0x2b, 0x2e, 0x54, 0x5b, 0x22, 0x6d, 0x93, 0xef, 0xe1, - 0x34, 0x2f, 0x4b, 0x81, 0x52, 0xa2, 0xa4, 0x28, 0x79, 0xf5, 0x81, 0x65, 0xec, 0x27, 0x7e, 0x16, - 0xd1, 0xc3, 0x03, 0x92, 0xc0, 0xb0, 0x21, 0x5f, 0x24, 0x96, 0x71, 0x3f, 0xf1, 0xb2, 0x88, 0xba, - 0x94, 0xf1, 0xb0, 0x75, 0x51, 0x0c, 0x65, 0x1c, 0x24, 0x7e, 0x16, 0x52, 0x97, 0xb2, 0xc5, 0xaf, - 0x1a, 0x45, 0xb4, 0x49, 0x2e, 0x60, 0xb4, 0x7d, 0xea, 0x59, 0x30, 0x2c, 0xe3, 0x81, 0x09, 0x60, - 0x8f, 0x4d, 0x7f, 0x87, 0x51, 0xb7, 0x12, 0xfa, 0xb5, 0x95, 0x65, 0x9e, 0xb5, 0xf6, 0x36, 0x61, - 0x97, 0xd2, 0x2d, 0x50, 0x1a, 0xe7, 0x26, 0xeb, 0x06, 0x91, 0x73, 0x80, 0x85, 0x52, 0xab, 0x99, - 0x6d, 0x0f, 0xad, 0x7a, 0x40, 0x1d, 0x26, 0x4d, 0xf5, 0x5b, 0x58, 0xa0, 0x50, 0xec, 0x8d, 0x15, - 0xb9, 0x42, 0x1d, 0x77, 0x89, 0xc2, 0xbc, 0x11, 0x51, 0x6d, 0xa6, 0xf7, 0x70, 0x3a, 0x9b, 0x3e, - 0xdf, 0xa3, 0x2a, 0x16, 0xac, 0x9e, 0x4f, 0x79, 0xfd, 0xc6, 0xe6, 0xe4, 0x47, 0x18, 0x54, 0x7c, - 0x3e, 0x43, 0x25, 0x63, 0xcf, 0xa8, 0xff, 0x8d, 0xd5, 0xd0, 0xf1, 0x7c, 0x34, 0xe7, 0xb4, 0xf5, - 0x4b, 0x7f, 0xe8, 0xdc, 0x63, 0x4f, 0x49, 0x6c, 0xee, 0x79, 0xa1, 0x8f, 0xf6, 0x9e, 0x90, 0xb6, - 0x30, 0xfd, 0xdb, 0x83, 0xe1, 0xd4, 0x09, 0xec, 0x02, 0x46, 0x02, 0xe7, 0x4c, 0x2a, 0x61, 0x1a, - 0xe1, 0xe1, 0xae, 0x99, 0x8a, 0x3d, 0xd6, 0x4c, 0x03, 0x0a, 0x96, 0x6f, 0x4b, 0x61, 0x91, 0x29, - 0x11, 0x9b, 0xa3, 0x54, 0x4d, 0xf3, 0x37, 0xa8, 0x4d, 0xb8, 0xbf, 0x4d, 0x58, 0x7b, 0x32, 0x29, - 0xd7, 0x58, 0x9a, 0x29, 0xf0, 0x69, 0x83, 0x74, 0xac, 0xf8, 0xc7, 0x8a, 0x09, 0xb4, 0x83, 0xe6, - 0xd3, 0x16, 0xa6, 0xff, 0x78, 0x10, 0x51, 0x27, 0x8c, 0x83, 0xb1, 0x1d, 0x83, 0xff, 0x8e, 0x1b, - 0x13, 0x51, 0x44, 0xb5, 0xa9, 0x2f, 0x2b, 0x78, 0xad, 0xf2, 0x42, 0x99, 0x3e, 0x0c, 0x69, 0x0b, - 0x49, 0x06, 0x27, 0x8d, 0x29, 0x9f, 0x04, 0x4a, 0xac, 0x95, 0x09, 0xee, 0x98, 0xee, 0xd3, 0xe4, - 0x3b, 0x08, 0xf3, 0xb9, 0x40, 0x5c, 0x6a, 0x1f, 0x3b, 0xb1, 0x3b, 0x42, 0x9f, 0xb2, 0x9a, 0x29, - 0x96, 0x57, 0x0f, 0x4f, 0x26, 0xe0, 0x88, 0xee, 0x08, 0x7d, 0x5a, 0x08, 0xcc, 0x15, 0x96, 0xb7, - 0xca, 0x8c, 0xa1, 0x4f, 0x77, 0x84, 0xb3, 0x52, 0x8e, 0xdd, 0x95, 0xa2, 0x87, 0xf1, 0xab, 0xee, - 0x42, 0xd8, 0x65, 0x1a, 0x9a, 0x4c, 0xcf, 0x01, 0x58, 0x89, 0xb5, 0x96, 0x0d, 0x45, 0x23, 0x81, - 0xc3, 0x7c, 0x22, 0xa3, 0xff, 0x45, 0x19, 0x6d, 0x04, 0xfd, 0xce, 0x52, 0x73, 0x44, 0x08, 0x3a, - 0x22, 0x90, 0x6b, 0x80, 0xa2, 0xdd, 0x9b, 0x5a, 0x21, 0xdd, 0x95, 0x27, 0xb6, 0x2b, 0xb7, 0xfb, - 0x94, 0x3a, 0x2e, 0x24, 0x85, 0xa8, 0xe0, 0xcb, 0x57, 0x56, 0x9b, 0x37, 0xa5, 0xa9, 0x42, 0x44, - 0x3b, 0x5c, 0xfa, 0x9f, 0x07, 0xc1, 0x2f, 0x42, 0x77, 0xc5, 0xbe, 0xa4, 0x87, 0x89, 0xf4, 0x3e, - 0x4d, 0xc4, 0x09, 0xd8, 0xef, 0x06, 0x3c, 0x69, 0xb7, 0xa0, 0xed, 0x3d, 0x0b, 0xf4, 0xaa, 0x72, - 0xe6, 0x71, 0x66, 0x5b, 0xd9, 0x8a, 0x7b, 0x78, 0x60, 0x96, 0x8a, 0xab, 0x87, 0x4d, 0x3c, 0xa4, - 0x7b, 0xac, 0x53, 0xce, 0x41, 0xa7, 0x9c, 0x13, 0x08, 0xf4, 0xd2, 0xd4, 0x3a, 0xeb, 0xcf, 0x2c, - 0x48, 0x07, 0x10, 0xfc, 0xbc, 0x5c, 0xa9, 0xcd, 0x4f, 0x83, 0xdf, 0x02, 0xf3, 0x6f, 0xfa, 0x3f, - 0x00, 0x00, 0xff, 0xff, 0x57, 0xf3, 0x24, 0x5e, 0xb3, 0x06, 0x00, 0x00, + // 786 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x74, 0x55, 0x51, 0x8e, 0xdb, 0x36, + 0x10, 0x85, 0x2c, 0x2b, 0x5e, 0xcd, 0xaa, 0x9b, 0x5d, 0xc2, 0x4d, 0x85, 0xa2, 0x08, 0x04, 0x7d, + 0x04, 0x42, 0xd0, 0x66, 0xd1, 0xdc, 0x20, 0xf5, 0x36, 0x40, 0x80, 0x00, 0x35, 0xe8, 0x4d, 0x3f, + 0xfa, 0xa7, 0x95, 0x26, 0x32, 0xbb, 0xb2, 0x68, 0x90, 0x74, 0x50, 0xf7, 0x0e, 0x3d, 0x42, 0x0f, + 0xd0, 0x8b, 0xf5, 0xb3, 0x67, 0x28, 0x38, 0x94, 0x6c, 0xca, 0x4e, 0xfe, 0xe6, 0x3d, 0x8e, 0xc8, + 0x99, 0x79, 0x33, 0x23, 0xf8, 0xba, 0x92, 0x0a, 0x6f, 0xb7, 0x4a, 0x1a, 0x79, 0x6b, 0xcd, 0x57, + 0x64, 0xb2, 0xa9, 0xb5, 0xf3, 0xbf, 0x26, 0x10, 0x2f, 0xd6, 0x65, 0xdb, 0x62, 0xd7, 0x20, 0xbb, + 0x82, 0x89, 0xa8, 0xd3, 0x20, 0x0b, 0x8a, 0x90, 0x4f, 0x44, 0xcd, 0x18, 0x4c, 0xcd, 0x7e, 0x8b, + 0xe9, 0x24, 0x0b, 0x8a, 0x98, 0x93, 0xcd, 0x9e, 0xc1, 0x13, 0x6d, 0x4a, 0xb3, 0xd3, 0xe9, 0x13, + 0x62, 0x7b, 0xc4, 0xae, 0x21, 0xdc, 0x29, 0x91, 0xc6, 0x44, 0x5a, 0x93, 0xcd, 0x21, 0x32, 0xf2, + 0x11, 0xbb, 0x34, 0x24, 0xce, 0x01, 0xf6, 0x12, 0xae, 0x1f, 0x71, 0xff, 0x66, 0x67, 0xd6, 0x52, + 0x89, 0x3f, 0x4b, 0x23, 0x64, 0x97, 0x46, 0xe4, 0x70, 0xc6, 0xb3, 0x3b, 0xb8, 0xf9, 0x54, 0xb6, + 0xa2, 0x26, 0xa4, 0xb0, 0x92, 0xaa, 0xd6, 0x29, 0x64, 0x61, 0x71, 0xf9, 0xfa, 0xd9, 0x2b, 0xca, + 0xe5, 0xd7, 0xc3, 0x31, 0xa7, 0x63, 0x7e, 0xfe, 0x01, 0x7b, 0x09, 0x11, 0x2a, 0x25, 0x55, 0x3a, + 0xcb, 0x82, 0xe2, 0xf2, 0xf5, 0xdc, 0x7d, 0xb9, 0x54, 0xf2, 0xa1, 0xc5, 0xcd, 0x1d, 0x9a, 0x52, + 0xb4, 0x9a, 0x3b, 0x97, 0xfc, 0xbf, 0x00, 0xae, 0x4f, 0xef, 0x64, 0xdf, 0xc2, 0xc5, 0x5a, 0x6a, + 0xd3, 0x95, 0x1b, 0xa4, 0xe2, 0xc4, 0xfc, 0x80, 0x6d, 0x89, 0xb6, 0x52, 0x99, 0xa1, 0x44, 0xd6, + 0x66, 0xdf, 0xc3, 0x4d, 0x59, 0xd7, 0x0a, 0xb5, 0x46, 0xcd, 0x51, 0xcb, 0xf6, 0x13, 0xd6, 0x69, + 0x98, 0x85, 0x45, 0xc2, 0xcf, 0x0f, 0x58, 0x06, 0x97, 0x3d, 0xf9, 0x41, 0x63, 0x9d, 0x4e, 0xb3, + 0xa0, 0x48, 0xb8, 0x4f, 0x91, 0x87, 0xab, 0x8b, 0x11, 0xa8, 0xd3, 0x28, 0x0b, 0x8b, 0x98, 0xfb, + 0x94, 0x2b, 0x7e, 0xdb, 0x2b, 0x62, 0x4d, 0xf6, 0x02, 0xae, 0x0e, 0x4f, 0xdd, 0x2b, 0x81, 0x75, + 0x3a, 0xa3, 0x00, 0x4e, 0xd8, 0xfc, 0x77, 0xb8, 0x1a, 0x57, 0xc2, 0xbe, 0xb6, 0x75, 0xcc, 0xbd, + 0xd5, 0xde, 0x25, 0xec, 0x53, 0xb6, 0x05, 0x6a, 0x72, 0xee, 0xb3, 0xee, 0x11, 0x7b, 0x0e, 0xb0, + 0x36, 0x66, 0xbb, 0x72, 0xed, 0x61, 0x55, 0x8f, 0xb8, 0xc7, 0xe4, 0xb9, 0x7d, 0x0b, 0x2b, 0x54, + 0x46, 0x7c, 0x14, 0x55, 0x69, 0xd0, 0xc6, 0x5d, 0xa3, 0xa2, 0x37, 0x12, 0x6e, 0xcd, 0xfc, 0x2d, + 0xdc, 0xac, 0x16, 0xf7, 0x6f, 0xd1, 0x54, 0x6b, 0xd1, 0x35, 0x0b, 0xd9, 0x7d, 0x14, 0x0d, 0xfb, + 0x11, 0x66, 0xad, 0x6c, 0x56, 0x68, 0x74, 0x1a, 0x90, 0xfa, 0xdf, 0x38, 0x0d, 0x3d, 0xcf, 0xf7, + 0x74, 0xce, 0x07, 0xbf, 0xfc, 0x87, 0xd1, 0x3d, 0xee, 0x94, 0xa5, 0x74, 0xcf, 0x07, 0xfe, 0xde, + 0xdd, 0x13, 0xf3, 0x01, 0xe6, 0xff, 0x04, 0x70, 0xb9, 0xf0, 0x02, 0x7b, 0x01, 0x57, 0x0a, 0x1b, + 0xa1, 0x8d, 0xa2, 0x46, 0x78, 0x77, 0xd7, 0x4f, 0xc5, 0x09, 0x4b, 0xd3, 0x80, 0x4a, 0x94, 0x87, + 0x52, 0x38, 0x44, 0x25, 0x12, 0x0d, 0x6a, 0xd3, 0x37, 0x7f, 0x8f, 0x86, 0x84, 0xa7, 0x87, 0x84, + 0xad, 0xa7, 0xd0, 0x7a, 0x87, 0x35, 0x4d, 0x41, 0xc8, 0x7b, 0x64, 0x63, 0xc5, 0x3f, 0xb6, 0x42, + 0xa1, 0x1b, 0xb4, 0x90, 0x0f, 0x30, 0xff, 0x37, 0x80, 0x84, 0x7b, 0x61, 0x9c, 0x8d, 0xed, 0x35, + 0x84, 0x8f, 0xb8, 0xa7, 0x88, 0x12, 0x6e, 0x4d, 0x7b, 0x59, 0x25, 0x3b, 0x53, 0x56, 0x86, 0xfa, + 0x30, 0xe6, 0x03, 0x64, 0x05, 0x3c, 0xed, 0x4d, 0xbd, 0x54, 0xa8, 0xb1, 0x33, 0x14, 0xdc, 0x05, + 0x3f, 0xa5, 0xd9, 0x77, 0x10, 0x97, 0x8d, 0x42, 0xdc, 0x58, 0x1f, 0x37, 0xb1, 0x47, 0xc2, 0x9e, + 0x8a, 0x4e, 0x18, 0x51, 0xb6, 0xef, 0x96, 0x14, 0x70, 0xc2, 0x8f, 0x84, 0x3d, 0xad, 0x14, 0x96, + 0x06, 0xeb, 0x37, 0x86, 0xc6, 0x30, 0xe4, 0x47, 0xc2, 0x5b, 0x29, 0x17, 0xfe, 0x4a, 0xb1, 0xc3, + 0xf8, 0xd5, 0x78, 0x21, 0x1c, 0x33, 0x8d, 0x29, 0xd3, 0xe7, 0x00, 0xa2, 0xc6, 0xce, 0xca, 0x86, + 0xaa, 0x97, 0xc0, 0x63, 0x3e, 0x23, 0x63, 0xf8, 0x45, 0x19, 0x5d, 0x04, 0xd3, 0xd1, 0x52, 0xf3, + 0x44, 0x88, 0x46, 0x22, 0xb0, 0x5b, 0x80, 0x6a, 0xd8, 0x9b, 0x56, 0x21, 0xdb, 0x95, 0x4f, 0x5d, + 0x57, 0x1e, 0xf6, 0x29, 0xf7, 0x5c, 0x58, 0x0e, 0x49, 0x25, 0x37, 0x0f, 0xa2, 0xa3, 0x37, 0x35, + 0x55, 0x21, 0xe1, 0x23, 0x2e, 0xff, 0x7b, 0x02, 0xd1, 0x2f, 0xca, 0x76, 0xc5, 0xa9, 0xa4, 0xe7, + 0x89, 0x4c, 0x3e, 0x9b, 0x88, 0x17, 0x70, 0x38, 0x0e, 0x78, 0x3e, 0x6c, 0x41, 0xd7, 0x7b, 0x0e, + 0xd8, 0x55, 0xe5, 0xcd, 0xe3, 0xca, 0xb5, 0xb2, 0x13, 0xf7, 0xfc, 0x80, 0x96, 0x8a, 0xaf, 0x87, + 0x4b, 0x3c, 0xe6, 0x27, 0xac, 0x57, 0xce, 0xd9, 0xa8, 0x9c, 0x73, 0x88, 0xec, 0xd2, 0xb4, 0x3a, + 0xdb, 0xcf, 0x1c, 0xb0, 0x2d, 0xf8, 0x80, 0x4d, 0xd9, 0x2d, 0x95, 0xac, 0x50, 0x6b, 0xd1, 0x35, + 0xf4, 0x17, 0xb9, 0xe0, 0xa7, 0x74, 0x3e, 0x83, 0xe8, 0xe7, 0xcd, 0xd6, 0xec, 0x7f, 0x9a, 0xfd, + 0x16, 0xd1, 0x5f, 0xec, 0xff, 0x00, 0x00, 0x00, 0xff, 0xff, 0xa1, 0xbc, 0xed, 0xcb, 0xdd, 0x06, + 0x00, 0x00, } diff --git a/core/proto/core.proto b/core/proto/core.proto index 2ed2ad99824..01c11a4964d 100644 --- a/core/proto/core.proto +++ b/core/proto/core.proto @@ -85,6 +85,7 @@ message Order { repeated string authorizations = 6; optional string status = 7; repeated string names = 8; + optional bool beganProcessing = 9; } message Empty {} diff --git a/grpc/pb-marshalling.go b/grpc/pb-marshalling.go index 9b32ad50f1a..3da3b44e628 100644 --- a/grpc/pb-marshalling.go +++ b/grpc/pb-marshalling.go @@ -407,19 +407,20 @@ func registrationValid(reg *corepb.Registration) bool { } // orderValid checks that a corepb.Order is valid. In addition to the checks -// from `newOrderValid` it ensures the order ID is not nil. +// from `newOrderValid` it ensures the order ID and the BeganProcessing fields +// are not nil. func orderValid(order *corepb.Order) bool { - return order.Id != nil && newOrderValid(order) + return order.Id != nil && order.BeganProcessing != nil && newOrderValid(order) } // newOrderValid checks that a corepb.Order is valid. It allows for a nil // `order.Id` because the order has not been assigned an ID yet when it is being -// created initially. It also allows `order.CertificateSerial` to be nil such -// that it can be used in places where the order has not been finalized yet. -// Callers must additionally ensure the `CertificateSerial` field is non-nil if -// they intend to use it. +// created initially. It allows `order.BeganProcessing` to be nil because +// `sa.NewOrder` explicitly sets it to the default value. It also allows +// `order.CertificateSerial` to be nil such that it can be used in places where +// the order has not been finalized yet. func newOrderValid(order *corepb.Order) bool { - return !(order.RegistrationID == nil || order.Expires == nil || order.Authorizations == nil || order.Status == nil || order.Names == nil) + return !(order.RegistrationID == nil || order.Expires == nil || order.Authorizations == nil || order.Names == nil) } func authorizationValid(authz *corepb.Authorization) bool { diff --git a/grpc/pb-marshalling_test.go b/grpc/pb-marshalling_test.go index 011fd468afd..f0cf67e9de9 100644 --- a/grpc/pb-marshalling_test.go +++ b/grpc/pb-marshalling_test.go @@ -339,6 +339,7 @@ func TestOrderValid(t *testing.T) { testID := int64(1) testExpires := int64(1) emptyString := "" + falseBool := false testCases := []struct { Name string @@ -353,20 +354,20 @@ func TestOrderValid(t *testing.T) { Expires: &testExpires, CertificateSerial: &emptyString, Authorizations: []string{}, - Status: &emptyString, Names: []string{}, + BeganProcessing: &falseBool, }, ExpectedValid: true, }, { Name: "Serial nil", Order: &corepb.Order{ - Id: &testID, - RegistrationID: &testID, - Expires: &testExpires, - Authorizations: []string{}, - Status: &emptyString, - Names: []string{}, + Id: &testID, + RegistrationID: &testID, + Expires: &testExpires, + Authorizations: []string{}, + Names: []string{}, + BeganProcessing: &falseBool, }, ExpectedValid: true, }, @@ -381,8 +382,8 @@ func TestOrderValid(t *testing.T) { Expires: &testExpires, CertificateSerial: &emptyString, Authorizations: []string{}, - Status: &emptyString, Names: []string{}, + BeganProcessing: &falseBool, }, }, { @@ -392,8 +393,8 @@ func TestOrderValid(t *testing.T) { Expires: &testExpires, CertificateSerial: &emptyString, Authorizations: []string{}, - Status: &emptyString, Names: []string{}, + BeganProcessing: &falseBool, }, }, { @@ -403,8 +404,8 @@ func TestOrderValid(t *testing.T) { RegistrationID: &testID, CertificateSerial: &emptyString, Authorizations: []string{}, - Status: &emptyString, Names: []string{}, + BeganProcessing: &falseBool, }, }, { @@ -414,12 +415,12 @@ func TestOrderValid(t *testing.T) { RegistrationID: &testID, Expires: &testExpires, CertificateSerial: &emptyString, - Status: &emptyString, Names: []string{}, + BeganProcessing: &falseBool, }, }, { - Name: "Status nil", + Name: "BeganProcessing nil", Order: &corepb.Order{ Id: &testID, RegistrationID: &testID, @@ -437,7 +438,7 @@ func TestOrderValid(t *testing.T) { Expires: &testExpires, CertificateSerial: &emptyString, Authorizations: []string{}, - Status: &emptyString, + BeganProcessing: &falseBool, }, }, } diff --git a/ra/ra.go b/ra/ra.go index 02f3938865c..a8e0838ff99 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -1582,12 +1582,10 @@ func (ra *RegistrationAuthorityImpl) DeactivateAuthorization(ctx context.Context // NewOrder creates a new order object func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.NewOrderRequest) (*corepb.Order, error) { expires := ra.clk.Now().Add(ra.orderLifetime).UnixNano() - status := string(core.StatusPending) order := &corepb.Order{ RegistrationID: req.RegistrationID, Names: core.UniqueLowerNames(req.Names), Expires: &expires, - Status: &status, } // Check that the registration ID in question has rate limit space for another @@ -1657,10 +1655,9 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New } authz := nameToExistingAuthz[name] // If the existing authz isn't valid, note that its missing and continue. - // Since we reuse whole pending orders there isn't any benefit to reusing - // individual pending authorizations between orders. Worse, it makes the - // process of invalidating an order when an authorization is invalidated - // confusing because multiple orders would be invalidated simultaneously. + // Reusing pending authorizations between orders is primarily an effort to + // prevent clients hitting the pending authz limit, but in a V2 world order + // reuse accomplishes the same thing. if *authz.Status != string(core.StatusValid) { missingAuthzNames = append(missingAuthzNames, name) continue @@ -1717,6 +1714,7 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New } storedOrder, err := ra.SA.NewOrder(ctx, order) + fmt.Printf("ra.SA.NewOrder err: %#v\n", err) if err != nil { return nil, err } diff --git a/ra/ra_test.go b/ra/ra_test.go index 3df1c5395bf..ece199fd5a8 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -1238,6 +1238,11 @@ func TestNewOrderRateLimiting(t *testing.T) { validCSRBlock, _ := pem.Decode(CSRPEM) validCSR, _ := x509.ParseCertificateRequest(validCSRBlock.Bytes) + // Artificially set the order's status to pending since it didn't come from + // a sa.GetOrder call that populates the field. + pendingStatus := string(core.StatusPending) + order.Status = &pendingStatus + // Finalize the order with a CSR to change it from pending status to valid status _, err = ra.FinalizeOrder(ctx, &rapb.FinalizeOrderRequest{ Order: order, diff --git a/sa/_db-next/migrations/20180124141109_DropOrderStatusField.sql b/sa/_db-next/migrations/20180124141109_DropOrderStatusField.sql new file mode 100644 index 00000000000..549542adfc3 --- /dev/null +++ b/sa/_db-next/migrations/20180124141109_DropOrderStatusField.sql @@ -0,0 +1,11 @@ + +-- +goose Up +-- SQL in section 'Up' is executed when this migration is applied + +ALTER TABLE `orders` DROP COLUMN `status`; +ALTER TABLE `orders` ADD COLUMN `beganProcessing` BOOL NOT NULL DEFAULT FALSE; + +-- +goose Down +-- SQL section 'Down' is executed when this migration is rolled back + +ALTER TABLE `orders` ADD COLUMN `status` varchar(255) NOT NULL; diff --git a/sa/model.go b/sa/model.go index 3f11afe4894..8cf5e99be72 100644 --- a/sa/model.go +++ b/sa/model.go @@ -312,7 +312,7 @@ type orderModel struct { Expires time.Time Error []byte CertificateSerial string - Status core.AcmeStatus + BeganProcessing bool } type requestedNameModel struct { @@ -328,13 +328,12 @@ type orderToAuthzModel struct { func modelToOrder(om *orderModel) *corepb.Order { expires := om.Expires.UnixNano() - status := string(om.Status) return &corepb.Order{ Id: &om.ID, RegistrationID: &om.RegistrationID, Expires: &expires, Error: om.Error, CertificateSerial: &om.CertificateSerial, - Status: &status, + BeganProcessing: &om.BeganProcessing, } } diff --git a/sa/sa.go b/sa/sa.go index ffb91d8fc9e..9639a7eb1ab 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -832,30 +832,6 @@ func (ssa *SQLStorageAuthority) FinalizeAuthorization(ctx context.Context, authz return Rollback(tx, err) } - // When an authorization is being finalized to an invalid state we need to see - // if there is an order associated with this authorization that itself should - // now become invalid as a result of the authz being invalid. - if authz.Status == core.StatusInvalid { - // Try to find an order associated with this authz ID. There may not be one if - // this is a legacy V1 authorization from the new-authz endpoint. - orderID, err := ssa.orderIDForAuthz(tx, authz.ID) - // If there was an error, and it wasn't a no-result error, then we encountered - // something unexpected and must rollback - if err != nil && err != sql.ErrNoRows { - return Rollback(tx, err) - } - // If the err was nil then orderID is an associated order for this authz - if err == nil { - // Set the order to invalid - err := ssa.setOrderInvalid(ctx, tx, &corepb.Order{ - Id: &orderID, - }) - if err != nil { - return Rollback(tx, err) - } - } - } - return tx.Commit() } @@ -1002,17 +978,36 @@ func (ssa *SQLStorageAuthority) CountPendingAuthorizations(ctx context.Context, // orders for the given registration. func (ssa *SQLStorageAuthority) CountPendingOrders(ctx context.Context, regID int64) (int, error) { var count int - err := ssa.dbMap.SelectOne(&count, - `SELECT count(1) FROM orders + + // Find all of the unexpired order IDs for the given account + var orderIDs []int64 + _, err := ssa.dbMap.Select(&orderIDs, + `SELECT ID FROM orders WHERE registrationID = :regID AND - expires > :now AND - status = :pending`, + expires > :now`, map[string]interface{}{ - "regID": regID, - "now": ssa.clk.Now(), - "pending": string(core.StatusPending), + "regID": regID, + "now": ssa.clk.Now(), }) - return count, err + if err != nil { + return 0, err + } + + // Iterate the order IDs, fetching the full order & associated authorizations + // for each + for _, orderID := range orderIDs { + order, err := ssa.GetOrder(ctx, &sapb.OrderRequest{Id: &orderID}) + if err != nil { + return 0, err + } + + // If the order is pending, increment the pending count + if *order.Status == string(core.StatusPending) { + count++ + } + } + + return count, nil } // CountInvalidAuthorizations counts invalid authorizations for a user expiring @@ -1401,7 +1396,6 @@ func (ssa *SQLStorageAuthority) NewOrder(ctx context.Context, req *corepb.Order) order := &orderModel{ RegistrationID: *req.RegistrationID, Expires: time.Unix(0, *req.Expires), - Status: core.AcmeStatus(*req.Status), } tx, err := ssa.dbMap.Begin() @@ -1445,13 +1439,19 @@ func (ssa *SQLStorageAuthority) NewOrder(ctx context.Context, req *corepb.Order) // Update the request with the ID that the order received req.Id = &order.ID + + // Update the request with pending status (No need to calculate the status + // based on authzs here, we know a brand new order is always pending) + pendingStatus := string(core.StatusPending) + req.Status = &pendingStatus + processingStatus := false + req.BeganProcessing = &processingStatus return req, nil } // SetOrderProcessing updates a provided *corepb.Order in pending status to be -// in processing status by updating the status field of the corresponding Order -// table row in the DB. We avoid introducing a general purpose "Update this -// order" RPC to ensure we have minimally permissive RPCs. +// in processing status by updating the `beganProcessing` field of the +// corresponding Order table row in the DB. func (ssa *SQLStorageAuthority) SetOrderProcessing(ctx context.Context, req *corepb.Order) error { tx, err := ssa.dbMap.Begin() if err != nil { @@ -1460,49 +1460,26 @@ func (ssa *SQLStorageAuthority) SetOrderProcessing(ctx context.Context, req *cor result, err := tx.Exec(` UPDATE orders - SET status = ? + SET beganProcessing = ? WHERE id = ? - AND status = ?`, - string(core.StatusProcessing), + AND beganProcessing = ?`, + true, *req.Id, - string(core.StatusPending)) + false) if err != nil { - err = berrors.InternalServerError("error updating order to processing status") + err = berrors.InternalServerError("error updating order to beganProcessing status") return Rollback(tx, err) } n, err := result.RowsAffected() if err != nil || n == 0 { - err = berrors.InternalServerError("no order updated to processing status") + err = berrors.InternalServerError("no order updated to beganProcessing status") return Rollback(tx, err) } return tx.Commit() } -// setOrderInvalid updates a provided *corepb.Order in pending status to be in -// invalid status by updating the status field of the corresponding row in the -// DB. -func (ssa *SQLStorageAuthority) setOrderInvalid(ctx context.Context, tx *gorp.Transaction, req *corepb.Order) error { - result, err := tx.Exec(` - UPDATE orders - SET status = ? - WHERE id = ? - AND status = ?`, - string(core.StatusInvalid), - *req.Id, - string(core.StatusPending)) - if err != nil { - return err - } - - n, err := result.RowsAffected() - if err != nil || n == 0 { - return berrors.InternalServerError("no order updated to invalid status") - } - return nil -} - // FinalizeOrder finalizes a provided *corepb.Order by persisting the // CertificateSerial and a valid status to the database. No fields other than // CertificateSerial and the order ID on the provided order are processed (e.g. @@ -1515,13 +1492,11 @@ func (ssa *SQLStorageAuthority) FinalizeOrder(ctx context.Context, req *corepb.O result, err := tx.Exec(` UPDATE orders - SET certificateSerial = ?, status = ? - WHERE id = ? - AND status = ?`, + SET certificateSerial = ? + WHERE id = ? AND + beganProcessing = true`, *req.CertificateSerial, - string(core.StatusValid), - *req.Id, - string(core.StatusProcessing)) + *req.Id) if err != nil { err = berrors.InternalServerError("error updating order for finalization") return Rollback(tx, err) @@ -1551,26 +1526,6 @@ func (ssa *SQLStorageAuthority) authzForOrder(orderID int64) ([]string, error) { return ids, nil } -// orderForAuthz finds an order ID associated with a given authz (If any). -func (ssa *SQLStorageAuthority) orderIDForAuthz(tx *gorp.Transaction, authzID string) (int64, error) { - var orderID int64 - - err := tx.SelectOne(&orderID, ` - SELECT orderID - FROM orderToAuthz - WHERE authzID = ?`, - authzID) - - // NOTE(@cpu): orderIDForAuthz does not handle the sql.ErrNoRows that could be - // returned by `SelectOne`. The caller must do so. - if err != nil { - // If there is an err, return it as-is. - return 0, err - } - - return orderID, nil -} - // namesForOrder finds all of the requested names associated with an order. The // names are returned in their reversed form (see `sa.ReverseName`). func (ssa *SQLStorageAuthority) namesForOrder(orderID int64) ([]string, error) { @@ -1616,9 +1571,164 @@ func (ssa *SQLStorageAuthority) GetOrder(ctx context.Context, req *sapb.OrderReq } order.Names = reversedNames + // Calculate the status for the order + status, err := ssa.statusForOrder(ctx, order) + if err != nil { + return nil, err + } + order.Status = &status + return order, nil } +// statusForOrder examines the status of a provided order's authorizations to +// determine what the overall status of the order should be. In summary: +// * If any of the order's authorizations are invalid, 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 +// a certificate serial, the order is valid. +// * If all of the order's authorizations are valid, and we have began +// processing, but there is no certificate serial, the order is processing. +// * If all of the order's authorizations are valid, and we haven't begun +// processing, then the order is pending waiting a finalization request. +// An error is returned for any other case. +func (ssa *SQLStorageAuthority) statusForOrder(ctx context.Context, order *corepb.Order) (string, error) { + // Get the full Authorization objects for the order + authzs, err := ssa.getAllOrderAuthorizations(ctx, *order.Id, *order.RegistrationID) + // If there was an error getting the authorizations, return it immediately + if err != nil { + return "", err + } + + // If GetOrderAuthorizations returned a different number of authorization + // objects than the order's slice of authorization IDs something has gone + // wrong worth raising an internal error about. + if len(authzs) != len(order.Authorizations) { + return "", berrors.InternalServerError( + "GetOrderAuthorizations returned the wrong number of authorizations "+ + "(%d vs expected %d) for order %d", + len(authzs), len(order.Authorizations), *order.Id) + } + + // Keep a count of the authorizations seen + invalidAuthzs := 0 + deactivatedAuthzs := 0 + pendingAuthzs := 0 + validAuthzs := 0 + + // Loop over each of the order's authorization objects to examine the authz status + for _, authz := range authzs { + switch authz.Status { + case core.StatusInvalid: + invalidAuthzs++ + case core.StatusDeactivated: + deactivatedAuthzs++ + case core.StatusPending: + pendingAuthzs++ + case core.StatusValid: + validAuthzs++ + default: + return "", berrors.InternalServerError( + "Order is in an invalid state. Authz %s has invalid status %q", + authz.ID, authz.Status) + } + } + + // An order is invalid if **any** of its authzs are invalid + if invalidAuthzs > 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 + } + // An order is pending if **any** of its authzs are pending + if pendingAuthzs > 0 { + return string(core.StatusPending), nil + } + + // An order is fully authorizsed if it has valid authzs for each of the order + // names + fullyAuthorized := len(order.Names) == validAuthzs + + // If the order isn't fully authorized we've encountered an internal error: + // Above we checked for any invalid or pending authzs and should have returned + // early. Somehow we made it this far but also don't have the correct number + // of valid authzs. + if !fullyAuthorized { + return "", berrors.InternalServerError( + "Order has the incorrect number of valid authorizations & no pending, " + + "deactivated or invalid authorizations") + } + + // If the order is fully authorized and the certificate serial is set then the + // order is valid + if fullyAuthorized && order.CertificateSerial != nil && *order.CertificateSerial != "" { + return string(core.StatusValid), nil + } + + // If the order is fully authorized, and we have began processing it, then the + // order is processing. + if fullyAuthorized && order.BeganProcessing != nil && *order.BeganProcessing { + return string(core.StatusProcessing), nil + } + + // If the order is fully authorized, and we haven't begun processing it, then + // the order is still pending waiting a finalization request. + if fullyAuthorized && order.BeganProcessing != nil && !*order.BeganProcessing { + return string(core.StatusPending), nil + } + + return "", berrors.InternalServerError( + "Order %d is in an invalid state. No state known for this order's "+ + "authorizations", *order.Id) +} + +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 { + var authzs []*core.Authorization + _, err := ssa.dbMap.Select( + &authzs, + fmt.Sprintf(`SELECT %s from %s AS authz + LEFT 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 + } + + allAuthzs = append(allAuthzs, authzs...) + } + + // Collapse & dedupe the returned authorizations into a mapping from name to + // authorization + byName := make(map[string]*core.Authorization) + for _, auth := range allAuthzs { + // We only expect to get back DNS identifiers + if auth.Identifier.Type != core.IdentifierDNS { + return nil, fmt.Errorf("unknown identifier type: %q on authz id %q", auth.Identifier.Type, auth.ID) + } + existing, present := byName[auth.Identifier.Value] + if !present || auth.Expires.After(*existing.Expires) { + byName[auth.Identifier.Value] = auth + } + } + return byName, nil +} + +// TODO(@cpu): Rename this to `GetValidOrderAuthorizations` // GetOrderAuthorizations is used to find the valid, unexpired authorizations // associated with a specific order and account ID. func (ssa *SQLStorageAuthority) GetOrderAuthorizations( diff --git a/sa/sa_test.go b/sa/sa_test.go index 7790af97e58..21661dd05cf 100644 --- a/sa/sa_test.go +++ b/sa/sa_test.go @@ -1377,7 +1377,7 @@ func TestSetOrderProcessing(t *testing.T) { return } - sa, _, cleanup := initSA(t) + sa, fc, cleanup := initSA(t) defer cleanup() // Create a test registration to reference @@ -1387,14 +1387,28 @@ func TestSetOrderProcessing(t *testing.T) { }) test.AssertNotError(t, err, "Couldn't create test registration") + // Add one pending authz + authzExpires := fc.Now().Add(time.Hour) + newAuthz := core.Authorization{ + Identifier: core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "example.com"}, + RegistrationID: reg.ID, + Status: core.StatusPending, + Expires: &authzExpires, + } + authz, err := sa.NewPendingAuthorization(ctx, newAuthz) + test.AssertNotError(t, err, "Couldn't create new pending authorization") + + // Update the pending authz to be valid + authz.Status = core.StatusValid + err = sa.FinalizeAuthorization(ctx, authz) + test.AssertNotError(t, err, "Couldn't finalize pending authz to valid") + i := int64(1337) - status := string(core.StatusPending) order := &corepb.Order{ RegistrationID: ®.ID, Expires: &i, Names: []string{"example.com"}, - Authorizations: []string{"a", "b", "c"}, - Status: &status, + Authorizations: []string{authz.ID}, } // Add a new order in pending status with no certificate serial @@ -1412,6 +1426,7 @@ func TestSetOrderProcessing(t *testing.T) { &sapb.OrderRequest{Id: order.Id}) test.AssertNotError(t, err, "GetOrder failed") test.AssertEquals(t, *updatedOrder.Status, string(core.StatusProcessing)) + test.AssertEquals(t, *updatedOrder.BeganProcessing, true) } func TestFinalizeOrder(t *testing.T) { @@ -1421,7 +1436,7 @@ func TestFinalizeOrder(t *testing.T) { return } - sa, _, cleanup := initSA(t) + sa, fc, cleanup := initSA(t) defer cleanup() // Create a test registration to reference @@ -1431,20 +1446,38 @@ func TestFinalizeOrder(t *testing.T) { }) test.AssertNotError(t, err, "Couldn't create test registration") + // Add one pending authz + authzExpires := fc.Now().Add(time.Hour) + newAuthz := core.Authorization{ + Identifier: core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "example.com"}, + RegistrationID: reg.ID, + Status: core.StatusPending, + Expires: &authzExpires, + } + authz, err := sa.NewPendingAuthorization(ctx, newAuthz) + test.AssertNotError(t, err, "Couldn't create new pending authorization") + + // Set the authz to valid + authz.Status = core.StatusValid + err = sa.FinalizeAuthorization(ctx, authz) + test.AssertNotError(t, err, "Couldn't finalize pending authorization") + i := int64(1337) - status := string(core.StatusProcessing) order := &corepb.Order{ RegistrationID: ®.ID, Expires: &i, Names: []string{"example.com"}, - Authorizations: []string{"a", "b", "c"}, - Status: &status, + Authorizations: []string{authz.ID}, } - // Add a new order in processing status with an empty certificate serial + // Add a new order with an empty certificate serial order, err = sa.NewOrder(context.Background(), order) test.AssertNotError(t, err, "NewOrder failed") + // Set the order to processing so it can be finalized + err = sa.SetOrderProcessing(ctx, order) + test.AssertNotError(t, err, "SetOrderProcessing failed") + // Finalize the order with a certificate serial serial := "eat.serial.for.breakfast" order.CertificateSerial = &serial @@ -1468,7 +1501,7 @@ func TestOrder(t *testing.T) { return } - sa, _, cleanup := initSA(t) + sa, fc, cleanup := initSA(t) defer cleanup() // Create a test registration to reference @@ -1478,23 +1511,51 @@ func TestOrder(t *testing.T) { }) test.AssertNotError(t, err, "Couldn't create test registration") + // Add one pending authz + authzExpires := fc.Now().Add(time.Hour) + newAuthz := core.Authorization{ + Identifier: core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "example.com"}, + RegistrationID: reg.ID, + Status: core.StatusPending, + Expires: &authzExpires, + } + authz, err := sa.NewPendingAuthorization(ctx, newAuthz) + test.AssertNotError(t, err, "Couldn't create new pending authorization") + expires := time.Now().Truncate(time.Second).UnixNano() - status := string(core.StatusPending) empty := "" - order, err := sa.NewOrder(context.Background(), &corepb.Order{ - RegistrationID: ®.ID, - Expires: &expires, - Names: []string{"example.com"}, - Authorizations: []string{"a"}, - Status: &status, - CertificateSerial: &empty, - }) + + inputOrder := &corepb.Order{ + RegistrationID: ®.ID, + Expires: &expires, + Names: []string{"example.com"}, + Authorizations: []string{authz.ID}, + } + + order, err := sa.NewOrder(context.Background(), inputOrder) test.AssertNotError(t, err, "sa.NewOrder failed") test.AssertEquals(t, *order.Id, int64(1)) + pendingStatus := string(core.StatusPending) + falseBool := false + one := int64(1) + expectedOrder := &corepb.Order{ + // The expected order matches the input order + RegistrationID: inputOrder.RegistrationID, + Expires: inputOrder.Expires, + Names: inputOrder.Names, + Authorizations: inputOrder.Authorizations, + // And should also have an empty serial and the correct status and + // processing state, and an ID of 1 + Id: &one, + CertificateSerial: &empty, + BeganProcessing: &falseBool, + Status: &pendingStatus, + } + storedOrder, err := sa.GetOrder(context.Background(), &sapb.OrderRequest{Id: order.Id}) test.AssertNotError(t, err, "sa.Order failed") - test.AssertDeepEquals(t, storedOrder, order) + test.AssertDeepEquals(t, storedOrder, expectedOrder) } func TestGetOrderAuthorizations(t *testing.T) { @@ -1667,21 +1728,30 @@ func TestCountPendingOrders(t *testing.T) { defer cleanUp() reg := satest.CreateWorkingRegistration(t, sa) - expires := fc.Now().Add(time.Hour).UnixNano() - status := string(core.StatusPending) + authzExpires := fc.Now().Add(time.Hour) + expires := authzExpires.UnixNano() // Counting pending orders for a reg ID that doesn't exist should return 0 count, err := sa.CountPendingOrders(ctx, 12345) test.AssertNotError(t, err, "Couldn't count pending authorizations for fake reg ID") test.AssertEquals(t, count, 0) + // Add one pending authz + newAuthz := core.Authorization{ + Identifier: core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "example.com"}, + RegistrationID: reg.ID, + Status: core.StatusPending, + Expires: &authzExpires, + } + pendingAuthz, err := sa.NewPendingAuthorization(ctx, newAuthz) + test.AssertNotError(t, err, "Couldn't create new pending authorization") + // Add one pending order _, err = sa.NewOrder(ctx, &corepb.Order{ RegistrationID: ®.ID, Expires: &expires, Names: []string{"example.com"}, - Authorizations: []string{"abcd"}, - Status: &status, + Authorizations: []string{pendingAuthz.ID}, }) test.AssertNotError(t, err, "Couldn't create new pending order") @@ -1690,14 +1760,17 @@ func TestCountPendingOrders(t *testing.T) { test.AssertNotError(t, err, "Couldn't count pending authorizations") test.AssertEquals(t, count, 1) + // Create another fresh pending authz + secondPendingAuthz, err := sa.NewPendingAuthorization(ctx, newAuthz) + test.AssertNotError(t, err, "Couldn't create 2nd new pending authorization") + // Create a pending order that expired an hour ago expires = fc.Now().Add(-time.Hour).UnixNano() _, err = sa.NewOrder(ctx, &corepb.Order{ RegistrationID: ®.ID, Expires: &expires, Names: []string{"example.com"}, - Authorizations: []string{"abcd"}, - Status: &status, + Authorizations: []string{secondPendingAuthz.ID}, }) test.AssertNotError(t, err, "Couldn't create new expired pending order") @@ -1707,15 +1780,20 @@ func TestCountPendingOrders(t *testing.T) { test.AssertNotError(t, err, "Couldn't count pending authorizations") test.AssertEquals(t, count, 1) - // Create a non-pending order + // Create another fresh pending authz + thirdPendingAuthz, err := sa.NewPendingAuthorization(ctx, newAuthz) + test.AssertNotError(t, err, "Couldn't create third new pending authorization") + // And then deactivate it + err = sa.DeactivateAuthorization(ctx, thirdPendingAuthz.ID) + test.AssertNotError(t, err, "Couldn't deactivate third new pending authorization") + + // Create a deactivated order by referencing the deactivated authz expires = fc.Now().Add(time.Hour).UnixNano() - status = "off-the-hook" _, err = sa.NewOrder(ctx, &corepb.Order{ RegistrationID: ®.ID, Expires: &expires, Names: []string{"example.com"}, - Authorizations: []string{"abcd"}, - Status: &status, + Authorizations: []string{thirdPendingAuthz.ID}, }) test.AssertNotError(t, err, "Couldn't create new non-pending order") @@ -1755,8 +1833,28 @@ func TestGetOrderForNames(t *testing.T) { }) test.AssertNotError(t, err, "Couldn't create test registration") + // Add one pending authz for the first name for regA + authzExpires := fc.Now().Add(time.Hour) + newAuthzA := core.Authorization{ + Identifier: core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "example.com"}, + RegistrationID: regA.ID, + Status: core.StatusPending, + Expires: &authzExpires, + } + pendingAuthzA, err := sa.NewPendingAuthorization(ctx, newAuthzA) + test.AssertNotError(t, err, "Couldn't create new pending authorization for regA") + + // Add one pending authz for the second name for regA + newAuthzB := core.Authorization{ + Identifier: core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "just.another.example.com"}, + RegistrationID: regA.ID, + Status: core.StatusPending, + Expires: &authzExpires, + } + pendingAuthzB, err := sa.NewPendingAuthorization(ctx, newAuthzB) + test.AssertNotError(t, err, "Couldn't create new pending authorization for regA") + ctx := context.Background() - status := string(core.StatusPending) names := []string{"example.com", "just.another.example.com"} // Call GetOrderForNames for a set of names we haven't created an order for @@ -1776,8 +1874,7 @@ func TestGetOrderForNames(t *testing.T) { order, err := sa.NewOrder(ctx, &corepb.Order{ RegistrationID: ®A.ID, Expires: &expires, - Status: &status, - Authorizations: []string{"a", "b", "c"}, + Authorizations: []string{pendingAuthzA.ID, pendingAuthzB.ID}, Names: names, }) // It shouldn't error @@ -1827,16 +1924,16 @@ func TestGetOrderForNames(t *testing.T) { // to return expired orders test.Assert(t, result == nil, "sa.GetOrderForNames returned non-nil result for expired order case") - // Add a fresh order for a different of names. Put its status as Processing so - // we can finalize it + // Add a fresh order for a different of names. expires = fc.Now().Add(orderLifetime).UnixNano() - statusProcessing := string(core.StatusProcessing) names = []string{"zombo.com", "welcome.to.zombo.com"} order, err = sa.NewOrder(ctx, &corepb.Order{ RegistrationID: ®A.ID, Expires: &expires, - Status: &statusProcessing, - Authorizations: []string{"a", "b", "c"}, + // We can use the same pending authz's as above - it doesn't matter that + // they are for different names for this test, we just need some authz IDs + // that exist. + Authorizations: []string{pendingAuthzA.ID, pendingAuthzB.ID}, Names: names, }) // It shouldn't error @@ -1844,6 +1941,10 @@ func TestGetOrderForNames(t *testing.T) { // The order ID shouldn't be nil test.AssertNotNil(t, *order.Id, "NewOrder returned with a nil Id") + // Set the order processing so it can be finalized + err = sa.SetOrderProcessing(ctx, order) + test.AssertNotError(t, err, "sa.SetOrderProcessing failed") + // Finalize the order serial := "cinnamon toast crunch" order.CertificateSerial = &serial @@ -1884,6 +1985,7 @@ func TestUpdatePendingAuthorizationInvalidOrder(t *testing.T) { RegistrationID: reg.ID, Expires: &expires, Status: core.StatusPending, + Identifier: core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "your.order.is.up"}, } pendingAuthz, err := sa.NewPendingAuthorization(ctx, authz) test.AssertNotError(t, err, "Couldn't create new pending authorization") @@ -1898,17 +2000,16 @@ func TestUpdatePendingAuthorizationInvalidOrder(t *testing.T) { RegistrationID: reg.ID, Expires: &expires, Status: core.StatusPending, + Identifier: core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "your.order.is.up"}, } pendingAuthz, err = sa.NewPendingAuthorization(ctx, authz) test.AssertNotError(t, err, "Couldn't create new pending authorization") // Add a new order that references the above pending authz - status := string(core.StatusPending) expiresNano := expires.UnixNano() order, err := sa.NewOrder(ctx, &corepb.Order{ RegistrationID: ®.ID, Expires: &expiresNano, - Status: &status, Authorizations: []string{pendingAuthz.ID}, Names: []string{"your.order.is.up"}, }) @@ -1935,3 +2036,132 @@ func TestUpdatePendingAuthorizationInvalidOrder(t *testing.T) { // We expect the updated order status to be invalid test.AssertEquals(t, *updatedOrder.Status, string(core.StatusInvalid)) } + +func TestStatusForOrder(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + + sa, fc, cleanUp := initSA(t) + defer cleanUp() + + ctx := context.Background() + expires := fc.Now().Add(time.Hour) + expiresNano := expires.UnixNano() + + // Create a registration to work with + reg := satest.CreateWorkingRegistration(t, sa) + + // Create a pending authz + newAuthz := core.Authorization{ + RegistrationID: reg.ID, + Expires: &expires, + Status: core.StatusPending, + Identifier: core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "pending.your.order.is.up"}, + } + pendingAuthz, err := sa.NewPendingAuthorization(ctx, newAuthz) + test.AssertNotError(t, err, "Couldn't create new pending authorization") + + // Create an invalid authz + invalidAuthz, err := sa.NewPendingAuthorization(ctx, newAuthz) + invalidAuthz.Status = core.StatusInvalid + invalidAuthz.Identifier.Value = "invalid.your.order.is.up" + err = sa.FinalizeAuthorization(ctx, invalidAuthz) + test.AssertNotError(t, err, "Couldn't finalize pending authz to invalid") + + // Create a deactivate authz + deactivatedAuthz, err := sa.NewPendingAuthorization(ctx, newAuthz) + deactivatedAuthz.Status = core.StatusDeactivated + deactivatedAuthz.Identifier.Value = "deactivated.your.order.is.up" + err = sa.FinalizeAuthorization(ctx, deactivatedAuthz) + test.AssertNotError(t, err, "Couldn't finalize pending authz to deactivated") + + // Create a valid authz + validAuthz, err := sa.NewPendingAuthorization(ctx, newAuthz) + validAuthz.Status = core.StatusValid + validAuthz.Identifier.Value = "valid.your.order.is.up" + err = sa.FinalizeAuthorization(ctx, validAuthz) + test.AssertNotError(t, err, "Couldn't finalize pending authz to valid") + + testCases := []struct { + Name string + AuthorizationIDs []string + OrderNames []string + ExpectedStatus string + SetProcessing bool + Finalize bool + }{ + { + Name: "Order with an invalid authz", + OrderNames: []string{"pending.your.order.is.up", "invalid.your.order.is.up", "deactivated.your.order.is.up", "valid.your.order.is.up"}, + AuthorizationIDs: []string{pendingAuthz.ID, invalidAuthz.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"}, + AuthorizationIDs: []string{pendingAuthz.ID, deactivatedAuthz.ID, validAuthz.ID}, + ExpectedStatus: string(core.StatusDeactivated), + }, + { + Name: "Order with a pending authz", + OrderNames: []string{"valid.your.order.is.up", "pending.your.order.is.up"}, + AuthorizationIDs: []string{validAuthz.ID, pendingAuthz.ID}, + ExpectedStatus: string(core.StatusPending), + }, + { + Name: "Order with only valid authzs, not yet processed or finalized", + OrderNames: []string{"valid.your.order.is.up"}, + AuthorizationIDs: []string{validAuthz.ID}, + ExpectedStatus: string(core.StatusPending), + }, + { + Name: "Order with only valid authzs, set processing", + OrderNames: []string{"valid.your.order.is.up"}, + AuthorizationIDs: []string{validAuthz.ID}, + SetProcessing: true, + ExpectedStatus: string(core.StatusProcessing), + }, + { + Name: "Order with only valid authzs, set processing and finalized", + OrderNames: []string{"valid.your.order.is.up"}, + AuthorizationIDs: []string{validAuthz.ID}, + SetProcessing: true, + Finalize: true, + ExpectedStatus: string(core.StatusValid), + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + // Add a new order with the testcase authz IDs + newOrder, err := sa.NewOrder(ctx, &corepb.Order{ + RegistrationID: ®.ID, + Expires: &expiresNano, + Authorizations: tc.AuthorizationIDs, + Names: tc.OrderNames, + }) + test.AssertNotError(t, err, "NewOrder errored unexpectedly") + // If requested, set the order to processing + if tc.SetProcessing { + err := sa.SetOrderProcessing(ctx, newOrder) + test.AssertNotError(t, err, "Error setting order to processing status") + } + // If requested, finalize the order + if tc.Finalize { + cereal := "lucky charms" + newOrder.CertificateSerial = &cereal + err := sa.FinalizeOrder(ctx, newOrder) + test.AssertNotError(t, err, "Error finalizing order") + } + // Fetch the order by ID to get its calculated status + storedOrder, err := sa.GetOrder(ctx, &sapb.OrderRequest{Id: newOrder.Id}) + test.AssertNotError(t, err, "GetOrder failed") + // The status shouldn't be nil + test.AssertNotNil(t, storedOrder.Status, "Order status was nil") + // The status should match expected + test.AssertEquals(t, *storedOrder.Status, tc.ExpectedStatus) + }) + } + +} From c495c99dbfa72fdf042177230b90c5753cdf0f37 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 27 Jan 2018 12:17:41 -0500 Subject: [PATCH 05/19] Reimport OS --- sa/sa_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sa/sa_test.go b/sa/sa_test.go index f292483e834..532d17b61a3 100644 --- a/sa/sa_test.go +++ b/sa/sa_test.go @@ -11,6 +11,7 @@ import ( "io/ioutil" "math/big" "net" + "os" "reflect" "sync" "testing" From 6c251e00e743e309f28031e567fd1f6d176d505d Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 27 Jan 2018 12:24:35 -0500 Subject: [PATCH 06/19] Remove debug printf --- ra/ra.go | 1 - 1 file changed, 1 deletion(-) diff --git a/ra/ra.go b/ra/ra.go index a8e0838ff99..9bbc3e7729f 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -1714,7 +1714,6 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New } storedOrder, err := ra.SA.NewOrder(ctx, order) - fmt.Printf("ra.SA.NewOrder err: %#v\n", err) if err != nil { return nil, err } From ccca70c8e60ae083a6118df4630400420ddbee43 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 27 Jan 2018 12:25:24 -0500 Subject: [PATCH 07/19] Drop new column in migration down stage --- sa/_db-next/migrations/20180124141109_DropOrderStatusField.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/sa/_db-next/migrations/20180124141109_DropOrderStatusField.sql b/sa/_db-next/migrations/20180124141109_DropOrderStatusField.sql index 549542adfc3..c18703e1231 100644 --- a/sa/_db-next/migrations/20180124141109_DropOrderStatusField.sql +++ b/sa/_db-next/migrations/20180124141109_DropOrderStatusField.sql @@ -9,3 +9,4 @@ ALTER TABLE `orders` ADD COLUMN `beganProcessing` BOOL NOT NULL DEFAULT FALSE; -- SQL section 'Down' is executed when this migration is rolled back ALTER TABLE `orders` ADD COLUMN `status` varchar(255) NOT NULL; +ALTER TABLE `orders` DROP COLUMN `beganProcessing`; From c88017a51363eb88daecf089853dcbb6f0143223 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 27 Jan 2018 12:25:53 -0500 Subject: [PATCH 08/19] Fix typo --- sa/sa.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sa/sa.go b/sa/sa.go index 9639a7eb1ab..038bda6ec78 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -791,7 +791,7 @@ func (ssa *SQLStorageAuthority) UpdatePendingAuthorization(ctx context.Context, // FinalizeAuthorization converts a Pending Authorization to a final one. If the // Authorization is not found a berrors.NotFound result is returned. If the -// Authorization is status pending an berrors.InternalServer error is returned. +// Authorization is status pending a berrors.InternalServer error is returned. func (ssa *SQLStorageAuthority) FinalizeAuthorization(ctx context.Context, authz core.Authorization) error { tx, err := ssa.dbMap.Begin() if err != nil { From b8f89ceceaad013f7cf2a60426f2ec62d3f52943 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 27 Jan 2018 12:58:26 -0500 Subject: [PATCH 09/19] Reintroduce V2 migration test gating --- ra/ra_test.go | 20 ++++++++++++++++++++ sa/sa_test.go | 16 ++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/ra/ra_test.go b/ra/ra_test.go index f2ceab169e4..b72748ffb81 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -2106,6 +2106,10 @@ func TestRecheckCAAFail(t *testing.T) { } func TestNewOrder(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + _, _, ra, fc, cleanUp := initAuthorities(t) defer cleanUp() ra.orderLifetime = time.Hour @@ -2181,6 +2185,10 @@ func TestNewOrder(t *testing.T) { // an identical order results in only one order being created & subsequently // reused. func TestNewOrderReuse(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + _, _, ra, fc, cleanUp := initAuthorities(t) defer cleanUp() @@ -2336,6 +2344,10 @@ func TestNewOrderReuseInvalidAuthz(t *testing.T) { } func TestNewOrderWildcard(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + _, _, ra, _, cleanUp := initAuthorities(t) defer cleanUp() ra.orderLifetime = time.Hour @@ -2529,6 +2541,10 @@ func TestNewOrderWildcard(t *testing.T) { } func TestFinalizeOrder(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + _, sa, ra, _, cleanUp := initAuthorities(t) defer cleanUp() ra.orderLifetime = time.Hour @@ -2769,6 +2785,10 @@ func TestFinalizeOrder(t *testing.T) { } func TestFinalizeOrderWildcard(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + _, sa, ra, _, cleanUp := initAuthorities(t) defer cleanUp() diff --git a/sa/sa_test.go b/sa/sa_test.go index 532d17b61a3..8f7c5918009 100644 --- a/sa/sa_test.go +++ b/sa/sa_test.go @@ -1328,6 +1328,10 @@ func TestGetNewIssuancesByFQDNSet(t *testing.T) { } func TestNewOrder(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + sa, _, cleanup := initSA(t) defer cleanup() @@ -1547,6 +1551,10 @@ func TestOrder(t *testing.T) { } func TestGetOrderAuthorizations(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + sa, _, cleanup := initSA(t) defer cleanup() @@ -1700,6 +1708,10 @@ func TestAddPendingAuthorizations(t *testing.T) { } func TestCountPendingOrders(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + sa, fc, cleanUp := initSA(t) defer cleanUp() @@ -1789,6 +1801,10 @@ func TestCountPendingOrders(t *testing.T) { } func TestGetOrderForNames(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + sa, fc, cleanUp := initSA(t) defer cleanUp() From 868dfc810a8e435b6796f571ba6051114b7d91b8 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 27 Jan 2018 13:10:13 -0500 Subject: [PATCH 10/19] Add missing config-next gate --- ra/ra_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ra/ra_test.go b/ra/ra_test.go index b72748ffb81..b1e05340c32 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -1162,6 +1162,10 @@ func TestAuthzRateLimiting(t *testing.T) { } func TestNewOrderRateLimiting(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + _, _, ra, fc, cleanUp := initAuthorities(t) defer cleanUp() From c6598b1656c89296d4976f6081fe4dc292d79efd Mon Sep 17 00:00:00 2001 From: Daniel Date: Mon, 29 Jan 2018 16:57:02 -0500 Subject: [PATCH 11/19] Gate TestFinalizeOrderWithMixedSANAndCN test --- ra/ra_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ra/ra_test.go b/ra/ra_test.go index fe0610b9d94..9fabe5b646f 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -2790,6 +2790,10 @@ func TestFinalizeOrder(t *testing.T) { } func TestFinalizeOrderWithMixedSANAndCN(t *testing.T) { + if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { + return + } + _, sa, ra, _, cleanUp := initAuthorities(t) defer cleanUp() ra.orderLifetime = time.Hour From 0c339ba60a36ccba5e0eb56237e5c12bc6041264 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 30 Jan 2018 11:53:55 -0500 Subject: [PATCH 12/19] Mark follow-up for pending order rl optimization --- sa/sa.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sa/sa.go b/sa/sa.go index 038bda6ec78..d5f8cdce24f 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -995,6 +995,8 @@ func (ssa *SQLStorageAuthority) CountPendingOrders(ctx context.Context, regID in // Iterate the order IDs, fetching the full order & associated authorizations // for each + // TODO(@cpu): This is not performant and we should optimize: + // https://github.com/letsencrypt/boulder/issues/3410 for _, orderID := range orderIDs { order, err := ssa.GetOrder(ctx, &sapb.OrderRequest{Id: &orderID}) if err != nil { From 68884d293e17ec3e19f5d411067c7cbd7d62fd0b Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 30 Jan 2018 11:54:34 -0500 Subject: [PATCH 13/19] Fix SA comment typo. --- sa/sa.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sa/sa.go b/sa/sa.go index d5f8cdce24f..b2b8a71e628 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -1650,7 +1650,7 @@ func (ssa *SQLStorageAuthority) statusForOrder(ctx context.Context, order *corep return string(core.StatusPending), nil } - // An order is fully authorizsed if it has valid authzs for each of the order + // An order is fully authorized if it has valid authzs for each of the order // names fullyAuthorized := len(order.Names) == validAuthzs From 8b6dd338d6dd757601462738888377b3a9b965c0 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 30 Jan 2018 12:03:18 -0500 Subject: [PATCH 14/19] Use INNER JOIN in getAllOrderAuthorizations --- sa/sa.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sa/sa.go b/sa/sa.go index b2b8a71e628..7eefb935012 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -1699,7 +1699,7 @@ func (ssa *SQLStorageAuthority) getAllOrderAuthorizations( _, err := ssa.dbMap.Select( &authzs, fmt.Sprintf(`SELECT %s from %s AS authz - LEFT JOIN orderToAuthz + INNER JOIN orderToAuthz ON authz.ID = orderToAuthz.authzID WHERE authz.registrationID = ? AND authz.expires > ? AND From 4d5dbac4a8cb5c51e867411f4c0875b6f5b39715 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 30 Jan 2018 12:03:40 -0500 Subject: [PATCH 15/19] Error on repeated order authzs --- sa/sa.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/sa/sa.go b/sa/sa.go index 7eefb935012..6bdad5e046b 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -1722,10 +1722,14 @@ func (ssa *SQLStorageAuthority) getAllOrderAuthorizations( if auth.Identifier.Type != core.IdentifierDNS { return nil, fmt.Errorf("unknown identifier type: %q on authz id %q", auth.Identifier.Type, auth.ID) } - existing, present := byName[auth.Identifier.Value] - if !present || auth.Expires.After(*existing.Expires) { - byName[auth.Identifier.Value] = auth + // We don't expect there to be multiple authorizations for the same name + // within the same order + if _, present := byName[auth.Identifier.Value]; present { + return nil, berrors.InternalServerError( + "Found multiple authorizations within one order for identifier %q", + auth.Identifier.Value) } + byName[auth.Identifier.Value] = auth } return byName, nil } From 30c498ba29347f1af7ccbeead9dbeaea06493c8a Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 30 Jan 2018 12:21:11 -0500 Subject: [PATCH 16/19] Fix test broken in merge conflict resolve --- test/integration-test-v2.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/integration-test-v2.py b/test/integration-test-v2.py index 6fd29902058..8dc5216e89a 100644 --- a/test/integration-test-v2.py +++ b/test/integration-test-v2.py @@ -231,6 +231,11 @@ def test_revoke_by_privkey(): key_pem = OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM, key) csr_pem = acme_crypto_util.make_csr(key_pem, domains, False) order = client.new_order(csr_pem) + cleanup = do_http_challenges(client, order.authorizations) + try: + order = client.poll_order_and_request_issuance(order) + finally: + cleanup() # Create a new client with the JWK as the cert private key jwk = jose.JWKRSA(key=key) From bee6f5acbfd450d9ee80ccf082733fdbc0fbd9f8 Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 1 Feb 2018 11:28:05 -0500 Subject: [PATCH 17/19] Restore reuse of pending authorizations in orders --- ra/ra.go | 8 -------- ra/ra_test.go | 10 +--------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/ra/ra.go b/ra/ra.go index 63a7216c2d6..8b0fcd9b897 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -1657,14 +1657,6 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New continue } authz := nameToExistingAuthz[name] - // If the existing authz isn't valid, note that its missing and continue. - // Reusing pending authorizations between orders is primarily an effort to - // prevent clients hitting the pending authz limit, but in a V2 world order - // reuse accomplishes the same thing. - if *authz.Status != string(core.StatusValid) { - missingAuthzNames = append(missingAuthzNames, name) - continue - } // If the identifier is a wildcard and the existing authz only has one // DNS-01 type challenge we can reuse it. In theory we will // never get back an authorization for a domain with a wildcard prefix diff --git a/ra/ra_test.go b/ra/ra_test.go index 9fabe5b646f..52d6b15ffd9 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -14,7 +14,6 @@ import ( "net" "net/url" "os" - "reflect" "sort" "strings" "sync" @@ -2169,14 +2168,7 @@ func TestNewOrder(t *testing.T) { // Abuse the order of the queries used to extract the reused authorizations existing := orderC.Authorizations[:3] sort.Strings(existing) - - // We expect the pending authorizations were not reused between separate - // orders - // NOTE(@cpu): There's no test.AssertNotDeepEquals to use here so we call - // reflect.DeepEqual ourselves. - if reflect.DeepEqual(existing, orderA.Authorizations) { - t.Fatal("existing authorizations matched orderA authorizations") - } + test.AssertDeepEquals(t, existing, orderA.Authorizations) _, err = ra.NewOrder(context.Background(), &rapb.NewOrderRequest{ RegistrationID: &id, From c2b7c3f0f33e361cdaf4f42edc63e7d94ca0a965 Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 1 Feb 2018 15:52:42 -0500 Subject: [PATCH 18/19] remove extra whitespace --- sa/sa.go | 1 - 1 file changed, 1 deletion(-) diff --git a/sa/sa.go b/sa/sa.go index 6bdad5e046b..fbf16298caf 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -1690,7 +1690,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 From 7cb8c00ac93ccafa819786d6b10dda13d692eeb6 Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 1 Feb 2018 15:54:10 -0500 Subject: [PATCH 19/19] Update sa.GetOrder function doc comment --- sa/sa.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sa/sa.go b/sa/sa.go index fbf16298caf..e74d7048921 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -1713,7 +1713,7 @@ func (ssa *SQLStorageAuthority) getAllOrderAuthorizations( allAuthzs = append(allAuthzs, authzs...) } - // Collapse & dedupe the returned authorizations into a mapping from name to + // Collapse the returned authorizations into a mapping from name to // authorization byName := make(map[string]*core.Authorization) for _, auth := range allAuthzs { @@ -1785,9 +1785,10 @@ func (ssa *SQLStorageAuthority) GetOrderAuthorizations( return byName, nil } -// GetOrderForNames tries to find an order with the exact set of names -// requested, associated with the given accountID. Only unexpired orders are -// considered. If no order is found a nil corepb.Order pointer is returned. +// GetOrderForNames tries to find a **pending** order with the exact set of +// names requested, associated with the given accountID. Only unexpired orders +// with status pending are considered. If no order meeting these requirements is +// found a nil corepb.Order pointer is returned. func (ssa *SQLStorageAuthority) GetOrderForNames( ctx context.Context, req *sapb.GetOrderForNamesRequest) (*corepb.Order, error) {