mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(email): use shared default provider as fallback
Preserve explicit and per-user provider precedence while allowing multi-user installations to use a configured shared default deterministically. Closes #1387
This commit is contained in:
@@ -50,6 +50,7 @@ describe('EmailProviderResolver', () => {
|
||||
provide: EmailProviderService,
|
||||
useValue: {
|
||||
getProviderWithDecryptedPassword: vi.fn(),
|
||||
getSharedDefaultWithDecryptedPassword: vi.fn(),
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -79,6 +80,8 @@ describe('EmailProviderResolver', () => {
|
||||
expect(result.config.fromName).toBe('BookOrbit Bot');
|
||||
expect(result.config.fromAddress).toBe('bot@example.com');
|
||||
expect(result.config.tlsRejectUnauthorized).toBe(true);
|
||||
expect(preferencesService.getForUser).not.toHaveBeenCalled();
|
||||
expect(providerService.getSharedDefaultWithDecryptedPassword).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should include tlsRejectUnauthorized=false in resolved config when provider has it disabled', async () => {
|
||||
@@ -97,13 +100,49 @@ describe('EmailProviderResolver', () => {
|
||||
|
||||
expect(preferencesService.getForUser).toHaveBeenCalledWith(mockUser.id);
|
||||
expect(providerService.getProviderWithDecryptedPassword).toHaveBeenCalledWith(20, mockUser);
|
||||
expect(providerService.getSharedDefaultWithDecryptedPassword).not.toHaveBeenCalled();
|
||||
expect(result.providerId).toBe(20);
|
||||
});
|
||||
|
||||
it('should throw BadRequestException if no provider is requested and no default is set', async () => {
|
||||
it('should resolve from the shared default if the user has no provider preference', async () => {
|
||||
(preferencesService.getForUser as vi.Mock).mockResolvedValue(null);
|
||||
(providerService.getSharedDefaultWithDecryptedPassword as vi.Mock).mockResolvedValue({
|
||||
...mockProvider,
|
||||
id: 30,
|
||||
isShared: true,
|
||||
isDefault: true,
|
||||
});
|
||||
|
||||
await expect(resolver.resolve(mockUser, null)).rejects.toThrow(BadRequestException);
|
||||
const result = await resolver.resolve(mockUser, null);
|
||||
|
||||
expect(providerService.getSharedDefaultWithDecryptedPassword).toHaveBeenCalledOnce();
|
||||
expect(providerService.getProviderWithDecryptedPassword).not.toHaveBeenCalled();
|
||||
expect(result.providerId).toBe(30);
|
||||
expect(result.config.password).toBe('decrypted-password');
|
||||
});
|
||||
|
||||
it('should resolve from the shared default if other preferences exist without a provider preference', async () => {
|
||||
(preferencesService.getForUser as vi.Mock).mockResolvedValue({ defaultProviderId: null, defaultRecipientId: 40, defaultTemplateId: 50 });
|
||||
(providerService.getSharedDefaultWithDecryptedPassword as vi.Mock).mockResolvedValue({
|
||||
...mockProvider,
|
||||
id: 31,
|
||||
isShared: true,
|
||||
isDefault: true,
|
||||
});
|
||||
|
||||
const result = await resolver.resolve(mockUser);
|
||||
|
||||
expect(providerService.getSharedDefaultWithDecryptedPassword).toHaveBeenCalledOnce();
|
||||
expect(result.providerId).toBe(31);
|
||||
});
|
||||
|
||||
it('should throw BadRequestException if no requested, preferred, or shared default provider exists', async () => {
|
||||
(preferencesService.getForUser as vi.Mock).mockResolvedValue(null);
|
||||
(providerService.getSharedDefaultWithDecryptedPassword as vi.Mock).mockResolvedValue(null);
|
||||
|
||||
await expect(resolver.resolve(mockUser, null)).rejects.toThrow(
|
||||
new BadRequestException('No email provider configured. Add a provider in email settings.'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw BadRequestException if requested provider is missing', async () => {
|
||||
|
||||
@@ -35,6 +35,12 @@ export class EmailProviderResolver {
|
||||
return this.fromProvider(provider);
|
||||
}
|
||||
|
||||
const sharedDefault = await this.providerService.getSharedDefaultWithDecryptedPassword();
|
||||
if (sharedDefault) {
|
||||
this.logger.debug(`[${this.event}] [end] userId=${user.id} providerId=${sharedDefault.id} source=shared-default - provider resolved`);
|
||||
return this.fromProvider(sharedDefault);
|
||||
}
|
||||
|
||||
throw new BadRequestException('No email provider configured. Add a provider in email settings.');
|
||||
}
|
||||
|
||||
|
||||
@@ -1,11 +1,13 @@
|
||||
vi.mock('drizzle-orm', () => ({
|
||||
and: vi.fn((...clauses: unknown[]) => ({ op: 'and', clauses })),
|
||||
asc: vi.fn((value: unknown) => ({ op: 'asc', value })),
|
||||
desc: vi.fn((value: unknown) => ({ op: 'desc', value })),
|
||||
eq: vi.fn((left: unknown, right: unknown) => ({ op: 'eq', left, right })),
|
||||
or: vi.fn((...clauses: unknown[]) => ({ op: 'or', clauses })),
|
||||
sql: vi.fn((strings: TemplateStringsArray, ...values: unknown[]) => ({ op: 'sql', text: strings.join(''), values })),
|
||||
}));
|
||||
|
||||
import { and, eq, or, sql } from 'drizzle-orm';
|
||||
import { and, asc, desc, eq, or, sql } from 'drizzle-orm';
|
||||
|
||||
import { emailProviders } from '../../db/schema';
|
||||
import { EmailProviderRepository } from './email-provider.repository';
|
||||
@@ -20,6 +22,7 @@ describe('EmailProviderRepository', () => {
|
||||
};
|
||||
selectBuilder.from.mockReturnValue(selectBuilder);
|
||||
selectBuilder.where.mockReturnValue(selectBuilder);
|
||||
selectBuilder.orderBy.mockReturnValue(selectBuilder);
|
||||
|
||||
const insertBuilder = { values: vi.fn(), returning: vi.fn() };
|
||||
insertBuilder.values.mockReturnValue(insertBuilder);
|
||||
@@ -67,6 +70,25 @@ describe('EmailProviderRepository', () => {
|
||||
expect(selectBuilder.limit).toHaveBeenCalledWith(1);
|
||||
});
|
||||
|
||||
it('findSharedDefault only returns shared defaults with deterministic precedence', () => {
|
||||
const { db, selectBuilder } = makeDb();
|
||||
const repo = new EmailProviderRepository(db as never);
|
||||
|
||||
void repo.findSharedDefault();
|
||||
|
||||
expect(and).toHaveBeenCalledWith(
|
||||
{ op: 'eq', left: emailProviders.isShared, right: true },
|
||||
{ op: 'eq', left: emailProviders.isDefault, right: true },
|
||||
);
|
||||
expect(desc).toHaveBeenCalledWith(emailProviders.isSystemProvider);
|
||||
expect(asc).toHaveBeenCalledWith(emailProviders.id);
|
||||
expect(selectBuilder.orderBy).toHaveBeenCalledWith(
|
||||
{ op: 'desc', value: emailProviders.isSystemProvider },
|
||||
{ op: 'asc', value: emailProviders.id },
|
||||
);
|
||||
expect(selectBuilder.limit).toHaveBeenCalledWith(1);
|
||||
});
|
||||
|
||||
it('insert forwards values and returns inserted row', () => {
|
||||
const { db, insertBuilder } = makeDb();
|
||||
insertBuilder.returning.mockResolvedValue([{ id: 5 }]);
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { Inject, Injectable } from '@nestjs/common';
|
||||
import { and, eq, ne, or, sql } from 'drizzle-orm';
|
||||
import { and, asc, desc, eq, ne, or, sql } from 'drizzle-orm';
|
||||
import { NodePgDatabase } from 'drizzle-orm/node-postgres';
|
||||
|
||||
import { DB } from '../../db';
|
||||
@@ -24,6 +24,15 @@ export class EmailProviderRepository {
|
||||
return this.db.select().from(emailProviders).where(eq(emailProviders.id, id)).limit(1);
|
||||
}
|
||||
|
||||
findSharedDefault() {
|
||||
return this.db
|
||||
.select()
|
||||
.from(emailProviders)
|
||||
.where(and(eq(emailProviders.isShared, true), eq(emailProviders.isDefault, true)))
|
||||
.orderBy(desc(emailProviders.isSystemProvider), asc(emailProviders.id))
|
||||
.limit(1);
|
||||
}
|
||||
|
||||
insert(values: typeof emailProviders.$inferInsert) {
|
||||
return this.db.insert(emailProviders).values(values).returning();
|
||||
}
|
||||
|
||||
@@ -63,6 +63,7 @@ describe('EmailProviderService', () => {
|
||||
repo = {
|
||||
findAllForUser: vi.fn().mockResolvedValue([mockProvider]),
|
||||
findById: vi.fn().mockResolvedValue([mockProvider]),
|
||||
findSharedDefault: vi.fn().mockResolvedValue([]),
|
||||
insert: vi.fn().mockResolvedValue([mockProvider]),
|
||||
update: vi.fn().mockResolvedValue([mockProvider]),
|
||||
delete: vi.fn(),
|
||||
@@ -201,6 +202,34 @@ describe('EmailProviderService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('getSharedDefaultWithDecryptedPassword', () => {
|
||||
it('returns the shared default with its decrypted password', async () => {
|
||||
(repo.findSharedDefault as vi.Mock).mockResolvedValue([{ ...mockProvider, id: 22, userId: 9, isShared: true, isDefault: true }]);
|
||||
|
||||
const result = await service.getSharedDefaultWithDecryptedPassword();
|
||||
|
||||
expect(repo.findSharedDefault).toHaveBeenCalledOnce();
|
||||
expect(encryption.decrypt).toHaveBeenCalledWith('encrypted');
|
||||
expect(result).toMatchObject({ id: 22, plainPassword: 'decrypted' });
|
||||
});
|
||||
|
||||
it('returns null without decrypting when no shared default exists', async () => {
|
||||
const result = await service.getSharedDefaultWithDecryptedPassword();
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(encryption.decrypt).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('preserves a missing password as null', async () => {
|
||||
(repo.findSharedDefault as vi.Mock).mockResolvedValue([{ ...mockProvider, passwordEnc: null, isShared: true, isDefault: true }]);
|
||||
|
||||
const result = await service.getSharedDefaultWithDecryptedPassword();
|
||||
|
||||
expect(result?.plainPassword).toBeNull();
|
||||
expect(encryption.decrypt).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('toggleShared', () => {
|
||||
it('allows a superuser to toggle sharing', async () => {
|
||||
const result = await service.toggleShared(10, mockAdmin);
|
||||
|
||||
@@ -162,6 +162,13 @@ export class EmailProviderService {
|
||||
return { ...provider, plainPassword };
|
||||
}
|
||||
|
||||
async getSharedDefaultWithDecryptedPassword(): Promise<(EmailProvider & { plainPassword: string | null }) | null> {
|
||||
const [provider] = await this.repo.findSharedDefault();
|
||||
if (!provider) return null;
|
||||
const plainPassword = provider.passwordEnc ? this.encryption.decrypt(provider.passwordEnc) : null;
|
||||
return { ...provider, plainPassword };
|
||||
}
|
||||
|
||||
private async getOwned(id: number, user: RequestUser): Promise<EmailProvider> {
|
||||
const [provider] = await this.repo.findById(id);
|
||||
if (!provider) throw new NotFoundException('Provider not found');
|
||||
|
||||
Reference in New Issue
Block a user