PR review fixes / updates

acme/account.go

@@ -11,11 +11,11 @@ import (
 // Account is a subset of the internal account type containing only those
 // attributes required for responses in the ACME protocol.
 type Account struct {
+	ID        string           `json:"-"`
+	Key       *jose.JSONWebKey `json:"-"`
 	Contact   []string         `json:"contact,omitempty"`
 	Status    Status           `json:"status"`
 	OrdersURL string           `json:"orders"`
-	ID        string           `json:"-"`
-	Key       *jose.JSONWebKey `json:"-"`
 }
 
 // ToLog enables response logging.

acme/api/handler.go

@@ -21,14 +21,14 @@ func link(url, typ string) string {
 }
 
 // Clock that returns time in UTC rounded to seconds.
-type Clock int
+type Clock struct{}
 
 // Now returns the UTC time rounded to seconds.
 func (c *Clock) Now() time.Time {
 	return time.Now().UTC().Round(time.Second)
 }
 
-var clock = new(Clock)
+var clock Clock
 
 type payloadInfo struct {
 	value       []byte
@@ -65,7 +65,7 @@ type HandlerOptions struct {
 // NewHandler returns a new ACME API handler.
 func NewHandler(ops HandlerOptions) api.RouterHandler {
 	client := http.Client{
-		Timeout: time.Duration(30 * time.Second),
+		Timeout: 30 * time.Second,
 	}
 	dialer := &net.Dialer{
 		Timeout: 30 * time.Second,
@@ -89,8 +89,8 @@ func NewHandler(ops HandlerOptions) api.RouterHandler {
 func (h *Handler) Route(r api.Router) {
 	getLink := h.linker.GetLinkExplicit
 	// Standard ACME API
-	r.MethodFunc("GET", getLink(NewNonceLinkType, "{provisionerID}", false, nil), h.baseURLFromRequest(h.lookupProvisioner(h.addNonce(h.GetNonce))))
+	r.MethodFunc("GET", getLink(NewNonceLinkType, "{provisionerID}", false, nil), h.baseURLFromRequest(h.lookupProvisioner(h.addNonce(h.addDirLink(h.GetNonce)))))
-	r.MethodFunc("HEAD", getLink(NewNonceLinkType, "{provisionerID}", false, nil), h.baseURLFromRequest(h.lookupProvisioner(h.addNonce(h.GetNonce))))
+	r.MethodFunc("HEAD", getLink(NewNonceLinkType, "{provisionerID}", false, nil), h.baseURLFromRequest(h.lookupProvisioner(h.addNonce(h.addDirLink(h.GetNonce)))))
 	r.MethodFunc("GET", getLink(DirectoryLinkType, "{provisionerID}", false, nil), h.baseURLFromRequest(h.lookupProvisioner(h.addNonce(h.GetDirectory))))
 	r.MethodFunc("HEAD", getLink(DirectoryLinkType, "{provisionerID}", false, nil), h.baseURLFromRequest(h.lookupProvisioner(h.addNonce(h.GetDirectory))))
 
@@ -218,6 +218,7 @@ func (h *Handler) GetChallenge(w http.ResponseWriter, r *http.Request) {
 		api.WriteError(w, acme.WrapErrorISE(err, "error retrieving challenge"))
 		return
 	}
+	ch.AuthorizationID = azID
 	if acc.ID != ch.AccountID {
 		api.WriteError(w, acme.NewError(acme.ErrorUnauthorizedType,
 			"account '%s' does not own challenge '%s'", acc.ID, ch.ID))

acme/api/handler_test.go

@@ -582,6 +582,7 @@ func TestHandler_GetChallenge(t *testing.T) {
 						assert.Equals(t, ch.Status, acme.StatusPending)
 						assert.Equals(t, ch.Type, "http-01")
 						assert.Equals(t, ch.AccountID, "accID")
+						assert.Equals(t, ch.AuthorizationID, "authzID")
 						assert.HasSuffix(t, ch.Error.Type, acme.ErrorConnectionType.String())
 						return acme.NewErrorISE("force")
 					},
@@ -623,17 +624,19 @@ func TestHandler_GetChallenge(t *testing.T) {
 						assert.Equals(t, ch.Status, acme.StatusPending)
 						assert.Equals(t, ch.Type, "http-01")
 						assert.Equals(t, ch.AccountID, "accID")
+						assert.Equals(t, ch.AuthorizationID, "authzID")
 						assert.HasSuffix(t, ch.Error.Type, acme.ErrorConnectionType.String())
 						return nil
 					},
 				},
 				ch: &acme.Challenge{
-					ID:        "chID",
+					ID:              "chID",
-					Status:    acme.StatusPending,
+					Status:          acme.StatusPending,
-					Type:      "http-01",
+					AuthorizationID: "authzID",
-					AccountID: "accID",
+					Type:            "http-01",
-					URL:       url,
+					AccountID:       "accID",
-					Error:     acme.NewError(acme.ErrorConnectionType, "force"),
+					URL:             url,
+					Error:           acme.NewError(acme.ErrorConnectionType, "force"),
 				},
 				vco: &acme.ValidateChallengeOptions{
 					HTTPGet: func(string) (*http.Response, error) {

acme/authorization.go

@@ -8,15 +8,15 @@ import (
 
 // Authorization representst an ACME Authorization.
 type Authorization struct {
-	Identifier Identifier   `json:"identifier"`
-	Status     Status       `json:"status"`
-	ExpiresAt  time.Time    `json:"expires"`
-	Challenges []*Challenge `json:"challenges"`
-	Wildcard   bool         `json:"wildcard"`
-	Error      *Error       `json:"error,omitempty"`
 	ID         string       `json:"-"`
 	AccountID  string       `json:"-"`
 	Token      string       `json:"-"`
+	Identifier Identifier   `json:"identifier"`
+	Status     Status       `json:"status"`
+	Challenges []*Challenge `json:"challenges"`
+	Wildcard   bool         `json:"wildcard"`
+	ExpiresAt  time.Time    `json:"expires"`
+	Error      *Error       `json:"error,omitempty"`
 }
 
 // ToLog enables response logging.

acme/challenge.go

@@ -22,15 +22,16 @@ import (
 
 // Challenge represents an ACME response Challenge type.
 type Challenge struct {
-	Type        string `json:"type"`
+	ID              string `json:"-"`
-	Status      Status `json:"status"`
+	AccountID       string `json:"-"`
-	Token       string `json:"token"`
+	AuthorizationID string `json:"-"`
-	ValidatedAt string `json:"validated,omitempty"`
+	Value           string `json:"-"`
-	URL         string `json:"url"`
+	Type            string `json:"type"`
-	Error       *Error `json:"error,omitempty"`
+	Status          Status `json:"status"`
-	ID          string `json:"-"`
+	Token           string `json:"token"`
-	AccountID   string `json:"-"`
+	ValidatedAt     string `json:"validated,omitempty"`
-	Value       string `json:"-"`
+	URL             string `json:"url"`
+	Error           *Error `json:"error,omitempty"`
 }
 
 // ToLog enables response logging.

acme/common.go

@@ -15,14 +15,14 @@ type CertificateAuthority interface {
 }
 
 // Clock that returns time in UTC rounded to seconds.
-type Clock int
+type Clock struct{}
 
 // Now returns the UTC time rounded to seconds.
 func (c *Clock) Now() time.Time {
 	return time.Now().UTC().Round(time.Second)
 }
 
-var clock = new(Clock)
+var clock Clock
 
 // Provisioner is an interface that implements a subset of the provisioner.Interface --
 // only those methods required by the ACME api/authority.

acme/db/nosql/account.go

@@ -14,11 +14,11 @@ import (
 // dbAccount represents an ACME account.
 type dbAccount struct {
 	ID            string           `json:"id"`
-	CreatedAt     time.Time        `json:"createdAt"`
-	DeactivatedAt time.Time        `json:"deactivatedAt"`
 	Key           *jose.JSONWebKey `json:"key"`
 	Contact       []string         `json:"contact,omitempty"`
 	Status        acme.Status      `json:"status"`
+	CreatedAt     time.Time        `json:"createdAt"`
+	DeactivatedAt time.Time        `json:"deactivatedAt"`
 }
 
 func (dba *dbAccount) clone() *dbAccount {

acme/db/nosql/authz.go

@@ -16,12 +16,12 @@ type dbAuthz struct {
 	AccountID    string          `json:"accountID"`
 	Identifier   acme.Identifier `json:"identifier"`
 	Status       acme.Status     `json:"status"`
-	ExpiresAt    time.Time       `json:"expiresAt"`
+	Token        string          `json:"token"`
 	ChallengeIDs []string        `json:"challengeIDs"`
 	Wildcard     bool            `json:"wildcard"`
 	CreatedAt    time.Time       `json:"createdAt"`
+	ExpiresAt    time.Time       `json:"expiresAt"`
 	Error        *acme.Error     `json:"error"`
-	Token        string          `json:"token"`
 }
 
 func (ba *dbAuthz) clone() *dbAuthz {

acme/db/nosql/nonce.go

@@ -3,12 +3,12 @@ package nosql
 import (
 	"context"
 	"encoding/base64"
-	"encoding/json"
 	"time"
 
 	"github.com/pkg/errors"
 	"github.com/smallstep/certificates/acme"
 	"github.com/smallstep/nosql"
+	"github.com/smallstep/nosql/database"
 )
 
 // dbNonce contains nonce metadata used in the ACME protocol.
@@ -45,24 +45,27 @@ func (db *DB) CreateNonce(ctx context.Context) (acme.Nonce, error) {
 // DeleteNonce verifies that the nonce is valid (by checking if it exists),
 // and if so, consumes the nonce resource by deleting it from the database.
 func (db *DB) DeleteNonce(ctx context.Context, nonce acme.Nonce) error {
-	id := string(nonce)
+	err := db.db.Update(&database.Tx{
-	b, err := db.db.Get(nonceTable, []byte(nonce))
+		Operations: []*database.TxEntry{
-	if nosql.IsErrNotFound(err) {
+			{
-		return errors.Wrapf(err, "nonce %s not found", id)
+				Bucket: nonceTable,
-	} else if err != nil {
+				Key:    []byte(nonce),
-		return errors.Wrapf(err, "error loading nonce %s", id)
+				Cmd:    database.Get,
-	}
+			},
+			{
+				Bucket: nonceTable,
+				Key:    []byte(nonce),
+				Cmd:    database.Delete,
+			},
+		},
+	})
 
-	dbn := new(dbNonce)
+	switch {
-	if err := json.Unmarshal(b, dbn); err != nil {
+	case nosql.IsErrNotFound(err):
-		return errors.Wrapf(err, "error unmarshaling nonce %s", string(nonce))
+		return acme.NewError(acme.ErrorBadNonceType, "nonce %s not found", string(nonce))
+	case err != nil:
+		return errors.Wrapf(err, "error deleting nonce %s", string(nonce))
+	default:
+		return nil
 	}
-	if !dbn.DeletedAt.IsZero() {
-		return acme.NewError(acme.ErrorBadNonceType, "nonce %s already deleted", id)
-	}
-
-	nu := dbn.clone()
-	nu.DeletedAt = clock.Now()
-
-	return db.save(ctx, id, nu, dbn, "nonce", nonceTable)
 }

acme/db/nosql/nonce_test.go

@@ -11,7 +11,7 @@ import (
 	"github.com/smallstep/certificates/acme"
 	"github.com/smallstep/certificates/db"
 	"github.com/smallstep/nosql"
-	nosqldb "github.com/smallstep/nosql/database"
+	"github.com/smallstep/nosql/database"
 )
 
 func TestDB_CreateNonce(t *testing.T) {
@@ -85,108 +85,57 @@ func TestDB_DeleteNonce(t *testing.T) {
 
 	nonceID := "nonceID"
 	type test struct {
-		db  nosql.DB
+		db      nosql.DB
-		err error
+		err     error
+		acmeErr *acme.Error
 	}
 	var tests = map[string]func(t *testing.T) test{
 		"fail/not-found": func(t *testing.T) test {
 			return test{
 				db: &db.MockNoSQLDB{
-					MGet: func(bucket, key []byte) ([]byte, error) {
+					MUpdate: func(tx *database.Tx) error {
-						assert.Equals(t, bucket, nonceTable)
+						assert.Equals(t, tx.Operations[0].Bucket, nonceTable)
-						assert.Equals(t, string(key), nonceID)
+						assert.Equals(t, tx.Operations[0].Key, []byte(nonceID))
+						assert.Equals(t, tx.Operations[0].Cmd, database.Get)
 
-						return nil, nosqldb.ErrNotFound
+						assert.Equals(t, tx.Operations[1].Bucket, nonceTable)
+						assert.Equals(t, tx.Operations[1].Key, []byte(nonceID))
+						assert.Equals(t, tx.Operations[1].Cmd, database.Delete)
+						return database.ErrNotFound
 					},
 				},
-				err: errors.New("nonce nonceID not found"),
+				acmeErr: acme.NewError(acme.ErrorBadNonceType, "nonce %s not found", nonceID),
 			}
 		},
-		"fail/db.Get-error": func(t *testing.T) test {
+		"fail/db.Update-error": func(t *testing.T) test {
 			return test{
 				db: &db.MockNoSQLDB{
-					MGet: func(bucket, key []byte) ([]byte, error) {
+					MUpdate: func(tx *database.Tx) error {
-						assert.Equals(t, bucket, nonceTable)
+						assert.Equals(t, tx.Operations[0].Bucket, nonceTable)
-						assert.Equals(t, string(key), nonceID)
+						assert.Equals(t, tx.Operations[0].Key, []byte(nonceID))
+						assert.Equals(t, tx.Operations[0].Cmd, database.Get)
 
-						return nil, errors.Errorf("force")
+						assert.Equals(t, tx.Operations[1].Bucket, nonceTable)
+						assert.Equals(t, tx.Operations[1].Key, []byte(nonceID))
+						assert.Equals(t, tx.Operations[1].Cmd, database.Delete)
+						return errors.New("force")
 					},
 				},
-				err: errors.New("error loading nonce nonceID: force"),
+				err: errors.New("error deleting nonce nonceID: force"),
-			}
-		},
-		"fail/unmarshal-error": func(t *testing.T) test {
-			return test{
-				db: &db.MockNoSQLDB{
-					MGet: func(bucket, key []byte) ([]byte, error) {
-						assert.Equals(t, bucket, nonceTable)
-						assert.Equals(t, string(key), nonceID)
-
-						a := []string{"foo", "bar", "baz"}
-						b, err := json.Marshal(a)
-						assert.FatalError(t, err)
-
-						return b, nil
-					},
-				},
-				err: errors.New("error unmarshaling nonce nonceID"),
-			}
-		},
-		"fail/already-used": func(t *testing.T) test {
-			return test{
-				db: &db.MockNoSQLDB{
-					MGet: func(bucket, key []byte) ([]byte, error) {
-						assert.Equals(t, bucket, nonceTable)
-						assert.Equals(t, string(key), nonceID)
-
-						nonce := dbNonce{
-							ID:        nonceID,
-							CreatedAt: clock.Now().Add(-5 * time.Minute),
-							DeletedAt: clock.Now(),
-						}
-						b, err := json.Marshal(nonce)
-						assert.FatalError(t, err)
-
-						return b, nil
-					},
-				},
-				err: acme.NewError(acme.ErrorBadNonceType, "nonce already deleted"),
 			}
 		},
 		"ok": func(t *testing.T) test {
-			nonce := dbNonce{
-				ID:        nonceID,
-				CreatedAt: clock.Now().Add(-5 * time.Minute),
-			}
-			b, err := json.Marshal(nonce)
-			assert.FatalError(t, err)
 			return test{
 				db: &db.MockNoSQLDB{
-					MGet: func(bucket, key []byte) ([]byte, error) {
+					MUpdate: func(tx *database.Tx) error {
-						assert.Equals(t, bucket, nonceTable)
+						assert.Equals(t, tx.Operations[0].Bucket, nonceTable)
-						assert.Equals(t, string(key), nonceID)
+						assert.Equals(t, tx.Operations[0].Key, []byte(nonceID))
+						assert.Equals(t, tx.Operations[0].Cmd, database.Get)
 
-						return b, nil
+						assert.Equals(t, tx.Operations[1].Bucket, nonceTable)
-					},
+						assert.Equals(t, tx.Operations[1].Key, []byte(nonceID))
-					MCmpAndSwap: func(bucket, key, old, nu []byte) ([]byte, bool, error) {
+						assert.Equals(t, tx.Operations[1].Cmd, database.Delete)
-						assert.Equals(t, bucket, nonceTable)
+						return nil
-						assert.Equals(t, old, b)
-
-						dbo := new(dbNonce)
-						assert.FatalError(t, json.Unmarshal(old, dbo))
-						assert.Equals(t, dbo.ID, string(key))
-						assert.True(t, clock.Now().Add(-6*time.Minute).Before(dbo.CreatedAt))
-						assert.True(t, clock.Now().Add(-4*time.Minute).After(dbo.CreatedAt))
-						assert.True(t, dbo.DeletedAt.IsZero())
-
-						dbn := new(dbNonce)
-						assert.FatalError(t, json.Unmarshal(nu, dbn))
-						assert.Equals(t, dbn.ID, string(key))
-						assert.True(t, clock.Now().Add(-6*time.Minute).Before(dbn.CreatedAt))
-						assert.True(t, clock.Now().Add(-4*time.Minute).After(dbn.CreatedAt))
-						assert.True(t, clock.Now().Add(-time.Minute).Before(dbn.DeletedAt))
-						assert.True(t, clock.Now().Add(time.Minute).After(dbn.DeletedAt))
-						return nil, true, nil
 					},
 				},
 			}
@@ -197,8 +146,19 @@ func TestDB_DeleteNonce(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			db := DB{db: tc.db}
 			if err := db.DeleteNonce(context.Background(), acme.Nonce(nonceID)); err != nil {
-				if assert.NotNil(t, tc.err) {
+				switch k := err.(type) {
-					assert.HasPrefix(t, err.Error(), tc.err.Error())
+				case *acme.Error:
+					if assert.NotNil(t, tc.acmeErr) {
+						assert.Equals(t, k.Type, tc.acmeErr.Type)
+						assert.Equals(t, k.Detail, tc.acmeErr.Detail)
+						assert.Equals(t, k.Status, tc.acmeErr.Status)
+						assert.Equals(t, k.Err.Error(), tc.acmeErr.Err.Error())
+						assert.Equals(t, k.Detail, tc.acmeErr.Detail)
+					}
+				default:
+					if assert.NotNil(t, tc.err) {
+						assert.HasPrefix(t, err.Error(), tc.err.Error())
+					}
 				}
 			} else {
 				assert.Nil(t, tc.err)

acme/db/nosql/order.go

@@ -18,15 +18,15 @@ type dbOrder struct {
 	ID               string            `json:"id"`
 	AccountID        string            `json:"accountID"`
 	ProvisionerID    string            `json:"provisionerID"`
-	CreatedAt        time.Time         `json:"createdAt"`
-	ExpiresAt        time.Time         `json:"expiresAt,omitempty"`
-	Status           acme.Status       `json:"status"`
 	Identifiers      []acme.Identifier `json:"identifiers"`
+	AuthorizationIDs []string          `json:"authorizationIDs"`
+	Status           acme.Status       `json:"status"`
 	NotBefore        time.Time         `json:"notBefore,omitempty"`
 	NotAfter         time.Time         `json:"notAfter,omitempty"`
-	Error            *acme.Error       `json:"error,omitempty"`
+	CreatedAt        time.Time         `json:"createdAt"`
-	AuthorizationIDs []string          `json:"authorizationIDs"`
+	ExpiresAt        time.Time         `json:"expiresAt,omitempty"`
 	CertificateID    string            `json:"certificate,omitempty"`
+	Error            *acme.Error       `json:"error,omitempty"`
 }
 
 func (a *dbOrder) clone() *dbOrder {

acme/order.go

@@ -21,6 +21,8 @@ type Identifier struct {
 // Order contains order metadata for the ACME protocol order type.
 type Order struct {
 	ID                string       `json:"id"`
+	AccountID         string       `json:"-"`
+	ProvisionerID     string       `json:"-"`
 	Status            Status       `json:"status"`
 	ExpiresAt         time.Time    `json:"expires,omitempty"`
 	Identifiers       []Identifier `json:"identifiers"`
@@ -32,8 +34,6 @@ type Order struct {
 	FinalizeURL       string       `json:"finalize"`
 	CertificateID     string       `json:"-"`
 	CertificateURL    string       `json:"certificate,omitempty"`
-	AccountID         string       `json:"-"`
-	ProvisionerID     string       `json:"-"`
 }
 
 // ToLog enables response logging.