Skip to content

Commit 0763a5b

Browse files
committed
Avoids showing incorrect GitLab avatars
- Prevents showing a stranger's avatar when multiple GitLab users share the same name - Prioritizes matching authors by their public email address for maximum confidence - Falls back to matching by name only when there is a single, unambiguous match - Performs a secondary email-based search to resolve ambiguous name matches before falling back to Gravatar Fixes #2205
1 parent 3e1af8a commit 0763a5b

3 files changed

Lines changed: 108 additions & 13 deletions

File tree

src/plus/integrations/providers/gitlab/__tests__/gitlab.utils.test.ts

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import * as assert from 'assert';
2-
import { getGitLabPullRequestIdentityFromMaybeUrl, isMaybeGitLabPullRequestUrl } from '../gitlab.utils.js';
2+
import {
3+
getGitLabPullRequestIdentityFromMaybeUrl,
4+
isMaybeGitLabPullRequestUrl,
5+
selectGitLabUserForCommit,
6+
} from '../gitlab.utils.js';
7+
import type { GitLabUser } from '../models.js';
38

49
suite('Test GitLab PR URL parsing to identity: getPullRequestIdentityFromMaybeUrl()', () => {
510
function t(message: string, query: string, prNumber: string | undefined, ownerAndRepo?: string) {
@@ -110,3 +115,63 @@ suite('Test GitLab PR URL parsing to identity: getPullRequestIdentityFromMaybeUr
110115
);
111116
});
112117
});
118+
119+
suite('Test GitLab commit author resolution: selectGitLabUserForCommit()', () => {
120+
function user(id: number, name: string, publicEmail?: string, state: string = 'active'): GitLabUser {
121+
return {
122+
id: id,
123+
name: name,
124+
username: name.toLowerCase().replace(/\s+/g, ''),
125+
publicEmail: publicEmail,
126+
state: state,
127+
avatarUrl: `https://gitlab.com/avatars/${id}.png`,
128+
webUrl: `https://gitlab.com/u${id}`,
129+
};
130+
}
131+
132+
test('returns undefined for no results', () => {
133+
assert.strictEqual(selectGitLabUserForCommit([], 'Craig Wilson', 'craig@example.com'), undefined);
134+
});
135+
136+
test('returns the single exact name match when there is no email to match', () => {
137+
const u = user(1, 'Craig Wilson');
138+
assert.strictEqual(selectGitLabUserForCommit([u], 'Craig Wilson', undefined), u);
139+
});
140+
141+
test('returns the single exact name match when email does not match any public email', () => {
142+
const u = user(1, 'Craig Wilson');
143+
assert.strictEqual(selectGitLabUserForCommit([u], 'Craig Wilson', 'craig@example.com'), u);
144+
});
145+
146+
test('prefers the public email match over a name match', () => {
147+
const wrong = user(1, 'Craig Wilson');
148+
const right = user(2, 'Craig Wilson', 'craig@example.com');
149+
assert.strictEqual(selectGitLabUserForCommit([wrong, right], 'Craig Wilson', 'craig@example.com'), right);
150+
});
151+
152+
test('email match wins even when a different user is the exact name match', () => {
153+
const nameMatch = user(1, 'Craig Wilson');
154+
const emailMatch = user(2, 'Craig W', 'craig@example.com');
155+
assert.strictEqual(
156+
selectGitLabUserForCommit([nameMatch, emailMatch], 'Craig Wilson', 'craig@example.com'),
157+
emailMatch,
158+
);
159+
});
160+
161+
test('returns undefined when multiple users share the name and none match the email (#2205)', () => {
162+
const a = user(1, 'Craig Wilson', 'someoneelse@example.com');
163+
const b = user(2, 'Craig Wilson');
164+
assert.strictEqual(selectGitLabUserForCommit([a, b], 'Craig Wilson', 'craig@example.com'), undefined);
165+
});
166+
167+
test('matches name case-insensitively', () => {
168+
const u = user(1, 'CRAIG WILSON');
169+
assert.strictEqual(selectGitLabUserForCommit([u], 'craig wilson', undefined), u);
170+
});
171+
172+
test('matches public email case-insensitively', () => {
173+
const a = user(1, 'Craig Wilson');
174+
const b = user(2, 'Craig Wilson', 'Craig@Example.com');
175+
assert.strictEqual(selectGitLabUserForCommit([a, b], 'Craig Wilson', 'craig@example.com'), b);
176+
});
177+
});

