[CXH-1571] - Add License management - Zoom Connector#28
[CXH-1571] - Add License management - Zoom Connector#28mateoHernandez123 wants to merge 3 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Connector PR Review: [CXH-1571] - Add License management - Zoom ConnectorBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThis PR adds a Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
|
Thanks for the review. Triage of the 2 suggestions + the N2 finding in the summary:
|
mateoHernandez123
dismissed
github-actions[bot]’s stale review
Bot re-review reasserted the same 2 suggestions already triaged: both inline threads are resolved with rationale (url.JoinPath consistency belongs to a separate refactor; Revoke-Basic short-circuit would contradict the connector-wide "don't pre-judge user state" principle). The third finding (Metadata().Description) was applied in 63055c2. No new findings.
| // counts from GET /v2/accounts/me/plans/usage. Requires the | ||
| // `billing:read:plan_usage:admin` scope (or the legacy `billing:read`). | ||
| func (c *Client) GetAccountPlanUsage(ctx context.Context) (*PlanUsage, *http.Response, error) { | ||
| requestURL := fmt.Sprint(c.baseURL, "/accounts/me/plans/usage") |
There was a problem hiding this comment.
Not applying in this PR — keeping consistency with the existing client. All 12 read endpoints in pkg/zoom/client.go use fmt.Sprint(c.baseURL, "/path") (GetUsers, GetGroups, GetRoles, GetUser, etc.); url.JoinPath is only used by the 3 mutating endpoints (InviteUser, DeleteUser, PatchUserLicense) where it pre-existed. Migrating GetAccountPlanUsage alone would be inconsistent — it's a GET, so it follows the GET convention. If we want to standardize on url.JoinPath across the whole client, happy to do it as a separate refactor PR (touches all 12 GET callsites).
mateoHernandez123
force-pushed
the
mateohernandez/baton-zoom-add-license-management
branch
from
b35f862 to
8f2e3c6
Compare
| if user.Type != grantedType { | ||
| return annotations.New(&v2.GrantAlreadyRevoked{}), nil | ||
| } | ||
|
|
There was a problem hiding this comment.
What if grantedType == 1 (Basic) and user.Type == 1 ? the guard does not fire in that case IMO
mateoHernandez123
force-pushed
the
mateohernandez/baton-zoom-add-license-management
branch
from
2691297 to
a604a90
Compare
| "baton-zoom: failed to fetch plan usage; emitting licenses without seat counts", | ||
| zap.Error(err), | ||
| ) | ||
| } else if usage != nil { |
There was a problem hiding this comment.
no usage specified by license? how it works?
There was a problem hiding this comment.
Good catch — explained in detail in the header comment of pkg/connector/license.go:1-72. Short version: LicenseProfileTrait was wired in ec305db and each tier gets a different shape depending on what Zoom actually exposes.
| Tier | Seats surfaced? | Why |
|---|---|---|
Basic (type=1) |
No | Uncapped — Zoom doesn't track it. |
Licensed (type=2) |
purchased / consumed from plan_base.hosts / plan_base.usage |
Paid base-plan seats. |
On-Prem (type=3) |
No | Self-hosted via Meeting Connector; the billing API has no visibility into it. |
WithLicenseSeats is only added on Licensed when purchased > 0. The plan_base fetch is best-effort — if the billing:read:plan_usage:admin scope is missing the tiers still emit, just without seat numbers. EntitlementIDs are set to license:<id>:assigned so the C1 App Utilization feature (CE-720) can correlate consumed_seats to user grants. Validated against a live Business-Monthly tenant: plan_base.usage matches count(User.type == 2) on every state transition.
| // and emits one grant per user whose User.type matches the license tier | ||
| // being synced. The same status-stack pagination pattern used by userResourceType | ||
| // is used here to avoid an extra GET /v2/users/{id} per user. | ||
| func (l *licenseResourceType) Grants(ctx context.Context, res *v2.Resource, opts resource.SyncOpAttrs) ([]*v2.Grant, *resource.SyncOpResults, error) { |
There was a problem hiding this comment.
could we emit license grants from users?
| logger := ctxzap.Extract(ctx) | ||
|
|
||
| if principal.Id.ResourceType != resourceTypeUser.Id { | ||
| logger.Warn( |
There was a problem hiding this comment.
we don't need this warn log here. let's keep connectors clean from logs that are not giving us extra information
mateoHernandez123
force-pushed
the
mateohernandez/baton-zoom-add-license-management
branch
from
480f70f to
a604a90
Compare
mateoHernandez123
force-pushed
the
mateohernandez/baton-zoom-add-license-management
branch
from
ec305db to
e2da258
Compare
mateoHernandez123
force-pushed
the
mateohernandez/baton-zoom-add-license-management
branch
from
e2da258 to
9e7bb21
Compare
5 participants
Description
Adds a
licenseresource type that models Zoom's three license tiers (Basic, Licensed, On-Prem) as static resources, with user grants emitted principal-side fromuserBuilder.Grantsand seat counts surfaced viaLicenseProfileTraitfor the C1 App Utilization feature (CE-720).Closes CXH-1571.
Implementation
/licensesendpoint in Zoom) — tiers are fixed integer codes onUser.type: Basic (1), Licensed (2), On-Prem (3).userBuilder.Grants(principal-side) usingUser.typestashed in the user resource profile duringList().licenseResourceType.Grantsis a no-op. This keeps sync atO(users)instead ofO(users × tiers).PATCH /v2/users/{userId}with{"type": N}. Revoke downgrades to Basic (Zoom has no "no license" state — Basic is the floor tier).GetUser(no extraPATCH):GrantAlreadyExistsGrantAlreadyRevokedGrantAlreadyRevokedLicenseProfileTraitis wired on every tier (LicenseName+EntitlementIDs=license:<id>:assignedso App Utilization can correlateconsumed_seatsto user grants). Seat counts (WithLicenseSeats) are added only on Licensed whenpurchased > 0— Basic is uncapped (Zoom doesn't track it) and On-Prem is provisioned outside Zoom Cloud (no billing-API visibility).GET /v2/accounts/me/plans/usage(plan_base.hosts/plan_base.usage) — best-effort: if the billing scope is missing or the call fails, the three tiers still emit, just without seat numbers on Licensed.pkg/connector/license.go:1-72.Scopes required
Granular Server-to-Server OAuth scopes:
user:update:user:admin—PATCH /v2/users/{userId}(required for Grant/Revoke).billing:read:plan_usage:admin—GET /v2/accounts/me/plans/usage(optional; enables seat-count enrichment on Licensed).Validation
End-to-end against a live Business-Monthly tenant: sync, Grant, idempotent re-Grant, Revoke, idempotent re-Revoke (non-held tier and floor tier), and license assignment from the C1 platform UI after email-invitation acceptance.
plan_base.usagematchescount(User.type == 2)on every state transition.Useful links