feat(security): harden the API, trust Hiboutik for deletions, lock syncs #3

Merged
quentin_duwa merged 4 commits from feature/input-validation into main 2026-11-08 09:36:50 +00:00
12 changed files with 115 additions and 36 deletions
Showing only changes of commit 30c56f9330 - Show all commits

View File

@@ -0,0 +1,66 @@
import { of, throwError } from 'rxjs';
import { HiboutikClient } from './hiboutik.client';
describe('HiboutikClient', () => {
const httpService = { get: jest.fn() };
const configService = { get: jest.fn((key: string) => (key === 'HIBOUTIK_BASE_URL' ? 'https://shop.hiboutik.test/api' : 'x')) };
let client: HiboutikClient;
const answer = (data: unknown) => of({ data });
const page = (size: number) => Array.from({ length: size }, (_, index) => ({ id: index }));
beforeEach(() => {
jest.clearAllMocks();
client = new HiboutikClient(httpService as never, configService as never);
});
describe('getList', () => {
it('returns the list of a 2xx answer, empty or not', async () => {
httpService.get.mockReturnValueOnce(answer([{ id: 1 }])).mockReturnValueOnce(answer([]));
await expect(client.getList('/x')).resolves.toEqual([{ id: 1 }]);
await expect(client.getList('/x')).resolves.toEqual([]);
});
it('treats a 2xx answer without body as an empty list', async () => {
httpService.get.mockReturnValueOnce(answer('')).mockReturnValueOnce(answer(null));
await expect(client.getList('/x')).resolves.toEqual([]);
await expect(client.getList('/x')).resolves.toEqual([]);
});
it('rejects a 2xx answer that is not a list, without leaking its content', async () => {
httpService.get.mockReturnValueOnce(answer({ error: 'secret details' }));
const result = client.getList('/x');
await expect(result).rejects.toThrow('a list was expected');
await expect(result).rejects.not.toThrow('secret details');
});
it('lets HTTP errors through', async () => {
httpService.get.mockReturnValueOnce(throwError(() => new Error('Request failed with status code 500')));
await expect(client.getList('/x')).rejects.toThrow('500');
});
});
describe('getAllPages', () => {
it('walks the pages until a short one', async () => {
httpService.get.mockReturnValueOnce(answer(page(250))).mockReturnValueOnce(answer(page(10)));
const results = await client.getAllPages('/x');
expect(results).toHaveLength(260);
expect(httpService.get).toHaveBeenCalledTimes(2);
});
it('fails as a whole when a later page fails, instead of returning a truncated list', async () => {
httpService.get
.mockReturnValueOnce(answer(page(250)))
.mockReturnValueOnce(throwError(() => new Error('Request failed with status code 502')));
await expect(client.getAllPages('/x')).rejects.toThrow('502');
});
});
});

View File