src/plus/integrations/providers/gitlab/gitlab.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
} from '../../../../messages.js';
3030
import { configuration } from '../../../../system/-webview/configuration.js';
3131
import type { TokenWithInfo } from '../../authentication/models.js';
32+
import { selectGitLabUserForCommit } from './gitlab.utils.js';
3233
import type {
3334
GitLabCommit,
3435
GitLabIssue,
@@ -114,18 +115,16 @@ export class GitLabApi implements Disposable {
114115
scope,
115116
);
116117

117-
let user: GitLabUser | undefined;
118-
119-
const users = await this.findUser(provider, token, commit.author_name, options);
120-
for (const u of users) {
121-
if (u.name === commit.author_name || (u.publicEmail && u.publicEmail === commit.author_email)) {
122-
user = u;
123-
if (u.state === 'active') break;
124-
} else if (
125-
equalsIgnoreCase(u.name, commit.author_name) ||
126-
(u.publicEmail && equalsIgnoreCase(u.publicEmail, commit.author_email))
127-
) {
128-
user = u;
118+
let users = await this.findUser(provider, token, commit.author_name, options, cancellation);
119+
let user = selectGitLabUserForCommit(users, commit.author_name, commit.author_email);
120+
121+
// If the name search was ambiguous (multiple users share the name), try an email-scoped search to
122+
// disambiguate before giving up — otherwise we'd show a stranger's avatar (see #2205)
123+
if (user == null && commit.author_email) {
124+
const exactNameMatches = users.filter(u => equalsIgnoreCase(u.name, commit.author_name)).length;
125+
if (exactNameMatches > 1) {
126+
users = await this.findUser(provider, token, commit.author_email, options, cancellation);
127+
user = selectGitLabUserForCommit(users, commit.author_name, commit.author_email);
129128
}
130129
}
131130

src/plus/integrations/providers/gitlab/gitlab.utils.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import type { PullRequestUrlIdentity } from '@gitlens/git/utils/pullRequest.utils.js';
2+
import { equalsIgnoreCase } from '@gitlens/utils/string.js';
23
import type { GitCloudHostIntegrationId, GitSelfManagedHostIntegrationId } from '../../../../constants.integrations.js';
4+
import type { GitLabUser } from './models.js';
35

46
export type GitLabIntegrationIds =
57
| GitCloudHostIntegrationId.GitLab
@@ -39,3 +41,32 @@ export function getGitLabPullRequestIdentityFromMaybeUrl(
3941

4042
return prNumber != null ? { ownerAndRepo: ownerAndRepo, prNumber: prNumber, provider: id } : undefined;
4143
}
44+
45+
/**
46+
* Selects the GitLab user that authored a commit from a set of search results.
47+
*
48+
* GitLab has no way to resolve a commit's author to a user directly, so we search users by name/email and
49+
* disambiguate here. To avoid showing a stranger's avatar when multiple users share a name (see #2205), we only
50+
* return a user when we can identify them confidently — a matching public email, or a single exact name match.
51+
* Otherwise we return `undefined` so avatar resolution falls back to the commit email's Gravatar.
52+
*/
53+
export function selectGitLabUserForCommit(
54+
users: GitLabUser[],
55+
authorName: string,
56+
authorEmail: string | undefined,
57+
): GitLabUser | undefined {
58+
if (users.length === 0) return undefined;
59+
60+
// Strongest signal: a public email that matches the commit email — unambiguous identity
61+
if (authorEmail) {
62+
const byEmail = users.find(u => u.publicEmail && equalsIgnoreCase(u.publicEmail, authorEmail));
63+
if (byEmail != null) return byEmail;
64+
}
65+
66+
// Otherwise fall back to an exact name match — but only when it's unambiguous
67+
const nameMatches = users.filter(u => equalsIgnoreCase(u.name, authorName));
68+
if (nameMatches.length === 1) return nameMatches[0];
69+
70+
// No matches, or multiple name matches we can't disambiguate by email — don't guess
71+
return undefined;
72+
}

0 commit comments

Comments
 (0)