feat: extend private address books to OIDC users by katspaugh · Pull Request #3083 · safe-global/safe-client-gateway

katspaugh · 2026-05-07T14:38:49Z

Summary

Allow OIDC-authenticated users (members and admins) to perform every private-address-book and address-book-request write that SiWE users can today
Replace wallet-string actor columns (user_address_book_items.created_by, address_book_requests.requested_by_wallet/reviewed_by) with users.id references; mirrors the recent space-AB migration (#3056)
Resolve display identity (email or wallet, with Unknown user / Deleted user fallbacks) at read time via a new shared UserIdentityResolverService

Behavior change

The five write endpoints below previously returned 403 Forbidden for OIDC auth and now accept it (member/admin assertions still apply):

PUT /v1/spaces/:spaceId/address-book/private
DELETE /v1/spaces/:spaceId/address-book/private/:address
POST /v1/spaces/:spaceId/address-book/requests
PUT /v1/spaces/:spaceId/address-book/requests/:id/approve
PUT /v1/spaces/:spaceId/address-book/requests/:id/reject

DTO changes (pre-prod, single-version, no v2):

UserAddressBookItemDto.createdBy semantic shifts from "wallet" to "identity string" (email/wallet/fallback). Field name unchanged. Adds createdByUserId: number.
AddressBookRequestItemDto.requestedBy same shift; adds requestedByUserId: number, reviewedBy: string | null, reviewedByUserId: number | null.

For SiWE users the identity string still resolves to the wallet, so observable output is unchanged.

Schema migration

migrations/1778000000000-private-address-book-attribution-to-user-id.ts:

Drops user_address_book_items.created_by
Drops address_book_requests.requested_by_wallet and reviewed_by
Adds address_book_requests.reviewed_by_id integer NULL with FK to users(id) ON DELETE SET NULL

Pre-prod, no backfill. The reverse migration restores the dropped columns as nullable varchar(42).

Design and plan

Committed alongside the implementation:

docs/superpowers/specs/2026-05-07-oidc-private-address-books-design.md
docs/superpowers/plans/2026-05-07-oidc-private-address-books.md

Happy to drop these from the merge if the team prefers PR-only artifacts.

Test plan

Unit suite: yarn test:unit — 4755 passed locally.

End-to-end (require Postgres via Docker — please verify in CI):

OIDC user creates / reads / deletes a private contact (assert createdBy, createdByUserId)
OIDC member creates an address book request (assert requestedBy, requestedByUserId, reviewedBy: null, reviewedByUserId: null)
OIDC admin approves a request → contact lands in shared space AB with correct createdByUserId (member) and lastUpdatedByUserId (admin)
OIDC admin rejects a request → request is no longer in PENDING list
SiWE user paths still work and createdBy / requestedBy resolve to wallet (regression)
Migration up / revert / up round-trip cleanly
Non-member OIDC → 403 on write paths; member OIDC → 403 on approve/reject

Follow-ups (non-blocking, surfaced by the final reviewer)

Tighten approve e2e to assert createdByUserId equals the requesting member's id
reject returns 400 instead of 404 for missing request id (pre-existing pattern)
No dedicated unit spec for UserAddressBookService / AddressBookRequestsService (covered by e2e + repository specs)

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ests

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR extends private address book + address book request write operations to OIDC-authenticated users by removing SiWE-only write gates, migrating attribution columns from wallet strings to users.id, and resolving display identity (wallet/email/fallback) at read time via a shared resolver.

Changes:

Introduces UserIdentityResolverService (+ module + unit tests) and refactors shared space address book identity mapping to use it.
Removes SiWE-only guards from private address book + request write flows; updates DTOs to expose both identity strings and corresponding *UserId fields.
Applies a DB migration to drop wallet-string attribution columns and add reviewed_by_id with FK semantics; updates repositories/entities and adds OIDC E2E coverage.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/modules/users/domain/user-identity-resolver.service.ts	New shared service to resolve user display identities (wallet/email/fallback).
src/modules/users/domain/user-identity-resolver.module.ts	Nest module exporting the resolver for reuse across spaces routes.
src/modules/users/domain/tests/user-identity-resolver.service.spec.ts	Unit tests covering resolver behavior and fallbacks.
src/modules/spaces/spaces.module.ts	Wires the resolver module into SpacesModule; removes now-unneeded direct wallets repo provider.
src/modules/spaces/routes/user-address-book.service.ts	Removes SiWE-only write restriction; maps `createdBy` from resolved identity and adds `createdByUserId`.
src/modules/spaces/routes/user-address-book.controller.ts	Updates OpenAPI forbidden descriptions (drops “wallet authentication required”).
src/modules/spaces/routes/user-address-book.controller.e2e-spec.ts	Adds/updates E2E assertions for `createdByUserId` and OIDC write/read/delete paths.
src/modules/spaces/routes/entities/space-address-book.dto.entity.ts	Updates `createdBy` semantics docs and adds `createdByUserId` to DTO.
src/modules/spaces/routes/entities/address-book-request.dto.entity.ts	Adds `requestedByUserId`, `reviewedBy`, `reviewedByUserId` and updates field descriptions.
src/modules/spaces/routes/address-books.service.ts	Refactors shared space address book identity resolution to use `UserIdentityResolverService`.
src/modules/spaces/routes/address-books.service.spec.ts	Updates unit test wiring to construct AddressBooksService with the new resolver.
src/modules/spaces/routes/address-book-requests.service.ts	Removes SiWE-only restriction; switches reviewer attribution to userId and resolves identity strings at read time.
src/modules/spaces/routes/address-book-requests.controller.ts	Updates OpenAPI forbidden descriptions (drops “wallet authentication required”).
src/modules/spaces/routes/address-book-requests.controller.e2e-spec.ts	Adds OIDC E2E coverage for create/approve/reject and asserts new DTO fields.
src/modules/spaces/domain/address-books/user-address-book-items.repository.ts	Drops signer-address attribution on insert; relies on creator user FK only.
src/modules/spaces/domain/address-books/user-address-book-items.repository.interface.ts	Updates repository contract to remove signerAddress.
src/modules/spaces/domain/address-books/entities/user-address-book-item.entity.ts	Updates domain schema to remove `createdBy` wallet field.
src/modules/spaces/domain/address-books/entities/address-book-request.entity.ts	Updates domain schema to remove `requestedByWallet` and switch to `reviewedById`.
src/modules/spaces/domain/address-books/address-book-requests.repository.ts	Removes `requestedByWallet`, switches `reviewedBy` to `reviewedById` in transitions.
src/modules/spaces/domain/address-books/address-book-requests.repository.interface.ts	Updates repository contract for new attribution fields.
src/modules/spaces/datasources/entities/user-address-book-item.entity.db.ts	Removes `created_by` column mapping.
src/modules/spaces/datasources/entities/address-book-request.entity.db.ts	Removes `requested_by_wallet` + wallet reviewer column mapping; adds `reviewedById`.
migrations/1778000000000-private-address-book-attribution-to-user-id.ts	Migration dropping wallet-string attribution columns and adding `reviewed_by_id` FK.
docs/superpowers/specs/2026-05-07-oidc-private-address-books-design.md	Design spec documenting the approach and schema/service/DTO changes.
docs/superpowers/plans/2026-05-07-oidc-private-address-books.md	Implementation plan artifact for the change set.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    const [users, wallets] = await Promise.all([
+      this.usersRepository.find({ id: In(unique) }),
+      this.walletsRepository.find({
+        where: { user: { id: In(unique) } },
+        relations: { user: true },
+      }),
+    ]);
+
+    const walletByUserId = new Map<number, string>();
+    for (const wallet of wallets) {
+      if (!walletByUserId.has(wallet.user.id)) {
+        walletByUserId.set(wallet.user.id, wallet.address);
+      }
+    }


+- DROP COLUMN `requested_by_wallet` (`varchar(42)`).
+- DROP COLUMN `reviewed_by` (`varchar(42)`), ADD COLUMN `reviewed_by_id integer NULL` with FK to `users(id)` `ON DELETE SET NULL`.
+
+The TypeORM entity files (`user-address-book-item.entity.db.ts`, `address-book-request.entity.db.ts`) are updated to match: drop the wallet `@Column`s, add a `@ManyToOne(() => User)` for `reviewedBy`.


+2. User has a verified OIDC email → return that email.
+3. User has a wallet → return the preferred/first wallet address.


- Sort wallets by id ASC in UserIdentityResolverService.resolveMany so users with multiple wallets always render the same display address - Update the design doc to match the implemented resolution order (wallet → email → Unknown) and the raw-integer reviewed_by_id column (no @manytoone relation), matching the space-AB precedent Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

vseehausen · 2026-05-11T10:07:52Z

@@ -0,0 +1,1708 @@
+<!--


I have a preference for keeping planning documents out of the repo, as these are one-time, task-specific documents, with many lines.

Kept it for context for reviewers, will remove at the end.

vseehausen

LGTM, but pls remove the plan files.

LucieFaire

small remarks, otherwise good. Thank you for extracting the resolver!

LucieFaire · 2026-05-12T10:23:36Z

+
+    await queryRunner.query(`
+      ALTER TABLE address_book_requests
+        ADD COLUMN reviewed_by_id integer;


nit, but i think we could have kept it reviewied_by and just switched the type, as we have with other fields: invited_by,created_by

LucieFaire · 2026-05-12T10:27:28Z

    });

-    return this.mapRequestItem(request);
+    return await this.mapSingleRequest(request);


wonder if we really need a separate method or we can reuse the mapRequestItem as it was? we do have spaceid available

LucieFaire · 2026-05-12T10:29:47Z

+
+  it('falls back to email when no wallet', async () => {
+    mockUsersRepository.find.mockResolvedValue([
+      { id: 2, email: 'c@d.com' } as never,


could we maybe use a proper type instead of never in these tests?

katspaugh and others added 11 commits May 7, 2026 14:52

docs: spec for OIDC private address books

880e809

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

docs: implementation plan for OIDC private address books

008132e

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor: extract UserIdentityResolverService

515b7b2

refactor: drop orphaned IWalletsRepository provider; fix JSDoc link

d2af48d

feat: drop wallet-string attribution columns from private AB and requ…

96c5fc3

…ests

refactor: drop signerAddress from UserAddressBookItemsRepository

d98e461

feat(private-AB): allow OIDC writes and surface user identity

a879e4c

refactor: switch AddressBookRequestsRepository to userId attribution

05c1c8b

feat(AB-requests): allow OIDC writes and surface reviewer identity

072f95a

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

test(private-AB): cover OIDC paths and assert createdByUserId

9b06dc8

test(AB-requests): cover OIDC create/approve/reject paths

60149b9

katspaugh requested a review from a team as a code owner May 7, 2026 14:38

katspaugh requested review from LucieFaire, Copilot and vseehausen May 7, 2026 14:39

chore: add SPDX license headers to design docs

7ff35d7

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot started reviewing on behalf of katspaugh May 7, 2026 14:41 View session

Copilot AI reviewed May 7, 2026

View reviewed changes

vseehausen reviewed May 11, 2026

View reviewed changes

vseehausen requested changes May 11, 2026

View reviewed changes

LucieFaire reviewed May 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: extend private address books to OIDC users#3083

feat: extend private address books to OIDC users#3083
katspaugh wants to merge 13 commits into
mainfrom
oidc-private-address-books

katspaugh commented May 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

vseehausen May 11, 2026

Uh oh!

katspaugh May 12, 2026

Uh oh!

vseehausen left a comment

Uh oh!

LucieFaire left a comment

Uh oh!

LucieFaire May 12, 2026

Uh oh!

LucieFaire May 12, 2026

Uh oh!

LucieFaire May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		2. User has a verified OIDC email → return that email.
		3. User has a wallet → return the preferred/first wallet address.

Conversation

katspaugh commented May 7, 2026

Summary

Behavior change

Schema migration

Design and plan

Test plan

Follow-ups (non-blocking, surfaced by the final reviewer)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

vseehausen May 11, 2026

Choose a reason for hiding this comment

Uh oh!

katspaugh May 12, 2026

Choose a reason for hiding this comment

Uh oh!

vseehausen left a comment

Choose a reason for hiding this comment

Uh oh!

LucieFaire left a comment

Choose a reason for hiding this comment

Uh oh!

LucieFaire May 12, 2026

Choose a reason for hiding this comment

Uh oh!

LucieFaire May 12, 2026

Choose a reason for hiding this comment

Uh oh!

LucieFaire May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants