From 442efa6d8cb129707c47d43d4cda66ceed292f00 Mon Sep 17 00:00:00 2001 From: Geoff Greer Date: Wed, 26 Feb 2025 15:48:57 -0800 Subject: [PATCH 1/5] Add CI test for granting/revoking. --- .github/workflows/ci.yaml | 11 +++++---- scripts/grant-revoke.sh | 48 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) create mode 100755 scripts/grant-revoke.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 6dd6cfb2..42722f8b 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -48,8 +48,8 @@ jobs: BATON_WORKSPACES: ${{ secrets.BATON_WORKSPACES }} BATON_WORKSPACE_TOKENS: ${{ secrets.BATON_WORKSPACE_TOKENS }} # The following parameters are passed to grant/revoke commands - CONNECTOR_GRANT: 'group:778441812670942:member:user:5346803201281760' - CONNECTOR_ENTITLEMENT: 'group:778441812670942:member' + # CONNECTOR_GRANT: 'group:778441812670942:member:user:5346803201281760' + CONNECTOR_ENTITLEMENT: 'group:account/8c6f99ec-78ce-4654-8f92-e716b3dd67a7/group/778441812670942:member' CONNECTOR_PRINCIPAL_TYPE: 'user' CONNECTOR_PRINCIPAL: '5346803201281760' steps: @@ -63,5 +63,8 @@ jobs: run: ./scripts/get-baton.sh && mv baton /usr/local/bin - name: Build baton-databricks run: go build ./cmd/baton-databricks - - name: Run baton-databricks - run: ./baton-databricks + - name: Test grant/revoking entitlements + env: + BATON: baton + BATON_DATABRICKS: ./baton-databricks + run: ./scripts/grant-revoke.sh diff --git a/scripts/grant-revoke.sh b/scripts/grant-revoke.sh new file mode 100755 index 00000000..236746ff --- /dev/null +++ b/scripts/grant-revoke.sh @@ -0,0 +1,48 @@ +#!/bin/bash + +set -exo pipefail + +if [ -z "$BATON_DATABRICKS" ]; then + echo "BATON_DATABRICKS not set. using baton-databricks" + BATON_DATABRICKS=baton-databricks +fi +if [ -z "$BATON" ]; then + echo "BATON not set. using baton" + BATON=baton +fi + +# Error on unbound variables now that we've set BATON & BATON_DATABRICKS +set -u + +# Sync +$BATON_DATABRICKS + +# Grant entitlement +$BATON_DATABRICKS --grant-entitlement="$CONNECTOR_ENTITLEMENT" --grant-principal="$CONNECTOR_PRINCIPAL" --grant-principal-type="$CONNECTOR_PRINCIPAL_TYPE" + +# Check for grant before revoking +$BATON_DATABRICKS +$BATON grants --entitlement="$CONNECTOR_ENTITLEMENT" --output-format=json | jq --exit-status ".grants[] | select( .principal.id.resource == \"$CONNECTOR_PRINCIPAL\" )" + +# Grant already-granted entitlement +$BATON_DATABRICKS --grant-entitlement="$CONNECTOR_ENTITLEMENT" --grant-principal="$CONNECTOR_PRINCIPAL" --grant-principal-type="$CONNECTOR_PRINCIPAL_TYPE" + +# Get grant ID +CONNECTOR_GRANT=$($BATON grants --entitlement="$CONNECTOR_ENTITLEMENT" --output-format=json | jq --raw-output --exit-status ".grants[] | select( .principal.id.resource == \"$CONNECTOR_PRINCIPAL\" ).grant.id") + +# Revoke grant +$BATON_DATABRICKS --revoke-grant="$CONNECTOR_GRANT" + +# Revoke already-revoked grant +$BATON_DATABRICKS --revoke-grant="$CONNECTOR_GRANT" + +# Check grant was revoked +$BATON_DATABRICKS +$BATON grants --entitlement="$CONNECTOR_ENTITLEMENT" --output-format=json | jq --exit-status "if .grants then [ .grants[] | select( .principal.id.resource == \"$CONNECTOR_PRINCIPAL\" ) ] | length == 0 else . end" + +# Re-grant entitlement +$BATON_DATABRICKS --grant-entitlement="$CONNECTOR_ENTITLEMENT" --grant-principal="$CONNECTOR_PRINCIPAL" --grant-principal-type="$CONNECTOR_PRINCIPAL_TYPE" + +# Check grant was re-granted +$BATON_DATABRICKS +$BATON grants --entitlement="$CONNECTOR_ENTITLEMENT" --output-format=json | jq --exit-status ".grants[] | select( .principal.id.resource == \"$CONNECTOR_PRINCIPAL\" )" From a7427795630e03d2daaf006a62a03874d994abf6 Mon Sep 17 00:00:00 2001 From: Geoff Greer Date: Wed, 26 Feb 2025 16:07:10 -0800 Subject: [PATCH 2/5] Fix provisioning error if Grant() gets a user ID (which has no slashes in it) --- pkg/connector/groups.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pkg/connector/groups.go b/pkg/connector/groups.go index 2595d140..9cd3feb0 100644 --- a/pkg/connector/groups.go +++ b/pkg/connector/groups.go @@ -273,13 +273,21 @@ func (g *groupBuilder) Grant(ctx context.Context, principal *v2.Resource, entitl return nil, fmt.Errorf("databricks-connector: only users, groups and service principals can be granted group permissions") } - parentId, principalId, err := parseResourceId(principal.Id.Resource) - if err != nil { - return nil, fmt.Errorf("databricks-connector: failed to parse principal resource id: %w", err) + var parentId *v2.ResourceId + var principalId *v2.ResourceId + var err error + if principal.Id.ResourceType == groupResourceType.Id { + parentId, principalId, err = parseResourceId(principal.Id.Resource) + if err != nil { + return nil, fmt.Errorf("databricks-connector: failed to parse principal resource id: %w", err) + } + } else { + parentId = principal.ParentResourceId + principalId = principal.Id } var workspaceId string - if parentId.ResourceType == workspaceResourceType.Id { + if parentId != nil && parentId.ResourceType == workspaceResourceType.Id { workspaceId = parentId.Resource } From fd5ebf69114da6a4d4efb09dfc6192b23ac60399 Mon Sep 17 00:00:00 2001 From: Geoff Greer Date: Wed, 26 Feb 2025 16:14:00 -0800 Subject: [PATCH 3/5] Use correct entitlement ID for test. --- .github/workflows/ci.yaml | 8 +++----- scripts/grant-revoke.sh | 6 ++++++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 42722f8b..0759cd2c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -47,11 +47,6 @@ jobs: BATON_ACCOUNT_ID: ${{ secrets.BATON_ACCOUNT_ID }} BATON_WORKSPACES: ${{ secrets.BATON_WORKSPACES }} BATON_WORKSPACE_TOKENS: ${{ secrets.BATON_WORKSPACE_TOKENS }} - # The following parameters are passed to grant/revoke commands - # CONNECTOR_GRANT: 'group:778441812670942:member:user:5346803201281760' - CONNECTOR_ENTITLEMENT: 'group:account/8c6f99ec-78ce-4654-8f92-e716b3dd67a7/group/778441812670942:member' - CONNECTOR_PRINCIPAL_TYPE: 'user' - CONNECTOR_PRINCIPAL: '5346803201281760' steps: - name: Install Go uses: actions/setup-go@v5 @@ -67,4 +62,7 @@ jobs: env: BATON: baton BATON_DATABRICKS: ./baton-databricks + CONNECTOR_ENTITLEMENT: 'group:account/8c6f99ec-78ce-4654-8f92-e716b3dd67a7/group/66271399072731:member' + CONNECTOR_PRINCIPAL_TYPE: 'user' + CONNECTOR_PRINCIPAL: '5346803201281760' run: ./scripts/grant-revoke.sh diff --git a/scripts/grant-revoke.sh b/scripts/grant-revoke.sh index 236746ff..e3952e67 100755 --- a/scripts/grant-revoke.sh +++ b/scripts/grant-revoke.sh @@ -17,6 +17,12 @@ set -u # Sync $BATON_DATABRICKS +$BATON resources + +$BATON entitlements + +$BATON grants + # Grant entitlement $BATON_DATABRICKS --grant-entitlement="$CONNECTOR_ENTITLEMENT" --grant-principal="$CONNECTOR_PRINCIPAL" --grant-principal-type="$CONNECTOR_PRINCIPAL_TYPE" From a3c1c93d85f9c21989641239366046f5f752680d Mon Sep 17 00:00:00 2001 From: Geoff Greer Date: Wed, 26 Feb 2025 17:11:46 -0800 Subject: [PATCH 4/5] Sync everything, not just specific workspaces. --- .github/workflows/ci.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 0759cd2c..5a6884e5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -42,11 +42,11 @@ jobs: env: BATON_LOG_LEVEL: debug # Add any environment variables needed to run baton-bitbucket-datacenter - DATABRICKS_CLIENT_ID: ${{ secrets.DATABRICKS_CLIENT_ID }} - DATABRICKS_CLIENT_SECRET: ${{ secrets.DATABRICKS_CLIENT_SECRET }} + BATON_DATABRICKS_CLIENT_ID: ${{ secrets.DATABRICKS_CLIENT_ID }} + BATON_DATABRICKS_CLIENT_SECRET: ${{ secrets.DATABRICKS_CLIENT_SECRET }} BATON_ACCOUNT_ID: ${{ secrets.BATON_ACCOUNT_ID }} - BATON_WORKSPACES: ${{ secrets.BATON_WORKSPACES }} - BATON_WORKSPACE_TOKENS: ${{ secrets.BATON_WORKSPACE_TOKENS }} + # BATON_WORKSPACES: ${{ secrets.BATON_WORKSPACES }} + # BATON_WORKSPACE_TOKENS: ${{ secrets.BATON_WORKSPACE_TOKENS }} steps: - name: Install Go uses: actions/setup-go@v5 From 943809b5d7adce2464b0d7f133241e4bd7ca48f5 Mon Sep 17 00:00:00 2001 From: Geoff Greer Date: Wed, 26 Feb 2025 17:48:51 -0800 Subject: [PATCH 5/5] Parse group IDs better. --- pkg/connector/groups.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/pkg/connector/groups.go b/pkg/connector/groups.go index 9cd3feb0..8d52d21b 100644 --- a/pkg/connector/groups.go +++ b/pkg/connector/groups.go @@ -395,6 +395,17 @@ func (g *groupBuilder) Revoke(ctx context.Context, grant *v2.Grant) (annotations return nil, fmt.Errorf("databricks-connector: only users, groups and service principals can have group permissions revoked") } + principalId := principal.Id.Resource + + var err error + if principal.Id.ResourceType == groupResourceType.Id { + _, principal, err := parseResourceId(principal.Id.Resource) + if err != nil { + return nil, fmt.Errorf("databricks-connector: failed to parse principal resource id: %w", err) + } + principalId = principal.Resource + } + parentResourceId, groupId, err := parseResourceId(entitlement.Resource.Id.Resource) if err != nil { return nil, fmt.Errorf("databricks-connector: failed to parse entitlement resource id: %w", err) @@ -418,7 +429,7 @@ func (g *groupBuilder) Revoke(ctx context.Context, grant *v2.Grant) (annotations } for i, member := range group.Members { - if member.ID == principal.Id.Resource { + if member.ID == principalId { group.Members = slices.Delete(group.Members, i, i+1) break } @@ -444,7 +455,7 @@ func (g *groupBuilder) Revoke(ctx context.Context, grant *v2.Grant) (annotations return nil, nil } - principalID, err := preparePrincipalId(ctx, g.client, workspaceId, principal.Id.ResourceType, principal.Id.Resource) + principalId, err := preparePrincipalId(ctx, g.client, workspaceId, principal.Id.ResourceType, principal.Id.Resource) if err != nil { return nil, fmt.Errorf("databricks-connector: failed to prepare principal id: %w", err) } @@ -455,12 +466,12 @@ func (g *groupBuilder) Revoke(ctx context.Context, grant *v2.Grant) (annotations } // check if it contains the principals and remove the principal to the rule set - if slices.Contains(ruleSet.Principals, principalID) { + if slices.Contains(ruleSet.Principals, principalId) { // if there is only one principal, remove the whole rule set if len(ruleSet.Principals) == 1 { ruleSets = slices.Delete(ruleSets, i, i+1) } else { - pI := slices.Index(ruleSet.Principals, principalID) + pI := slices.Index(ruleSet.Principals, principalId) ruleSets[i].Principals = slices.Delete(ruleSet.Principals, pI, pI+1) } break @@ -468,7 +479,7 @@ func (g *groupBuilder) Revoke(ctx context.Context, grant *v2.Grant) (annotations l.Info( "databricks-connector: group already does not have the entitlement", - zap.String("principal_id", principalID), + zap.String("principal_id", principalId), zap.String("entitlement", entitlement.Slug), )