Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@ build: ## Build the zeroid binary
run: build ## Build and run zeroid locally
./$(BINARY) -config zeroid.yaml

test: ## Run all tests (unit + integration)
go test ./... -v -race -count=1 -timeout=120s
test: ## Run unit tests (matches CI unit-test step; integration requires Docker — use make test-integration)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion

Consider increasing timeout beyond 300s for larger test suites

While 300s is a reasonable increase from 120s, Go test suites with race detection can sometimes exceed this as the codebase grows. Consider 600s to future-proof against flaky timeouts in CI, or make it configurable via an environment variable.

Suggested fix:

Suggested change
test: ## Run unit tests (matches CI unit-test step; integration requires Docker — use make test-integration)
test: ## Run unit tests (matches CI unit-test step; integration requires Docker — use make test-integration)
go test $$(go list ./... | grep -v '/tests') -v -race -count=1 -timeout=$${TEST_TIMEOUT:-300s}

go test $$(go list ./... | grep -v '/tests') -v -race -count=1 -timeout=300s

test-integration: ## Run integration tests only (requires Docker)
go test ./tests/integration/ -v -count=1 -timeout=120s
test-integration: ## Run integration tests only (requires Docker, testcontainers)
go test ./tests/integration/ -v -count=1 -timeout=300s

test-all: ## Run unit + integration tests (requires Docker)
go test ./... -v -race -count=1 -timeout=300s

lint: ## Run go vet
go vet ./...
Expand Down
36 changes: 36 additions & 0 deletions domain/identity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,42 @@ func TestBuildWIMSEURI_LengthCap(t *testing.T) {
}
}

// TestBuildWIMSEURI_TypeSegmentVaries pins that identity_type is embedded as a
// path segment and differs for each type — the invariant that UpdateIdentity
// must maintain when retyping: the same (domain, account, project, external_id)
// produces different URIs for different identity types.
func TestBuildWIMSEURI_TypeSegmentVaries(t *testing.T) {
const (
wimseDomain = "highflame.dev"
account = "acct"
project = "proj"
extID = "my-identity"
)
cases := []struct {
typ IdentityType
segment string
}{
{IdentityTypeAgent, "/agent/"},
{IdentityTypeApplication, "/application/"},
{IdentityTypeMCPServer, "/mcp_server/"},
{IdentityTypeService, "/service/"},
}
uris := make(map[string]bool)
for _, tc := range cases {
uri, err := BuildWIMSEURI(wimseDomain, account, project, tc.typ, extID)
if err != nil {
t.Fatalf("BuildWIMSEURI(%q) returned error: %v", tc.typ, err)
}
if !strings.Contains(uri, tc.segment) {
t.Errorf("URI for type %q = %q, want segment %q", tc.typ, uri, tc.segment)
}
if uris[uri] {
t.Errorf("duplicate URI across identity types: %q", uri)
}
uris[uri] = true
}
}

