Improve policy bad request handling

authority/admin/api/policy.go

@@ -105,11 +105,8 @@ func (par *PolicyAdminResponder) CreateAuthorityPolicy(w http.ResponseWriter, r
 
 	var createdPolicy *linkedca.Policy
 	if createdPolicy, err = par.auth.CreateAuthorityPolicy(ctx, adm, newPolicy); err != nil {
-		var pe *authority.PolicyError
+		if isBadRequest(err) {
-		isPolicyError := errors.As(err, &pe)
+			render.Error(w, admin.WrapError(admin.ErrorBadRequestType, err, "error storing authority policy"))
-
-		if isPolicyError && pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure || pe.Typ == authority.ConfigurationFailure {
-			render.Error(w, admin.WrapError(admin.ErrorBadRequestType, pe, "error storing authority policy"))
 			return
 		}
 
@@ -153,10 +150,8 @@ func (par *PolicyAdminResponder) UpdateAuthorityPolicy(w http.ResponseWriter, r
 
 	var updatedPolicy *linkedca.Policy
 	if updatedPolicy, err = par.auth.UpdateAuthorityPolicy(ctx, adm, newPolicy); err != nil {
-		var pe *authority.PolicyError
+		if isBadRequest(err) {
-		isPolicyError := errors.As(err, &pe)
+			render.Error(w, admin.WrapError(admin.ErrorBadRequestType, err, "error updating authority policy"))
-		if isPolicyError && pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure || pe.Typ == authority.ConfigurationFailure {
-			render.Error(w, admin.WrapError(admin.ErrorBadRequestType, pe, "error updating authority policy"))
 			return
 		}
 
@@ -246,10 +241,8 @@ func (par *PolicyAdminResponder) CreateProvisionerPolicy(w http.ResponseWriter,
 	prov.Policy = newPolicy
 
 	if err := par.auth.UpdateProvisioner(ctx, prov); err != nil {
-		var pe *authority.PolicyError
+		if isBadRequest(err) {
-		isPolicyError := errors.As(err, &pe)
+			render.Error(w, admin.WrapError(admin.ErrorBadRequestType, err, "error creating provisioner policy"))
-		if isPolicyError && pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure || pe.Typ == authority.ConfigurationFailure {
-			render.Error(w, admin.WrapError(admin.ErrorBadRequestType, pe, "error creating provisioner policy"))
 			return
 		}
 
@@ -286,10 +279,8 @@ func (par *PolicyAdminResponder) UpdateProvisionerPolicy(w http.ResponseWriter,
 
 	prov.Policy = newPolicy
 	if err := par.auth.UpdateProvisioner(ctx, prov); err != nil {
-		var pe *authority.PolicyError
+		if isBadRequest(err) {
-		isPolicyError := errors.As(err, &pe)
+			render.Error(w, admin.WrapError(admin.ErrorBadRequestType, err, "error updating provisioner policy"))
-		if isPolicyError && pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure || pe.Typ == authority.ConfigurationFailure {
-			render.Error(w, admin.WrapError(admin.ErrorBadRequestType, pe, "error updating provisioner policy"))
 			return
 		}
 
@@ -456,6 +447,14 @@ func (par *PolicyAdminResponder) blockLinkedCA() error {
 	return nil
 }
 
+// isBadRequest checks if an error should result in a bad request error
+// returned to the client.
+func isBadRequest(err error) bool {
+	var pe *authority.PolicyError
+	isPolicyError := errors.As(err, &pe)
+	return isPolicyError && (pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure || pe.Typ == authority.ConfigurationFailure)
+}
+
 // applyConditionalDefaults applies default settings in case they're not provided
 // in the request body.
 func applyConditionalDefaults(p *linkedca.Policy) {

authority/admin/api/policy_test.go

@@ -25,6 +25,7 @@ import (
 func TestPolicyAdminResponder_GetAuthorityPolicy(t *testing.T) {
 	type test struct {
 		auth           adminAuthority
+		deploymentType string
 		adminDB        admin.DB
 		ctx            context.Context
 		err            *admin.Error
@@ -32,6 +33,17 @@ func TestPolicyAdminResponder_GetAuthorityPolicy(t *testing.T) {
 		statusCode     int
 	}
 	var tests = map[string]func(t *testing.T) test{
+		"fail/linkedca": func(t *testing.T) test {
+			ctx := context.Background()
+			err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments")
+			err.Message = "policy operations not yet supported in linked deployments"
+			return test{
+				ctx:            ctx,
+				deploymentType: "linked",
+				err:            err,
+				statusCode:     501,
+			}
+		},
 		"fail/auth.GetAuthorityPolicy-error": func(t *testing.T) test {
 			ctx := context.Background()
 			err := admin.WrapErrorISE(errors.New("force"), "error retrieving authority policy")
@@ -89,6 +101,7 @@ func TestPolicyAdminResponder_GetAuthorityPolicy(t *testing.T) {
 			par := &PolicyAdminResponder{
 				auth:           tc.auth,
 				adminDB:        tc.adminDB,
+				deploymentType: tc.deploymentType,
 			}
 
 			req := httptest.NewRequest("GET", "/foo", nil)
@@ -128,6 +141,7 @@ func TestPolicyAdminResponder_GetAuthorityPolicy(t *testing.T) {
 func TestPolicyAdminResponder_CreateAuthorityPolicy(t *testing.T) {
 	type test struct {
 		auth           adminAuthority
+		deploymentType string
 		adminDB        admin.DB
 		body           []byte
 		ctx            context.Context
@@ -137,6 +151,17 @@ func TestPolicyAdminResponder_CreateAuthorityPolicy(t *testing.T) {
 		statusCode     int
 	}
 	var tests = map[string]func(t *testing.T) test{
+		"fail/linkedca": func(t *testing.T) test {
+			ctx := context.Background()
+			err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments")
+			err.Message = "policy operations not yet supported in linked deployments"
+			return test{
+				ctx:            ctx,
+				deploymentType: "linked",
+				err:            err,
+				statusCode:     501,
+			}
+		},
 		"fail/auth.GetAuthorityPolicy-error": func(t *testing.T) test {
 			ctx := context.Background()
 			err := admin.WrapErrorISE(errors.New("force"), "error retrieving authority policy")
@@ -323,6 +348,7 @@ func TestPolicyAdminResponder_CreateAuthorityPolicy(t *testing.T) {
 				auth:           tc.auth,
 				adminDB:        tc.adminDB,
 				acmeDB:         tc.acmeDB,
+				deploymentType: tc.deploymentType,
 			}
 
 			req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body)))
@@ -371,6 +397,7 @@ func TestPolicyAdminResponder_CreateAuthorityPolicy(t *testing.T) {
 func TestPolicyAdminResponder_UpdateAuthorityPolicy(t *testing.T) {
 	type test struct {
 		auth           adminAuthority
+		deploymentType string
 		adminDB        admin.DB
 		body           []byte
 		ctx            context.Context
@@ -380,6 +407,17 @@ func TestPolicyAdminResponder_UpdateAuthorityPolicy(t *testing.T) {
 		statusCode     int
 	}
 	var tests = map[string]func(t *testing.T) test{
+		"fail/linkedca": func(t *testing.T) test {
+			ctx := context.Background()
+			err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments")
+			err.Message = "policy operations not yet supported in linked deployments"
+			return test{
+				ctx:            ctx,
+				deploymentType: "linked",
+				err:            err,
+				statusCode:     501,
+			}
+		},
 		"fail/auth.GetAuthorityPolicy-error": func(t *testing.T) test {
 			ctx := context.Background()
 			err := admin.WrapErrorISE(errors.New("force"), "error retrieving authority policy")
@@ -573,6 +611,7 @@ func TestPolicyAdminResponder_UpdateAuthorityPolicy(t *testing.T) {
 				auth:           tc.auth,
 				adminDB:        tc.adminDB,
 				acmeDB:         tc.acmeDB,
+				deploymentType: tc.deploymentType,
 			}
 
 			req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body)))
@@ -621,6 +660,7 @@ func TestPolicyAdminResponder_UpdateAuthorityPolicy(t *testing.T) {
 func TestPolicyAdminResponder_DeleteAuthorityPolicy(t *testing.T) {
 	type test struct {
 		auth           adminAuthority
+		deploymentType string
 		adminDB        admin.DB
 		body           []byte
 		ctx            context.Context
@@ -630,6 +670,17 @@ func TestPolicyAdminResponder_DeleteAuthorityPolicy(t *testing.T) {
 	}
 
 	var tests = map[string]func(t *testing.T) test{
+		"fail/linkedca": func(t *testing.T) test {
+			ctx := context.Background()
+			err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments")
+			err.Message = "policy operations not yet supported in linked deployments"
+			return test{
+				ctx:            ctx,
+				deploymentType: "linked",
+				err:            err,
+				statusCode:     501,
+			}
+		},
 		"fail/auth.GetAuthorityPolicy-error": func(t *testing.T) test {
 			ctx := context.Background()
 			err := admin.WrapErrorISE(errors.New("force"), "error retrieving authority policy")
@@ -716,6 +767,7 @@ func TestPolicyAdminResponder_DeleteAuthorityPolicy(t *testing.T) {
 				auth:           tc.auth,
 				adminDB:        tc.adminDB,
 				acmeDB:         tc.acmeDB,
+				deploymentType: tc.deploymentType,
 			}
 
 			req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body)))
@@ -759,6 +811,7 @@ func TestPolicyAdminResponder_DeleteAuthorityPolicy(t *testing.T) {
 func TestPolicyAdminResponder_GetProvisionerPolicy(t *testing.T) {
 	type test struct {
 		auth           adminAuthority
+		deploymentType string
 		adminDB        admin.DB
 		ctx            context.Context
 		acmeDB         acme.DB
@@ -767,6 +820,17 @@ func TestPolicyAdminResponder_GetProvisionerPolicy(t *testing.T) {
 		statusCode     int
 	}
 	var tests = map[string]func(t *testing.T) test{
+		"fail/linkedca": func(t *testing.T) test {
+			ctx := context.Background()
+			err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments")
+			err.Message = "policy operations not yet supported in linked deployments"
+			return test{
+				ctx:            ctx,
+				deploymentType: "linked",
+				err:            err,
+				statusCode:     501,
+			}
+		},
 		"fail/prov-no-policy": func(t *testing.T) test {
 			prov := &linkedca.Provisioner{}
 			ctx := linkedca.NewContextWithProvisioner(context.Background(), prov)
@@ -804,6 +868,7 @@ func TestPolicyAdminResponder_GetProvisionerPolicy(t *testing.T) {
 				auth:           tc.auth,
 				adminDB:        tc.adminDB,
 				acmeDB:         tc.acmeDB,
+				deploymentType: tc.deploymentType,
 			}
 
 			req := httptest.NewRequest("GET", "/foo", nil)
@@ -843,6 +908,7 @@ func TestPolicyAdminResponder_GetProvisionerPolicy(t *testing.T) {
 func TestPolicyAdminResponder_CreateProvisionerPolicy(t *testing.T) {
 	type test struct {
 		auth           adminAuthority
+		deploymentType string
 		body           []byte
 		ctx            context.Context
 		err            *admin.Error
@@ -850,6 +916,17 @@ func TestPolicyAdminResponder_CreateProvisionerPolicy(t *testing.T) {
 		statusCode     int
 	}
 	var tests = map[string]func(t *testing.T) test{
+		"fail/linkedca": func(t *testing.T) test {
+			ctx := context.Background()
+			err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments")
+			err.Message = "policy operations not yet supported in linked deployments"
+			return test{
+				ctx:            ctx,
+				deploymentType: "linked",
+				err:            err,
+				statusCode:     501,
+			}
+		},
 		"fail/existing-policy": func(t *testing.T) test {
 			policy := &linkedca.Policy{
 				X509: &linkedca.X509Policy{
@@ -993,6 +1070,7 @@ func TestPolicyAdminResponder_CreateProvisionerPolicy(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			par := &PolicyAdminResponder{
 				auth:           tc.auth,
+				deploymentType: tc.deploymentType,
 			}
 
 			req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body)))
@@ -1041,6 +1119,7 @@ func TestPolicyAdminResponder_CreateProvisionerPolicy(t *testing.T) {
 func TestPolicyAdminResponder_UpdateProvisionerPolicy(t *testing.T) {
 	type test struct {
 		auth           adminAuthority
+		deploymentType string
 		body           []byte
 		ctx            context.Context
 		err            *admin.Error
@@ -1048,6 +1127,17 @@ func TestPolicyAdminResponder_UpdateProvisionerPolicy(t *testing.T) {
 		statusCode     int
 	}
 	var tests = map[string]func(t *testing.T) test{
+		"fail/linkedca": func(t *testing.T) test {
+			ctx := context.Background()
+			err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments")
+			err.Message = "policy operations not yet supported in linked deployments"
+			return test{
+				ctx:            ctx,
+				deploymentType: "linked",
+				err:            err,
+				statusCode:     501,
+			}
+		},
 		"fail/no-existing-policy": func(t *testing.T) test {
 			prov := &linkedca.Provisioner{
 				Name: "provName",
@@ -1193,6 +1283,7 @@ func TestPolicyAdminResponder_UpdateProvisionerPolicy(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			par := &PolicyAdminResponder{
 				auth:           tc.auth,
+				deploymentType: tc.deploymentType,
 			}
 
 			req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body)))
@@ -1241,6 +1332,7 @@ func TestPolicyAdminResponder_UpdateProvisionerPolicy(t *testing.T) {
 func TestPolicyAdminResponder_DeleteProvisionerPolicy(t *testing.T) {
 	type test struct {
 		auth           adminAuthority
+		deploymentType string
 		adminDB        admin.DB
 		body           []byte
 		ctx            context.Context
@@ -1250,6 +1342,17 @@ func TestPolicyAdminResponder_DeleteProvisionerPolicy(t *testing.T) {
 	}
 
 	var tests = map[string]func(t *testing.T) test{
+		"fail/linkedca": func(t *testing.T) test {
+			ctx := context.Background()
+			err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments")
+			err.Message = "policy operations not yet supported in linked deployments"
+			return test{
+				ctx:            ctx,
+				deploymentType: "linked",
+				err:            err,
+				statusCode:     501,
+			}
+		},
 		"fail/no-existing-policy": func(t *testing.T) test {
 			prov := &linkedca.Provisioner{
 				Name: "provName",
@@ -1306,6 +1409,7 @@ func TestPolicyAdminResponder_DeleteProvisionerPolicy(t *testing.T) {
 				auth:           tc.auth,
 				adminDB:        tc.adminDB,
 				acmeDB:         tc.acmeDB,
+				deploymentType: tc.deploymentType,
 			}
 
 			req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body)))
@@ -1348,6 +1452,7 @@ func TestPolicyAdminResponder_DeleteProvisionerPolicy(t *testing.T) {
 
 func TestPolicyAdminResponder_GetACMEAccountPolicy(t *testing.T) {
 	type test struct {
+		deploymentType string
 		ctx            context.Context
 		acmeDB         acme.DB
 		err            *admin.Error
@@ -1355,6 +1460,17 @@ func TestPolicyAdminResponder_GetACMEAccountPolicy(t *testing.T) {
 		statusCode     int
 	}
 	var tests = map[string]func(t *testing.T) test{
+		"fail/linkedca": func(t *testing.T) test {
+			ctx := context.Background()
+			err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments")
+			err.Message = "policy operations not yet supported in linked deployments"
+			return test{
+				ctx:            ctx,
+				deploymentType: "linked",
+				err:            err,
+				statusCode:     501,
+			}
+		},
 		"fail/no-policy": func(t *testing.T) test {
 			prov := &linkedca.Provisioner{
 				Name: "provName",
@@ -1401,6 +1517,7 @@ func TestPolicyAdminResponder_GetACMEAccountPolicy(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			par := &PolicyAdminResponder{
 				acmeDB:         tc.acmeDB,
+				deploymentType: tc.deploymentType,
 			}
 
 			req := httptest.NewRequest("GET", "/foo", nil)
@@ -1439,6 +1556,7 @@ func TestPolicyAdminResponder_GetACMEAccountPolicy(t *testing.T) {
 
 func TestPolicyAdminResponder_CreateACMEAccountPolicy(t *testing.T) {
 	type test struct {
+		deploymentType string
 		acmeDB         acme.DB
 		body           []byte
 		ctx            context.Context
@@ -1447,6 +1565,17 @@ func TestPolicyAdminResponder_CreateACMEAccountPolicy(t *testing.T) {
 		statusCode     int
 	}
 	var tests = map[string]func(t *testing.T) test{
+		"fail/linkedca": func(t *testing.T) test {
+			ctx := context.Background()
+			err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments")
+			err.Message = "policy operations not yet supported in linked deployments"
+			return test{
+				ctx:            ctx,
+				deploymentType: "linked",
+				err:            err,
+				statusCode:     501,
+			}
+		},
 		"fail/existing-policy": func(t *testing.T) test {
 			policy := &linkedca.Policy{
 				X509: &linkedca.X509Policy{
@@ -1565,6 +1694,7 @@ func TestPolicyAdminResponder_CreateACMEAccountPolicy(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			par := &PolicyAdminResponder{
 				acmeDB:         tc.acmeDB,
+				deploymentType: tc.deploymentType,
 			}
 
 			req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body)))
@@ -1612,6 +1742,7 @@ func TestPolicyAdminResponder_CreateACMEAccountPolicy(t *testing.T) {
 
 func TestPolicyAdminResponder_UpdateACMEAccountPolicy(t *testing.T) {
 	type test struct {
+		deploymentType string
 		acmeDB         acme.DB
 		body           []byte
 		ctx            context.Context
@@ -1620,6 +1751,17 @@ func TestPolicyAdminResponder_UpdateACMEAccountPolicy(t *testing.T) {
 		statusCode     int
 	}
 	var tests = map[string]func(t *testing.T) test{
+		"fail/linkedca": func(t *testing.T) test {
+			ctx := context.Background()
+			err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments")
+			err.Message = "policy operations not yet supported in linked deployments"
+			return test{
+				ctx:            ctx,
+				deploymentType: "linked",
+				err:            err,
+				statusCode:     501,
+			}
+		},
 		"fail/no-existing-policy": func(t *testing.T) test {
 			prov := &linkedca.Provisioner{
 				Name: "provName",
@@ -1740,6 +1882,7 @@ func TestPolicyAdminResponder_UpdateACMEAccountPolicy(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			par := &PolicyAdminResponder{
 				acmeDB:         tc.acmeDB,
+				deploymentType: tc.deploymentType,
 			}
 
 			req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body)))
@@ -1787,6 +1930,7 @@ func TestPolicyAdminResponder_UpdateACMEAccountPolicy(t *testing.T) {
 
 func TestPolicyAdminResponder_DeleteACMEAccountPolicy(t *testing.T) {
 	type test struct {
+		deploymentType string
 		body           []byte
 		ctx            context.Context
 		acmeDB         acme.DB
@@ -1795,6 +1939,17 @@ func TestPolicyAdminResponder_DeleteACMEAccountPolicy(t *testing.T) {
 	}
 
 	var tests = map[string]func(t *testing.T) test{
+		"fail/linkedca": func(t *testing.T) test {
+			ctx := context.Background()
+			err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments")
+			err.Message = "policy operations not yet supported in linked deployments"
+			return test{
+				ctx:            ctx,
+				deploymentType: "linked",
+				err:            err,
+				statusCode:     501,
+			}
+		},
 		"fail/no-existing-policy": func(t *testing.T) test {
 			prov := &linkedca.Provisioner{
 				Name: "provName",
@@ -1881,6 +2036,7 @@ func TestPolicyAdminResponder_DeleteACMEAccountPolicy(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			par := &PolicyAdminResponder{
 				acmeDB:         tc.acmeDB,
+				deploymentType: tc.deploymentType,
 			}
 
 			req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body)))
@@ -2000,3 +2156,45 @@ func Test_applyConditionalDefaults(t *testing.T) {
 		})
 	}
 }
+
+func Test_isBadRequest(t *testing.T) {
+	tests := []struct {
+		name string
+		err  error
+		want bool
+	}{
+		{
+			name: "nil",
+			err:  nil,
+			want: false,
+		},
+		{
+			name: "no-policy-error",
+			err:  errors.New("some error"),
+			want: false,
+		},
+		{
+			name: "no-bad-request",
+			err: &authority.PolicyError{
+				Typ: authority.InternalFailure,
+				Err: errors.New("error"),
+			},
+			want: false,
+		},
+		{
+			name: "bad-request",
+			err: &authority.PolicyError{
+				Typ: authority.AdminLockOut,
+				Err: errors.New("admin lock out"),
+			},
+			want: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isBadRequest(tt.err); got != tt.want {
+				t.Errorf("isBadRequest() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}