Remove unnecessary usage of UpdatePendingAuthorization in RA (#3927)

Removes superfluous usage of `UpdatePendingAuthorization` in the RA to update the key authorization and test if the authorization is pending and instead uses the result of the initial `GetAuthorization` call in the WFE.

Fixes #3923.

ra/ra.go

@@ -14,10 +14,6 @@ import (
 	"time"
 
 	"github.com/jmhodges/clock"
-	"github.com/prometheus/client_golang/prometheus"
-	"github.com/weppos/publicsuffix-go/publicsuffix"
-	"golang.org/x/net/context"
-
 	caPB "github.com/letsencrypt/boulder/ca/proto"
 	"github.com/letsencrypt/boulder/core"
 	corepb "github.com/letsencrypt/boulder/core/proto"
@@ -37,6 +33,9 @@ import (
 	sapb "github.com/letsencrypt/boulder/sa/proto"
 	vaPB "github.com/letsencrypt/boulder/va/proto"
 	"github.com/letsencrypt/boulder/web"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/weppos/publicsuffix-go/publicsuffix"
+	"golang.org/x/net/context"
 	grpc "google.golang.org/grpc"
 )
 
@@ -1455,6 +1454,10 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
 		return authz, nil
 	}
 
+	if authz.Status != core.StatusPending {
+		return core.Authorization{}, berrors.WrongAuthorizationStateError("authorization must be pending")
+	}
+
 	// Look up the account key for this authorization
 	reg, err := ra.SA.GetRegistration(ctx, authz.RegistrationID)
 	if err != nil {
@@ -1491,15 +1494,9 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
 		return core.Authorization{}, berrors.MalformedError(cErr.Error())
 	}
 
-	// Store the updated version
-	if err = ra.SA.UpdatePendingAuthorization(ctx, authz); err != nil {
-		ra.log.Warningf("Error calling ra.SA.UpdatePendingAuthorization: %s", err)
-		return core.Authorization{}, err
-	}
 	ra.stats.Inc("NewPendingAuthorizations", 1)
 
 	// Dispatch to the VA for service
-
 	vaCtx := context.Background()
 	go func(authz core.Authorization) {
 		// We will mutate challenges later in this goroutine to change status and

ra/ra_test.go

@@ -23,6 +23,11 @@ import (
 	"testing"
 	"time"
 
+	"github.com/golang/protobuf/proto"
+	ctasn1 "github.com/google/certificate-transparency-go/asn1"
+	ctx509 "github.com/google/certificate-transparency-go/x509"
+	ctpkix "github.com/google/certificate-transparency-go/x509/pkix"
+	"github.com/jmhodges/clock"
 	capb "github.com/letsencrypt/boulder/ca/proto"
 	"github.com/letsencrypt/boulder/cmd"
 	"github.com/letsencrypt/boulder/core"
@@ -45,12 +50,6 @@ import (
 	"github.com/letsencrypt/boulder/test"
 	"github.com/letsencrypt/boulder/test/vars"
 	vaPB "github.com/letsencrypt/boulder/va/proto"
-
-	"github.com/golang/protobuf/proto"
-	ctasn1 "github.com/google/certificate-transparency-go/asn1"
-	ctx509 "github.com/google/certificate-transparency-go/x509"
-	ctpkix "github.com/google/certificate-transparency-go/x509/pkix"
-	"github.com/jmhodges/clock"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/weppos/publicsuffix-go/publicsuffix"
 	"golang.org/x/net/context"
@@ -927,7 +926,7 @@ func TestUpdateAuthorizationAlreadyValid(t *testing.T) {
 	// A subsequent call to update the authorization should return the expected error
 	_, err = ra.UpdateAuthorization(ctx, finalAuthz, ResponseIndex, response)
 	test.Assert(t, berrors.Is(err, berrors.WrongAuthorizationState),
-		"FinalizeAuthorization of valid authz didn't return a berrors.WrongAuthorizationState")
+		"UpdateAuthorization of valid authz (with reuseValidAuthz disabled) didn't return a berrors.WrongAuthorizationState")
 }
 
 func TestUpdateAuthorizationNewRPC(t *testing.T) {

test/integration-test.py

@@ -550,7 +550,6 @@ def test_stats():
     expect_stat(8000, "\ngo_goroutines ")
     expect_stat(8000, '\ngrpc_client_handling_seconds_count{grpc_method="NewRegistration",grpc_service="ra.RegistrationAuthority",grpc_type="unary"} ')
     expect_stat(8002, '\ngrpc_server_handling_seconds_sum{grpc_method="UpdateAuthorization",grpc_service="ra.RegistrationAuthority",grpc_type="unary"} ')
-    expect_stat(8002, '\ngrpc_client_handling_seconds_count{grpc_method="UpdatePendingAuthorization",grpc_service="sa.StorageAuthority",grpc_type="unary"} ')
     expect_stat(8001, "\ngo_goroutines ")
 
 def test_sct_embedding():

wfe2/wfe.go

@@ -16,10 +16,6 @@ import (
 	"time"
 
 	"github.com/jmhodges/clock"
-	"github.com/prometheus/client_golang/prometheus"
-	"golang.org/x/net/context"
-	jose "gopkg.in/square/go-jose.v2"
-
 	"github.com/letsencrypt/boulder/core"
 	corepb "github.com/letsencrypt/boulder/core/proto"
 	berrors "github.com/letsencrypt/boulder/errors"
@@ -34,6 +30,9 @@ import (
 	"github.com/letsencrypt/boulder/revocation"
 	sapb "github.com/letsencrypt/boulder/sa/proto"
 	"github.com/letsencrypt/boulder/web"
+	"github.com/prometheus/client_golang/prometheus"
+	"golang.org/x/net/context"
+	jose "gopkg.in/square/go-jose.v2"
 )
 
 // Paths are the ACME-spec identified URL path-segments for various methods.
@@ -994,9 +993,9 @@ func (wfe *WebFrontEndImpl) postChallenge(
 		return
 	}
 
-	// Ask the RA to update this authorization. Send an empty `core.Challenge{}`
+	// Send the authorization to the RA for validation (the name of this RPC is somewhat
-	// as the challenge update because we do not care about the KeyAuthorization
+	// misleading, the RA sends the authorization to the VA for validation. Once the validation
-	// (if any) sent in the challengeUpdate.
+	// is complete the VA returns back to the RA to finalize the authorization)
 	updatedAuthorization, err := wfe.RA.UpdateAuthorization(ctx, authz, challengeIndex, core.Challenge{})
 	if err != nil {
 		wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to update challenge"), err)