From e99b618d6e6fdb1eae2773bedddef9f19f43d38a Mon Sep 17 00:00:00 2001 From: Kieron Browne Date: Fri, 23 Jun 2023 15:49:59 +0100 Subject: [PATCH] Move role api param testing to payloads package We also migrated all role validation to jellydator Issue: https://github.com/cloudfoundry/korifi/issues/1996 Co-authored-by: Danail Branekov --- api/handlers/role.go | 36 ++------- api/handlers/role_test.go | 74 +++++++++--------- api/handlers/validator.go | 60 --------------- api/payloads/role.go | 126 ++++++++++++++++++++++++------- api/payloads/role_test.go | 121 ++++++++++++++--------------- api/payloads/validation/rules.go | 2 +- 6 files changed, 199 insertions(+), 220 deletions(-) diff --git a/api/handlers/role.go b/api/handlers/role.go index 1a55b8ad2..13011a895 100644 --- a/api/handlers/role.go +++ b/api/handlers/role.go @@ -21,22 +21,6 @@ const ( RolePath = RolesPath + "/{guid}" ) -type RoleName string - -const ( - RoleAdmin RoleName = "admin" - RoleAdminReadOnly RoleName = "admin_read_only" - RoleGlobalAuditor RoleName = "global_auditor" - RoleOrganizationAuditor RoleName = "organization_auditor" - RoleOrganizationBillingManager RoleName = "organization_billing_manager" - RoleOrganizationManager RoleName = "organization_manager" - RoleOrganizationUser RoleName = "organization_user" - RoleSpaceAuditor RoleName = "space_auditor" - RoleSpaceDeveloper RoleName = "space_developer" - RoleSpaceManager RoleName = "space_manager" - RoleSpaceSupporter RoleName = "space_supporter" -) - //counterfeiter:generate -o fake -fake-name CFRoleRepository . CFRoleRepository type CFRoleRepository interface { @@ -49,14 +33,14 @@ type CFRoleRepository interface { type Role struct { apiBaseURL url.URL roleRepo CFRoleRepository - decoderValidator RequestValidator + requestValidator RequestValidator } -func NewRole(apiBaseURL url.URL, roleRepo CFRoleRepository, decoderValidator RequestValidator) *Role { +func NewRole(apiBaseURL url.URL, roleRepo CFRoleRepository, requestValidator RequestValidator) *Role { return &Role{ apiBaseURL: apiBaseURL, roleRepo: roleRepo, - decoderValidator: decoderValidator, + requestValidator: requestValidator, } } @@ -65,7 +49,7 @@ func (h *Role) create(r *http.Request) (*routing.Response, error) { logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.role.create") var payload payloads.RoleCreate - if err := h.decoderValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { + if err := h.requestValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to decode payload") } @@ -84,12 +68,8 @@ func (h *Role) list(r *http.Request) (*routing.Response, error) { authInfo, _ := authorization.InfoFromContext(r.Context()) logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.role.list") - if err := r.ParseForm(); err != nil { - return nil, apierrors.LogAndReturn(logger, err, "Unable to parse request query parameters") - } - - roleListFilter := new(payloads.RoleListFilter) - err := payloads.Decode(roleListFilter, r.Form) + roleListFilter := new(payloads.RoleList) + err := h.requestValidator.DecodeAndValidateURLValues(r, roleListFilter) if err != nil { return nil, apierrors.LogAndReturn(logger, err, "Unable to decode request query parameters") } @@ -101,14 +81,14 @@ func (h *Role) list(r *http.Request) (*routing.Response, error) { filteredRoles := filterRoles(roleListFilter, roles) - if err := h.sortList(filteredRoles, r.FormValue("order_by")); err != nil { + if err := h.sortList(filteredRoles, roleListFilter.OrderBy); err != nil { return nil, apierrors.LogAndReturn(logger, err, "unable to parse order by request") } return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForRole, filteredRoles, h.apiBaseURL, *r.URL)), nil } -func filterRoles(roleListFilter *payloads.RoleListFilter, roles []repositories.RoleRecord) []repositories.RoleRecord { +func filterRoles(roleListFilter *payloads.RoleList, roles []repositories.RoleRecord) []repositories.RoleRecord { var filteredRoles []repositories.RoleRecord for _, role := range roles { if match(roleListFilter.GUIDs, role.GUID) && diff --git a/api/handlers/role_test.go b/api/handlers/role_test.go index c50d50864..bebf128ff 100644 --- a/api/handlers/role_test.go +++ b/api/handlers/role_test.go @@ -2,6 +2,7 @@ package handlers_test import ( "errors" + "io" "net/http" "net/http/httptest" "strings" @@ -45,7 +46,7 @@ var _ = Describe("Role", func() { roleCreate = &payloads.RoleCreate{ Type: "space_developer", Relationships: payloads.RoleRelationships{ - User: &payloads.UserRelationship{ + User: payloads.UserRelationship{ Data: payloads.UserRelationshipData{ Username: "my-user", }, @@ -62,12 +63,18 @@ var _ = Describe("Role", func() { }) JustBeforeEach(func() { - req, err := http.NewRequestWithContext(ctx, "POST", rolesBase, strings.NewReader("")) + req, err := http.NewRequestWithContext(ctx, "POST", rolesBase, strings.NewReader("payload-data")) Expect(err).NotTo(HaveOccurred()) routerBuilder.Build().ServeHTTP(rr, req) }) It("creates the role", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + req, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + bodyBytes, err := io.ReadAll(req.Body) + Expect(err).NotTo(HaveOccurred()) + Expect(string(bodyBytes)).To(Equal("payload-data")) + Expect(roleRepo.CreateRoleCallCount()).To(Equal(1)) _, actualAuthInfo, roleMessage := roleRepo.CreateRoleArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) @@ -153,23 +160,25 @@ var _ = Describe("Role", func() { }) Describe("List roles", func() { - var query string - BeforeEach(func() { - query = "" roleRepo.ListRolesReturns([]repositories.RoleRecord{ {GUID: "role-1"}, {GUID: "role-2"}, }, nil) + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.RoleList{}) }) JustBeforeEach(func() { - req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+query, nil) + req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+"?foo=bar", nil) Expect(err).NotTo(HaveOccurred()) routerBuilder.Build().ServeHTTP(rr, req) }) It("lists roles", func() { + Expect(requestValidator.DecodeAndValidateURLValuesCallCount()).To(Equal(1)) + req, _ := requestValidator.DecodeAndValidateURLValuesArgsForCall(0) + Expect(req.URL.String()).To(HaveSuffix(rolesBase + "?foo=bar")) + Expect(roleRepo.ListRolesCallCount()).To(Equal(1)) _, actualAuthInfo := roleRepo.ListRolesArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) @@ -178,7 +187,7 @@ var _ = Describe("Role", func() { Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json")) Expect(rr).To(HaveHTTPBody(SatisfyAll( MatchJSONPath("$.pagination.total_results", BeEquivalentTo(2)), - MatchJSONPath("$.pagination.first.href", "https://api.example.org/v3/roles"), + MatchJSONPath("$.pagination.first.href", "https://api.example.org/v3/roles?foo=bar"), MatchJSONPath("$.resources", HaveLen(2)), MatchJSONPath("$.resources[0].guid", "role-1"), MatchJSONPath("$.resources[0].links.self.href", "https://api.example.org/v3/roles/role-1"), @@ -186,17 +195,6 @@ var _ = Describe("Role", func() { ))) }) - When("include is specified", func() { - BeforeEach(func() { - query = "?include=user" - }) - - It("does not fail but has no effect on the result", func() { - Expect(rr).To(HaveHTTPStatus(http.StatusOK)) - Expect(rr).To(HaveHTTPBody(MatchJSONPath("$.pagination.total_results", BeEquivalentTo(2)))) - }) - }) - Describe("filtering and ordering", func() { BeforeEach(func() { roleRepo.ListRolesReturns([]repositories.RoleRecord{ @@ -207,31 +205,35 @@ var _ = Describe("Role", func() { }, nil) }) - DescribeTable("filtering", func(filter string, expectedGUIDs ...any) { - req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+"?"+filter, nil) + DescribeTable("filtering", func(filter payloads.RoleList, expectedGUIDs ...any) { + req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+"?foo=bar", nil) Expect(err).NotTo(HaveOccurred()) + + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&filter) rr = httptest.NewRecorder() routerBuilder.Build().ServeHTTP(rr, req) Expect(rr).To(HaveHTTPStatus(http.StatusOK)) Expect(rr).To(HaveHTTPBody(MatchJSONPath("$.resources[*].guid", expectedGUIDs))) }, - Entry("no filter", "", "1", "2", "3", "4"), - Entry("guids1", "guids=4", "4"), - Entry("guids2", "guids=1,3", "1", "3"), - Entry("types1", "types=a", "1"), - Entry("types2", "types=b,c", "2", "3", "4"), - Entry("space_guids1", "space_guids=space1", "1"), - Entry("space_guids2", "space_guids=space1,space2", "1", "2"), - Entry("organization_guids1", "organization_guids=org1", "3"), - Entry("organization_guids2", "organization_guids=org1,org2", "3", "4"), - Entry("user_guids1", "user_guids=user1", "1", "2", "3"), - Entry("user_guids2", "user_guids=user1,user2", "1", "2", "3", "4"), + Entry("no filter", payloads.RoleList{}, "1", "2", "3", "4"), + Entry("guids1", payloads.RoleList{GUIDs: map[string]bool{"4": true}}, "4"), + Entry("guids2", payloads.RoleList{GUIDs: map[string]bool{"1": true, "3": true}}, "1", "3"), + Entry("types1", payloads.RoleList{Types: map[string]bool{"a": true}}, "1"), + Entry("types2", payloads.RoleList{Types: map[string]bool{"b": true, "c": true}}, "2", "3", "4"), + Entry("space_guids1", payloads.RoleList{SpaceGUIDs: map[string]bool{"space1": true}}, "1"), + Entry("space_guids2", payloads.RoleList{SpaceGUIDs: map[string]bool{"space1": true, "space2": true}}, "1", "2"), + Entry("organization_guids1", payloads.RoleList{OrgGUIDs: map[string]bool{"org1": true}}, "3"), + Entry("organization_guids2", payloads.RoleList{OrgGUIDs: map[string]bool{"org1": true, "org2": true}}, "3", "4"), + Entry("user_guids1", payloads.RoleList{UserGUIDs: map[string]bool{"user1": true}}, "1", "2", "3"), + Entry("user_guids2", payloads.RoleList{UserGUIDs: map[string]bool{"user1": true, "user2": true}}, "1", "2", "3", "4"), ) DescribeTable("ordering", func(order string, expectedGUIDs ...any) { - req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+"?order_by="+order, nil) + req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+"?order_by=not-used-in-test", nil) Expect(err).NotTo(HaveOccurred()) + + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.RoleList{OrderBy: order}) rr = httptest.NewRecorder() routerBuilder.Build().ServeHTTP(rr, req) @@ -245,13 +247,13 @@ var _ = Describe("Role", func() { ) }) - When("order_by is not a valid field", func() { + When("decoding the url values fails", func() { BeforeEach(func() { - query = "?order_by=not_valid" + requestValidator.DecodeAndValidateURLValuesReturns(errors.New("boom")) }) - It("returns an Unknown key error", func() { - expectUnknownKeyError("The query parameter is invalid: Order by can only be: .*") + It("returns an Unknown error", func() { + expectUnknownError() }) }) diff --git a/api/handlers/validator.go b/api/handlers/validator.go index ccb46a1ac..26819bfc2 100644 --- a/api/handlers/validator.go +++ b/api/handlers/validator.go @@ -11,7 +11,6 @@ import ( "strings" apierrors "code.cloudfoundry.org/korifi/api/errors" - "code.cloudfoundry.org/korifi/api/payloads" "github.com/go-playground/locales/en" ut "github.com/go-playground/universal-translator" @@ -227,28 +226,6 @@ func wireValidator() (*validator.Validate, ut.Translator, error) { return nil, nil, err } - v.RegisterStructValidation(checkRoleTypeAndOrgSpace, payloads.RoleCreate{}) - - err = v.RegisterTranslation("cannot_have_both_org_and_space_set", trans, func(ut ut.Translator) error { - return ut.Add("cannot_have_both_org_and_space_set", "Cannot pass both 'organization' and 'space' in a create role request", false) - }, func(ut ut.Translator, fe validator.FieldError) string { - t, _ := ut.T("cannot_have_both_org_and_space_set", fe.Field()) - return t - }) - if err != nil { - return nil, nil, err - } - - err = v.RegisterTranslation("valid_role", trans, func(ut ut.Translator) error { - return ut.Add("valid_role", "{0} is not a valid role", false) - }, func(ut ut.Translator, fe validator.FieldError) string { - t, _ := ut.T("valid_role", fmt.Sprintf("%v", fe.Value())) - return t - }) - if err != nil { - return nil, nil, err - } - return v, trans, nil } @@ -265,43 +242,6 @@ func registerDefaultTranslator(v *validator.Validate) (ut.Translator, error) { return trans, nil } -func checkRoleTypeAndOrgSpace(sl validator.StructLevel) { - roleCreate := sl.Current().Interface().(payloads.RoleCreate) - - if roleCreate.Relationships.Organization != nil && roleCreate.Relationships.Space != nil { - sl.ReportError(roleCreate.Relationships.Organization, "relationships.organization", "Organization", "cannot_have_both_org_and_space_set", "") - } - - roleType := RoleName(roleCreate.Type) - - switch roleType { - case RoleSpaceManager: - fallthrough - case RoleSpaceAuditor: - fallthrough - case RoleSpaceDeveloper: - fallthrough - case RoleSpaceSupporter: - if roleCreate.Relationships.Space == nil { - sl.ReportError(roleCreate.Relationships.Space, "relationships.space", "Space", "required", "") - } - case RoleOrganizationUser: - fallthrough - case RoleOrganizationAuditor: - fallthrough - case RoleOrganizationManager: - fallthrough - case RoleOrganizationBillingManager: - if roleCreate.Relationships.Organization == nil { - sl.ReportError(roleCreate.Relationships.Organization, "relationships.organization", "Organization", "required", "") - } - - case RoleName(""): - default: - sl.ReportError(roleCreate.Type, "type", "Role type", "valid_role", "") - } -} - func serviceInstanceTagLength(fl validator.FieldLevel) bool { tags, ok := fl.Field().Interface().([]string) if !ok { diff --git a/api/payloads/role.go b/api/payloads/role.go index 909a30ca2..35553bfca 100644 --- a/api/payloads/role.go +++ b/api/payloads/role.go @@ -1,44 +1,53 @@ package payloads import ( + "context" "net/url" "strings" "code.cloudfoundry.org/korifi/api/authorization" + "code.cloudfoundry.org/korifi/api/payloads/validation" + jellidation "github.com/jellydator/validation" "code.cloudfoundry.org/korifi/api/repositories" rbacv1 "k8s.io/api/rbac/v1" ) -type ( - RoleCreate struct { - Type string `json:"type" validate:"required"` - Relationships RoleRelationships `json:"relationships" validate:"required"` - } +const ( + RoleAdmin = "admin" + RoleAdminReadOnly = "admin_read_only" + RoleGlobalAuditor = "global_auditor" + RoleOrganizationAuditor = "organization_auditor" + RoleOrganizationBillingManager = "organization_billing_manager" + RoleOrganizationManager = "organization_manager" + RoleOrganizationUser = "organization_user" + RoleSpaceAuditor = "space_auditor" + RoleSpaceDeveloper = "space_developer" + RoleSpaceManager = "space_manager" + RoleSpaceSupporter = "space_supporter" +) - UserRelationship struct { - Data UserRelationshipData `json:"data" validate:"required"` - } +type RoleCreate struct { + Type string `json:"type"` + Relationships RoleRelationships `json:"relationships"` +} - UserRelationshipData struct { - Username string `json:"username" validate:"required_without=GUID"` - GUID string `json:"guid" validate:"required_without=Username"` - } +type ctxType string - RoleRelationships struct { - User *UserRelationship `json:"user" validate:"required"` - Space *Relationship `json:"space"` - Organization *Relationship `json:"organization"` - } +const typeKey ctxType = "type" - RoleListFilter struct { - GUIDs map[string]bool - Types map[string]bool - SpaceGUIDs map[string]bool - OrgGUIDs map[string]bool - UserGUIDs map[string]bool - } -) +func (p RoleCreate) Validate() error { + ctx := context.WithValue(context.Background(), typeKey, p.Type) + + return jellidation.ValidateStructWithContext(ctx, &p, + jellidation.Field(&p.Type, + jellidation.Required, + validation.OneOf(RoleSpaceManager, RoleSpaceAuditor, RoleSpaceDeveloper, RoleSpaceSupporter, + RoleOrganizationUser, RoleOrganizationAuditor, RoleOrganizationManager, RoleOrganizationBillingManager), + ), + jellidation.Field(&p.Relationships, validation.StrictlyRequired), + ) +} func (p RoleCreate) ToMessage() repositories.CreateRoleMessage { record := repositories.CreateRoleMessage{ @@ -70,25 +79,84 @@ func (p RoleCreate) ToMessage() repositories.CreateRoleMessage { return record } -func (r *RoleListFilter) SupportedKeys() []string { +type RoleRelationships struct { + User UserRelationship `json:"user"` + Space *Relationship `json:"space"` + Organization *Relationship `json:"organization"` +} + +func (r RoleRelationships) ValidateWithContext(ctx context.Context) error { + roleType := ctx.Value(typeKey) + + return jellidation.ValidateStruct(&r, + jellidation.Field(&r.User, validation.StrictlyRequired), + + jellidation.Field(&r.Space, + jellidation.When(r.Organization != nil && r.Organization.Data != nil && r.Organization.Data.GUID != "", + jellidation.Nil.Error("cannot pass both 'organization' and 'space' in a create role request"))), + + jellidation.Field(&r.Space, + jellidation.When( + roleType == RoleSpaceAuditor || roleType == RoleSpaceDeveloper || + roleType == RoleSpaceManager || roleType == RoleSpaceSupporter, + jellidation.NotNil, + validation.StrictlyRequired, + )), + + jellidation.Field(&r.Organization, + jellidation.When( + roleType == RoleOrganizationAuditor || roleType == RoleOrganizationBillingManager || + roleType == RoleOrganizationManager || roleType == RoleOrganizationUser, + jellidation.NotNil, + validation.StrictlyRequired, + )), + ) +} + +type UserRelationship struct { + Data UserRelationshipData `json:"data"` +} + +type UserRelationshipData struct { + Username string `json:"username"` + GUID string `json:"guid"` +} + +type RoleList struct { + GUIDs map[string]bool + Types map[string]bool + SpaceGUIDs map[string]bool + OrgGUIDs map[string]bool + UserGUIDs map[string]bool + OrderBy string +} + +func (r RoleList) SupportedKeys() []string { return []string{"guids", "types", "space_guids", "organization_guids", "user_guids", "order_by", "include", "per_page", "page"} } -func (r *RoleListFilter) DecodeFromURLValues(values url.Values) error { +func (r *RoleList) DecodeFromURLValues(values url.Values) error { r.GUIDs = commaSepToSet(values.Get("guids")) r.Types = commaSepToSet(values.Get("types")) r.SpaceGUIDs = commaSepToSet(values.Get("space_guids")) r.OrgGUIDs = commaSepToSet(values.Get("organization_guids")) r.UserGUIDs = commaSepToSet(values.Get("user_guids")) + r.OrderBy = values.Get("order_by") return nil } +func (r RoleList) Validate() error { + return jellidation.ValidateStruct(&r, + jellidation.Field(&r.OrderBy, validation.OneOf("created_at", "updated_at", "-created_at", "-updated_at")), + ) +} + func commaSepToSet(in string) map[string]bool { - out := map[string]bool{} if in == "" { - return out + return nil } + out := map[string]bool{} for _, s := range strings.Split(in, ",") { out[s] = true } diff --git a/api/payloads/role_test.go b/api/payloads/role_test.go index dcf75c3ce..1783d0a92 100644 --- a/api/payloads/role_test.go +++ b/api/payloads/role_test.go @@ -8,7 +8,6 @@ import ( rbacv1 "k8s.io/api/rbac/v1" "code.cloudfoundry.org/korifi/api/errors" - "code.cloudfoundry.org/korifi/api/handlers" "code.cloudfoundry.org/korifi/api/payloads" . "github.com/onsi/ginkgo/v2" @@ -29,7 +28,7 @@ var _ = Describe("RoleCreate", func() { createPayload = payloads.RoleCreate{ Type: "space_manager", Relationships: payloads.RoleRelationships{ - User: &payloads.UserRelationship{ + User: payloads.UserRelationship{ Data: payloads.UserRelationshipData{ Username: "cf-service-account", }, @@ -53,33 +52,15 @@ var _ = Describe("RoleCreate", func() { Expect(roleCreate).To(PointTo(Equal(createPayload))) }) - When("the user name is missing", func() { - BeforeEach(func() { - createPayload.Relationships.User = nil - }) - - It("fails", func() { - Expect(apiError).To(HaveOccurred()) - Expect(apiError.Detail()).To(ContainSubstring("User is a required field")) - }) - }) - When("the user name and GUID are missing", func() { BeforeEach(func() { - createPayload.Relationships.User = &payloads.UserRelationship{ - Data: payloads.UserRelationshipData{ - Username: "", - GUID: "", - }, - } + createPayload.Relationships.User.Data.GUID = "" + createPayload.Relationships.User.Data.Username = "" }) It("fails", func() { Expect(apiError).To(HaveOccurred()) - Expect(apiError.Detail()).To(SatisfyAll( - ContainSubstring("Field validation for 'GUID' failed"), - ContainSubstring("Field validation for 'Username' failed"), - )) + Expect(apiError.Detail()).To(ContainSubstring("user cannot be blank")) }) }) @@ -90,29 +71,7 @@ var _ = Describe("RoleCreate", func() { It("fails", func() { Expect(apiError).To(HaveOccurred()) - Expect(apiError.Detail()).To(ContainSubstring("Type is a required field")) - }) - }) - - When("a space role is missing a space relationship", func() { - BeforeEach(func() { - createPayload.Relationships.Space = nil - }) - - It("fails", func() { - Expect(apiError).To(HaveOccurred()) - Expect(apiError.Detail()).To(ContainSubstring("relationships.space is a required field")) - }) - }) - - When("an org role is missing an org relationship", func() { - BeforeEach(func() { - createPayload.Type = "organization_manager" - }) - - It("fails", func() { - Expect(apiError).To(HaveOccurred()) - Expect(apiError.Detail()).To(ContainSubstring("relationships.organization is a required field")) + Expect(apiError.Detail()).To(ContainSubstring("type cannot be blank")) }) }) @@ -127,7 +86,7 @@ var _ = Describe("RoleCreate", func() { It("fails", func() { Expect(apiError).To(HaveOccurred()) - Expect(apiError.Detail()).To(ContainSubstring("Cannot pass both 'organization' and 'space' in a create role request")) + Expect(apiError.Detail()).To(ContainSubstring("cannot pass both 'organization' and 'space' in a create role request")) }) }) @@ -198,23 +157,53 @@ var _ = DescribeTable("Role org / space combination validation", } }, - Entry("org auditor w org", string(handlers.RoleOrganizationAuditor), "organization", true, ""), - Entry("org auditor w space", string(handlers.RoleOrganizationAuditor), "space", false, "relationships.organization is a required field"), - Entry("org billing manager w org", string(handlers.RoleOrganizationBillingManager), "organization", true, ""), - Entry("org billing manager w space", string(handlers.RoleOrganizationBillingManager), "space", false, "relationships.organization is a required field"), - Entry("org manager w org", string(handlers.RoleOrganizationManager), "organization", true, ""), - Entry("org manager w space", string(handlers.RoleOrganizationManager), "space", false, "relationships.organization is a required field"), - Entry("org user w org", string(handlers.RoleOrganizationUser), "organization", true, ""), - Entry("org user w space", string(handlers.RoleOrganizationUser), "space", false, "relationships.organization is a required field"), - - Entry("space auditor w org", string(handlers.RoleSpaceAuditor), "organization", false, "relationships.space is a required field"), - Entry("space auditor w space", string(handlers.RoleSpaceAuditor), "space", true, ""), - Entry("space developer w org", string(handlers.RoleSpaceDeveloper), "organization", false, "relationships.space is a required field"), - Entry("space developer w space", string(handlers.RoleSpaceDeveloper), "space", true, ""), - Entry("space manager w org", string(handlers.RoleSpaceManager), "organization", false, "relationships.space is a required field"), - Entry("space manager w space", string(handlers.RoleSpaceManager), "space", true, ""), - Entry("space supporter w org", string(handlers.RoleSpaceSupporter), "organization", false, "relationships.space is a required field"), - Entry("space supporter w space", string(handlers.RoleSpaceSupporter), "space", true, ""), - - Entry("invalid role name", "does-not-exist", "organization", false, "does-not-exist is not a valid role"), + Entry("org auditor w org", payloads.RoleOrganizationAuditor, "organization", true, ""), + Entry("org auditor w space", payloads.RoleOrganizationAuditor, "space", false, "relationships.organization is required"), + Entry("org billing manager w org", payloads.RoleOrganizationBillingManager, "organization", true, ""), + Entry("org billing manager w space", payloads.RoleOrganizationBillingManager, "space", false, "relationships.organization is required"), + Entry("org manager w org", payloads.RoleOrganizationManager, "organization", true, ""), + Entry("org manager w space", payloads.RoleOrganizationManager, "space", false, "relationships.organization is required"), + Entry("org user w org", payloads.RoleOrganizationUser, "organization", true, ""), + Entry("org user w space", payloads.RoleOrganizationUser, "space", false, "relationships.organization is required"), + + Entry("space auditor w org", payloads.RoleSpaceAuditor, "organization", false, "relationships.space is required"), + Entry("space auditor w space", payloads.RoleSpaceAuditor, "space", true, ""), + Entry("space developer w org", payloads.RoleSpaceDeveloper, "organization", false, "relationships.space is required"), + Entry("space developer w space", payloads.RoleSpaceDeveloper, "space", true, ""), + Entry("space manager w org", payloads.RoleSpaceManager, "organization", false, "relationships.space is required"), + Entry("space manager w space", payloads.RoleSpaceManager, "space", true, ""), + Entry("space supporter w org", payloads.RoleSpaceSupporter, "organization", false, "relationships.space is required"), + Entry("space supporter w space", payloads.RoleSpaceSupporter, "space", true, ""), + + Entry("invalid role name", "does-not-exist", "organization", false, "type value must be one of"), ) + +var _ = Describe("role list", func() { + DescribeTable("valid query", + func(query string, expectedRoleListQueryParameters payloads.RoleList) { + actualRoleListQueryParameters, decodeErr := decodeQuery[payloads.RoleList](query) + + Expect(decodeErr).NotTo(HaveOccurred()) + Expect(*actualRoleListQueryParameters).To(Equal(expectedRoleListQueryParameters)) + }, + + Entry("guids", "guids=g1,g2", payloads.RoleList{GUIDs: map[string]bool{"g1": true, "g2": true}}), + Entry("types", "types=g1,g2", payloads.RoleList{Types: map[string]bool{"g1": true, "g2": true}}), + Entry("space_guids", "space_guids=g1,g2", payloads.RoleList{SpaceGUIDs: map[string]bool{"g1": true, "g2": true}}), + Entry("organization_guids", "organization_guids=g1,g2", payloads.RoleList{OrgGUIDs: map[string]bool{"g1": true, "g2": true}}), + Entry("user_guids", "user_guids=g1,g2", payloads.RoleList{UserGUIDs: map[string]bool{"g1": true, "g2": true}}), + Entry("order_by1", "order_by=created_at", payloads.RoleList{OrderBy: "created_at"}), + Entry("order_by2", "order_by=-created_at", payloads.RoleList{OrderBy: "-created_at"}), + Entry("order_by3", "order_by=updated_at", payloads.RoleList{OrderBy: "updated_at"}), + Entry("order_by4", "order_by=-updated_at", payloads.RoleList{OrderBy: "-updated_at"}), + Entry("include", "include=foo", payloads.RoleList{}), + ) + + DescribeTable("invalid query", + func(query string, expectedErrMsg string) { + _, decodeErr := decodeQuery[payloads.RoleList](query) + Expect(decodeErr).To(MatchError(ContainSubstring(expectedErrMsg))) + }, + Entry("invalid order_by", "order_by=foo", "value must be one of"), + ) +}) diff --git a/api/payloads/validation/rules.go b/api/payloads/validation/rules.go index 3ee11138b..04c558cf6 100644 --- a/api/payloads/validation/rules.go +++ b/api/payloads/validation/rules.go @@ -26,7 +26,7 @@ type strictlyRequiredRule struct { validation.RequiredRule } -// We wrap tje original validation.RequiredRule in order to workaround +// We wrap the original validation.RequiredRule in order to workaround // incorrect zero type check: // https://github.com/jellydator/validation/blob/44595f5c48dd0da8bbeff0f56ceaa581631e55b1/util.go#L151-L156 func (r strictlyRequiredRule) Validate(value interface{}) error {