Skip to content

Commit

Permalink
Allow colons in object id (#108)
Browse files Browse the repository at this point in the history 
* Update warrant object ids to accept colons

* Update user and tenant ids to accept colons

* Remove unused ObjectSpec and StringToObjectSpec method

* Add valid_object_id validation to client provided id attribute for tenant, user, role, permission, pricing-tier, and feature

* Update validation error messages for valid_object_type, valid_object_id, and valid_inherit_if validations

---------

Co-authored-by: Karan Kajla <[email protected]>
  • Loading branch information
@stanleyphu @kkajla12
stanleyphu and kkajla12 authored May 4, 2023
1 parent c24e6d8 commit a48b448
Show file tree
Hide file tree
Showing 12 changed files with 57 additions and 101 deletions.
2 changes: 1 addition & 1 deletion pkg/authz/feature/spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

type FeatureSpec struct {
FeatureId string `json:"featureId" validate:"required"`
FeatureId string `json:"featureId" validate:"required,valid_object_id"`
Name *string `json:"name"`
Description *string `json:"description"`
CreatedAt time.Time `json:"createdAt"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/authz/permission/spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

type PermissionSpec struct {
PermissionId string `json:"permissionId" validate:"required"`
PermissionId string `json:"permissionId" validate:"required,valid_object_id"`
Name *string `json:"name"`
Description *string `json:"description"`
CreatedAt time.Time `json:"createdAt"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/authz/pricingtier/spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

type PricingTierSpec struct {
PricingTierId string `json:"pricingTierId" validate:"required"`
PricingTierId string `json:"pricingTierId" validate:"required,valid_object_id"`
Name *string `json:"name"`
Description *string `json:"description"`
CreatedAt time.Time `json:"createdAt"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/authz/role/spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

type RoleSpec struct {
RoleId string `json:"roleId" validate:"required"`
RoleId string `json:"roleId" validate:"required,valid_object_id"`
Name *string `json:"name"`
Description *string `json:"description"`
CreatedAt time.Time `json:"createdAt"`
Expand Down
32 changes: 9 additions & 23 deletions pkg/authz/tenant/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package tenant

import (
"context"
"regexp"

"github.com/google/uuid"
"github.com/pkg/errors"
object "github.com/warrant-dev/warrant/pkg/authz/object"
objecttype "github.com/warrant-dev/warrant/pkg/authz/objecttype"
"github.com/warrant-dev/warrant/pkg/event"
Expand All @@ -31,13 +31,17 @@ func NewService(env service.Env, repository TenantRepository, eventSvc event.Eve
}

func (svc TenantService) Create(ctx context.Context, tenantSpec TenantSpec) (*TenantSpec, error) {
err := validateOrGenerateTenantIdInSpec(&tenantSpec)
if err != nil {
return nil, err
if tenantSpec.TenantId == "" {
// generate an id for the tenant if one isn't supplied
generatedUUID, err := uuid.NewRandom()
if err != nil {
return nil, errors.New("unable to generate random UUID for tenant")
}
tenantSpec.TenantId = generatedUUID.String()
}

var newTenant Model
err = svc.Env().DB().WithinTransaction(ctx, func(txCtx context.Context) error {
err := svc.Env().DB().WithinTransaction(ctx, func(txCtx context.Context) error {
createdObject, err := svc.ObjectSvc.Create(txCtx, *tenantSpec.ToObjectSpec())
if err != nil {
switch err.(type) {
Expand Down Expand Up @@ -152,21 +156,3 @@ func (svc TenantService) DeleteByTenantId(ctx context.Context, tenantId string)

return nil
}

func validateOrGenerateTenantIdInSpec(tenantSpec *TenantSpec) error {
tenantIdRegExp := regexp.MustCompile(`^[a-zA-Z0-9_\-\.@\|]+$`)
if tenantSpec.TenantId != "" {
// Validate tenantId if provided
if !tenantIdRegExp.Match([]byte(tenantSpec.TenantId)) {
return service.NewInvalidParameterError("tenantId", "must be provided and can only contain alphanumeric characters and/or '-', '_', '@', and '|'")
}
} else {
// Generate a TenantId for the tenant if one isn't supplied
generatedUUID, err := uuid.NewRandom()
if err != nil {
return service.NewInternalError("unable to generate random UUID for tenant")
}
tenantSpec.TenantId = generatedUUID.String()
}
return nil
}
2 changes: 1 addition & 1 deletion pkg/authz/tenant/spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

type TenantSpec struct {
TenantId string `json:"tenantId"`
TenantId string `json:"tenantId" validate:"omitempty,valid_object_id"`
Name *string `json:"name"`
CreatedAt time.Time `json:"createdAt"`
}
Expand Down
32 changes: 9 additions & 23 deletions pkg/authz/user/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package authz

import (
"context"
"regexp"

"github.com/google/uuid"
"github.com/pkg/errors"
object "github.com/warrant-dev/warrant/pkg/authz/object"
objecttype "github.com/warrant-dev/warrant/pkg/authz/objecttype"
"github.com/warrant-dev/warrant/pkg/event"
Expand All @@ -31,13 +31,17 @@ func NewService(env service.Env, repository UserRepository, eventSvc event.Event
}

func (svc UserService) Create(ctx context.Context, userSpec UserSpec) (*UserSpec, error) {
err := validateOrGenerateUserIdInSpec(&userSpec)
if err != nil {
return nil, err
if userSpec.UserId == "" {
// generate an id for the user if one isn't provided
generatedUUID, err := uuid.NewRandom()
if err != nil {
return nil, errors.New("unable to generate random UUID for user")
}
userSpec.UserId = generatedUUID.String()
}

var newUser Model
err = svc.Env().DB().WithinTransaction(ctx, func(txCtx context.Context) error {
err := svc.Env().DB().WithinTransaction(ctx, func(txCtx context.Context) error {
createdObject, err := svc.ObjectSvc.Create(txCtx, *userSpec.ToObjectSpec())
if err != nil {
switch err.(type) {
Expand Down Expand Up @@ -149,21 +153,3 @@ func (svc UserService) DeleteByUserId(ctx context.Context, userId string) error

return err
}

func validateOrGenerateUserIdInSpec(userSpec *UserSpec) error {
userIdRegExp := regexp.MustCompile(`^[a-zA-Z0-9_\-\.@\|]+$`)
if userSpec.UserId != "" {
// Validate userId if provided
if !userIdRegExp.Match([]byte(userSpec.UserId)) {
return service.NewInvalidParameterError("userId", "must be provided and can only contain alphanumeric characters and/or '-', '_', '@', and '|'")
}
} else {
// Generate a UserID for the user if one isn't supplied
generatedUUID, err := uuid.NewRandom()
if err != nil {
return service.NewInternalError("unable to generate random UUID for user")
}
userSpec.UserId = generatedUUID.String()
}
return nil
}
2 changes: 1 addition & 1 deletion pkg/authz/user/spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

type UserSpec struct {
UserId string `json:"userId"`
UserId string `json:"userId" validate:"omitempty,valid_object_id"`
Email *string `json:"email" validate:"omitempty,email"`
CreatedAt time.Time `json:"createdAt"`
}
Expand Down
42 changes: 13 additions & 29 deletions pkg/authz/warrant/spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,6 @@ type SortOptions struct {
IsAscending bool
}

type ObjectSpec struct {
ObjectType string `json:"objectType" validate:"required,valid_object_type"`
ObjectId string `json:"objectId" validate:"required,valid_object_id"`
}

func StringToObjectSpec(str string) (*ObjectSpec, error) {
objectTypeId := strings.Split(str, ":")

if len(objectTypeId) != 2 {
return nil, fmt.Errorf("invalid object")
}

return &ObjectSpec{
ObjectType: objectTypeId[0],
ObjectId: objectTypeId[1],
}, nil
}

type SubjectSpec struct {
ObjectType string `json:"objectType,omitempty" validate:"required_with=ObjectId,valid_object_type"`
ObjectId string `json:"objectId,omitempty" validate:"required_with=ObjectType,valid_object_id"`
Expand All @@ -60,23 +42,25 @@ func (spec *SubjectSpec) String() string {
func StringToSubjectSpec(str string) (*SubjectSpec, error) {
objectRelation := strings.Split(str, "#")
if len(objectRelation) < 2 {
objectTypeId := strings.Split(str, ":")
objectType, objectId, colonFound := strings.Cut(str, ":")

if len(objectTypeId) != 2 {
if !colonFound {
return nil, fmt.Errorf("invalid subject")
}

return &SubjectSpec{
ObjectType: objectTypeId[0],
ObjectId: objectTypeId[1],
ObjectType: objectType,
ObjectId: objectId,
}, nil
}

object := objectRelation[0]
relation := objectRelation[1]
objectTypeId := strings.Split(object, ":")
objectType := objectTypeId[0]
objectId := objectTypeId[1]

objectType, objectId, colonFound := strings.Cut(object, ":")
if !colonFound {
return nil, fmt.Errorf("invalid subject")
}

subjectSpec := &SubjectSpec{
ObjectType: objectType,
Expand Down Expand Up @@ -147,8 +131,8 @@ func StringToWarrantSpec(warrantString string) (*WarrantSpec, error) {
return nil, fmt.Errorf("invalid warrant")
}

objectTypeAndObjectId := strings.Split(objectAndRelation[0], ":")
if len(objectTypeAndObjectId) != 2 {
objectType, objectId, colonFound := strings.Cut(objectAndRelation[0], ":")
if !colonFound {
return nil, fmt.Errorf("invalid warrant")
}

Expand Down Expand Up @@ -176,8 +160,8 @@ func StringToWarrantSpec(warrantString string) (*WarrantSpec, error) {
}

return &WarrantSpec{
ObjectType: objectTypeAndObjectId[0],
ObjectId: objectTypeAndObjectId[1],
ObjectType: objectType,
ObjectId: objectId,
Relation: objectAndRelation[1],
Subject: subjectSpec,
Context: contextSetSpec,
Expand Down
8 changes: 4 additions & 4 deletions pkg/service/json.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func validObjectId(fl validator.FieldLevel) bool {
return true
}

regExp := regexp.MustCompile(`^[a-zA-Z0-9_\-\.@\|]+$`)
regExp := regexp.MustCompile(`^[a-zA-Z0-9_\-\.@\|:]+$`)
return regExp.Match([]byte(value))
}

Expand Down Expand Up @@ -183,11 +183,11 @@ func ValidateStruct(obj interface{}) error {
validValues := strings.Join(strings.Split(err.Param(), " "), ", ")
return NewInvalidParameterError(fieldName, fmt.Sprintf("must be one of %s", validValues))
case "valid_object_type", "valid_relation":
return NewInvalidParameterError(fieldName, "must be provided and can only contain lower-case alphanumeric characters and/or '-' and '_'")
return NewInvalidParameterError(fieldName, "can only contain lower-case alphanumeric characters and/or '-' and '_'")
case "valid_object_id":
return NewInvalidParameterError(fieldName, "must be provided and can only contain alphanumeric characters and/or '-', '_', '@', and '|'")
return NewInvalidParameterError(fieldName, "can only contain alphanumeric characters and/or '-', '_', '@', ':', and '|'")
case "valid_inheritif":
return NewInvalidParameterError(fieldName, "must be provided and can only be 'anyOf', 'allOf', 'noneOf', or a valid relation name")
return NewInvalidParameterError(fieldName, "can only be 'anyOf', 'allOf', 'noneOf', or a valid relation name")
default:
return NewInvalidRequestError("Invalid request body")
}
Expand Down
14 changes: 7 additions & 7 deletions tests/warrants.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,15 @@
"method": "POST",
"url": "/v1/permissions",
"body": {
"permissionId": "edit-balance-sheet",
"permissionId": "balance-sheet:edit",
"name": "Edit Balance Sheet",
"description": "Grants access to edit the balance sheet."
}
},
"expectedResponse": {
"statusCode": 200,
"body": {
"permissionId": "edit-balance-sheet",
"permissionId": "balance-sheet:edit",
"name": "Edit Balance Sheet",
"description": "Grants access to edit the balance sheet."
}
Expand Down Expand Up @@ -116,7 +116,7 @@
"url": "/v1/warrants",
"body": {
"objectType": "permission",
"objectId": "edit-balance-sheet",
"objectId": "balance-sheet:edit",
"relation": "member",
"subject": {
"objectType": "user",
Expand All @@ -128,7 +128,7 @@
"statusCode": 200,
"body": {
"objectType": "permission",
"objectId": "edit-balance-sheet",
"objectId": "balance-sheet:edit",
"relation": "member",
"subject": {
"objectType": "user",
Expand Down Expand Up @@ -203,7 +203,7 @@
"url": "/v1/warrants",
"body": {
"objectType": "permission",
"objectId": "edit-balance-sheet",
"objectId": "balance-sheet:edit",
"relation": "member",
"subject": {
"objectType": "user",
Expand Down Expand Up @@ -238,9 +238,9 @@
"name": "deletePermissionEditBalanceSheet",
"request": {
"method": "DELETE",
"url": "/v1/permissions/edit-balance-sheet",
"url": "/v1/permissions/balance-sheet:edit",
"body": {
"permissionId": "edit-balance-sheet"
"permissionId": "balance-sheet:edit"
}
},
"expectedResponse": {
Expand Down
18 changes: 9 additions & 9 deletions tests/zz-events.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,17 @@
"type": "deleted",
"source": "api",
"resourceType": "permission",
"resourceId": "edit-balance-sheet"
"resourceId": "balance-sheet:edit"
},
{
"type": "created",
"source": "api",
"resourceType": "permission",
"resourceId": "edit-balance-sheet",
"resourceId": "balance-sheet:edit",
"meta": {
"description": "Grants access to edit the balance sheet.",
"name": "Edit Balance Sheet",
"permissionId": "edit-balance-sheet"
"permissionId": "balance-sheet:edit"
}
}
],
Expand Down Expand Up @@ -114,7 +114,7 @@
"type": "deleted",
"source": "api",
"resourceType": "permission",
"resourceId": "edit-balance-sheet"
"resourceId": "balance-sheet:edit"
},
{
"type": "deleted",
Expand Down Expand Up @@ -252,7 +252,7 @@
"type": "access_revoked",
"source": "api",
"objectType": "permission",
"objectId": "edit-balance-sheet",
"objectId": "balance-sheet:edit",
"relation": "member",
"subjectType": "user",
"subjectId": "user-a"
Expand Down Expand Up @@ -294,7 +294,7 @@
"type": "access_granted",
"source": "api",
"objectType": "permission",
"objectId": "edit-balance-sheet",
"objectId": "balance-sheet:edit",
"relation": "member",
"subjectType": "user",
"subjectId": "user-a"
Expand Down Expand Up @@ -340,7 +340,7 @@
"type": "access_granted",
"source": "api",
"objectType": "permission",
"objectId": "edit-balance-sheet",
"objectId": "balance-sheet:edit",
"relation": "member",
"subjectType": "user",
"subjectId": "user-a"
Expand Down Expand Up @@ -562,7 +562,7 @@
"type": "access_revoked",
"source": "api",
"objectType": "permission",
"objectId": "edit-balance-sheet",
"objectId": "balance-sheet:edit",
"relation": "member",
"subjectType": "user",
"subjectId": "user-a"
Expand All @@ -589,7 +589,7 @@
"type": "access_granted",
"source": "api",
"objectType": "permission",
"objectId": "edit-balance-sheet",
"objectId": "balance-sheet:edit",
"relation": "member",
"subjectType": "user",
"subjectId": "user-a"
Expand Down

0 comments on commit a48b448

Please sign in to comment.