[LFXV2-504] Identity link/unlink functionality - auth0#27
Conversation
- Added support for unlinking identities in the identity linking service. - Updated message handler to include unlink identity operations. - Enhanced documentation to describe unlinking process and subjects. - Introduced new constants for unlink identity scope and subjects. Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-504 Assisted by [Claude Code](https://claude.ai/code) Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
WalkthroughAdds identity unlinking: new UnlinkIdentity model and port methods, message-handler routing and service wiring, Auth0/Authelia/mock provider implementations, new constants and docs, and JWT redaction applied to HTTP request logging. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant MH as Message Handler
participant Svc as Identity Service
participant Prov as Auth Provider<br/>(Auth0/Authelia)
Client->>MH: Publish `UserIdentityUnlinkSubject` message
MH->>Svc: Route -> UnlinkIdentity(ctx, msg)
Svc->>Svc: Unmarshal payload & lookup metadata (user token -> user_id)
alt valid & authorized
Svc->>Prov: DELETE /api/v2/users/{id}/identities/{provider}/{user_id} (using user token)
Prov-->>Svc: 200 OK (remaining identities)
Svc-->>MH: success response JSON
MH-->>Client: reply (identity unlinked successfully)
else invalid/failed
Svc-->>MH: error response JSON
MH-->>Client: reply (error)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip Migrating from UI to YAML configuration.Use the |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/service/message_handler.go (1)
408-412:⚠️ Potential issue | 🔴 CriticalEnforce required scope in
LinkIdentitymetadata lookup.
LinkIdentitydoes not passconstants.UserUpdateIdentityRequiredScopetoMetadataLookup, which can bypass intended authorization checks for identity-link operations.🔐 Proposed fix
- user, errMetadataLookup := m.userReader.MetadataLookup(ctx, linkRequest.User.AuthToken) + user, errMetadataLookup := m.userReader.MetadataLookup( + ctx, + linkRequest.User.AuthToken, + constants.UserUpdateIdentityRequiredScope, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/service/message_handler.go` around lines 408 - 412, LinkIdentity calls m.userReader.MetadataLookup(ctx, linkRequest.User.AuthToken) without enforcing the identity-update scope; update the MetadataLookup invocation in LinkIdentity to pass constants.UserUpdateIdentityRequiredScope (alongside ctx and linkRequest.User.AuthToken) so the lookup enforces the required authorization scope, ensuring m.userReader.MetadataLookup receives the scope parameter and returns metadata only when the token has UserUpdateIdentityRequiredScope.
🧹 Nitpick comments (1)
internal/infrastructure/auth0/jwt_parser_test.go (1)
78-78: Consider adding a sibling test for the identity scope constant.Line 78 now validates metadata scope via constant; adding one case for
constants.UserUpdateIdentityRequiredScopewould protect the new linking/unlinking authorization path from regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/infrastructure/auth0/jwt_parser_test.go` at line 78, Add a sibling unit test in jwt_parser_test.go that mirrors the existing metadata-scope assertion but calls jwtVerify.JWTVerify(ctx, user.Token, constants.UserUpdateIdentityRequiredScope) instead of constants.UserUpdateMetadataRequiredScope, asserting the same expected behavior (claims and error) to ensure the identity scope path is covered; locate the existing test using jwtVerify.JWTVerify and constants.UserUpdateMetadataRequiredScope and duplicate its structure, swapping in constants.UserUpdateIdentityRequiredScope and updating assertions/messages accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/infrastructure/auth0/identity.go`:
- Around line 129-137: The current error handling after apiRequest.Call(ctx,
&remainingIdentities) in identity.go returns a generic errors.NewUnexpected for
every failure; change this to preserve HTTP semantics by inspecting statusCode
and returning appropriate typed errors (e.g., errors.NewUnauthorized for 401,
errors.NewForbidden for 403, errors.NewNotFound for 404,
errors.NewBadRequest/validation error for 400/422) while still including errCall
as the underlying cause and logging via slog.ErrorContext (keep "user_id"
redaction and "error" details). Implement a switch or map on statusCode after
the call (referencing apiRequest.Call, statusCode, errCall, remainingIdentities,
primaryUserID) that returns the corresponding domain error instead of always
errors.NewUnexpected.
- Line 117: The URL is built by interpolating primaryUserID, provider, and
secondaryUserID directly (see the local variable url in identity.go), which can
contain reserved characters like '|' and break routing; update the construction
to percent-escape each path segment before formatting (e.g., call url.PathEscape
or equivalent on primaryUserID, provider, and secondaryUserID) and then build
the final string using ilf.domain and the escaped segments so the Auth0
Management API receives valid path segments.
In `@internal/infrastructure/auth0/user.go`:
- Around line 394-408: The validation currently uses equality checks against
empty string which allows whitespace-only values; update the validation in the
function handling the unlink request to trim and reject whitespace-only inputs
by using strings.TrimSpace on request.User.UserID, request.User.AuthToken,
request.Unlink.Provider, and request.Unlink.IdentityID and return the same
errors.NewValidation messages when TrimSpace(...) == "" so inputs like " " are
treated as missing.
In `@pkg/redaction/redaction_test.go`:
- Around line 135-136: The test contains JWT-shaped string literals (jwt1 and
jwt2 in pkg/redaction/redaction_test.go) that trigger secret scanners; replace
those hard-coded token-looking constants with non-secret placeholders or
generate safe dummy tokens at runtime (e.g., build strings that do not match JWT
regex or use a helper like makeTestToken()) so the tests exercise redaction
logic without committing secret-like values; update references to jwt1 and jwt2
accordingly.
---
Outside diff comments:
In `@internal/service/message_handler.go`:
- Around line 408-412: LinkIdentity calls m.userReader.MetadataLookup(ctx,
linkRequest.User.AuthToken) without enforcing the identity-update scope; update
the MetadataLookup invocation in LinkIdentity to pass
constants.UserUpdateIdentityRequiredScope (alongside ctx and
linkRequest.User.AuthToken) so the lookup enforces the required authorization
scope, ensuring m.userReader.MetadataLookup receives the scope parameter and
returns metadata only when the token has UserUpdateIdentityRequiredScope.
---
Nitpick comments:
In `@internal/infrastructure/auth0/jwt_parser_test.go`:
- Line 78: Add a sibling unit test in jwt_parser_test.go that mirrors the
existing metadata-scope assertion but calls jwtVerify.JWTVerify(ctx, user.Token,
constants.UserUpdateIdentityRequiredScope) instead of
constants.UserUpdateMetadataRequiredScope, asserting the same expected behavior
(claims and error) to ensure the identity scope path is covered; locate the
existing test using jwtVerify.JWTVerify and
constants.UserUpdateMetadataRequiredScope and duplicate its structure, swapping
in constants.UserUpdateIdentityRequiredScope and updating assertions/messages
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6ebf0568-0044-46c2-aa8d-8d2a8d906d4c
📒 Files selected for processing (19)
cmd/server/service/message_handler.gocmd/server/service/providers.godocs/identity_linking.mdinternal/domain/model/identity.gointernal/domain/port/message_handler.gointernal/domain/port/user.gointernal/infrastructure/auth0/identity.gointernal/infrastructure/auth0/jwt_parser_test.gointernal/infrastructure/auth0/user.gointernal/infrastructure/auth0/user_test.gointernal/infrastructure/authelia/user.gointernal/infrastructure/mock/user.gointernal/service/message_handler.gointernal/service/message_handler_test.gopkg/constants/subjects.gopkg/constants/user.gopkg/httpclient/request.gopkg/redaction/redaction.gopkg/redaction/redaction_test.go
Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-504 Assisted by [Claude Code](https://claude.ai/code) Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
There was a problem hiding this comment.
Pull request overview
Implements backend support for social identity linking/unlinking over NATS using Auth0 Management API endpoints, and hardens logging by redacting JWTs in request bodies.
Changes:
- Add NATS request/reply support for unlinking identities (
lfx.auth-service.user_identity.unlink) alongside existing linking support. - Redact JWTs found in HTTP request bodies before logging.
- Centralize Auth0 required-scope strings under
pkg/constantsand update docs accordingly.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/redaction/redaction.go | Adds JWT-pattern redaction helper used for log sanitization. |
| pkg/redaction/redaction_test.go | Adds unit tests validating JWT redaction behavior. |
| pkg/httpclient/request.go | Redacts JWTs from logged request bodies. |
| pkg/constants/user.go | Adds shared constants for Auth0 scope requirements. |
| pkg/constants/subjects.go | Adds NATS subject constant for identity unlinking. |
| internal/service/message_handler.go | Adds UnlinkIdentity handler and wires scope usage for unlinking. |
| internal/service/message_handler_test.go | Updates expected “service unavailable” error strings. |
| internal/infrastructure/auth0/identity.go | Adds Auth0 Management API call to unlink identities. |
| internal/infrastructure/auth0/user.go | Adds UnlinkIdentity implementation on Auth0 user reader/writer. |
| internal/infrastructure/auth0/user_test.go | Updates scope constant usage in JWT validation test. |
| internal/infrastructure/auth0/jwt_parser_test.go | Updates scope constant usage in JWT verification test. |
| internal/infrastructure/mock/user.go | Adds mock UnlinkIdentity stub returning service unavailable. |
| internal/infrastructure/authelia/user.go | Adds authelia UnlinkIdentity stub returning service unavailable. |
| internal/domain/model/identity.go | Introduces UnlinkIdentity request model. |
| internal/domain/port/user.go | Extends IdentityLinker port with UnlinkIdentity. |
| internal/domain/port/message_handler.go | Extends identity linking handler port with UnlinkIdentity. |
| cmd/server/service/providers.go | Wires unlinker dependency and subject subscription. |
| cmd/server/service/message_handler.go | Routes unlink subject to the new handler. |
| docs/identity_linking.md | Documents link/unlink subjects, payloads, and usage notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…quests; add comprehensive tests for unlinking functionality Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-504 Assisted by [Claude Code](https://claude.ai/code) Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/service/message_handler_test.go (1)
1160-1313: Add an assertion that unlink requests the required identity-update scope.These tests validate payload plumbing well, but they currently don’t verify that
MetadataLookupis called withconstants.UserUpdateIdentityRequiredScope, which is the key authorization contract for unlink.Suggested test-hardening diff
type mockUserServiceReader struct { getUserFunc func(ctx context.Context, user *model.User) (*model.User, error) searchUserFunc func(ctx context.Context, user *model.User, criteria string) (*model.User, error) - metadataLookupFunc func(ctx context.Context, input string) (*model.User, error) + metadataLookupFunc func(ctx context.Context, input string, requiredScopes ...string) (*model.User, error) } @@ func (m *mockUserServiceReader) MetadataLookup(ctx context.Context, input string, requiredScopes ...string) (*model.User, error) { if m.metadataLookupFunc != nil { - return m.metadataLookupFunc(ctx, input) + return m.metadataLookupFunc(ctx, input, requiredScopes...) } @@ { name: "successful unlink passes correct fields to unlinker", messageData: validPayload("linkedin", "QhNK44iR6W"), userReader: &mockUserServiceReader{ - metadataLookupFunc: func(ctx context.Context, input string) (*model.User, error) { + metadataLookupFunc: func(ctx context.Context, input string, requiredScopes ...string) (*model.User, error) { + if len(requiredScopes) != 1 || requiredScopes[0] != constants.UserUpdateIdentityRequiredScope { + t.Errorf("expected required scope %q, got %v", constants.UserUpdateIdentityRequiredScope, requiredScopes) + } return &model.User{UserID: "auth0|testuser"}, nil }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/service/message_handler_test.go` around lines 1160 - 1313, Add assertions in the tests that MetadataLookup is invoked with the required scope by updating the mockUserServiceReader.metadataLookupFunc implementations to check that their input equals constants.UserUpdateIdentityRequiredScope; specifically, in the cases "MetadataLookup failure", "unlinker error", "successful unlink passes correct fields to unlinker", and "user_id is populated from MetadataLookup not from payload" add something like if input != constants.UserUpdateIdentityRequiredScope { t.Fatalf(...) } before returning the mocked (*model.User, error) so the tests validate the authorization contract; reference the mockUserServiceReader.metadataLookupFunc and the constant constants.UserUpdateIdentityRequiredScope when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/service/message_handler_test.go`:
- Around line 1160-1313: Add assertions in the tests that MetadataLookup is
invoked with the required scope by updating the
mockUserServiceReader.metadataLookupFunc implementations to check that their
input equals constants.UserUpdateIdentityRequiredScope; specifically, in the
cases "MetadataLookup failure", "unlinker error", "successful unlink passes
correct fields to unlinker", and "user_id is populated from MetadataLookup not
from payload" add something like if input !=
constants.UserUpdateIdentityRequiredScope { t.Fatalf(...) } before returning the
mocked (*model.User, error) so the tests validate the authorization contract;
reference the mockUserServiceReader.metadataLookupFunc and the constant
constants.UserUpdateIdentityRequiredScope when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f236e888-13b2-4b60-99dd-bb43ffe399c1
📒 Files selected for processing (4)
internal/infrastructure/auth0/identity.gointernal/infrastructure/auth0/user.gointernal/service/message_handler_test.gopkg/redaction/redaction_test.go
andrest50
left a comment
There was a problem hiding this comment.
LGTM. I'm not as familiar with the codebase, but the PR description helps to see that it's working and the code looks overall clean.
Overview
Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-504
Implements the Auth Service backend for social account linking and unlinking via NATS, using Auth0's Management API.
What was added
lfx.auth-service.user_identity.link— links a social identity to a user account. Accepts the user's JWT (update:current_user_identitiesscope) and the ID token obtained from the SSR app's SPA popup flow. CallsPOST /api/v2/users/{id}/identities.lfx.auth-service.user_identity.unlink— removes a secondary identity from a user account. Accepts the user's JWT and the provider + identity ID to remove. CallsDELETE /api/v2/users/{id}/identities/{provider}/{identity_id}.[REDACTED]).pkg/constants(UserUpdateMetadataRequiredScope,UserUpdateIdentityRequiredScope).service unavailable.docs/identity_linking.md.Test Evidence
Before linking — 2 identities
Link LinkedIn identity
Service logs:
After linking — 3 identities
Unlink LinkedIn identity
Service logs:
After unlinking — back to 2 identities