Repository navigation
feat(resource-servers): support UCP type and release it before reassigning - #1530
Conversation
…gning Co-Authored-By: Claude <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1530 +/- ##
==========================================
+ Coverage 81.83% 82.12% +0.28%
==========================================
Files 190 190
Lines 8923 8977 +54
Branches 1977 1990 +13
==========================================
+ Hits 7302 7372 +70
+ Misses 839 818 -21
- Partials 782 787 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
| }; | ||
|
|
||
| // Only one resource server can hold the UCP type, so the holder giving it up is updated first | ||
| const holder = this.existing?.find((e) => e.type === RESOURCE_SERVER_TYPE_UCP); |
There was a problem hiding this comment.
Since this check only runs in processChanges(), does --dry-run ever show the warning? I tried a holder with no type plus another server setting it. The preview looked clean, then the real import hit a 409. Could we share this with a dryRunChanges override?
There was a problem hiding this comment.
Good catch. Added a dryRunChanges override that runs the same check. The dry-run spinner hides handler logs though, so it only shows with AUTH0_DEBUG=true. Documented that.
| } | ||
| } | ||
|
|
||
| if (releasing.length) { |
There was a problem hiding this comment.
If the release goes through but the new holder's update then fails, do we end up with no UCP server at all? That's what I saw with a mocked failure. Maybe worth saying in the error that the type was already released?
There was a problem hiding this comment.
Done, the error now says the type was already released and to re-run. Checked on a tenant: after the failure no server holds it, and the re-run fixes it.
| if (taker && !(deletes && changes.del.some((r) => r.id === holder.id))) { | ||
| const { name } = holder; | ||
| log.warn( | ||
| `'${name}' already has type '${RESOURCE_SERVER_TYPE_UCP}', so setting it on '${taker.name}' is likely to fail with a 409. Set 'type: null' on '${name}' in the same config to move it.` |
There was a problem hiding this comment.
Small one: if the holder is a skipped delete (deletes off), would pointing to AUTH0_ALLOW_DELETE=true be a better hint than type: null?
There was a problem hiding this comment.
Done, the hint now points to AUTH0_ALLOW_DELETE when the holder is a skipped delete.
|
|
||
| // The API allows only one UCP-typed resource server per tenant; fail early instead of on a 409 | ||
| const excluded = assets.exclude?.resourceServers || []; | ||
| const ucpResourceServers = resourceServers.filter( |
There was a problem hiding this comment.
type on a system API seems to get silently dropped in updateResourceServer, but it still counts here. Should we skip or reject is_system servers in this check? Not sure that can even happen, though.
There was a problem hiding this comment.
Skipped is_system servers in the count, same as excluded ones. They drop type on update, so I left that as is rather than rejecting it.
|
|
||
| // The API allows only one UCP-typed resource server per tenant; fail early instead of on a 409 | ||
| const excluded = assets.exclude?.resourceServers || []; | ||
| const ucpResourceServers = resourceServers.filter( |
There was a problem hiding this comment.
Since we're failing early here anyway, could we also catch type: null combined with dev.ucp* scopes, before the API 400s? Fine to skip if you'd rather leave it to the API.
There was a problem hiding this comment.
Added as a warning rather than an error. The API only enforces it behind a flag, so a hard error could block a config it would accept.
| @@ -36,6 +36,32 @@ describe('#directory context resourceServers', () => { | |||
| expect(context.assets.resourceServers).to.deep.equal(resourceServersTarget); | |||
| }); | |||
|
|
|||
| it('should process the resource server type, including null', async () => { | |||
There was a problem hiding this comment.
I think this file isn't picked up by the npm test glob (no .test in the name), so these new tests don't run in CI. Rename it? Heads up: the existing client_id test at line 164 fails once it actually runs
There was a problem hiding this comment.
Good catch, renamed to resourceServers.test.js so CI picks it up, and fixed the client_id test mock (getAll to list).
| expect(calls).to.deep.equal([`update:rs1:${UCP}`, 'update:rs2:undefined']); | ||
| }); | ||
|
|
||
| it('should clear the holder before another existing server takes the type', async () => { |
There was a problem hiding this comment.
All the desired configs here include id, but real ones won't. Could we add one case with just name/identifier? It works, I checked, but a test would keep it that way
There was a problem hiding this comment.
Added a case with just name and identifier.
|
|
||
| - Resource servers that don't hold the type have no `type` property in API responses, so exports are unchanged for them. | ||
| - `type: null` is accepted whether or not the tenant has the feature flag. Setting `universal_commerce_protocol_server` without the flag fails with `403`. | ||
| - A config with `type: null` on a server that doesn't hold the type shows as an `UPDATE` in every dry run. The update itself changes nothing. |
There was a problem hiding this comment.
Could dry run treat type: null the same as no type when the server doesn't hold it? Then we wouldn't need this note. Not a blocker!
There was a problem hiding this comment.
Done. dryRunChanges now drops type: null for servers that don't hold the type, so the note is gone.
| - value: "read:account" | ||
| description: "read account" | ||
| # client_id: "if linked to a resource server client (readonly)" | ||
| # type: "universal_commerce_protocol_server" # EA, one per tenant, null clears it |
There was a problem hiding this comment.
Worth adding a directory example too, or skipping it on purpose since JSON can't take the comment?
There was a problem hiding this comment.
Skipping on purpose. JSON can't take the comment, and a live UCP file would fail for anyone on a tenant without the flag. The docs have the directory snippet.
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
| if (!taker) return; | ||
|
|
||
| const holderInDel = changes.del.some((r) => r.id === holder.id); | ||
| // Mirrors the delete gating in DefaultHandler.processChanges |
There was a problem hiding this comment.
One catch: dry run doesn't apply exclusions, so an excluded holder still shows up in del. With deletes on, dry run stays quiet but the import 409s; with deletes off, the hint says AUTH0_ALLOW_DELETE, which won't help. Could we check excluded.includes(holder.name) first and give an "it's excluded" hint?
There was a problem hiding this comment.
Good catch. Dry run now filters excluded servers the same way the import does (filterExcluded), so an excluded holder no longer shows as a DELETE and the warning fires with deletes on or off. The hint now says it's excluded through AUTH0_EXCLUDED_RESOURCE_SERVERS instead of pointing at AUTH0_ALLOW_DELETE. Checked on a tenant. Excluded servers also drop out of the dry-run table, matching the import.
…he holder warning Co-Authored-By: Claude <noreply@anthropic.com>
🔧 Changes
Adds support for the resource server
typeproperty, the Early Access flag that declares an API as a Universal Commerce Protocol (UCP) server. The only accepted value isuniversal_commerce_protocol_server, andnullclears it. The Management API gates setting the type behind a tenant feature flag and allows only one UCP-typed resource server per tenant, so the CLI handles both rules explicitly.auth0to^7.7.0, which shipstypeandResourceServerTypeEnum(auth0/node-auth0#1436). The handler schema acceptstypeas an SDK enum value ornull. Both the YAML and directory contexts already pass fields through, so no parser changes were needed.validate()rejects a config with more than one UCP-typed resource server before any API call, instead of failing later with a409. Excluded and system resource servers are ignored, since theirtypeis never sent. It also warns whentype: nullis combined withdev.ucpscopes, which the API reserves for the UCP-typed server.processChanges()updates the current holder first when a config releases the type (type: null). Without this, moving the type from one resource server to another in a single deploy could fail with a409, because creates run before updates and updates run in parallel. If the update that follows the release fails, the error says the type was already released and to re-run. Imports that don't release a held type take the same single-call path as before.type, is excluded by name, or is removed from the config while deletes are off. The warning names the fix. AdryRunChangesoverride runs the same check for--dry-run, where the spinner hides handler logs, so it shows there withAUTH0_DEBUG=true.UPDATEfortype: nullon a server that doesn't hold the type, since the API omitstypefor those servers.resourceServers.test.jsso thetest/**/*.test*glob that CI uses runs it, and fixes theclient_idtest mock there (getAlltolist), which only failed once the file was actually run.Omitting
typeleaves the existing value unchanged (the API treats it as a PATCH), so clearing it requires an explicittype: null. A holder giving up the type must also empty itsdev.ucpscopes (scopes: []), because omittingscopesleaves them in place. This is documented indocs/resource-specific-documentation.md, with a commented example inexamples/yaml/tenant.yaml. No existing config is affected.Shape
YAML, before:
YAML, after:
Directory (JSON), before:
{ "name": "Commerce API", "identifier": "https://commerce.example.com" }Directory (JSON), after:
{ "name": "Commerce API", "identifier": "https://commerce.example.com", "type": "universal_commerce_protocol_server" }🔬 Testing
Unit tests
Added in
test/tools/auth0/handlers/resourceServers.tests.js:type: nulltype: nullcombined withdev.ucpscopes, and stays quiet for the UCP-typed server and for other scopesnull, and an omittedtype, and rejects any other valuenull, leaves it unset when omitted, and leaves a holder that keeps its type alonetype, is excluded, or is removed with deletes off, and not when it is deleted (including by an extension), released, or keeps the typetype: nullon a server that doesn't hold the type, and reports it on the holderAdded in
test/context/yaml/resourceServers.test.jsandtest/context/directory/resourceServers.test.js:type, includingnull(and an emptytype:in YAML, which becomesnull)typeonly when it is setManual verification
Run against a live tenant with the UCP feature flag enabled, scoped to resource servers only, and verified by reading the tenant back through the Management API in both YAML and directory formats:
typeand confirm the preview lists the expected changes and the tenant is unchanged. Confirmtype: nullon a server without the type shows no change.typeand others omit it. Import that export and confirm the state is preserved.type, and confirm the warning appears (withAUTH0_DEBUG=truein dry run) and the API returns409.token_lifetime, and confirm the error says the type was already released and that a re-run recovers.type: nulland confirm it clears. Omittypeon the holder and confirm it stays.typevalue fails schema validation.📝 Checklist