mirror of https://github.com/hashicorp/boundary
Request Validation logic moved into a helper (#296)
* Move all the validation logic into a helper tool. * Adding tests for verifier helpers.pull/290/head
Todd Knight
6 years ago
committed by
GitHub
parent
bc32272ca7
commit
e423b6589e
@ -0,0 +1,137 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"regexp"
|
||||
"strings"
|
||||
|
||||
"github.com/golang/protobuf/ptypes/timestamp"
|
||||
"github.com/golang/protobuf/ptypes/wrappers"
|
||||
"github.com/hashicorp/boundary/internal/gen/controller/api/resources/scopes"
|
||||
"google.golang.org/genproto/protobuf/field_mask"
|
||||
)
|
||||
|
||||
type CustomValidatorFunc func() map[string]string
|
||||
|
||||
var NoopValidatorFn CustomValidatorFunc = func() map[string]string { return nil }
|
||||
|
||||
type ApiResource interface {
|
||||
GetId() string
|
||||
GetScope() *scopes.ScopeInfo
|
||||
GetName() *wrappers.StringValue
|
||||
GetDescription() *wrappers.StringValue
|
||||
GetCreatedTime() *timestamp.Timestamp
|
||||
GetUpdatedTime() *timestamp.Timestamp
|
||||
GetVersion() uint32
|
||||
}
|
||||
|
||||
func ValidateCreateRequest(i ApiResource, fn CustomValidatorFunc) error {
|
||||
badFields := map[string]string{}
|
||||
if i.GetId() != "" {
|
||||
badFields["id"] = "This is a read only field."
|
||||
}
|
||||
if i.GetCreatedTime() != nil {
|
||||
badFields["created_time"] = "This is a read only field."
|
||||
}
|
||||
if i.GetUpdatedTime() != nil {
|
||||
badFields["updated_time"] = "This is a read only field."
|
||||
}
|
||||
if i.GetVersion() != 0 {
|
||||
badFields["version"] = "Cannot specify this field in a create request."
|
||||
}
|
||||
for k, v := range fn() {
|
||||
badFields[k] = v
|
||||
}
|
||||
if len(badFields) > 0 {
|
||||
return InvalidArgumentErrorf("Argument errors found in the request.", badFields)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
type UpdateRequest interface {
|
||||
GetId() string
|
||||
GetUpdateMask() *field_mask.FieldMask
|
||||
}
|
||||
|
||||
func ValidateUpdateRequest(prefix string, r UpdateRequest, i ApiResource, fn CustomValidatorFunc) error {
|
||||
badFields := map[string]string{}
|
||||
if !ValidId(prefix, r.GetId()) {
|
||||
badFields["id"] = "Improperly formatted path identifier."
|
||||
}
|
||||
if r.GetUpdateMask() == nil {
|
||||
badFields["update_mask"] = "UpdateMask not provided but is required to update this resource."
|
||||
}
|
||||
|
||||
if i == nil {
|
||||
// It is legitimate for no item to be specified in an update request as it indicates all fields provided in
|
||||
// the mask will be marked as unset.
|
||||
return nil
|
||||
}
|
||||
if i.GetVersion() == 0 {
|
||||
badFields["version"] = "Existing resource version is required for an update."
|
||||
}
|
||||
if i.GetId() != "" {
|
||||
badFields["id"] = "This is a read only field and cannot be specified in an update request."
|
||||
}
|
||||
if i.GetCreatedTime() != nil {
|
||||
badFields["created_time"] = "This is a read only field and cannot be specified in an update request."
|
||||
}
|
||||
if i.GetUpdatedTime() != nil {
|
||||
badFields["updated_time"] = "This is a read only field and cannot be specified in an update request."
|
||||
}
|
||||
|
||||
for k, v := range fn() {
|
||||
badFields[k] = v
|
||||
}
|
||||
|
||||
if len(badFields) > 0 {
|
||||
return InvalidArgumentErrorf("Errors in provided fields.", badFields)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
type GetRequest interface {
|
||||
GetId() string
|
||||
}
|
||||
|
||||
func ValidateGetRequest(prefix string, r GetRequest, fn CustomValidatorFunc) error {
|
||||
badFields := map[string]string{}
|
||||
if !ValidId(prefix, r.GetId()) {
|
||||
badFields["id"] = "Invalid formatted group id."
|
||||
}
|
||||
for k, v := range fn() {
|
||||
badFields[k] = v
|
||||
}
|
||||
if len(badFields) > 0 {
|
||||
return InvalidArgumentErrorf("Improperly formatted identifier.", badFields)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
type DeleteRequest interface {
|
||||
GetId() string
|
||||
}
|
||||
|
||||
func ValidateDeleteRequest(prefix string, r DeleteRequest, fn CustomValidatorFunc) error {
|
||||
badFields := map[string]string{}
|
||||
if !ValidId(prefix, r.GetId()) {
|
||||
badFields["id"] = "Incorrectly formatted identifier."
|
||||
}
|
||||
for k, v := range fn() {
|
||||
badFields[k] = v
|
||||
}
|
||||
if len(badFields) > 0 {
|
||||
return InvalidArgumentErrorf("Errors in provided fields.", badFields)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
var reInvalidID = regexp.MustCompile("[^A-Za-z0-9]")
|
||||
|
||||
func ValidId(prefix, id string) bool {
|
||||
prefix = prefix + "_"
|
||||
if !strings.HasPrefix(id, prefix) {
|
||||
return false
|
||||
}
|
||||
id = strings.TrimPrefix(id, prefix)
|
||||
return !reInvalidID.Match([]byte(id))
|
||||
}
|
||||
@ -0,0 +1,389 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
pb "github.com/hashicorp/boundary/internal/gen/controller/api/resources/users"
|
||||
pbs "github.com/hashicorp/boundary/internal/gen/controller/api/services"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"google.golang.org/genproto/googleapis/rpc/errdetails"
|
||||
"google.golang.org/grpc/status"
|
||||
"google.golang.org/protobuf/types/known/fieldmaskpb"
|
||||
"google.golang.org/protobuf/types/known/timestamppb"
|
||||
)
|
||||
|
||||
func errorIncludesFields(t *testing.T, err error, wantFields []string) {
|
||||
t.Helper()
|
||||
s, ok := status.FromError(err)
|
||||
require.True(t, ok)
|
||||
require.Len(t, s.Details(), 1)
|
||||
rd, ok := s.Details()[0].(*errdetails.BadRequest)
|
||||
require.True(t, ok)
|
||||
var gotFields []string
|
||||
for _, d := range rd.FieldViolations {
|
||||
gotFields = append(gotFields, d.Field)
|
||||
}
|
||||
assert.ElementsMatch(t, gotFields, wantFields)
|
||||
}
|
||||
|
||||
// Throughout this test we will use the User requests, but we could use any request
|
||||
// message. User was picked arbitrarily.
|
||||
|
||||
func TestValidId(t *testing.T) {
|
||||
assert.True(t, ValidId("prefix", "prefix_somerandomid"))
|
||||
assert.True(t, ValidId("prefix", "prefix_short"))
|
||||
assert.True(t, ValidId("prefix", "prefix_thisisalongidentifierwhichstillworks"))
|
||||
|
||||
assert.False(t, ValidId("prefix", "prefixsomerandomid"))
|
||||
assert.False(t, ValidId("prefix", "prefix_this has spaces"))
|
||||
assert.False(t, ValidId("prefix", "prefix_includes-dash"))
|
||||
assert.False(t, ValidId("prefix", "prefix_other@strange!characters"))
|
||||
assert.False(t, ValidId("short", "prefix_short"))
|
||||
}
|
||||
|
||||
func TestValidateGetRequest(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
prefix string
|
||||
req GetRequest
|
||||
valFn CustomValidatorFunc
|
||||
badFields []string
|
||||
}{
|
||||
{
|
||||
name: "noopvalidator no error",
|
||||
prefix: "prefix",
|
||||
req: &pbs.GetUserRequest{
|
||||
Id: "prefix_someidentifier",
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
},
|
||||
{
|
||||
name: "noopvalidator bad prefix",
|
||||
prefix: "bad",
|
||||
req: &pbs.GetUserRequest{
|
||||
Id: "prefix_someidentifier",
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"id"},
|
||||
},
|
||||
{
|
||||
name: "custom field error",
|
||||
prefix: "prefix",
|
||||
req: &pbs.GetUserRequest{
|
||||
Id: "prefix_someidentifier",
|
||||
},
|
||||
valFn: func() map[string]string {
|
||||
return map[string]string{"test": "test"}
|
||||
},
|
||||
badFields: []string{"test"},
|
||||
},
|
||||
{
|
||||
name: "both custom error and prefix",
|
||||
prefix: "bad",
|
||||
req: &pbs.GetUserRequest{
|
||||
Id: "prefix_someidentifier",
|
||||
},
|
||||
valFn: func() map[string]string {
|
||||
return map[string]string{"test": "test"}
|
||||
},
|
||||
badFields: []string{"id", "test"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
err := ValidateGetRequest(tc.prefix, tc.req, tc.valFn)
|
||||
if len(tc.badFields) == 0 {
|
||||
if !assert.NoError(t, err) {
|
||||
errorIncludesFields(t, err, []string{})
|
||||
}
|
||||
return
|
||||
}
|
||||
errorIncludesFields(t, err, tc.badFields)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateDeleteRequest(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
prefix string
|
||||
req DeleteRequest
|
||||
valFn CustomValidatorFunc
|
||||
badFields []string
|
||||
}{
|
||||
{
|
||||
name: "noopvalidator no error",
|
||||
prefix: "prefix",
|
||||
req: &pbs.DeleteUserRequest{
|
||||
Id: "prefix_someidentifier",
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
},
|
||||
{
|
||||
name: "noopvalidator bad prefix",
|
||||
prefix: "bad",
|
||||
req: &pbs.DeleteUserRequest{
|
||||
Id: "prefix_someidentifier",
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"id"},
|
||||
},
|
||||
{
|
||||
name: "custom field error",
|
||||
prefix: "prefix",
|
||||
req: &pbs.DeleteUserRequest{
|
||||
Id: "prefix_someidentifier",
|
||||
},
|
||||
valFn: func() map[string]string {
|
||||
return map[string]string{"test": "test"}
|
||||
},
|
||||
badFields: []string{"test"},
|
||||
},
|
||||
{
|
||||
name: "both custom error and prefix",
|
||||
prefix: "bad",
|
||||
req: &pbs.DeleteUserRequest{
|
||||
Id: "prefix_someidentifier",
|
||||
},
|
||||
valFn: func() map[string]string {
|
||||
return map[string]string{"test": "test"}
|
||||
},
|
||||
badFields: []string{"id", "test"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
err := ValidateDeleteRequest(tc.prefix, tc.req, tc.valFn)
|
||||
if len(tc.badFields) == 0 {
|
||||
if !assert.NoError(t, err) {
|
||||
errorIncludesFields(t, err, []string{})
|
||||
}
|
||||
return
|
||||
}
|
||||
errorIncludesFields(t, err, tc.badFields)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateCreateRequest(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
item ApiResource
|
||||
valFn CustomValidatorFunc
|
||||
badFields []string
|
||||
}{
|
||||
{
|
||||
name: "valid",
|
||||
item: &pb.User{},
|
||||
valFn: NoopValidatorFn,
|
||||
},
|
||||
{
|
||||
name: "disallow set id",
|
||||
item: &pb.User{
|
||||
Id: "anything",
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"id"},
|
||||
},
|
||||
{
|
||||
name: "disallow set created",
|
||||
item: &pb.User{
|
||||
CreatedTime: timestamppb.Now(),
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"created_time"},
|
||||
},
|
||||
{
|
||||
name: "disallow set updated",
|
||||
item: &pb.User{
|
||||
UpdatedTime: timestamppb.Now(),
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"updated_time"},
|
||||
},
|
||||
{
|
||||
name: "disallow set version",
|
||||
item: &pb.User{
|
||||
Version: 4,
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"version"},
|
||||
},
|
||||
{
|
||||
name: "custom validator error",
|
||||
item: &pb.User{},
|
||||
valFn: func() map[string]string {
|
||||
return map[string]string{"test": "test"}
|
||||
},
|
||||
badFields: []string{"test"},
|
||||
},
|
||||
{
|
||||
name: "disallow several fields",
|
||||
item: &pb.User{
|
||||
Id: "anything",
|
||||
CreatedTime: timestamppb.Now(),
|
||||
Version: 4,
|
||||
},
|
||||
valFn: func() map[string]string {
|
||||
return map[string]string{"test": "test"}
|
||||
},
|
||||
badFields: []string{"id", "created_time", "version", "test"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
err := ValidateCreateRequest(tc.item, tc.valFn)
|
||||
if len(tc.badFields) == 0 {
|
||||
if !assert.NoError(t, err) {
|
||||
errorIncludesFields(t, err, []string{})
|
||||
}
|
||||
return
|
||||
}
|
||||
errorIncludesFields(t, err, tc.badFields)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateUpdateRequest(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
prefix string
|
||||
req UpdateRequest
|
||||
item ApiResource
|
||||
valFn CustomValidatorFunc
|
||||
badFields []string
|
||||
}{
|
||||
{
|
||||
name: "valid",
|
||||
prefix: "prefix",
|
||||
req: &pbs.UpdateUserRequest{
|
||||
Id: "prefix_something",
|
||||
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"anything"}},
|
||||
},
|
||||
item: &pb.User{
|
||||
Version: 1,
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
},
|
||||
{
|
||||
name: "missing mask",
|
||||
prefix: "prefix",
|
||||
req: &pbs.UpdateUserRequest{
|
||||
Id: "prefix_something",
|
||||
},
|
||||
item: &pb.User{
|
||||
Version: 1,
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"update_mask"},
|
||||
},
|
||||
{
|
||||
name: "bad id",
|
||||
prefix: "prefix",
|
||||
req: &pbs.UpdateUserRequest{
|
||||
Id: "mismatched_prefix",
|
||||
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"anything"}},
|
||||
},
|
||||
item: &pb.User{
|
||||
Version: 1,
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"id"},
|
||||
},
|
||||
{
|
||||
name: "missing version",
|
||||
prefix: "prefix",
|
||||
req: &pbs.UpdateUserRequest{
|
||||
Id: "prefix_something",
|
||||
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"anything"}},
|
||||
},
|
||||
item: &pb.User{},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"version"},
|
||||
},
|
||||
{
|
||||
name: "bad create time",
|
||||
prefix: "prefix",
|
||||
req: &pbs.UpdateUserRequest{
|
||||
Id: "prefix_something",
|
||||
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"anything"}},
|
||||
},
|
||||
item: &pb.User{
|
||||
Version: 1,
|
||||
CreatedTime: timestamppb.Now(),
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"created_time"},
|
||||
},
|
||||
{
|
||||
name: "bad updated time",
|
||||
prefix: "prefix",
|
||||
req: &pbs.UpdateUserRequest{
|
||||
Id: "prefix_something",
|
||||
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"anything"}},
|
||||
},
|
||||
item: &pb.User{
|
||||
Version: 1,
|
||||
UpdatedTime: timestamppb.Now(),
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"updated_time"},
|
||||
},
|
||||
{
|
||||
name: "bad defined id on item",
|
||||
prefix: "prefix",
|
||||
req: &pbs.UpdateUserRequest{
|
||||
Id: "prefix_something",
|
||||
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"anything"}},
|
||||
},
|
||||
item: &pb.User{
|
||||
Id: "prefix_something",
|
||||
Version: 1,
|
||||
},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"id"},
|
||||
},
|
||||
{
|
||||
name: "custom validator error",
|
||||
prefix: "prefix",
|
||||
req: &pbs.UpdateUserRequest{
|
||||
Id: "prefix_something",
|
||||
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"anything"}},
|
||||
},
|
||||
item: &pb.User{
|
||||
Version: 1,
|
||||
},
|
||||
valFn: func() map[string]string {
|
||||
return map[string]string{
|
||||
"test": "test",
|
||||
}
|
||||
},
|
||||
badFields: []string{"test"},
|
||||
},
|
||||
{
|
||||
name: "multiple",
|
||||
prefix: "prefix",
|
||||
req: &pbs.UpdateUserRequest{},
|
||||
item: &pb.User{},
|
||||
valFn: NoopValidatorFn,
|
||||
badFields: []string{"id", "update_mask", "version"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
err := ValidateUpdateRequest(tc.prefix, tc.req, tc.item, tc.valFn)
|
||||
if len(tc.badFields) == 0 {
|
||||
if !assert.NoError(t, err) {
|
||||
errorIncludesFields(t, err, []string{})
|
||||
}
|
||||
return
|
||||
}
|
||||
errorIncludesFields(t, err, tc.badFields)
|
||||
})
|
||||
}
|
||||
}
|
||||
Loading…
Reference in new issue