@@ -33,6 +33,17 @@ export class HiboutikClient {
return response.data;
}
/**
* GET on a list endpoint. HTTP errors (4xx/5xx) throw, a 2xx answer without body is an empty list, and any other
* non-list body throws: an empty list triggers deletions in the sync, so it must never be inferred from a malformed answer.
*/
async getList<T>(path: string, params?: Record<string, unknown>): Promise<T[]> {
const data = await this.get<T[] | '' | null | undefined>(path, params);
if (data === '' || data == null) return [];
if (!Array.isArray(data)) throw new Error(`Unexpected answer from Hiboutik ${path}: a list was expected`);
return data;
}
/**
* Fetches every page of a paginated endpoint (`p` param, starting at 1) and concatenates the results.
* Only for paginated endpoints: on a non-paginated one, a full first page would loop forever on the same results.
@@ -42,8 +53,8 @@ export class HiboutikClient {
let page = 1;
while (true) {
const pageData = await this.get<T[]>(path, { ...baseParams, p: page });
if (!pageData || pageData.length === 0) break;
const pageData = await this.getList<T>(path, { ...baseParams, p: page });
if (pageData.length === 0) break;
results.push(...pageData);
// A page smaller than the page size is the last one: no need to request an empty page
if (pageData.length < HIBOUTIK_PAGE_SIZE) break;

View File

@@ -9,7 +9,7 @@ export class StockHiboutikRepository {
/** Stock of every product and size, all warehouses combined. Not paginated. */
async getStocks(): Promise<StockHiboutikDto[]> {
const stocks = await this.hiboutikClient.get<StockHiboutikDto[]>('/stock_available/all_wh', {});
const stocks = await this.hiboutikClient.getList<StockHiboutikDto>('/stock_available/all_wh', {});
// Hiboutik may return ids as strings: normalize them so strict comparisons work downstream
return stocks.map((stock) => ({
...stock,

View File

@@ -9,7 +9,7 @@ export class SupplierHiboutikRepository {
/** `/suppliers` is not paginated: everything comes in one call. */
async getSuppliers(): Promise<SupplierHiboutikDto[]> {
return this.hiboutikClient.get<SupplierHiboutikDto[]>('/suppliers', {
return this.hiboutikClient.getList<SupplierHiboutikDto>('/suppliers', {
order_by: 'supplier_id',
sort: 'ASC',
});

View File

@@ -25,11 +25,7 @@ export class BrandHiboutikService {
(brand) => `Hiboutik brand ${brand.hiboutikId}`,
);
// Safety net: an empty answer from Hiboutik most likely means an API issue
if (hiboutikBrands.length === 0) {
this.logger.warn('Hiboutik returned no brands: deletion skipped.');
return;
}
// Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
const nbDeleted = await this.brandDatabaseRepository.deleteMissing(hiboutikBrands.map((dto) => Number(dto.brand_id)));
this.logger.log(
`Brands sync ended: ${hiboutikBrands.length - nbFailed}/${hiboutikBrands.length} synced, ${nbFailed} failed, ${nbDeleted} deleted.`,

View File

@@ -79,11 +79,19 @@ describe('CategoryHiboutikService', () => {
expect(categoryDatabaseRepository.deleteMissing).toHaveBeenCalledWith([1, 2]);
});
it('does not delete anything when Hiboutik returns no categories', async () => {
it('deletes every category when Hiboutik answers with an empty list', async () => {
categoryHiboutikRepository.getCategories.mockResolvedValue([]);
await service.syncCategories();
expect(categoryDatabaseRepository.deleteMissing).toHaveBeenCalledWith([]);
});
it('deletes nothing when the Hiboutik call fails', async () => {
categoryHiboutikRepository.getCategories.mockRejectedValue(new Error('Request failed with status code 500'));
await expect(service.syncCategories()).rejects.toThrow('500');
expect(categoryDatabaseRepository.deleteMissing).not.toHaveBeenCalled();
});
});

View File

@@ -37,11 +37,7 @@ export class CategoryHiboutikService {
(dto) => `parent of Hiboutik category ${dto.category_id}`,
);
// Safety net: an empty answer from Hiboutik most likely means an API issue
if (hiboutikCategories.length === 0) {
this.logger.warn('Hiboutik returned no categories: deletion skipped.');
return;
}
// Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
const nbDeleted = await this.categoryDatabaseRepository.deleteMissing(hiboutikCategories.map((dto) => Number(dto.category_id)));
this.logger.log(
`Categories sync ended: ${hiboutikCategories.length - nbFailed}/${hiboutikCategories.length} synced, ${nbFailed} failed, ` +

View File

@@ -48,9 +48,17 @@ describe('CustomerHiboutikService', () => {
expect(customerDatabaseRepository.upsert).toHaveBeenCalledTimes(2);
});
it('does not delete customers when Hiboutik returns none', async () => {
it('deletes every customer when Hiboutik answers with an empty list', async () => {
await service.syncCustomers();
expect(customerDatabaseRepository.deleteMissing).toHaveBeenCalledWith([]);
});
it('deletes nothing when the Hiboutik call fails', async () => {
customerHiboutikRepository.getCustomers.mockRejectedValue(new Error('Request failed with status code 500'));
await expect(service.syncCustomers()).rejects.toThrow('500');
expect(customerDatabaseRepository.deleteMissing).not.toHaveBeenCalled();
});

View File

@@ -37,13 +37,8 @@ export class CustomerHiboutikService {
async syncCustomers() {
const hiboutikCustomers = await this.customerHiboutikRepository.getCustomers();
// Safety net: an empty answer from Hiboutik most likely means an API issue
let nbDeleted = 0;
if (hiboutikCustomers.length === 0) {
this.logger.warn('Hiboutik returned no customers: deletion skipped.');
} else {
nbDeleted = await this.customerDatabaseRepository.deleteMissing(hiboutikCustomers.map((dto) => Number(dto.customers_id)));
}
// Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
const nbDeleted = await this.customerDatabaseRepository.deleteMissing(hiboutikCustomers.map((dto) => Number(dto.customers_id)));
let nbFailed = 0;
let nbFailedAddresses = 0;

View File

@@ -89,12 +89,21 @@ describe('ProductHiboutikService', () => {
expect(calls[0]).toBe('deleteProducts');
});
it('does not delete anything when Hiboutik returns no products', async () => {
it('deletes every product when Hiboutik answers with an empty list', async () => {
productHiboutikRepository.getProducts.mockResolvedValue([]);
productDatabaseRepository.getAll.mockResolvedValue([{ id: 'uuid-1', hiboutikId: 1 }]);
await service.syncProducts();
expect(productDatabaseRepository.deleteAll).toHaveBeenCalledWith(['uuid-1']);
});
it('deletes nothing when the Hiboutik call fails', async () => {
productHiboutikRepository.getProducts.mockRejectedValue(new Error('Request failed with status code 500'));
productDatabaseRepository.getAll.mockResolvedValue([{ id: 'uuid-1', hiboutikId: 1 }]);
await expect(service.syncProducts()).rejects.toThrow('500');
expect(productDatabaseRepository.deleteAll).not.toHaveBeenCalled();
});

View File

@@ -27,7 +27,6 @@ export class ProductHiboutikService {
/** Mirrors Hiboutik products, their versions (sizes) and stocks. Products missing from Hiboutik are deleted. */
async syncProducts() {
this.logger.log('Starting products sync');
const [productsHiboutik, hiboutikStocks, existingProducts] = await Promise.all([
this.productHiboutikRepository.getProducts(),
this.stockHiboutikRepository.getStocks(),
@@ -56,12 +55,7 @@ export class ProductHiboutikService {
/** @returns the number of deleted products. */
private async deleteMissingProducts(productsHiboutik: ProductHiboutikDto[], existingProducts: Product[]): Promise<number> {
// Safety net: an empty answer from Hiboutik most likely means an API issue, not an empty catalog
if (productsHiboutik.length === 0 && existingProducts.length > 0) {
this.logger.warn(`Hiboutik returned no products while ${existingProducts.length} exist in database: deletion skipped.`);
return 0;
}
// Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
const hiboutikIds = new Set(productsHiboutik.map((product) => Number(product.product_id)));
const missingProductIds = existingProducts.filter((product) => !hiboutikIds.has(product.hiboutikId)).map((product) => product.id);
return this.productDatabaseRepository.deleteAll(missingProductIds);

View File

@@ -25,11 +25,7 @@ export class SupplierHiboutikService {
(supplier) => `Hiboutik supplier ${supplier.hiboutikId}`,
);
// Safety net: an empty answer from Hiboutik most likely means an API issue
if (hiboutikSuppliers.length === 0) {
this.logger.warn('Hiboutik returned no suppliers: deletion skipped.');
return;
}
// Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
const nbDeleted = await this.supplierDatabaseRepository.deleteMissing(hiboutikSuppliers.map((dto) => Number(dto.supplier_id)));
this.logger.log(
`Suppliers sync ended: ${hiboutikSuppliers.length - nbFailed}/${hiboutikSuppliers.length} synced, ${nbFailed} failed, ${nbDeleted} deleted.`,