fix(core): Support type filters on global credential lookups (#30002)

This commit is contained in:
Jaakko Husso
2026-05-07 20:50:58 +03:00
committed by GitHub
parent 75053fec93
commit 8e0f37d100
4 changed files with 139 additions and 7 deletions
@@ -186,39 +186,49 @@ export class CredentialsRepository extends Repository<CredentialsEntity> {
}
/**
* Find all global credentials
* Find all global credentials, optionally narrowed by credential type.
*/
async findAllGlobalCredentials(
options: {
includeData?: boolean;
type?: string;
filters?: {
dependency?: CredentialDependencyFilter;
};
} = {},
): Promise<CredentialsEntity[]> {
const { includeData = false, filters } = options;
const { includeData = false, type, filters } = options;
const dependencyFilter = filters?.dependency;
if (dependencyFilter) {
return await this.findAllGlobalCredentialsByDependencyFilter({
dependencyFilter,
includeData,
type,
});
}
const findManyOptions = this.toFindManyOptions({ includeData });
findManyOptions.where = { ...findManyOptions.where, isGlobal: true };
findManyOptions.where = {
...findManyOptions.where,
isGlobal: true,
...(type ? { type: Like(`%${type}%`) } : {}),
};
return await this.find(findManyOptions);
}
private async findAllGlobalCredentialsByDependencyFilter(options: {
dependencyFilter: CredentialDependencyFilter;
includeData?: boolean;
type?: string;
}): Promise<CredentialsEntity[]> {
const { includeData, dependencyFilter } = options;
const { includeData, dependencyFilter, type } = options;
const qb = this.createQueryBuilder('credential');
qb.where('credential.isGlobal = :isGlobal', { isGlobal: true });
if (type) {
qb.andWhere('credential.type LIKE :type', { type: `%${type}%` });
}
addCredentialDependencyExistsFilter(qb, dependencyFilter);
const defaultSelect: Array<keyof CredentialsEntity> = [
@@ -1583,6 +1583,61 @@ describe('CredentialsService', () => {
filters: { dependency: undefined },
});
});
it('should forward the credential type filter to the global credentials lookup (owner user)', async () => {
// ARRANGE
credentialsRepository.findMany.mockResolvedValue([]);
credentialsRepository.findAllGlobalCredentials.mockResolvedValue([]);
// ACT
await service.getMany(ownerUser, {
includeGlobal: true,
listQueryOptions: { filter: { type: 'slackOAuth2Api' } },
});
// ASSERT
expect(credentialsRepository.findAllGlobalCredentials).toHaveBeenCalledWith({
includeData: false,
type: 'slackOAuth2Api',
filters: { dependency: undefined },
});
});
it('should forward the credential type filter to the global credentials lookup (member user)', async () => {
// ARRANGE
credentialsRepository.getManyAndCountWithSharingSubquery.mockResolvedValue({
credentials: [],
count: 0,
});
credentialsRepository.findAllGlobalCredentials.mockResolvedValue([]);
// ACT
await service.getMany(memberUser, {
includeGlobal: true,
listQueryOptions: { filter: { type: 'slackOAuth2Api' } },
});
// ASSERT
expect(credentialsRepository.findAllGlobalCredentials).toHaveBeenCalledWith({
includeData: false,
type: 'slackOAuth2Api',
filters: { dependency: undefined },
});
});
it('should not pass a type filter when the listQueryOptions filter has no type', async () => {
// ARRANGE
credentialsRepository.findMany.mockResolvedValue([]);
credentialsRepository.findAllGlobalCredentials.mockResolvedValue([]);
// ACT
await service.getMany(ownerUser, { includeGlobal: true });
// ASSERT
const lastCall =
credentialsRepository.findAllGlobalCredentials.mock.calls.at(-1)?.[0] ?? {};
expect(lastCall).not.toHaveProperty('type');
});
});
describe('with includeGlobal = false', () => {
@@ -115,9 +115,11 @@ export class CredentialsService {
credentials: CredentialsEntity[],
includeData: boolean,
dependencyFilter?: CredentialDependencyFilter,
type?: string,
): Promise<CredentialsEntity[]> {
const globalCredentials = await this.credentialsRepository.findAllGlobalCredentials({
includeData,
...(type ? { type } : {}),
filters: { dependency: dependencyFilter },
});
@@ -128,6 +130,15 @@ export class CredentialsService {
return [...credentials, ...newGlobalCreds];
}
/**
* Read the credential `type` filter from listQueryOptions before any repo
* call mutates it (toFindManyOptions wraps it in a Like(...) in place).
*/
private extractTypeFilter(listQueryOptions: ListQuery.Options): string | undefined {
const filterType = listQueryOptions.filter?.type;
return typeof filterType === 'string' && filterType !== '' ? filterType : undefined;
}
async getMany(
user: User,
options: {
@@ -234,6 +245,7 @@ export class CredentialsService {
}: GetManyCredentialsOptions,
): Promise<CredentialsEntity[]> {
const { dependency: dependencyFilter } = filters ?? {};
const typeFilter = this.extractTypeFilter(listQueryOptions);
// If onlySharedWithMe or dependency filtering is requested, use subquery approach.
if (onlySharedWithMe || dependencyFilter) {
@@ -253,7 +265,12 @@ export class CredentialsService {
);
if (includeGlobal) {
return await this.addGlobalCredentials(credentials, includeData, dependencyFilter);
return await this.addGlobalCredentials(
credentials,
includeData,
dependencyFilter,
typeFilter,
);
}
return credentials;
@@ -267,7 +284,12 @@ export class CredentialsService {
});
if (includeGlobal) {
credentials = await this.addGlobalCredentials(credentials, includeData, dependencyFilter);
credentials = await this.addGlobalCredentials(
credentials,
includeData,
dependencyFilter,
typeFilter,
);
}
return credentials;
@@ -284,6 +306,7 @@ export class CredentialsService {
}: GetManyCredentialsOptions,
): Promise<CredentialsEntity[]> {
const { dependency: dependencyFilter } = filters ?? {};
const typeFilter = this.extractTypeFilter(listQueryOptions);
let isPersonalProject = false;
let personalProjectOwnerId: string | null = null;
@@ -344,7 +367,12 @@ export class CredentialsService {
);
if (includeGlobal) {
return await this.addGlobalCredentials(credentials, includeData, dependencyFilter);
return await this.addGlobalCredentials(
credentials,
includeData,
dependencyFilter,
typeFilter,
);
}
return credentials;
@@ -187,6 +187,45 @@ describe('CredentialsRepository', () => {
);
expect(credentials).toHaveLength(1);
});
test('should narrow results by credential type when type is provided', async () => {
// ARRANGE
const slackCred = mock<CredentialsEntity>({
id: 'global-slack',
isGlobal: true,
type: 'slackOAuth2Api',
});
entityManager.find.mockResolvedValueOnce([slackCred]);
// ACT
const credentials = await repository.findAllGlobalCredentials({ type: 'slackOAuth2Api' });
// ASSERT — the where clause must include both isGlobal AND a type matcher
expect(entityManager.find).toHaveBeenCalledWith(
CredentialsEntity,
expect.objectContaining({
where: expect.objectContaining({
isGlobal: true,
type: expect.anything(),
}),
}),
);
expect(credentials).toEqual([slackCred]);
});
test('should not add a type filter when type is omitted', async () => {
// ARRANGE
entityManager.find.mockResolvedValueOnce([]);
// ACT
await repository.findAllGlobalCredentials();
// ASSERT — where contains isGlobal but NOT type
const findCall = entityManager.find.mock.calls.find((call) => call[0] === CredentialsEntity);
const findArg = findCall?.[1] as { where?: Record<string, unknown> };
expect(findArg?.where).toBeDefined();
expect(findArg?.where).not.toHaveProperty('type');
});
});
describe('findAllPersonalCredentials', () => {