From 1cff30bcccb9d6357fd03c8495e6fd4c62260ab6 Mon Sep 17 00:00:00 2001 From: Abhinav Pappu Date: Thu, 3 Oct 2024 12:02:04 -0700 Subject: [PATCH 1/2] feat: support rbac v2 statements - added version field - updated validation to account for new v2 statement expectations --- .../meta/operation/rbac_statement.graphql | 1 + client/internal/meta/schema/rbac.graphql | 32 ++++++++ client/meta/genqlient.generated.go | 11 +++ docs/resources/rbac_statement.md | 1 + observe/helpers.go | 17 ++++ observe/resource_rbac_statement.go | 78 ++++++++++++++---- observe/resource_rbac_statement_test.go | 82 +++++++++++++++++++ 7 files changed, 204 insertions(+), 18 deletions(-) diff --git a/client/internal/meta/operation/rbac_statement.graphql b/client/internal/meta/operation/rbac_statement.graphql index b75ac43e..7d3eff24 100644 --- a/client/internal/meta/operation/rbac_statement.graphql +++ b/client/internal/meta/operation/rbac_statement.graphql @@ -16,6 +16,7 @@ fragment RbacStatement on RbacStatement { all } role + version } mutation createRbacStatement($config: RbacStatementInput!) { diff --git a/client/internal/meta/schema/rbac.graphql b/client/internal/meta/schema/rbac.graphql index ec66d26b..74418852 100644 --- a/client/internal/meta/schema/rbac.graphql +++ b/client/internal/meta/schema/rbac.graphql @@ -29,6 +29,17 @@ extend type Query { """ rbacStatements: [RbacStatement!]! + + """ + Get all RBAC Role Statements + """ + rbacRoleStatements: [RbacStatement!]! + + """ + Get all RBAC resource statements for a particular object + """ + rbacResourceStatement(id: ObjectId!): [RbacStatement!]! + """ Given a particular user, and a particular object/role request, return what would happen. Note that we assume that the customer owning the object is the current @@ -66,6 +77,11 @@ extend type Query { Get the group users will be assigned to by default """ rbacDefaultGroup: RbacGroup! + + """ + Get the group users will be assigned to by default + """ + rbacDefaultSharingGroups: [RbacDefaultSharingGroup!]! } extend type User { @@ -116,6 +132,8 @@ extend type Mutation { """ setRbacDefaultGroup(id: ORN!): ResultStatus! unsetRbacDefaultGroup: ResultStatus! + + setRbacDefaultSharingGroups(shares: [RbacDefaultSharingGroupInput!]): ResultStatus! } scalar ORN @goModel(model: "observe/authorization/id.ORN") @@ -212,6 +230,8 @@ type RbacStatement implements AuditedObject @goModel(model: "observe/rbac/policy object: RbacObject! role: RbacRole! + version: Int + createdBy: UserId! createdByInfo: UserInfo! @goField(forceResolver: true) createdDate: Time! @@ -225,6 +245,7 @@ input RbacStatementInput @goModel(model: "observe/rbac/policy.Statement") { subject: RbacSubjectInput! object: RbacObjectInput! role: RbacRole! + version: Int } input UpdateRbacStatementInput @goModel(model: "observe/rbac/policy.Statement") { @@ -233,6 +254,7 @@ input UpdateRbacStatementInput @goModel(model: "observe/rbac/policy.Statement") subject: RbacSubjectInput! object: RbacObjectInput! role: RbacRole! + version: Int } type MutateRbacStatementsResponse @goModel(model: "observe/meta/metatypes.MutateRbacStatementsResponse") { @@ -241,6 +263,16 @@ type MutateRbacStatementsResponse @goModel(model: "observe/meta/metatypes.Mutate deletedStatements: [ORN!]! } +type RbacDefaultSharingGroup @goModel(model: "observe/meta/metatypes.RbacDefaultSharingGroup") { + groupId: ORN! + allowEdit: Boolean! +} + +input RbacDefaultSharingGroupInput @goModel(model: "observe/meta/metatypes.RbacDefaultSharingGroup") { + groupId: ORN! + allowEdit: Boolean! +} + """ A RequestSubject is different from a Subject, because the RequestSubject provides all of the values, such that each Statement can match against diff --git a/client/meta/genqlient.generated.go b/client/meta/genqlient.generated.go index dcef88d6..3320d575 100644 --- a/client/meta/genqlient.generated.go +++ b/client/meta/genqlient.generated.go @@ -7214,6 +7214,7 @@ type RbacStatement struct { Subject RbacStatementSubjectRbacSubject `json:"subject"` Object RbacStatementObjectRbacObject `json:"object"` Role RbacRole `json:"role"` + Version *int `json:"version"` } // GetId returns RbacStatement.Id, and is useful for accessing the field via an interface. @@ -7231,11 +7232,15 @@ func (v *RbacStatement) GetObject() RbacStatementObjectRbacObject { return v.Obj // GetRole returns RbacStatement.Role, and is useful for accessing the field via an interface. func (v *RbacStatement) GetRole() RbacRole { return v.Role } +// GetVersion returns RbacStatement.Version, and is useful for accessing the field via an interface. +func (v *RbacStatement) GetVersion() *int { return v.Version } + type RbacStatementInput struct { Description string `json:"description"` Subject RbacSubjectInput `json:"subject"` Object RbacObjectInput `json:"object"` Role RbacRole `json:"role"` + Version *int `json:"version"` } // GetDescription returns RbacStatementInput.Description, and is useful for accessing the field via an interface. @@ -7250,6 +7255,9 @@ func (v *RbacStatementInput) GetObject() RbacObjectInput { return v.Object } // GetRole returns RbacStatementInput.Role, and is useful for accessing the field via an interface. func (v *RbacStatementInput) GetRole() RbacRole { return v.Role } +// GetVersion returns RbacStatementInput.Version, and is useful for accessing the field via an interface. +func (v *RbacStatementInput) GetVersion() *int { return v.Version } + // RbacStatementObjectRbacObject includes the requested fields of the GraphQL type RbacObject. type RbacStatementObjectRbacObject struct { ObjectId *string `json:"objectId"` @@ -13289,6 +13297,7 @@ fragment RbacStatement on RbacStatement { all } role + version } ` @@ -16689,6 +16698,7 @@ fragment RbacStatement on RbacStatement { all } role + version } ` @@ -20312,6 +20322,7 @@ fragment RbacStatement on RbacStatement { all } role + version } ` diff --git a/docs/resources/rbac_statement.md b/docs/resources/rbac_statement.md index 950ba372..28a56367 100644 --- a/docs/resources/rbac_statement.md +++ b/docs/resources/rbac_statement.md @@ -56,6 +56,7 @@ resource "observe_rbac_statement" "group_example" { ### Optional - `description` (String) +- `version` (Number) ### Read-Only diff --git a/observe/helpers.go b/observe/helpers.go index d4b83eab..c4512a25 100644 --- a/observe/helpers.go +++ b/observe/helpers.go @@ -196,6 +196,23 @@ func validateStringInSlice(valid []string, ignoreCase bool) schema.SchemaValidat } } +func validateInSlice[T comparable](valid []T) schema.SchemaValidateDiagFunc { + return func(i interface{}, path cty.Path) diag.Diagnostics { + t, ok := i.(T) + if !ok { + return diag.Errorf("expected type of %s to be %T", i, t) + } + + for _, v := range valid { + if t == v { + return nil + } + } + + return diag.Errorf("expected %v to be one of %v", t, i) + } +} + func validateTimeDuration(i interface{}, path cty.Path) diag.Diagnostics { s := i.(string) if _, err := time.ParseDuration(s); err != nil { diff --git a/observe/resource_rbac_statement.go b/observe/resource_rbac_statement.go index 5b5a7af8..43b54f4c 100644 --- a/observe/resource_rbac_statement.go +++ b/observe/resource_rbac_statement.go @@ -21,6 +21,7 @@ const ( schemaRbacStatementObjectTypeDescription = "The type of object such as dataset." schemaRbacStatementObjectNameDescription = "The name of object. Can be provided along with `type`." schemaRbacStatementObjectOwnerDescription = "True to bind to objects owned by the user. Can be provided along with `type`." + schemaRbacStatementVersionDescription = "The version of the statement. Defaults to V1" schemaRbacStatementSubjectUserDescription = "OID of a user." schemaRbacStatementSubjectGroupDescription = "OID of a RBAC Group." @@ -30,7 +31,8 @@ var rbacStatementObjectTypes = []string{ "object.0.id", "object.0.folder", "object.0.workspace", - "object.0.type", + // excluding type from ExactlyOneOf since it's required to specify both a type and and id for v2 resource statements + // "object.0.type", "object.0.all", } @@ -104,7 +106,6 @@ func resourceRbacStatement() *schema.Resource { }, "type": { Type: schema.TypeString, - ExactlyOneOf: rbacStatementObjectTypes, Optional: true, Description: schemaRbacStatementObjectTypeDescription, }, @@ -135,6 +136,12 @@ func resourceRbacStatement() *schema.Resource { Required: true, ValidateDiagFunc: validateEnums(gql.AllRbacRoles), }, + "version": { + Type: schema.TypeInt, + Optional: true, + Default: 1, + ValidateDiagFunc: validateInSlice([]int{1, 2}), + }, "oid": { Type: schema.TypeString, Computed: true, @@ -150,13 +157,22 @@ func newRbacStatementConfig(data *schema.ResourceData) (input *gql.RbacStatement input.Description = v.(string) } + var version int + if v, ok := data.GetOk("version"); ok { + version = v.(int) + // we only care about setting the version field if it's V2 + if version == 2 { + input.Version = &version + } + } + subject, err := newRbacSubjectInput(data) if err != nil { return nil, diag.Errorf(err.Error()) } input.Subject = subject - object, err := newRbacObjectInput(data) + object, err := newRbacObjectInput(data, version) if err != nil { return nil, diag.Errorf(err.Error()) } @@ -186,24 +202,36 @@ func newRbacSubjectInput(data *schema.ResourceData) (gql.RbacSubjectInput, error return subject, nil } -func newRbacObjectInput(data *schema.ResourceData) (gql.RbacObjectInput, error) { +func newRbacObjectInput(data *schema.ResourceData, version int) (gql.RbacObjectInput, error) { object := gql.RbacObjectInput{} if v, ok := data.GetOk("object.0.id"); ok { object.ObjectId = stringPtr(v.(string)) } if v, ok := data.GetOk("object.0.folder"); ok { + if version == 2 { + return object, fmt.Errorf("object.folder is not supported in V2 statements") + } object.FolderId = stringPtr(v.(string)) } if v, ok := data.GetOk("object.0.workspace"); ok { + if version == 2 { + return object, fmt.Errorf("object.workspace is not supported in V2 statements") + } object.WorkspaceId = stringPtr(v.(string)) } if v, ok := data.GetOk("object.0.type"); ok { object.Type = stringPtr(v.(string)) if oname, ok := data.GetOk("object.0.name"); ok { + if version == 2 { + return object, fmt.Errorf("object.name is not supported in V2 statements") + } object.Name = stringPtr(oname.(string)) } } object.Owner = boolPtr(data.Get("object.0.owner").(bool)) + if version == 2 && *object.Owner { + return object, fmt.Errorf("object.owner is not supported in V2 statements") + } object.All = boolPtr(data.Get("object.0.all").(bool)) return object, nil } @@ -284,22 +312,32 @@ func rbacStatementToResourceData(r *gql.RbacStatement, data *schema.ResourceData // object object := make(map[string]interface{}, 0) - if r.Object.ObjectId != nil { - object["id"] = *r.Object.ObjectId - } else if r.Object.FolderId != nil { - object["folder"] = *r.Object.FolderId - } else if r.Object.WorkspaceId != nil { - object["workspace"] = *r.Object.WorkspaceId - } else if r.Object.Type != nil { - object["type"] = *r.Object.Type - if r.Object.Name != nil { - object["name"] = *r.Object.Name + if r.Version != nil && *r.Version == 2 { + // RBAC v2 statements will have an id AND a type for resource statements, and neither for role statements + if r.Object.ObjectId != nil { + object["id"] = *r.Object.ObjectId + } + if r.Object.Type != nil { + object["type"] = *r.Object.Type } - if r.Object.Owner != nil { - object["owner"] = *r.Object.Owner + } else { + if r.Object.ObjectId != nil { + object["id"] = *r.Object.ObjectId + } else if r.Object.FolderId != nil { + object["folder"] = *r.Object.FolderId + } else if r.Object.WorkspaceId != nil { + object["workspace"] = *r.Object.WorkspaceId + } else if r.Object.Type != nil { + object["type"] = *r.Object.Type + if r.Object.Name != nil { + object["name"] = *r.Object.Name + } + if r.Object.Owner != nil { + object["owner"] = *r.Object.Owner + } + } else if r.Object.All != nil { + object["all"] = *r.Object.All } - } else if r.Object.All != nil { - object["all"] = *r.Object.All } if err := data.Set("object", []interface{}{object}); err != nil { diags = append(diags, diag.FromErr(err)...) @@ -310,6 +348,10 @@ func rbacStatementToResourceData(r *gql.RbacStatement, data *schema.ResourceData diags = append(diags, diag.FromErr(err)...) } + if err := data.Set("version", r.Version); err != nil { + diags = append(diags, diag.FromErr(err)...) + } + if err := data.Set("oid", r.Oid().String()); err != nil { diags = append(diags, diag.FromErr(err)...) } diff --git a/observe/resource_rbac_statement_test.go b/observe/resource_rbac_statement_test.go index b97770a8..359a55b9 100644 --- a/observe/resource_rbac_statement_test.go +++ b/observe/resource_rbac_statement_test.go @@ -289,3 +289,85 @@ func TestAccObserveRbacStatementTypeCreate(t *testing.T) { }, }) } + +func TestAccObserveRbacV2StatementWithGroupCreate(t *testing.T) { + randomPrefix := acctest.RandomWithPrefix("tf") + rbacV2Preamble := configPreamble + datastreamConfigPreamble + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(rbacV2Preamble + ` + resource "observe_rbac_group" "example" { + name = "%[1]s" + } + + resource "observe_rbac_statement" "example" { + description = "%[1]s" + subject { + group = observe_rbac_group.example.oid + } + object { + type = "datastream" + id = observe_datastream.test.id + } + role = "Editor" + version = 2 + } + `, randomPrefix), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("observe_rbac_statement.example", "description", randomPrefix), + resource.TestCheckResourceAttr("observe_rbac_statement.example", "subject.#", "1"), + resource.TestCheckResourceAttrSet("observe_rbac_statement.example", "subject.0.group"), + resource.TestCheckResourceAttr("observe_rbac_statement.example", "object.#", "1"), + resource.TestCheckResourceAttrSet("observe_rbac_statement.example", "object.0.id"), + resource.TestCheckResourceAttr("observe_rbac_statement.example", "role", "Editor"), + resource.TestCheckResourceAttr("observe_rbac_statement.example", "version", "2"), + ), + }, + }, + }) +} + +func TestAccObserveRbacV2StatementWithUserCreate(t *testing.T) { + randomPrefix := acctest.RandomWithPrefix("tf") + rbacV2Preamble := configPreamble + datastreamConfigPreamble + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(rbacV2Preamble + ` + data "observe_user" "system" { + email = "%[2]s" + } + + resource "observe_rbac_statement" "example" { + description = "%[1]s" + subject { + user = data.observe_user.system.oid + } + object { + type = "datastream" + id = observe_datastream.test.id + } + role = "Viewer" + version = 2 + } + `, randomPrefix, systemUser()), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("observe_rbac_statement.example", "description", randomPrefix), + resource.TestCheckResourceAttr("observe_rbac_statement.example", "subject.#", "1"), + resource.TestCheckResourceAttrSet("observe_rbac_statement.example", "subject.0.user"), + resource.TestCheckResourceAttr("observe_rbac_statement.example", "object.#", "1"), + resource.TestCheckResourceAttrSet("observe_rbac_statement.example", "object.0.id"), + resource.TestCheckResourceAttr("observe_rbac_statement.example", "role", "Viewer"), + resource.TestCheckResourceAttr("observe_rbac_statement.example", "version", "2"), + ), + }, + }, + }) +} From f5d19699cf739633fec7e3d184a1da73d57a5556 Mon Sep 17 00:00:00 2001 From: Abhinav Pappu Date: Thu, 3 Oct 2024 13:20:10 -0700 Subject: [PATCH 2/2] remove the exactlyoneof validation since it's not possible to support both v1 and v2 logic with it --- observe/resource_rbac_statement.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/observe/resource_rbac_statement.go b/observe/resource_rbac_statement.go index 43b54f4c..6a5aa609 100644 --- a/observe/resource_rbac_statement.go +++ b/observe/resource_rbac_statement.go @@ -27,15 +27,6 @@ const ( schemaRbacStatementSubjectGroupDescription = "OID of a RBAC Group." ) -var rbacStatementObjectTypes = []string{ - "object.0.id", - "object.0.folder", - "object.0.workspace", - // excluding type from ExactlyOneOf since it's required to specify both a type and and id for v2 resource statements - // "object.0.type", - "object.0.all", -} - func resourceRbacStatement() *schema.Resource { return &schema.Resource{ Description: "Manages a RBAC Statement.", @@ -88,19 +79,16 @@ func resourceRbacStatement() *schema.Resource { Schema: map[string]*schema.Schema{ "id": { Type: schema.TypeString, - ExactlyOneOf: rbacStatementObjectTypes, Optional: true, Description: schemaRbacStatementObjectIdDescription, }, "folder": { Type: schema.TypeString, - ExactlyOneOf: rbacStatementObjectTypes, Optional: true, Description: schemaRbacStatementObjectFolderDescription, }, "workspace": { Type: schema.TypeString, - ExactlyOneOf: rbacStatementObjectTypes, Optional: true, Description: schemaRbacStatementObjectWorkspaceDescription, }, @@ -124,7 +112,6 @@ func resourceRbacStatement() *schema.Resource { }, "all": { Type: schema.TypeBool, - ExactlyOneOf: rbacStatementObjectTypes, Optional: true, Default: false, }, @@ -348,7 +335,11 @@ func rbacStatementToResourceData(r *gql.RbacStatement, data *schema.ResourceData diags = append(diags, diag.FromErr(err)...) } - if err := data.Set("version", r.Version); err != nil { + version := 1 + if r.Version != nil { + version = *r.Version + } + if err := data.Set("version", version); err != nil { diags = append(diags, diag.FromErr(err)...) }