// TestSubTypeValidForIdentityType pins the sub_type ↔ identity_type matrix.
// Regression: code_agent must be valid for identity_type=agent (a delegated
// code agent, e.g. forge's per-sandbox mint) — it was previously accepted only
Expand Down
6 changes: 3 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ require (
golang.org/x/net v0.55.0 // indirect
golang.org/x/sys v0.45.0 // indirect
golang.org/x/text v0.37.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 // indirect
google.golang.org/grpc v1.80.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 // indirect
google.golang.org/grpc v1.82.1 // indirect
google.golang.org/protobuf v1.36.11 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
mellium.im/sasl v0.3.2 // indirect
Expand Down
12 changes: 6 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -231,12 +231,12 @@ golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4=
gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E=
google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9 h1:VPWxll4HlMw1Vs/qXtN7BvhZqsS9cdAittCNvVENElA=
google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9/go.mod h1:7QBABkRtR8z+TEnmXTqIqwJLlzrZKVfAUm7tY3yGv0M=
google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 h1:m8qni9SQFH0tJc1X0vmnpw/0t+AImlSvp30sEupozUg=
google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8=
google.golang.org/grpc v1.80.0 h1:Xr6m2WmWZLETvUNvIUmeD5OAagMw3FiKmMlTdViWsHM=
google.golang.org/grpc v1.80.0/go.mod h1:ho/dLnxwi3EDJA4Zghp7k2Ec1+c2jqup0bFkw07bwF4=
google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478 h1:yQugLulqltosq0B/f8l4w9VryjV+N/5gcW0jQ3N8Qec=
google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478/go.mod h1:C6ADNqOxbgdUUeRTU+LCHDPB9ttAMCTff6auwCVa4uc=
google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 h1:RmoJA1ujG+/lRGNfUnOMfhCy5EipVMyvUE+KNbPbTlw=
google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8=
google.golang.org/grpc v1.82.1 h1:NnAxzGRA0677vCa4BUkOAnO5+FfQqVl9iUXeD0IqcGE=
google.golang.org/grpc v1.82.1/go.mod h1:yzTZ1TB1Z3SG+LIYaI+WiE8D5+PZ3ArnrSp8zF3+/ZA=
google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE=
google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
24 changes: 24 additions & 0 deletions internal/service/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,7 @@ func (s *IdentityService) UpdateIdentity(ctx context.Context, id, accountID, pro
if req.TrustLevel != "" {
identity.TrustLevel = req.TrustLevel
}
priorType := identity.IdentityType
if req.IdentityType != "" {
if !req.IdentityType.Valid() {
return nil, fmt.Errorf("%w: invalid identity_type: %s", ErrInvalidIdentityField, req.IdentityType)
Expand All @@ -691,6 +692,19 @@ func (s *IdentityService) UpdateIdentity(ctx context.Context, id, accountID, pro
!identity.SubType.ValidForIdentityType(identity.IdentityType) {
return nil, fmt.Errorf("%w: invalid sub_type %q for identity_type %q", ErrInvalidIdentityField, identity.SubType, identity.IdentityType)
}
// When identity_type changes the WIMSE URI must be rebuilt in lockstep:
// the URI embeds identity_type as a path segment, so a stale URI breaks
// GetIdentityByWIMSEURI and leaves dangling JWT sub claims.
if identity.IdentityType != priorType {
newURI, err := domain.BuildWIMSEURI(
s.wimseDomain, identity.AccountID, identity.ProjectID,
identity.IdentityType, identity.ExternalID,
)
if err != nil {
return nil, err
}
identity.WIMSEURI = newURI
}
if req.OwnerUserID != "" {
identity.OwnerUserID = req.OwnerUserID
}
Expand Down Expand Up @@ -790,6 +804,16 @@ func (s *IdentityService) UpdateIdentity(ctx context.Context, id, accountID, pro
return nil, err
}

// Post-persist cleanups — all run after repo.Update succeeds so a DB
// failure cannot leave credentials revoked while the identity row is
// unchanged (the authoritative outcome must land before the sweep).

// Retype: cascade-revoke active credentials so old tokens (which carry
// the prior URI as sub) stop resolving — same sweep as deactivation,
// with a distinct audit reason.
if identity.IdentityType != priorType {
s.runDeactivationCleanup(ctx, identity, "identity_retyped")
}
// Fresh transition into deactivated or expired: sweep linked API keys,
// cascade-revoke active credentials, and emit a retirement/expiry signal.
// Centralized here so every update path (PUT /identities/{id},
Expand Down
53 changes: 53 additions & 0 deletions tests/integration/identity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"net/http"
"net/url"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -634,3 +635,55 @@ func TestUpdateIdentityRiskMetadata(t *testing.T) {
assert.Equal(t, "high", reclassified["risk_tier"], "risk_tier should be unchanged by capability_tier update")
assert.Equal(t, "ial3", reclassified["ial"], "ial should be unchanged by capability_tier update")
}

// TestUpdateIdentity_TypeChange_WIMSEURIRebuild verifies that PATCH
// /identities/{id} with a new identity_type rebuilds the WIMSE URI in
// lockstep. The old URI must no longer resolve (404) and the new URI must
// resolve (200) — otherwise GetIdentityByWIMSEURI and JWT sub-claim lookups
// break silently after a retype.
func TestUpdateIdentity_TypeChange_WIMSEURIRebuild(t *testing.T) {
externalID := uid("retype-agent")
identity := registerIdentity(t, externalID, nil)

oldURI := identity.WIMSEURI
require.Contains(t, oldURI, "/agent/", "initial URI must embed identity_type=agent")

// Retype agent → application (supply a compatible sub_type).
resp := doRequest(t, http.MethodPatch, adminPath("/identities/"+identity.ID), map[string]any{
"identity_type": "application",
"sub_type": "api_service",
}, adminHeaders())
require.Equal(t, http.StatusOK, resp.StatusCode)
body := decode(t, resp)

newURI, _ := body["wimse_uri"].(string)
assert.Contains(t, newURI, "/application/", "WIMSE URI must be rebuilt to reflect the new identity_type")
assert.NotContains(t, newURI, "/agent/", "old identity_type segment must not appear in the rebuilt URI")
assert.NotEqual(t, oldURI, newURI, "URI must change when identity_type changes")

// New URI resolves; stale URI does not.
newResp := get(t, adminPath("/identities/by-wimse")+"?uri="+url.QueryEscape(newURI), adminHeaders())
assert.Equal(t, http.StatusOK, newResp.StatusCode, "new wimse_uri must resolve after retype")
_ = newResp.Body.Close()

oldResp := get(t, adminPath("/identities/by-wimse")+"?uri="+url.QueryEscape(oldURI), adminHeaders())
assert.Equal(t, http.StatusNotFound, oldResp.StatusCode, "stale wimse_uri must no longer resolve after retype")
_ = oldResp.Body.Close()
}

// TestUpdateIdentity_TypeUnchanged_URIStable verifies that an update that
// does not touch identity_type leaves the WIMSE URI unchanged — the rebuild
// must be a no-op when the type is the same.
func TestUpdateIdentity_TypeUnchanged_URIStable(t *testing.T) {
externalID := uid("stable-uri-agent")
identity := registerIdentity(t, externalID, nil)
originalURI := identity.WIMSEURI

resp := doRequest(t, http.MethodPatch, adminPath("/identities/"+identity.ID), map[string]any{
"name": "renamed-agent",
}, adminHeaders())
require.Equal(t, http.StatusOK, resp.StatusCode)
body := decode(t, resp)

assert.Equal(t, originalURI, body["wimse_uri"], "URI must not change when identity_type is not updated")
}