diff --git a/README.md b/README.md index 507785be..848585c0 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,8 @@ baton resources - **Server Administrators**: Users with `ServerAdministrator` site role are skipped during group membership sync (server-level admins are not site-scoped). - **Grant expansion**: Group-based permissions on projects, workbooks, and views are supported. When a group has a permission, all members inherit it. Revoking such a permission from a single member does not remove their access: Tableau has no user-level assignment to remove, so the revoke returns `GrantAlreadyRevoked` and the next sync derives the permission from the group again. Revoke from the group, or remove the user from the group. - **"All Users" group**: Tableau automatically adds every user to the built-in "All Users" group. This group cannot be modified via the API: Grant is a no-op because the user is already a member, and Revoke returns an error. The connector syncs its membership to show each user's base site role. -- **License revoke constraints**: Revoking a license (setting a user to "Unlicensed") will fail if the user belongs to a group with a "Grant role on sign in" minimum site role. Remove the user from the constraining group first, then revoke the license. +- **License grant is tier-aware**: A tier is a view of the user's single site role (for example, `SiteAdministratorCreator` already satisfies Creator), and the four tiers form one exclusion group (`tableau-license`) with no default tier or ranking. Granting a tier the user already holds is a no-op that keeps their role and returns `GrantAlreadyExists`. Granting a different tier to a user holding `SiteAdministrator`, `SiteAdministratorCreator`, `SiteAdministratorExplorer`, or `ServerAdministrator` fails with `FailedPrecondition`: change those roles through the Site Role entitlement (it assigns `SiteAdministrator*`; this connector does not assign `ServerAdministrator`). Any other user is set to the tier's base role, with `GrantReplaced` only when the previous role belonged to a recognized tier. A missing user returns `NotFound`. +- **License revoke sets the user to Unlicensed**: A user who still holds the tier is set to `Unlicensed`, including the four admin roles above. A missing user, or one who no longer holds the tier, returns `GrantAlreadyRevoked` with no write; so does revoking Unlicensed from an Unlicensed user, and the next sync still shows that user as Unlicensed. Re-assign `SiteAdministrator*` roles through the Site Role entitlement. `ServerAdministrator` exists only on Tableau Server and this connector does not assign it, so a revoked Server Administrator stays Unlicensed until the role is restored in Tableau. Revoke also fails if the user belongs to a group with a "Grant role on sign in" minimum site role; remove them from that group first. - **Locked project names**: Inherited workbook grants for locked projects are found by filtering workbooks on the project name, and Tableau rejects filters containing commas (`,`) or ampersands (`&`). Locked projects whose names contain these characters are skipped for inherited workbook grants and a warning is logged. Rename those projects to sync their inherited access. - **Idempotency**: Granting a permission that already exists returns success (not an error). Revoking a permission that is already removed also returns success. diff --git a/docs/connector.mdx b/docs/connector.mdx index e7f12d5e..de273ede 100644 --- a/docs/connector.mdx +++ b/docs/connector.mdx @@ -39,6 +39,9 @@ In Tableau, licensing is role-based: each user's **site role** determines their When your Tableau site has per-tier seat capacities configured, C1 tracks the purchased and consumed seat counts for each license tier, so you can review license utilization alongside who holds each tier. +Because a tier is a view of the user's single site role, several roles can satisfy one tier (for example, a Site Administrator Creator counts as Creator), and the four tiers are mutually exclusive in C1 with no default tier or ranking. Granting a tier the user already holds changes nothing and keeps their current role. Administrators are refused when you grant them a different tier: change their role through the Site Role entitlement instead. For any other user, granting a tier sets their site role to that tier's base role, and C1 replaces their previous tier grant when the previous role belonged to a recognized tier. Granting to a user that no longer exists returns a not-found error. + +Revoking a tier a user still holds sets them to Unlicensed, including users with an administrator role. Revoking from a user who no longer exists, or who no longer holds the tier, changes nothing; the same applies to revoking Unlicensed from an Unlicensed user, and the next sync still shows that user as Unlicensed. To give a revoked administrator their role back, assign it through the Site Role entitlement. Server Administrator exists only on Tableau Server and this connector cannot assign it, so a revoked Server Administrator stays Unlicensed until a server administrator restores the role in Tableau. Revoke also fails if the user belongs to a group with a "Grant role on sign in" minimum site role; remove them from that group first. Revoking a license fails if the user belongs to a group with a **Grant role on sign in** minimum site role. Tableau returns `Site role of link user that belongs to license upon login cannot be lower than minimum site role of group`. Remove the user from that group first, then revoke the license. diff --git a/docs/docs-info.md b/docs/docs-info.md index 8bb8e29b..447cd699 100644 --- a/docs/docs-info.md +++ b/docs/docs-info.md @@ -135,7 +135,7 @@ > | Viewer | `Viewer` | Yes | | > | Unlicensed | `Unlicensed` | Yes | | > | Site Administrator | `SiteAdministrator` | Yes | Legacy role, maps to SiteAdministratorExplorer | -> | Server Administrator | `ServerAdministrator` | Sync only | Server-level role, appears in sync but cannot be assigned via the REST API | +> | Server Administrator | `ServerAdministrator` | Sync only | Exists only on Tableau Server; appears in sync, and this connector does not assign it | ### Users @@ -187,7 +187,7 @@ - `Viewer` — View and interact with published dashboards - `Unlicensed` — No access to Tableau site features - **Entitlements**: - - `member` — License is assigned to user (static entitlement) + - `member` — License is assigned to user (static entitlement); all four tiers share exclusion group `tableau-license` (unscoped, no default or order) - **Seat counts** (License Profile trait): - **Purchased** — read from the site's per-tier capacity fields (`tierCreatorCapacity`, `tierExplorerCapacity`, `tierViewerCapacity`), which the Tableau API returns as JSON strings. Attached only when the value is a positive number; a `0` or absent capacity attaches no seats (avoids rendering "0 of 0"). The `Unlicensed` floor tier is never capped. - **Consumed** — counted server-side per tier via the users `siteRole` filter, reading `pagination.totalAvailable` (one request per tier instead of walking every user page). @@ -205,7 +205,7 @@ > | Viewer | `Viewer` | > | Unlicensed | `Unlicensed` | > -> `SiteAdministrator` is a legacy role (maps to `SiteAdministratorExplorer`) that cannot be used with the Tableau REST API filter endpoint, so the connector fetches all users and filters client-side when processing Explorer license grants. `ServerAdministrator` consumes a Creator license per Tableau's licensing model and is counted under the Creator tier (sync only — it cannot be assigned via the sites API). See [Tableau REST API Site Roles](https://help.tableau.com/current/api/rest_api/en-us/REST/rest_api_concepts_new_site_roles.htm) for the full role reference. +> `SiteAdministrator` is a legacy role (maps to `SiteAdministratorExplorer`) that cannot be used with the Tableau REST API filter endpoint, so the connector fetches all users and filters client-side when processing Explorer license grants. `ServerAdministrator` consumes a Creator license per Tableau's licensing model and is counted under the Creator tier (sync only — this connector does not assign it). See [Tableau REST API Site Roles](https://help.tableau.com/current/api/rest_api/en-us/REST/rest_api_concepts_new_site_roles.htm) for the full role reference. > **Important — License Revoke Constraints**: Revoking a license (setting a user to "Unlicensed") will fail if the user belongs to a group with "Grant role on sign in" configured. Tableau enforces that a user's site role cannot be lower than the minimum site role required by any group they belong to. The error message from Tableau will be: *"Site role of link user that belongs to license upon login cannot be lower than minimum site role of group"*. To resolve this, remove the user from the constraining group first, then revoke the license. @@ -319,12 +319,15 @@ - Users with unknown or unexpected site roles (not in the connector's role map) are **skipped** during site grant sync instead of creating malformed grants. A warning is logged. - The role map covers all valid [Tableau REST API site roles](https://help.tableau.com/current/api/rest_api/en-us/REST/rest_api_concepts_new_site_roles.htm): `SiteAdministratorCreator`, `SiteAdministratorExplorer`, `SiteAdministrator`, `Creator`, `Explorer`, `ExplorerCanPublish`, `Viewer`, `Unlicensed`. -- `ServerAdministrator` is synced (read-only) and consumes a Creator license, but it is never a grant target — the assignable license is Creator, and the `ServerAdministrator` role itself cannot be set via the sites API. +- `ServerAdministrator` is synced (read-only) and consumes a Creator license, but it is never a grant target — the assignable license is Creator, and this connector does not assign the `ServerAdministrator` role itself. Revoking a tier from a Server Administrator leaves the user Unlicensed, and the role has to be restored by a server administrator in Tableau. ### License Grants - The Explorer license includes `SiteAdministrator` (legacy role). This role is not supported by the Tableau API's `siteRole` filter, so the connector falls back to fetching all users and filtering client-side for the Explorer license. - For Creator, Viewer, and Unlicensed licenses, server-side filtering is used for efficiency. +- **Grant** is tier-aware: a user whose site role already belongs to the target tier gets `GrantAlreadyExists` with no write and keeps their role. A different tier for a user holding `SiteAdministrator`, `SiteAdministratorCreator`, `SiteAdministratorExplorer`, or `ServerAdministrator` fails with `FailedPrecondition` (the Site Role entitlement assigns the `SiteAdministrator*` roles; this connector does not assign `ServerAdministrator`). Any other user is set to the tier's base role, with `GrantReplaced` only when the previous role belonged to a recognized tier. A missing user returns `NotFound`. +- **Revoke** sets a user who still holds the tier to `Unlicensed`, including the four admin roles above. A missing user, or one who no longer holds the tier, returns `GrantAlreadyRevoked` without a write. Revoking the Unlicensed tier from an already Unlicensed user is the same successful no-op; the next sync still shows Unlicensed in C1 because it is the floor role and current state. +- Grant and Revoke read the user first without caching and merge the read's rate-limit annotations into every result. - Consumed-seat counts reuse the same server-side `siteRole` filter but read only `pagination.totalAvailable` per tier (no full user walk). The Explorer count excludes the unfilterable legacy `SiteAdministrator` role, which does not exist on Tableau Cloud. ### View Grants and showTabs @@ -417,7 +420,7 @@ Tested against: `us-east-1.online.tableau.com` (Tableau Cloud) | viewer | OK | OK | | | unlicensed | OK | OK | | | site administrator | OK | OK | Legacy role (maps to SiteAdministratorExplorer) | -| server administrator | Blocked | N/A | Read-only: server-level role, cannot be assigned via REST API | +| server administrator | Blocked | N/A | Read-only: exists only on Tableau Server; this connector does not assign it | ### Group Membership diff --git a/pkg/client/client.go b/pkg/client/client.go index 49543cfe..4e89f4ef 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -10,7 +10,7 @@ // // Users: // - GET /api/{version}/sites/{siteId}/users - List users (paginated) -// - GET /api/{version}/sites/{siteId}/users/{userId} - Verify current user +// - GET /api/{version}/sites/{siteId}/users/{userId} - Verify current user / get a single user // - POST /api/{version}/sites/{siteId}/users - Add user to site // - PUT /api/{version}/sites/{siteId}/users/{userId} - Update user site role // - DELETE /api/{version}/sites/{siteId}/users/{userId} - Remove user from site @@ -73,6 +73,8 @@ import ( "github.com/conductorone/baton-sdk/pkg/annotations" "github.com/conductorone/baton-sdk/pkg/uhttp" "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) // ============================================================================= @@ -278,6 +280,25 @@ func (c *Client) AddUserToSite(ctx context.Context, user CreateUserRequest) (*Us return res.User, annos, nil } +// GetUser returns a single user by ID. Callers can distinguish a missing +// user from other failures with IsNotFoundError. +func (c *Client) GetUser(ctx context.Context, userId string) (*User, annotations.Annotations, error) { + urlGetUser, err := c.buildSiteURL(pathUsers, userId) + if err != nil { + return nil, nil, err + } + + var res userResponse + // Bypass the SDK response cache: provisioning reads the role it is about to + // act on, and a cached copy goes stale after this client's own writes. + annos, err := c.doRequest(ctx, http.MethodGet, urlGetUser, &res, nil, uhttp.WithNoCache()) + if err != nil { + return nil, annos, err + } + + return &res.User, annos, nil +} + // UpdateUserSiteRole updates a user's site role. func (c *Client) UpdateUserSiteRole(ctx context.Context, userId, siteRole string) (annotations.Annotations, error) { urlUpdateUserSiteRole, err := c.buildSiteURL(pathUsers, userId) @@ -752,8 +773,15 @@ func permissionBody(granteeType, granteeID, capabilityName, capabilityMode strin } } -// doRequest executes an HTTP request against the Tableau API. -func (c *Client) doRequest(ctx context.Context, method string, url *url.URL, res, body any) (annotations.Annotations, error) { +// IsNotFoundError reports whether err is the error doRequest returns for an +// HTTP 404 response (e.g. GetUser on a deleted/unknown user ID). +func IsNotFoundError(err error) bool { + return status.Code(err) == codes.NotFound +} + +// doRequest executes an HTTP request against the Tableau API. Extra request +// options are applied after the defaults. +func (c *Client) doRequest(ctx context.Context, method string, url *url.URL, res, body any, extraOpts ...uhttp.RequestOption) (annotations.Annotations, error) { reqOpts := []uhttp.RequestOption{ uhttp.WithAcceptJSONHeader(), uhttp.WithContentTypeJSONHeader(), @@ -762,6 +790,7 @@ func (c *Client) doRequest(ctx context.Context, method string, url *url.URL, res if body != nil { reqOpts = append(reqOpts, uhttp.WithJSONBody(body)) } + reqOpts = append(reqOpts, extraOpts...) req, err := c.httpClient.NewRequest(ctx, method, url, reqOpts...) if err != nil { diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go new file mode 100644 index 00000000..dc44dacd --- /dev/null +++ b/pkg/client/client_test.go @@ -0,0 +1,71 @@ +package client + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/conductorone/baton-sdk/pkg/uhttp" + "github.com/stretchr/testify/require" +) + +const testSiteId = "site-1" + +// newTestClient builds a Client wired directly to srv, bypassing New's +// Login/auth-signin flow so tests only exercise the endpoint under test. +func newTestClient(t *testing.T, srv *httptest.Server) *Client { + t.Helper() + + base, err := uhttp.NewBaseHttpClientWithContext(context.Background(), srv.Client()) + require.NoError(t, err) + + return &Client{ + httpClient: base, + baseUrl: srv.URL, + authToken: "test-token", + siteId: testSiteId, + } +} + +func TestGetUser_Found(t *testing.T) { + t.Parallel() + + wantPath := "/sites/" + testSiteId + "/users/user-1" + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != wantPath { + w.WriteHeader(http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"user":{"id":"user-1","email":"user1@example.com","siteRole":"SiteAdministratorCreator"}}`)) + })) + t.Cleanup(srv.Close) + + c := newTestClient(t, srv) + + user, _, err := c.GetUser(context.Background(), "user-1") + require.NoError(t, err) + require.NotNil(t, user) + require.Equal(t, "user-1", user.ID) + require.Equal(t, "SiteAdministratorCreator", user.SiteRole) +} + +func TestGetUser_NotFound(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":{"summary":"Not Found","detail":"User not found","code":"0x1"}}`)) + })) + t.Cleanup(srv.Close) + + c := newTestClient(t, srv) + + user, _, err := c.GetUser(context.Background(), "missing-user") + require.Error(t, err) + require.Nil(t, user) + require.True(t, IsNotFoundError(err), "expected IsNotFoundError(err) to be true, got err: %v", err) +} diff --git a/pkg/connector/group.go b/pkg/connector/group.go index 80b512b2..6d1406c5 100644 --- a/pkg/connector/group.go +++ b/pkg/connector/group.go @@ -186,7 +186,7 @@ func (g *groupBuilder) Revoke(ctx context.Context, grant *v2.Grant) (annotations annos, err := g.client.RemoveUserFromGroup(ctx, entitlement.Resource.Id.Resource, principal.Id.Resource) if err != nil { - if isNotFoundError(err) { + if client.IsNotFoundError(err) { return annotations.New(&v2.GrantAlreadyRevoked{}), nil } return annos, fmt.Errorf("failed to remove user from group: %w", err) diff --git a/pkg/connector/helpers.go b/pkg/connector/helpers.go index bb6f013d..5ddf8b23 100644 --- a/pkg/connector/helpers.go +++ b/pkg/connector/helpers.go @@ -3,6 +3,7 @@ package connector import ( "context" "fmt" + "slices" "strings" v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" @@ -13,6 +14,7 @@ import ( "github.com/conductorone/baton-tableau/pkg/client" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/anypb" ) const ( @@ -208,9 +210,23 @@ func isAlreadyExistsError(err error) bool { return status.Code(err) == codes.AlreadyExists } -// isNotFoundError returns true if the error indicates the resource was not found (HTTP 404). -func isNotFoundError(err error) bool { - return status.Code(err) == codes.NotFound +// mergeAnnotations concatenates annotation sets in order. When the same +// message type appears more than once (e.g. a RateLimitDescription from each +// of several API calls), the most recent one wins so a result carries at most +// one entry per type. +func mergeAnnotations(sets ...annotations.Annotations) annotations.Annotations { + var merged annotations.Annotations + for _, set := range sets { + for _, a := range set { + idx := slices.IndexFunc(merged, func(m *anypb.Any) bool { return m.GetTypeUrl() == a.GetTypeUrl() }) + if idx >= 0 { + merged[idx] = a + continue + } + merged = append(merged, a) + } + } + return merged } // permissionFunc is the signature shared by all Add/Delete permission client methods. @@ -297,7 +313,7 @@ func revokePermission( } if err != nil { - if isNotFoundError(err) { + if client.IsNotFoundError(err) { return annotations.New(&v2.GrantAlreadyRevoked{}), nil } return annos, fmt.Errorf("failed to revoke permission: %w", err) diff --git a/pkg/connector/license.go b/pkg/connector/license.go index a8bac42b..40fec2a1 100644 --- a/pkg/connector/license.go +++ b/pkg/connector/license.go @@ -3,21 +3,31 @@ package connector import ( "context" "fmt" + "slices" "strconv" "strings" v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" "github.com/conductorone/baton-sdk/pkg/annotations" + "github.com/conductorone/baton-sdk/pkg/connectorbuilder" ent "github.com/conductorone/baton-sdk/pkg/types/entitlement" grant "github.com/conductorone/baton-sdk/pkg/types/grant" rs "github.com/conductorone/baton-sdk/pkg/types/resource" "github.com/conductorone/baton-tableau/pkg/client" "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" "go.uber.org/zap" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) var licenses = []string{creator, explorer, viewer, unlicensed} +// licenseExclusionGroup marks the four tier entitlements as mutually exclusive: +// a user's single site role puts them in exactly one tier. It is deliberately +// unscoped, so the SDK shares one group across the tier resources, and sets no +// default or order, so C1 gets no fallback tier or privilege ranking. +const licenseExclusionGroup = "tableau-license" + var licensesMap = map[string]string{ "creator": creator, "explorer": explorer, @@ -34,6 +44,24 @@ var RolesPerLicense = map[string][]string{ unlicensed: {unlicensed}, } +// adminLicenseRoles are refused on a cross-tier license Grant. +var adminLicenseRoles = map[string]bool{ + siteAdministrator: true, + siteAdministratorCreator: true, + siteAdministratorExplorer: true, + serverAdministrator: true, +} + +// tierForRole returns the license tier whose RolesPerLicense set contains role. +func tierForRole(role string) (string, bool) { + for _, tier := range licenses { + if slices.Contains(RolesPerLicense[tier], role) { + return tier, true + } + } + return "", false +} + // licenseCapacity returns the raw per-tier capacity string, or nil when the site exposes none. func licenseCapacity(license string, site *client.Site) *string { if site == nil { @@ -190,6 +218,7 @@ func (l *licenseBuilder) StaticEntitlements(_ context.Context, _ rs.SyncOpAttrs) ent.WithGrantableTo(resourceTypeUser), ent.WithDisplayName("License member"), ent.WithDescription("Member of License in Tableau"), + ent.WithExclusionGroup(licenseExclusionGroup), ) return []*v2.Entitlement{en}, nil, nil @@ -237,26 +266,95 @@ func (l *licenseBuilder) Grants(ctx context.Context, resource *v2.Resource, opts return rv, &rs.SyncOpResults{NextPageToken: nextToken}, nil } -// Grant assigns the target license tier by setting the user's site role. -func (l *licenseBuilder) Grant(ctx context.Context, principal *v2.Resource, entitlement *v2.Entitlement) (annotations.Annotations, error) { - licenseName, ok := licensesMap[entitlement.Resource.Id.Resource] +// Grant sets the user's site role to the tier's base role after a fresh read +// and returns the new grant on a write (nil on every no-op or error path): +// NotFound for a missing user, GrantAlreadyExists (no write) for a role already +// in the tier, FailedPrecondition for an admin role, else a write that reports +// GrantReplaced when the previous role belonged to a known tier. +func (l *licenseBuilder) Grant(ctx context.Context, principal *v2.Resource, entitlement *v2.Entitlement) ([]*v2.Grant, annotations.Annotations, error) { + log := ctxzap.Extract(ctx) + + targetTier, ok := licensesMap[entitlement.Resource.Id.Resource] if !ok { - return nil, fmt.Errorf("unknown license %s", entitlement.Resource.Id.Resource) + return nil, nil, status.Errorf(codes.InvalidArgument, "unknown license %s", entitlement.Resource.Id.Resource) } - annos, err := l.client.UpdateUserSiteRole(ctx, principal.Id.Resource, licenseName) + user, getAnnos, err := l.client.GetUser(ctx, principal.Id.Resource) if err != nil { - return annos, fmt.Errorf("failed to grant %s license to user %s: %w", licenseName, principal.Id.Resource, err) + if client.IsNotFoundError(err) { + return nil, getAnnos, fmt.Errorf("license: cannot grant %s: user %s not found: %w", targetTier, principal.Id.Resource, err) + } + return nil, getAnnos, fmt.Errorf("failed to read user %s for license grant: %w", principal.Id.Resource, err) } - return annos, nil + if slices.Contains(RolesPerLicense[targetTier], user.SiteRole) { + log.Debug("license grant: user already in target tier, no write needed", + zap.String("user_id", principal.Id.Resource), zap.String("tier", targetTier), zap.String("site_role", user.SiteRole)) + return nil, mergeAnnotations(getAnnos, annotations.New(&v2.GrantAlreadyExists{})), nil + } + + if adminLicenseRoles[user.SiteRole] { + return nil, getAnnos, status.Errorf(codes.FailedPrecondition, + "license: cannot grant %s to user %s: current site role %q is an administrative role; use the Site Role entitlement to change it explicitly", + targetTier, principal.Id.Resource, user.SiteRole) + } + + putAnnos, err := l.client.UpdateUserSiteRole(ctx, principal.Id.Resource, targetTier) + annos := mergeAnnotations(getAnnos, putAnnos) + if err != nil { + return nil, annos, fmt.Errorf("failed to grant %s license to user %s: %w", targetTier, principal.Id.Resource, err) + } + + if oldTier, ok := tierForRole(user.SiteRole); ok { + oldResource := &v2.Resource{ + Id: &v2.ResourceId{ + ResourceType: resourceTypeLicense.Id, + Resource: strings.ToLower(oldTier), + }, + } + oldEntitlement := ent.NewAssignmentEntitlement(oldResource, memberEntitlement) + annos.Append(&v2.GrantReplaced{ReplacedGrantId: grant.NewGrantID(principal.Id, oldEntitlement)}) + } + + // Same shape as Grants() emits, so the grant ID matches the one sync reports. + return []*v2.Grant{grant.NewGrant(entitlement.Resource, memberEntitlement, principal.Id)}, annos, nil } -// Revoke removes the license by setting the user's site role to Unlicensed. -func (l *licenseBuilder) Revoke(ctx context.Context, grant *v2.Grant) (annotations.Annotations, error) { - principalID := grant.Principal.Id.Resource +// Revoke sets the user's site role to Unlicensed after a fresh read, returning +// GrantAlreadyRevoked without writing when the user is missing, no longer in the +// tier, or already Unlicensed. +func (l *licenseBuilder) Revoke(ctx context.Context, g *v2.Grant) (annotations.Annotations, error) { + log := ctxzap.Extract(ctx) + principalID := g.Principal.Id.Resource - annos, err := l.client.UpdateUserSiteRole(ctx, principalID, unlicensed) + tier, ok := licensesMap[g.Entitlement.Resource.Id.Resource] + if !ok { + return nil, status.Errorf(codes.InvalidArgument, "unknown license %s", g.Entitlement.Resource.Id.Resource) + } + + user, getAnnos, err := l.client.GetUser(ctx, principalID) + if err != nil { + if client.IsNotFoundError(err) { + log.Debug("license revoke: user not found, nothing to revoke", zap.String("user_id", principalID)) + return mergeAnnotations(getAnnos, annotations.New(&v2.GrantAlreadyRevoked{})), nil + } + return getAnnos, fmt.Errorf("failed to read user %s for license revoke: %w", principalID, err) + } + + if !slices.Contains(RolesPerLicense[tier], user.SiteRole) { + log.Debug("license revoke: user no longer in tier, nothing to revoke", + zap.String("user_id", principalID), zap.String("tier", tier), zap.String("site_role", user.SiteRole)) + return mergeAnnotations(getAnnos, annotations.New(&v2.GrantAlreadyRevoked{})), nil + } + + if tier == unlicensed { + log.Debug("license revoke: user already Unlicensed (floor tier), nothing to remove", + zap.String("user_id", principalID)) + return mergeAnnotations(getAnnos, annotations.New(&v2.GrantAlreadyRevoked{})), nil + } + + putAnnos, err := l.client.UpdateUserSiteRole(ctx, principalID, unlicensed) + annos := mergeAnnotations(getAnnos, putAnnos) if err != nil { return annos, fmt.Errorf("failed to revoke license from user %s: %w", principalID, err) } @@ -264,6 +362,8 @@ func (l *licenseBuilder) Revoke(ctx context.Context, grant *v2.Grant) (annotatio return annos, nil } +var _ connectorbuilder.ResourceProvisionerV2 = (*licenseBuilder)(nil) + func newLicenseBuilder(client *client.Client) *licenseBuilder { return &licenseBuilder{ client: client, diff --git a/pkg/connector/license_test.go b/pkg/connector/license_test.go index 3b65087d..00de592c 100644 --- a/pkg/connector/license_test.go +++ b/pkg/connector/license_test.go @@ -1,10 +1,29 @@ package connector import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "path/filepath" + "strconv" + "strings" + "sync" "testing" + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/annotations" + "github.com/conductorone/baton-sdk/pkg/connectorbuilder" + "github.com/conductorone/baton-sdk/pkg/dotc1z" + sdksync "github.com/conductorone/baton-sdk/pkg/sync" + "github.com/conductorone/baton-sdk/pkg/types" + "github.com/conductorone/baton-sdk/pkg/types/grant" "github.com/conductorone/baton-sdk/pkg/types/resource" + "github.com/conductorone/baton-tableau/pkg/client" "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) // TestLicenseResource_Trait asserts the LicenseProfileTrait is attached with the @@ -107,3 +126,758 @@ func TestCapacitySeats(t *testing.T) { }) } } + +// ============================================================================= +// Grant / Revoke +// ============================================================================= + +const ( + licenseTestSiteID = "site-1" + + // licenseTestServerError is a pseudo role that makes the mock GetUser return HTTP 500. + licenseTestServerError = "__server_error__" + + licenseTestRateLimit = 100 + licenseTestRateRemaining = 42 +) + +// licenseTestUsersMu guards the users maps handed to newLicenseTestClient, which +// the mock server's goroutine reads and writes while a test may change a role. +var licenseTestUsersMu sync.Mutex + +type recordedPut struct { + userID string + siteRole string +} + +// newLicenseTestClient starts a mock Tableau API that accepts any login and +// serves GetUser from users (user ID -> site role, updated by each PUT; a +// missing key is a 404, licenseTestServerError a 500). Every GET and PUT carries +// rate-limit headers, and every PUT is recorded in puts. +func newLicenseTestClient(t *testing.T, users map[string]string, puts *[]recordedPut) *client.Client { + t.Helper() + + usersPathPrefix := "/sites/" + licenseTestSiteID + "/users/" + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.Path == "/auth/signin": + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "credentials": map[string]any{ + "site": map[string]any{"id": licenseTestSiteID}, + "user": map[string]any{"id": "me"}, + "token": "test-token", + }, + }) + case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, usersPathPrefix): + userID := strings.TrimPrefix(r.URL.Path, usersPathPrefix) + licenseTestUsersMu.Lock() + role, ok := users[userID] + licenseTestUsersMu.Unlock() + if !ok { + w.Header().Set("Content-Type", "application/json") + w.Header().Set("X-Ratelimit-Limit", strconv.Itoa(licenseTestRateLimit)) + w.Header().Set("X-Ratelimit-Remaining", strconv.Itoa(licenseTestRateRemaining)) + w.Header().Set("X-Ratelimit-Reset", "30") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":{"summary":"Not Found","detail":"user not found","code":"0x1"}}`)) + return + } + w.Header().Set("Content-Type", "application/json") + w.Header().Set("X-Ratelimit-Limit", strconv.Itoa(licenseTestRateLimit)) + w.Header().Set("X-Ratelimit-Remaining", strconv.Itoa(licenseTestRateRemaining)) + w.Header().Set("X-Ratelimit-Reset", "30") + if role == licenseTestServerError { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":{"summary":"Internal Server Error","detail":"boom","code":"0x3"}}`)) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{ + "user": map[string]any{"id": userID, "siteRole": role}, + }) + case r.Method == http.MethodPut && strings.HasPrefix(r.URL.Path, usersPathPrefix): + userID := strings.TrimPrefix(r.URL.Path, usersPathPrefix) + var body struct { + User struct { + SiteRole string `json:"siteRole"` + } `json:"user"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + licenseTestUsersMu.Lock() + *puts = append(*puts, recordedPut{userID: userID, siteRole: body.User.SiteRole}) + users[userID] = body.User.SiteRole + licenseTestUsersMu.Unlock() + w.Header().Set("Content-Type", "application/json") + w.Header().Set("X-Ratelimit-Limit", strconv.Itoa(licenseTestRateLimit)) + w.Header().Set("X-Ratelimit-Remaining", strconv.Itoa(licenseTestRateRemaining)) + w.Header().Set("X-Ratelimit-Reset", "30") + _ = json.NewEncoder(w).Encode(map[string]any{ + "user": map[string]any{"id": userID, "siteRole": body.User.SiteRole}, + }) + default: + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":{"summary":"Not Found","detail":"unexpected request","code":"0x2"}}`)) + } + })) + t.Cleanup(srv.Close) + + c, err := client.New(context.Background(), "unused", licenseTestSiteID, "token-name", "token-secret", "3.19", srv.URL) + require.NoError(t, err) + return c +} + +func testUserPrincipal(id string) *v2.Resource { + return &v2.Resource{Id: &v2.ResourceId{ResourceType: resourceTypeUser.Id, Resource: id}} +} + +func testLicenseEntitlement(tier string) *v2.Entitlement { + return &v2.Entitlement{ + Resource: &v2.Resource{ + Id: &v2.ResourceId{ResourceType: resourceTypeLicense.Id, Resource: strings.ToLower(tier)}, + }, + } +} + +// syncGrantID returns the ID Grants() emits for the user holding the tier, built +// from the same license resource and user resource the sync uses. +func syncGrantID(t *testing.T, tier, userID string) string { + t.Helper() + + lic, err := licenseResource(tier, nil, 0) + require.NoError(t, err) + u, err := userResource(&client.User{ID: userID}, lic.Id) + require.NoError(t, err) + return grant.NewGrant(lic, memberEntitlement, u.Id).GetId() +} + +// requireWriteGrant asserts that Grant returned exactly the grant sync would emit. +func requireWriteGrant(t *testing.T, grants []*v2.Grant, tier, userID string) { + t.Helper() + + require.Len(t, grants, 1) + require.Equal(t, strings.ToLower(tier), grants[0].GetEntitlement().GetResource().GetId().GetResource()) + require.Equal(t, "license:"+strings.ToLower(tier)+":"+memberEntitlement, grants[0].GetEntitlement().GetId()) + require.Equal(t, userID, grants[0].GetPrincipal().GetId().GetResource()) + require.Equal(t, syncGrantID(t, tier, userID), grants[0].GetId()) +} + +// TestLicenseGrant_AlreadyInTier asserts that granting a tier to a user whose +// current site role already satisfies that tier performs no write and reports +// GrantAlreadyExists. +func TestLicenseGrant_AlreadyInTier(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + tier string + curRole string + }{ + {"creator tier: Creator", creator, creator}, + {"creator tier: SiteAdministratorCreator", creator, siteAdministratorCreator}, + {"creator tier: ServerAdministrator", creator, serverAdministrator}, + {"explorer tier: Explorer", explorer, explorer}, + {"explorer tier: SiteAdministratorExplorer", explorer, siteAdministratorExplorer}, + {"explorer tier: ExplorerCanPublish", explorer, explorerCanPublish}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + c := newLicenseTestClient(t, map[string]string{"user-1": tt.curRole}, &puts) + l := newLicenseBuilder(c) + + grants, annos, err := l.Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(tt.tier)) + require.NoError(t, err) + require.Empty(t, grants, "no-op grant returns no grants") + require.Empty(t, puts, "already-in-tier grant must not write") + require.True(t, annos.Contains(&v2.GrantAlreadyExists{})) + require.False(t, annos.Contains(&v2.GrantReplaced{}), "same-tier no-op replaces nothing") + }) + } +} + +// TestLicenseGrant_CrossTierNonAdmin asserts that a non-admin role outside the +// target tier is replaced, with exactly one PUT and a GrantReplaced annotation +// naming the user's previous license grant. +func TestLicenseGrant_CrossTierNonAdmin(t *testing.T) { + t.Parallel() + + var puts []recordedPut + c := newLicenseTestClient(t, map[string]string{"user-1": viewer}, &puts) + l := newLicenseBuilder(c) + + grants, annos, err := l.Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(creator)) + require.NoError(t, err) + requireWriteGrant(t, grants, creator, "user-1") + + require.Len(t, puts, 1, "cross-tier grant must write exactly once") + require.Equal(t, "user-1", puts[0].userID) + require.Equal(t, creator, puts[0].siteRole) + + require.False(t, annos.Contains(&v2.GrantAlreadyExists{})) + + var replaced v2.GrantReplaced + found, err := annos.Pick(&replaced) + require.NoError(t, err) + require.True(t, found, "expected a GrantReplaced annotation") + require.Equal(t, "license:viewer:member:user:user-1", replaced.GetReplacedGrantId()) +} + +// TestLicenseGrant_CrossTierAdmin asserts that granting a different tier to a +// user currently holding an admin role is refused with FailedPrecondition +// and does not write. +func TestLicenseGrant_CrossTierAdmin(t *testing.T) { + t.Parallel() + + adminRolesUnderTest := []string{siteAdministrator, siteAdministratorCreator, siteAdministratorExplorer, serverAdministrator} + + for _, role := range adminRolesUnderTest { + t.Run(role, func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + c := newLicenseTestClient(t, map[string]string{"user-1": role}, &puts) + l := newLicenseBuilder(c) + + // Pick a target tier the role does not already satisfy. + target := explorer + if role == siteAdministratorExplorer || role == siteAdministrator { + target = creator + } + + grants, annos, err := l.Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(target)) + require.Error(t, err) + require.Empty(t, grants) + require.Equal(t, codes.FailedPrecondition, status.Code(err)) + require.False(t, annos.Contains(&v2.GrantReplaced{})) + require.Empty(t, puts, "refused admin cross-tier grant must not write") + }) + } +} + +// TestLicenseGrant_UnknownUser asserts that a 404 on the single-user GET means +// there is nothing to grant to. +func TestLicenseGrant_UnknownUser(t *testing.T) { + t.Parallel() + + var puts []recordedPut + c := newLicenseTestClient(t, map[string]string{}, &puts) + l := newLicenseBuilder(c) + + grants, _, err := l.Grant(context.Background(), testUserPrincipal("ghost"), testLicenseEntitlement(creator)) + require.Error(t, err) + require.Empty(t, grants) + require.Equal(t, codes.NotFound, status.Code(err)) + require.Empty(t, puts) +} + +// TestLicenseRevoke_Stale asserts that revoking a tier the user no longer +// holds (e.g. a later Grant already moved them elsewhere) does not write and +// reports GrantAlreadyRevoked. +func TestLicenseRevoke_Stale(t *testing.T) { + t.Parallel() + + var puts []recordedPut + c := newLicenseTestClient(t, map[string]string{"user-1": creator}, &puts) + l := newLicenseBuilder(c) + + gr := &v2.Grant{ + Entitlement: testLicenseEntitlement(viewer), + Principal: testUserPrincipal("user-1"), + } + + annos, err := l.Revoke(context.Background(), gr) + require.NoError(t, err) + require.Empty(t, puts, "stale revoke must not write") + require.True(t, annos.Contains(&v2.GrantAlreadyRevoked{})) +} + +// TestLicenseRevoke_HoldsTier asserts that a user who still holds the tier is +// unlicensed via exactly one PUT. +func TestLicenseRevoke_HoldsTier(t *testing.T) { + t.Parallel() + + var puts []recordedPut + c := newLicenseTestClient(t, map[string]string{"user-1": creator}, &puts) + l := newLicenseBuilder(c) + + gr := &v2.Grant{ + Entitlement: testLicenseEntitlement(creator), + Principal: testUserPrincipal("user-1"), + } + + _, err := l.Revoke(context.Background(), gr) + require.NoError(t, err) + require.Len(t, puts, 1) + require.Equal(t, "user-1", puts[0].userID) + require.Equal(t, unlicensed, puts[0].siteRole) +} + +// TestLicenseRevoke_UserNotFound asserts that a deleted user (404) has nothing +// left to revoke. +func TestLicenseRevoke_UserNotFound(t *testing.T) { + t.Parallel() + + var puts []recordedPut + c := newLicenseTestClient(t, map[string]string{}, &puts) + l := newLicenseBuilder(c) + + gr := &v2.Grant{ + Entitlement: testLicenseEntitlement(creator), + Principal: testUserPrincipal("ghost"), + } + + annos, err := l.Revoke(context.Background(), gr) + require.NoError(t, err) + require.Empty(t, puts) + require.True(t, annos.Contains(&v2.GrantAlreadyRevoked{})) +} + +// TestLicense_UnknownLicense asserts that an unknown license resource ID is +// rejected with InvalidArgument before any write. +func TestLicense_UnknownLicense(t *testing.T) { + t.Parallel() + + ent := testLicenseEntitlement("platinum") + + t.Run("grant", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + l := newLicenseBuilder(newLicenseTestClient(t, map[string]string{"user-1": creator}, &puts)) + + grants, _, err := l.Grant(context.Background(), testUserPrincipal("user-1"), ent) + require.Error(t, err) + require.Empty(t, grants) + require.Equal(t, codes.InvalidArgument, status.Code(err)) + require.Empty(t, puts) + }) + + t.Run("revoke", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + l := newLicenseBuilder(newLicenseTestClient(t, map[string]string{"user-1": creator}, &puts)) + + _, err := l.Revoke(context.Background(), &v2.Grant{ + Entitlement: ent, + Principal: testUserPrincipal("user-1"), + }) + require.Error(t, err) + require.Equal(t, codes.InvalidArgument, status.Code(err)) + require.Empty(t, puts) + }) +} + +// requireSingleRateLimit asserts annos carries exactly one RateLimitDescription +// and that it is the one the mock GET responses stamped. +func requireSingleRateLimit(t *testing.T, annos annotations.Annotations) { + t.Helper() + + count := 0 + for _, a := range annos { + if a.MessageIs(&v2.RateLimitDescription{}) { + count++ + } + } + require.Equal(t, 1, count, "expected exactly one RateLimitDescription") + + var rl v2.RateLimitDescription + found, err := annos.Pick(&rl) + require.NoError(t, err) + require.True(t, found) + require.EqualValues(t, licenseTestRateLimit, rl.GetLimit()) + require.EqualValues(t, licenseTestRateRemaining, rl.GetRemaining()) +} + +// TestLicense_GetUserServerError asserts that a non-404 failure reading the +// user surfaces as an error (not NotFound) and never falls through to a write. +func TestLicense_GetUserServerError(t *testing.T) { + t.Parallel() + + t.Run("grant", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + c := newLicenseTestClient(t, map[string]string{"user-1": licenseTestServerError}, &puts) + l := newLicenseBuilder(c) + + _, annos, err := l.Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(creator)) + require.Error(t, err) + require.NotEqual(t, codes.NotFound, status.Code(err)) + require.False(t, annos.Contains(&v2.GrantReplaced{})) + require.Empty(t, puts, "failed read must not write") + }) + + t.Run("revoke", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + c := newLicenseTestClient(t, map[string]string{"user-1": licenseTestServerError}, &puts) + l := newLicenseBuilder(c) + + annos, err := l.Revoke(context.Background(), &v2.Grant{ + Entitlement: testLicenseEntitlement(creator), + Principal: testUserPrincipal("user-1"), + }) + require.Error(t, err) + require.NotEqual(t, codes.NotFound, status.Code(err)) + require.False(t, annos.Contains(&v2.GrantAlreadyRevoked{}), "a server error must not be reported as already revoked") + require.Empty(t, puts, "failed read must not write") + }) +} + +// TestLicenseGrant_RoleOutsideEveryTier asserts that a current role belonging +// to no tier (e.g. Guest) is overwritten with one PUT and, since there is no +// previous license grant to name, no GrantReplaced. +func TestLicenseGrant_RoleOutsideEveryTier(t *testing.T) { + t.Parallel() + + var puts []recordedPut + c := newLicenseTestClient(t, map[string]string{"user-1": "Guest"}, &puts) + l := newLicenseBuilder(c) + + _, annos, err := l.Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(creator)) + require.NoError(t, err) + require.Equal(t, []recordedPut{{userID: "user-1", siteRole: creator}}, puts) + require.False(t, annos.Contains(&v2.GrantReplaced{})) + require.False(t, annos.Contains(&v2.GrantAlreadyExists{})) +} + +// TestLicenseGrant_ExplorerCanPublishToCreator asserts that ExplorerCanPublish +// counts as the Explorer tier: granting Creator writes Creator once and +// reports the Explorer license grant as replaced. +func TestLicenseGrant_ExplorerCanPublishToCreator(t *testing.T) { + t.Parallel() + + var puts []recordedPut + c := newLicenseTestClient(t, map[string]string{"user-1": explorerCanPublish}, &puts) + l := newLicenseBuilder(c) + + _, annos, err := l.Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(creator)) + require.NoError(t, err) + require.Equal(t, []recordedPut{{userID: "user-1", siteRole: creator}}, puts) + + var replaced v2.GrantReplaced + found, err := annos.Pick(&replaced) + require.NoError(t, err) + require.True(t, found, "expected a GrantReplaced annotation") + require.Equal(t, "license:explorer:member:user:user-1", replaced.GetReplacedGrantId()) +} + +// TestLicense_PropagatesGetUserAnnotations asserts that the rate-limit +// annotations from the initial GetUser read are present on every result, +// merged without duplicating the PUT's own RateLimitDescription. +func TestLicense_PropagatesGetUserAnnotations(t *testing.T) { + t.Parallel() + + revokeOf := func(tier string) *v2.Grant { + return &v2.Grant{Entitlement: testLicenseEntitlement(tier), Principal: testUserPrincipal("user-1")} + } + + t.Run("grant already exists", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + l := newLicenseBuilder(newLicenseTestClient(t, map[string]string{"user-1": creator}, &puts)) + _, annos, err := l.Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(creator)) + require.NoError(t, err) + require.True(t, annos.Contains(&v2.GrantAlreadyExists{})) + requireSingleRateLimit(t, annos) + }) + + t.Run("grant refused for admin role", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + l := newLicenseBuilder(newLicenseTestClient(t, map[string]string{"user-1": siteAdministratorCreator}, &puts)) + _, annos, err := l.Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(explorer)) + require.Equal(t, codes.FailedPrecondition, status.Code(err)) + requireSingleRateLimit(t, annos) + }) + + t.Run("grant user not found", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + l := newLicenseBuilder(newLicenseTestClient(t, map[string]string{}, &puts)) + _, annos, err := l.Grant(context.Background(), testUserPrincipal("ghost"), testLicenseEntitlement(creator)) + require.Equal(t, codes.NotFound, status.Code(err)) + requireSingleRateLimit(t, annos) + }) + + t.Run("grant write path", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + l := newLicenseBuilder(newLicenseTestClient(t, map[string]string{"user-1": viewer}, &puts)) + _, annos, err := l.Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(creator)) + require.NoError(t, err) + require.True(t, annos.Contains(&v2.GrantReplaced{})) + requireSingleRateLimit(t, annos) + }) + + t.Run("revoke already revoked", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + l := newLicenseBuilder(newLicenseTestClient(t, map[string]string{"user-1": creator}, &puts)) + annos, err := l.Revoke(context.Background(), revokeOf(viewer)) + require.NoError(t, err) + require.True(t, annos.Contains(&v2.GrantAlreadyRevoked{})) + requireSingleRateLimit(t, annos) + }) + + t.Run("revoke user not found", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + l := newLicenseBuilder(newLicenseTestClient(t, map[string]string{}, &puts)) + annos, err := l.Revoke(context.Background(), &v2.Grant{Entitlement: testLicenseEntitlement(creator), Principal: testUserPrincipal("ghost")}) + require.NoError(t, err) + require.True(t, annos.Contains(&v2.GrantAlreadyRevoked{})) + requireSingleRateLimit(t, annos) + }) + + t.Run("revoke write path", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + l := newLicenseBuilder(newLicenseTestClient(t, map[string]string{"user-1": creator}, &puts)) + annos, err := l.Revoke(context.Background(), revokeOf(creator)) + require.NoError(t, err) + require.Len(t, puts, 1) + requireSingleRateLimit(t, annos) + }) +} + +// TestLicenseRevoke_UnlicensedTier asserts that Unlicensed is the floor tier: +// revoking it from an Unlicensed user is an annotated no-op (GrantAlreadyRevoked, +// no write, nil error) because there is nothing left to remove, and revoking it +// from a user who holds some other tier is a stale revoke. +func TestLicenseRevoke_UnlicensedTier(t *testing.T) { + t.Parallel() + + t.Run("user is unlicensed", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + l := newLicenseBuilder(newLicenseTestClient(t, map[string]string{"user-1": unlicensed}, &puts)) + + annos, err := l.Revoke(context.Background(), &v2.Grant{ + Entitlement: testLicenseEntitlement(unlicensed), + Principal: testUserPrincipal("user-1"), + }) + require.NoError(t, err) + require.True(t, annos.Contains(&v2.GrantAlreadyRevoked{})) + require.Empty(t, puts, "floor-tier revoke must not write") + requireSingleRateLimit(t, annos) + }) + + t.Run("user holds another tier", func(t *testing.T) { + t.Parallel() + + var puts []recordedPut + l := newLicenseBuilder(newLicenseTestClient(t, map[string]string{"user-1": creator}, &puts)) + + annos, err := l.Revoke(context.Background(), &v2.Grant{ + Entitlement: testLicenseEntitlement(unlicensed), + Principal: testUserPrincipal("user-1"), + }) + require.NoError(t, err) + require.Empty(t, puts) + require.True(t, annos.Contains(&v2.GrantAlreadyRevoked{})) + }) +} + +// TestLicense_GrantThenRevokeSameClient asserts that a revoke following a +// grant on one client reads the role the grant wrote rather than a cached +// pre-grant response. +func TestLicense_GrantThenRevokeSameClient(t *testing.T) { + t.Parallel() + + var puts []recordedPut + c := newLicenseTestClient(t, map[string]string{"user-1": viewer}, &puts) + l := newLicenseBuilder(c) + + _, _, err := l.Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(creator)) + require.NoError(t, err) + require.Len(t, puts, 1) + require.Equal(t, creator, puts[0].siteRole) + + annos, err := l.Revoke(context.Background(), &v2.Grant{ + Entitlement: testLicenseEntitlement(creator), + Principal: testUserPrincipal("user-1"), + }) + require.NoError(t, err) + require.False(t, annos.Contains(&v2.GrantAlreadyRevoked{}), "revoke must see the granted role") + require.Len(t, puts, 2) + require.Equal(t, unlicensed, puts[1].siteRole) +} + +// TestLicenseGrant_RoleChangedBetweenGrants asserts that a second grant on the +// same client sees a role changed outside the connector after the first read. +func TestLicenseGrant_RoleChangedBetweenGrants(t *testing.T) { + t.Parallel() + + var puts []recordedPut + users := map[string]string{"user-1": siteAdministratorCreator} + c := newLicenseTestClient(t, users, &puts) + l := newLicenseBuilder(c) + + grants, annos, err := l.Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(creator)) + require.NoError(t, err) + require.Empty(t, grants) + require.True(t, annos.Contains(&v2.GrantAlreadyExists{})) + require.Empty(t, puts) + + licenseTestUsersMu.Lock() + users["user-1"] = viewer + licenseTestUsersMu.Unlock() + + grants, annos, err = l.Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(creator)) + require.NoError(t, err) + requireWriteGrant(t, grants, creator, "user-1") + require.False(t, annos.Contains(&v2.GrantAlreadyExists{}), "grant must see the externally changed role") + require.Len(t, puts, 1) + require.Equal(t, creator, puts[0].siteRole) +} + +// TestLicenseGrant_FailedWriteNoReplacement asserts that a cross-tier grant +// whose write fails reports no GrantReplaced, since nothing was replaced. +func TestLicenseGrant_FailedWriteNoReplacement(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodPost && r.URL.Path == "/auth/signin": + _ = json.NewEncoder(w).Encode(map[string]any{ + "credentials": map[string]any{"site": map[string]any{"id": licenseTestSiteID}, "user": map[string]any{"id": "me"}, "token": "t"}, + }) + case r.Method == http.MethodGet: + _ = json.NewEncoder(w).Encode(map[string]any{"user": map[string]any{"id": "user-1", "siteRole": viewer}}) + default: + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":{"summary":"Bad Request","detail":"rejected","code":"0x4"}}`)) + } + })) + t.Cleanup(srv.Close) + + c, err := client.New(context.Background(), "unused", licenseTestSiteID, "token-name", "token-secret", "3.19", srv.URL) + require.NoError(t, err) + + grants, annos, err := newLicenseBuilder(c).Grant(context.Background(), testUserPrincipal("user-1"), testLicenseEntitlement(creator)) + require.ErrorContains(t, err, "failed to grant Creator license") + require.Empty(t, grants) + require.False(t, annos.Contains(&v2.GrantReplaced{})) +} + +// licenseOnlyConnector syncs just the license resource type, so the test runs a +// real SDK sync without mocking the rest of the Tableau API. +type licenseOnlyConnector struct { + *Connector +} + +// Validate skips the site read the mock does not serve. +func (c *licenseOnlyConnector) Validate(_ context.Context) (annotations.Annotations, error) { + return nil, nil +} + +func (c *licenseOnlyConnector) ResourceSyncers(_ context.Context) []connectorbuilder.ResourceSyncerV2 { + return []connectorbuilder.ResourceSyncerV2{newLicenseBuilder(c.client)} +} + +// inProcessClient lets the SDK syncer call a connector server directly. Only +// the RPCs a license-only sync issues are forwarded; any other call panics on +// the nil embedded client, which flags a new syncer dependency. +type inProcessClient struct { + types.ConnectorClient + server types.ConnectorServer +} + +func (c *inProcessClient) GetMetadata(ctx context.Context, in *v2.ConnectorServiceGetMetadataRequest, _ ...grpc.CallOption) (*v2.ConnectorServiceGetMetadataResponse, error) { + return c.server.GetMetadata(ctx, in) +} + +func (c *inProcessClient) Validate(ctx context.Context, in *v2.ConnectorServiceValidateRequest, _ ...grpc.CallOption) (*v2.ConnectorServiceValidateResponse, error) { + return c.server.Validate(ctx, in) +} + +func (c *inProcessClient) Cleanup(ctx context.Context, in *v2.ConnectorServiceCleanupRequest, _ ...grpc.CallOption) (*v2.ConnectorServiceCleanupResponse, error) { + return c.server.Cleanup(ctx, in) +} + +func (c *inProcessClient) ListResourceTypes(ctx context.Context, in *v2.ResourceTypesServiceListResourceTypesRequest, _ ...grpc.CallOption) (*v2.ResourceTypesServiceListResourceTypesResponse, error) { + return c.server.ListResourceTypes(ctx, in) +} + +func (c *inProcessClient) ListResources(ctx context.Context, in *v2.ResourcesServiceListResourcesRequest, _ ...grpc.CallOption) (*v2.ResourcesServiceListResourcesResponse, error) { + return c.server.ListResources(ctx, in) +} + +func (c *inProcessClient) ListStaticEntitlements( + ctx context.Context, in *v2.EntitlementsServiceListStaticEntitlementsRequest, _ ...grpc.CallOption, +) (*v2.EntitlementsServiceListStaticEntitlementsResponse, error) { + return c.server.ListStaticEntitlements(ctx, in) +} + +func (c *inProcessClient) ListEntitlements(ctx context.Context, in *v2.EntitlementsServiceListEntitlementsRequest, _ ...grpc.CallOption) (*v2.EntitlementsServiceListEntitlementsResponse, error) { + return c.server.ListEntitlements(ctx, in) +} + +func (c *inProcessClient) ListGrants(ctx context.Context, in *v2.GrantsServiceListGrantsRequest, _ ...grpc.CallOption) (*v2.GrantsServiceListGrantsResponse, error) { + return c.server.ListGrants(ctx, in) +} + +// TestLicenseSync_TierEntitlementsShareExclusionGroup runs a real SDK sync and +// reads the stored entitlements back, so it covers how the SDK stamps the +// static member entitlement onto each tier: one shared, unscoped group with no +// default or order, and entitlement IDs unchanged. +func TestLicenseSync_TierEntitlementsShareExclusionGroup(t *testing.T) { + ctx := context.Background() + + var puts []recordedPut + // The mock only serves sign-in and single-user reads: the site read fails, so + // tiers sync without seats, and the user list 404s, so grants fail soft. + tc := newLicenseTestClient(t, map[string]string{}, &puts) + + server, err := connectorbuilder.NewConnector(ctx, &licenseOnlyConnector{Connector: &Connector{client: tc}}) + require.NoError(t, err) + + c1zPath := filepath.Join(t.TempDir(), "sync.c1z") + syncer, err := sdksync.NewSyncer(ctx, &inProcessClient{server: server}, sdksync.WithC1ZPath(c1zPath), sdksync.WithTmpDir(t.TempDir())) + require.NoError(t, err) + require.NoError(t, syncer.Sync(ctx)) + require.NoError(t, syncer.Close(ctx)) + + store, err := dotc1z.NewStore(ctx, c1zPath, dotc1z.WithReadOnly(true)) + require.NoError(t, err) + t.Cleanup(func() { _ = store.Close(ctx) }) + + resp, err := store.ListEntitlements(ctx, &v2.EntitlementsServiceListEntitlementsRequest{}) + require.NoError(t, err) + + got := map[string]*v2.EntitlementExclusionGroup{} + for _, e := range resp.GetList() { + group := &v2.EntitlementExclusionGroup{} + annos := annotations.Annotations(e.GetAnnotations()) + ok, err := annos.Pick(group) + require.NoError(t, err) + require.True(t, ok, "entitlement %s has no exclusion group", e.GetId()) + got[e.GetId()] = group + } + + require.Len(t, got, len(licenses)) + for _, id := range []string{"license:creator:member", "license:explorer:member", "license:viewer:member", "license:unlicensed:member"} { + group, ok := got[id] + require.True(t, ok, "missing entitlement ID %s", id) + require.Equal(t, "tableau-license", group.GetExclusionGroupId()) + require.False(t, group.GetScopeToResource(), "group must be shared across tiers, not scoped per resource") + require.False(t, group.GetIsDefault(), "no fallback tier is authorized") + require.Zero(t, group.GetOrder(), "no privilege order is authorized") + } +} diff --git a/pkg/connector/site.go b/pkg/connector/site.go index 4e3efc35..d99137d8 100644 --- a/pkg/connector/site.go +++ b/pkg/connector/site.go @@ -30,8 +30,8 @@ const ( // roles maps Tableau API role names to display slugs. // ServerAdministrator is included for sync (Tableau returns it for server-level -// admins) but cannot be granted via the REST API — it is a server-level role, -// not a site-level role. +// admins) but this connector does not assign it — it exists only on Tableau +// Server. var roles = map[string]string{ siteAdministrator: "site administrator", siteAdministratorCreator: "site administrator creator", @@ -44,8 +44,8 @@ var roles = map[string]string{ unlicensed: "unlicensed", } -// nonGrantableRoles are roles that can appear during sync but cannot be -// assigned via the REST API's Update User Site Role endpoint. +// nonGrantableRoles are roles that can appear during sync but that this +// connector does not assign. var nonGrantableRoles = map[string]bool{ serverAdministrator: true, } @@ -155,7 +155,7 @@ func (s *siteBuilder) Grant(ctx context.Context, principal *v2.Resource, entitle } if nonGrantableRoles[apiRoleName] { - return nil, fmt.Errorf("role %q cannot be assigned via the Tableau REST API (server-level role, not site-level)", apiRoleName) + return nil, fmt.Errorf("role %q is not assigned by this connector (it exists only on Tableau Server)", apiRoleName) } annos, err := s.client.UpdateUserSiteRole(ctx, principalID, apiRoleName)