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 }; } /**