From 6890b5c84314904d3968b44c2020a2454aac9bef Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Tue, 1 Sep 2026 09:03:23 -0400 Subject: [PATCH] fix(atproto): address the take-ownership reset by DID, not by email Take ownership sent a PDS password reset to the OpenMeet email address. Nothing keeps that address in step with the email on the AT Protocol account. Changing an OpenMeet email never writes the PDS side, so the two can drift apart. com.atproto.server.requestPasswordReset sends to whichever account owns the address it is given. If the OpenMeet address had drifted and another account owned it, the user got a working reset code for an account that was not theirs. If no account owned it, the reset failed with an unclear error. The account is now resolved from the identity DID through com.atproto.admin.getAccountInfo, and the reset is sent to the address on that account. A missing account or a missing address now fails with a clear error instead of reporting success, and the response reports the address the reset actually went to. The OpenMeet email is no longer required to start take ownership, since it is no longer the identifier. A mismatch between the two addresses is logged as a warning, without the addresses themselves. --- .../atproto-identity-recovery.service.spec.ts | 71 ++++++++++++++++++- .../atproto-identity-recovery.service.ts | 52 ++++++++++++-- 2 files changed, 114 insertions(+), 9 deletions(-) diff --git a/src/atproto-identity/atproto-identity-recovery.service.spec.ts b/src/atproto-identity/atproto-identity-recovery.service.spec.ts index 29d52006..24160c5b 100644 --- a/src/atproto-identity/atproto-identity-recovery.service.spec.ts +++ b/src/atproto-identity/atproto-identity-recovery.service.spec.ts @@ -52,6 +52,7 @@ describe('AtprotoIdentityRecoveryService', () => { const mockPdsAccountService = { searchAccountsByEmail: jest.fn(), + getAccountInfo: jest.fn(), adminUpdateAccountPassword: jest.fn(), requestPasswordReset: jest.fn(), createSession: jest.fn(), @@ -468,6 +469,7 @@ describe('AtprotoIdentityRecoveryService', () => { userAtprotoIdentityService.findByUserUlid.mockResolvedValue( mockIdentityEntity as UserAtprotoIdentityEntity, ); + pdsAccountService.getAccountInfo.mockResolvedValue(mockPdsAccount as any); pdsAccountService.requestPasswordReset.mockResolvedValue(undefined); // Act @@ -477,10 +479,75 @@ describe('AtprotoIdentityRecoveryService', () => { ); // Assert + expect(pdsAccountService.getAccountInfo).toHaveBeenCalledWith( + mockIdentityEntity.did, + ); expect(pdsAccountService.requestPasswordReset).toHaveBeenCalledWith( - mockUser.email, + mockPdsAccount.email, + ); + expect(result).toEqual({ email: mockPdsAccount.email }); + }); + + it('should address the reset to the account on the PDS when the OpenMeet email has drifted', async () => { + // Arrange - the OpenMeet email now belongs to somebody else on the PDS + userService.findByUlid.mockResolvedValue({ + ...mockUser, + email: 'someone-elses-address@example.com', + } as any); + userAtprotoIdentityService.findByUserUlid.mockResolvedValue( + mockIdentityEntity as UserAtprotoIdentityEntity, + ); + pdsAccountService.getAccountInfo.mockResolvedValue(mockPdsAccount as any); + pdsAccountService.requestPasswordReset.mockResolvedValue(undefined); + + // Act + const result = await service.initiateTakeOwnership( + 'test-tenant', + mockUser.ulid, + ); + + // Assert - the reset goes to the account the DID owns, not to the + // OpenMeet address, so no other account can be reset by this flow + expect(pdsAccountService.requestPasswordReset).toHaveBeenCalledWith( + mockPdsAccount.email, + ); + expect(pdsAccountService.requestPasswordReset).not.toHaveBeenCalledWith( + 'someone-elses-address@example.com', + ); + expect(result).toEqual({ email: mockPdsAccount.email }); + }); + + it('should throw NotFoundException when the DID has no account on the PDS', async () => { + // Arrange + userService.findByUlid.mockResolvedValue(mockUser as any); + userAtprotoIdentityService.findByUserUlid.mockResolvedValue( + mockIdentityEntity as UserAtprotoIdentityEntity, + ); + pdsAccountService.getAccountInfo.mockResolvedValue(null); + + // Act & Assert + await expect( + service.initiateTakeOwnership('test-tenant', mockUser.ulid), + ).rejects.toThrow(NotFoundException); + expect(pdsAccountService.requestPasswordReset).not.toHaveBeenCalled(); + }); + + it('should fail visibly when the account on the PDS has no email address', async () => { + // Arrange - nothing can be delivered, so this must not report success + userService.findByUlid.mockResolvedValue(mockUser as any); + userAtprotoIdentityService.findByUserUlid.mockResolvedValue( + mockIdentityEntity as UserAtprotoIdentityEntity, ); - expect(result).toEqual({ email: mockUser.email }); + pdsAccountService.getAccountInfo.mockResolvedValue({ + did: mockPdsAccount.did, + handle: mockPdsAccount.handle, + } as any); + + // Act & Assert + await expect( + service.initiateTakeOwnership('test-tenant', mockUser.ulid), + ).rejects.toThrow(BadRequestException); + expect(pdsAccountService.requestPasswordReset).not.toHaveBeenCalled(); }); it('should throw NotFoundException when user not found', async () => { diff --git a/src/atproto-identity/atproto-identity-recovery.service.ts b/src/atproto-identity/atproto-identity-recovery.service.ts index 8b6a6520..1683dd5c 100644 --- a/src/atproto-identity/atproto-identity-recovery.service.ts +++ b/src/atproto-identity/atproto-identity-recovery.service.ts @@ -207,19 +207,24 @@ export class AtprotoIdentityRecoveryService { * Initiate take ownership: trigger PDS password reset email. * User will receive email to set their own password. * + * The reset is addressed by DID. The email comes from the AT Protocol + * account itself, not from the OpenMeet user record, so a drifted OpenMeet + * email cannot send a reset code for a different account. + * * @param tenantId - The tenant ID * @param userUlid - The user's ULID * @returns Object with the email address the reset was sent to - * @throws NotFoundException if user not found - * @throws BadRequestException if no custodial identity exists + * @throws NotFoundException if the user or the AT Protocol account is not found + * @throws BadRequestException if no custodial identity exists, or the + * AT Protocol account has no email address to deliver a reset to */ async initiateTakeOwnership( tenantId: string, userUlid: string, ): Promise<{ email: string }> { const user = await this.userService.findByUlid(userUlid, tenantId); - if (!user || !user.email) { - throw new NotFoundException('User not found or has no email'); + if (!user) { + throw new NotFoundException('User not found'); } // Check if user has custodial identity @@ -239,14 +244,47 @@ export class AtprotoIdentityRecoveryService { ); } + // Resolve the address from the DID, never from the OpenMeet email. The + // two can drift apart, because changing an OpenMeet email does not write + // the PDS side. requestPasswordReset sends to whichever account owns the + // address it is given, so a drifted address would mail a reset code for + // an account the user does not own. The DID is the identity, so the + // address has to come from it. + const account = await this.pdsAccountService.getAccountInfo(identity.did); + if (!account) { + throw new NotFoundException( + 'No AT Protocol account was found for this identity, so no password ' + + 'reset can be sent.', + ); + } + if (!account.email) { + throw new BadRequestException( + 'This AT Protocol account has no email address on file, so a ' + + 'password reset cannot be delivered. Contact support to set one ' + + 'before taking ownership.', + ); + } + if ( + user.email && + account.email.toLowerCase() !== user.email.toLowerCase() + ) { + // The addresses themselves are not logged. Both are recoverable from + // the DID by an operator who needs them. + this.logger.warn( + `AT Protocol account email differs from the OpenMeet email for ` + + `${identity.did}; the reset was addressed to the account on the PDS`, + ); + } + // Trigger PDS password reset email - await this.pdsAccountService.requestPasswordReset(user.email); + await this.pdsAccountService.requestPasswordReset(account.email); this.logger.log( - `Initiated take ownership for user ${userUlid}: password reset email sent to ${user.email}`, + `Initiated take ownership for user ${userUlid}: password reset email ` + + `sent for ${identity.did}`, ); - return { email: user.email }; + return { email: account.email }; } /**