feat: support external_id in identity commands#444
Conversation
Bump github.com/ory/kratos to the master version that adds external_id support to the identity CLI commands (which the Ory CLI wraps), and bump github.com/ory/x accordingly (kratos now requires corsx.ClassifyOrigin). This exposes, with no further wiring: - get identity --external-id <externalID> to look up by external ID - an EXTERNAL ID column in identity get/list/import output Closes #413 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The ORY_RATE_LIMIT_HEADER value was only attached to the Ory Network SDK client, not to the project HTTP client used by the wrapped Ory Kratos and Ory Hydra admin CLI commands (identity import/get/list, etc.). As a result those admin API calls were still subject to rate limiting during E2E tests. Attach the header to the project HTTP client too. No-op unless the env var is set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Import an identity with an external_id and look it up via 'ory get identity --external-id', asserting the round trip against the Ory Network. Also covers the not-found path. Runs against staging with ORY_RATE_LIMIT_HEADER set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a ChangesRate-limit header transport
External ID lookup test
Dependency version updates
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Client as ProjectHTTPClient
participant SHT as setHeaderTransport
participant Base as BaseTransport
participant API as ProjectAPI
Client->>SHT: RoundTrip(request)
SHT->>SHT: clone request, set Ory-RateLimit-Action header
SHT->>Base: RoundTrip(clonedRequest)
Base->>API: send request with header
API-->>Client: response
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/cloudx/client/sdks.go (1)
128-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the hardcoded header name into a constant.
The string
"Ory-RateLimit-Action"is duplicated here and at line 61 innewSDKConfiguration. Extracting it into a shared constant (e.g.,rateLimitActionHeader) alongside the existingRateLimitHeaderKeyprevents drift if the header name changes.♻️ Suggested refactor
+const rateLimitActionHeader = "Ory-RateLimit-Action" + // ... in newSDKConfiguration: - if rateLimitHeader != "" { - conf.AddDefaultHeader("Ory-RateLimit-Action", rateLimitHeader) - } + if rateLimitHeader != "" { + conf.AddDefaultHeader(rateLimitActionHeader, rateLimitHeader) + } // ... in newProjectHTTPClient: if rateLimitHeader != "" { - c.Transport = &setHeaderTransport{base: c.Transport, key: "Ory-RateLimit-Action", value: rateLimitHeader} + c.Transport = &setHeaderTransport{base: c.Transport, key: rateLimitActionHeader, value: rateLimitHeader} }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/cloudx/client/sdks.go` around lines 128 - 130, The header name is hardcoded in both the transport setup and newSDKConfiguration, so extract it into a shared constant. Add a single constant for the Ory rate limit action header near the existing RateLimitHeaderKey definition, then update setHeaderTransport usage in the client setup to reference that constant instead of the literal string.cmd/cloudx/identity/get_external_id_test.go (1)
27-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using
encoding/jsonfor JSON construction.The import JSON is built via string concatenation with
testhelpers.FakeEmail()ever returns a value containing"or\, the JSON would be malformed and the test would fail with a confusing error. Usingencoding/json.Marshalwould eliminate this risk entirely.This is low-risk given typical fake email generators produce safe characters, so it's purely a defensive improvement.
♻️ Optional refactor: use encoding/json
import ( "os" "path/filepath" "testing" + "encoding/json" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tidwall/gjson" "github.com/ory/cli/cmd/cloudx/testhelpers" "github.com/ory/x/randx" ) + +type importPayload struct { + SchemaID string `json:"schema_id"` + Traits map[string]string `json:"traits"` + ExternalID string `json:"external_id"` +}Then in the test body:
- require.NoError(t, os.WriteFile(importPath, []byte(`{ - "schema_id": "preset://username", - "traits": { - "username": "`+email+`" - }, - "external_id": "`+externalID+`" -}`), 0o600)) + payload, err := json.Marshal(importPayload{ + SchemaID: "preset://username", + Traits: map[string]string{"username": email}, + ExternalID: externalID, + }) + require.NoError(t, err) + require.NoError(t, os.WriteFile(importPath, payload, 0o600))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/cloudx/identity/get_external_id_test.go` around lines 27 - 33, The test data in getExternalID is building JSON by string concatenation, which is fragile if FakeEmail() ever returns characters that need escaping. Update the import payload construction in getExternalID to use encoding/json marshalling instead of manual string assembly, so the JSON written by os.WriteFile is always valid. Keep the same fields (schema_id, traits.username, external_id) and replace the inline literal construction with a marshaled struct/map near the existing importPath setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/cloudx/client/sdks.go`:
- Around line 128-130: The header name is hardcoded in both the transport setup
and newSDKConfiguration, so extract it into a shared constant. Add a single
constant for the Ory rate limit action header near the existing
RateLimitHeaderKey definition, then update setHeaderTransport usage in the
client setup to reference that constant instead of the literal string.
In `@cmd/cloudx/identity/get_external_id_test.go`:
- Around line 27-33: The test data in getExternalID is building JSON by string
concatenation, which is fragile if FakeEmail() ever returns characters that need
escaping. Update the import payload construction in getExternalID to use
encoding/json marshalling instead of manual string assembly, so the JSON written
by os.WriteFile is always valid. Keep the same fields (schema_id,
traits.username, external_id) and replace the inline literal construction with a
marshaled struct/map near the existing importPath setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a8ed5bc2-cff0-42c7-8582-c7402e09e0ae
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
cmd/cloudx/client/http_client.gocmd/cloudx/client/sdks.gocmd/cloudx/identity/get_external_id_test.gogo.mod
playwright-community/playwright-go v0.4702.0 downloads its driver from the retired playwright.azureedge.net CDN, which now 404s and breaks the 'Run tests' CI job before any test runs. Upgrade to github.com/mxschmitt/playwright-go v0.6100.0 (the module moved back to that path), which fetches the driver via npm and downloads browsers from cdn.playwright.dev instead of azureedge. Update the two imports and the CI install command accordingly. Verified locally: driver + chromium install succeeds and the identity external-id E2E test passes against staging. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
Related Issue or Design Document
Closes #413
The
external_idfield is part of the Ory identity API but was not usable fromthe CLI:
ory identity importdid not surface it and there was no way to lookan identity up by its external ID.
The Ory CLI identity commands wrap Ory Kratos'
CLI commands, so the feature itself landed in Kratos. This PR pulls it in and
exposes it:
ory get identity --external-id <externalID>looks an identity up by itsexternal ID (via
GET /admin/identities/by/external/{externalID}).get/list/importoutput now includes anEXTERNAL IDcolumn.
importcontinues to forward theexternal_idfield.Changes
github.com/ory/kratosto the master version that addsexternal_idsupport to the identity CLI commands, and bump
github.com/ory/xaccordingly(Kratos now requires
corsx.ClassifyOrigin).fix(client): attach theOry-RateLimit-Actionheader(
ORY_RATE_LIMIT_HEADER) to the project HTTP client as well, not just the OryNetwork SDK client. Previously the admin API calls made by the wrapped Kratos
and Hydra CLI commands were not covered by the rate-limit bypass header during
E2E tests. No-op unless the env var is set.
external_idand looks it upvia
ory get identity --external-id, asserting the round trip against the OryNetwork (plus the not-found path).
Checklist
and signed the CLA.
introduces a new feature.
vulnerability.
works.
appropriate).
Further comments
The
external_idcolumn isVARCHAR(64); the E2E test keeps generated externalIDs well under that limit.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests