fix(azure): filter pull request queries by Renovate author (#45359)

* implement plan

* add tests

* fix initialization order

* cast contexts to WebApi for type safety in authHandler checks

* fix(azure): scope PR filtering to effective credentials (#1)

* fix(azure): scope PR filtering to effective credentials

* fix(azure): address authentication review feedback

* fix: reorder import statements

* Address review comments

* fetch renovate user id in initPlatform

---------

Co-authored-by: Sergei Zharinov <zharinov@users.noreply.github.com>

Author: aluqmanj-ni

lib/modules/platform/azure/azure-got-wrapper.spec.ts

@@ -1,5 +1,7 @@
+import type { WebApi } from 'azure-devops-node-api';
 import type { DeploymentFlags } from 'azure-devops-node-api/interfaces/common/VSSInterfaces.js';
 import { buildTestJwt } from '~test/jwt-util.ts';
+import { logger } from '~test/util.ts';
 import type * as _hostRules from '../../../util/host-rules.ts';
 
 describe('modules/platform/azure/azure-got-wrapper', () => {
@@ -199,4 +201,84 @@ describe('modules/platform/azure/azure-got-wrapper', () => {
       expect(await azure.isHosted()).toBe(false);
     });
   });
+
+  describe('getAuthenticatedUserId', () => {
+    let sdk: typeof import('azure-devops-node-api');
+
+    beforeEach(async () => {
+      sdk = await vi.importActual('azure-devops-node-api');
+      azure.setEndpoint('https://dev.azure.com/renovate8');
+    });
+
+    it('returns the authenticated user ID using PAT credentials', async () => {
+      const connect = vi
+        .spyOn(sdk.WebApi.prototype, 'connect')
+        .mockResolvedValue({ authenticatedUser: { id: 'user-id' } });
+
+      expect(await azure.getAuthenticatedUserId({ token: '123test' })).toBe(
+        'user-id',
+      );
+      const context = connect.mock.contexts[0] as WebApi;
+      expect(context.authHandler.constructor.name).toBe(
+        'PersonalAccessTokenCredentialHandler',
+      );
+    });
+
+    it('returns the authenticated user ID using JWT credentials', async () => {
+      const token = buildTestJwt(
+        { typ: 'JWT', alg: 'RS256' },
+        { aud: '499b84ac', sub: 'test', exp: 9999999999 },
+        'fake-sig',
+      );
+      const connect = vi
+        .spyOn(sdk.WebApi.prototype, 'connect')
+        .mockResolvedValue({ authenticatedUser: { id: 'user-id' } });
+
+      expect(await azure.getAuthenticatedUserId({ token })).toBe('user-id');
+      const context = connect.mock.contexts[0] as WebApi;
+      expect(context.authHandler.constructor.name).toBe(
+        'BearerCredentialHandler',
+      );
+    });
+
+    it('returns the authenticated user ID using username and password', async () => {
+      const connect = vi
+        .spyOn(sdk.WebApi.prototype, 'connect')
+        .mockResolvedValue({ authenticatedUser: { id: 'user-id' } });
+
+      expect(
+        await azure.getAuthenticatedUserId({
+          username: 'user',
+          password: 'pass',
+        }),
+      ).toBe('user-id');
+      const context = connect.mock.contexts[0] as WebApi;
+      expect(context.authHandler).toMatchObject({
+        username: 'user',
+        password: 'pass',
+      });
+    });
+
+    it('returns undefined when the authenticated user ID is unavailable', async () => {
+      vi.spyOn(sdk.WebApi.prototype, 'connect').mockResolvedValue({});
+
+      expect(
+        await azure.getAuthenticatedUserId({ token: '123test' }),
+      ).toBeUndefined();
+    });
+
+    it('returns undefined when connection data cannot be read', async () => {
+      vi.spyOn(sdk.WebApi.prototype, 'connect').mockRejectedValue(
+        new Error('boom'),
+      );
+
+      expect(
+        await azure.getAuthenticatedUserId({ token: '123test' }),
+      ).toBeUndefined();
+      expect(logger.logger.debug).toHaveBeenCalledWith(
+        { err: new Error('boom') },
+        'Azure: could not determine authenticated user ID',
+      );
+    });
+  });
 });

lib/modules/platform/azure/azure-got-wrapper.ts

@@ -31,8 +31,8 @@ function getAuthenticationHandler(config: HostRule): IRequestHandler {
   return getPersonalAccessTokenHandler(config.token!, true);
 }
 
-export function azureObj(): azure.WebApi {
-  const config = hostRules.find({ hostType, url: endpoint });
+export function azureObj(credentials?: HostRule): azure.WebApi {
+  const config = credentials ?? hostRules.find({ hostType, url: endpoint });
   if (!config.token && !(config.username && config.password)) {
     throw new Error(`No config found for azure`);
   }
@@ -43,8 +43,8 @@ export function azureObj(): azure.WebApi {
   });
 }
 
-export function gitApi(): Promise<IGitApi> {
-  return azureObj().getGitApi();
+export function gitApi(credentials?: HostRule): Promise<IGitApi> {
+  return azureObj(credentials).getGitApi();
 }
 
 export function coreApi(): Promise<ICoreApi> {
@@ -59,6 +59,21 @@ export function workItemTrackingApi(): Promise<IWorkItemTrackingApi> {
   return azureObj().getWorkItemTrackingApi();
 }
 
+export async function getAuthenticatedUserId(
+  credentials: HostRule,
+): Promise<string | undefined> {
+  try {
+    const { authenticatedUser } = await azureObj(credentials).connect();
+    if (!authenticatedUser?.id) {
+      logger.debug('Azure: authenticated user ID is unavailable');
+    }
+    return authenticatedUser?.id;
+  } catch (err) {
+    logger.debug({ err }, 'Azure: could not determine authenticated user ID');
+    return undefined;
+  }
+}
+
 /**
  * Whether the endpoint is Azure DevOps Services (cloud) rather than Azure
  * DevOps Server (on-premises). Read from the location service's

lib/modules/platform/azure/index.spec.ts

@@ -23,6 +23,7 @@ import type { Mocked, MockedObject } from 'vitest';
 import { vi } from 'vitest';
 import { mockDeep } from 'vitest-mock-extended';
 import { partial } from '~test/util.ts';
+import { GlobalConfig } from '../../../config/global.ts';
 import {
   REPOSITORY_ARCHIVED,
   REPOSITORY_NOT_FOUND,
@@ -33,6 +34,9 @@ import type * as _hostRules from '../../../util/host-rules.ts';
 import type { Platform, RepoParams } from '../types.ts';
 import { AzurePrVote } from './types.ts';
 
+vi.mock('../../../config/global.ts', async (importOriginal) =>
+  importOriginal<typeof import('../../../config/global.ts')>(),
+);
 vi.mock('./azure-got-wrapper.ts', () => mockDeep());
 vi.mock('./azure-helper.ts', () => mockDeep());
 vi.mock('../../../util/sanitize.ts', () =>
@@ -51,6 +55,7 @@ describe('modules/platform/azure/index', () => {
   beforeEach(async () => {
     // reset module
     vi.resetModules();
+    GlobalConfig.reset();
     hostRules = await vi.importActual('../../../util/host-rules.ts');
     azure = await vi.importActual('./index.ts');
     azureApi = await vi.importMock('./azure-got-wrapper.ts');
@@ -66,6 +71,7 @@ describe('modules/platform/azure/index', () => {
     hostRules.clear();
     hostRules.add({ token: 'token' });
     azureHelper.getPolicyEvaluations.mockResolvedValue([]);
+    azureApi.getAuthenticatedUserId.mockResolvedValue('renovate-user-id');
     // Default to the hosted (cloud) endpoint used across these tests.
     azureApi.isHosted.mockResolvedValue(true);
     await azure.initPlatform({
@@ -156,6 +162,22 @@ describe('modules/platform/azure/index', () => {
           token: 'token',
         }),
       ).toMatchSnapshot();
+      expect(azureApi.getAuthenticatedUserId).toHaveBeenLastCalledWith({
+        token: 'token',
+      });
+    });
+
+    it('should discover the authenticated user with basic credentials', async () => {
+      await azure.initPlatform({
+        endpoint: 'https://dev.azure.com/renovate12345',
+        username: 'user',
+        password: 'pass',
+      });
+
+      expect(azureApi.getAuthenticatedUserId).toHaveBeenLastCalledWith({
+        username: 'user',
+        password: 'pass',
+      });
     });
   });
 
@@ -499,6 +521,81 @@ describe('modules/platform/azure/index', () => {
       });
     });
 
+    it('queries the exact branch when including other authors', async () => {
+      await initRepo();
+      const getPullRequests = vi.fn().mockResolvedValue([
+        {
+          pullRequestId: 1,
+          sourceRefName: 'refs/heads/branch-a',
+          targetRefName: 'refs/heads/branch-b',
+          title: 'branch a pr',
+          status: 1,
+        },
+      ]);
+      azureApi.gitApi.mockResolvedValueOnce(
+        partial<IGitApi>({
+          getPullRequests,
+        }),
+      );
+
+      const res = await azure.findPr({
+        branchName: 'branch-a',
+        state: 'open',
+        targetBranch: 'branch-b',
+        includeOtherAuthors: true,
+      });
+
+      expect(res).toMatchObject({
+        number: 1,
+        sourceBranch: 'branch-a',
+        state: 'open',
+        targetBranch: 'branch-b',
+      });
+      expect(getPullRequests).toHaveBeenCalledExactlyOnceWith(
+        '1',
+        {
+          sourceRefName: 'refs/heads/branch-a',
+          sourceRepositoryId: '1',
+          status: 1,
+          targetRefName: 'refs/heads/branch-b',
+        },
+        'some',
+        0,
+        0,
+        1,
+      );
+    });
+
+    it('returns null when no PR from another author matches', async () => {
+      await initRepo();
+      const getPullRequests = vi.fn().mockResolvedValue([]);
+      azureApi.gitApi.mockResolvedValueOnce(
+        partial<IGitApi>({
+          getPullRequests,
+        }),
+      );
+
+      const res = await azure.findPr({
+        branchName: 'branch-a',
+        state: 'open',
+        includeOtherAuthors: true,
+      });
+
+      expect(res).toBeNull();
+      expect(getPullRequests).toHaveBeenCalledExactlyOnceWith(
+        '1',
+        {
+          sourceRefName: 'refs/heads/branch-a',
+          sourceRepositoryId: '1',
+          status: 1,
+        },
+        'some',
+        0,
+        0,
+        1,
+      );
+    });
+
     it('catches errors', async () => {
       azureApi.gitApi.mockResolvedValueOnce(
         partial<IGitApi>({
@@ -514,13 +611,96 @@ describe('modules/platform/azure/index', () => {
   });
 
   describe('getPrList()', () => {
-    it('returns empty array', async () => {
+    it('filters PRs by repository and authenticated user', async () => {
+      await initRepo();
+      const getPullRequests = vi.fn().mockResolvedValue([]);
       azureApi.gitApi.mockResolvedValueOnce(
         partial<IGitApi>({
-          getPullRequests: vi.fn().mockResolvedValue([]),
+          getPullRequests,
+        }),
+      );
+      expect(await azure.getPrList()).toEqual([]);
+      expect(azureApi.getAuthenticatedUserId).toHaveBeenCalledExactlyOnceWith({
+        token: 'token',
+      });
+      expect(azureApi.gitApi).toHaveBeenLastCalledWith();
+      expect(getPullRequests).toHaveBeenCalledExactlyOnceWith(
+        '1',
+        {
+          creatorId: 'renovate-user-id',
+          sourceRepositoryId: '1',
+          status: 4,
+        },
+        'some',
+        0,
+        0,
+        100,
+      );
+    });
+
+    it('does not filter by authenticated user when ignorePrAuthor is enabled', async () => {
+      GlobalConfig.set({ ignorePrAuthor: true });
+      await initRepo();
+      const getPullRequests = vi.fn().mockResolvedValue([]);
+      azureApi.gitApi.mockResolvedValueOnce(
+        partial<IGitApi>({
+          getPullRequests,
+        }),
+      );
+
+      expect(await azure.getPrList()).toEqual([]);
+      expect(getPullRequests).toHaveBeenCalledExactlyOnceWith(
+        '1',
+        {
+          sourceRepositoryId: '1',
+          status: 4,
+        },
+        'some',
+        0,
+        0,
+        100,
+      );
+    });
+
+    it('does not filter by authenticated user when the ID is unavailable', async () => {
+      azureApi.getAuthenticatedUserId.mockResolvedValueOnce(undefined);
+      await azure.initPlatform({
+        endpoint: 'https://dev.azure.com/renovate12345',
+        token: 'token',
+      });
+      await initRepo();
+      const getPullRequests = vi.fn().mockResolvedValue([]);
+      azureApi.gitApi.mockResolvedValueOnce(
+        partial<IGitApi>({
+          getPullRequests,
         }),
       );
+
       expect(await azure.getPrList()).toEqual([]);
+      expect(getPullRequests).toHaveBeenCalledExactlyOnceWith(
+        '1',
+        {
+          sourceRepositoryId: '1',
+          status: 4,
+        },
+        'some',
+        0,
+        0,
+        100,
+      );
+    });
+
+    it('reuses the cached PR list', async () => {
+      await initRepo();
+      const getPullRequests = vi.fn().mockResolvedValue([]);
+      azureApi.gitApi.mockResolvedValueOnce(
+        partial<IGitApi>({ getPullRequests }),
+      );
+
+      await azure.getPrList();
+      await azure.getPrList();
+
+      expect(getPullRequests).toHaveBeenCalledOnce();
     });
   });