Skip to content

fix: use NES user id for cli amplitude events #526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
facundo-herodevs merged 1 commit into from
Mar 25, 2026
Merged

fix: use NES user id for cli amplitude events #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 24 additions & 13 deletions src/service/analytics.svc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,12 @@ export async function refreshIdentityFromStoredToken(): Promise<boolean> {
return false;
}

const entries = toIdentityEntries(claims);
const signature = buildIdentitySignature(entries);
const signature = buildIdentitySignature(claims);
if (signature === lastIdentitySignature) {
return false;
}
applyIdentityClaims(claims, signature);
emitIdentify(entries, claims.user_id);
emitIdentify(claims);
return true;
} catch (error) {
logAnalyticsError('refreshIdentityFromStoredToken', error);
Expand Down Expand Up @@ -151,10 +150,15 @@ function buildIdentifyEventOptions(userId?: string) {
}

function normalizeClaim(value: unknown): string {
if (typeof value !== 'string') {
return '';
if (typeof value === 'string') {
return value.trim();
}
return value.trim();

if (typeof value === 'number' && Number.isFinite(value)) {
return String(value);
}

return '';
}

function extractIdentityClaims(accessToken: string | undefined): IdentityClaims | undefined {
Expand All @@ -163,8 +167,10 @@ function extractIdentityClaims(accessToken: string | undefined): IdentityClaims
return;
}

const nesClaims = payload.nes as { identity?: unknown } | undefined;

const identity: IdentityClaims = {
user_id: normalizeClaim(payload.sub) || undefined,
user_id: normalizeClaim(nesClaims?.identity) || undefined,
email: normalizeClaim(payload.email) || undefined,
organization_name: normalizeClaim(payload.company) || undefined,
role: normalizeClaim(payload.role) || undefined,
Expand Down Expand Up @@ -203,8 +209,10 @@ function toIdentityEntries(identity: IdentityClaims): Array<[keyof IdentityClaim
return entries;
}

function buildIdentitySignature(entries: Array<[keyof IdentityClaims, string]>): string {
return entries.map(([field, value]) => `${field}:${value}`).join('|');
function buildIdentitySignature(identity: IdentityClaims): string {
return toIdentityEntries(identity)
.map(([field, value]) => `${field}:${value}`)
.join('|');
}

function applyIdentityClaims(claims: IdentityClaims, signature: string): void {
Expand All @@ -213,13 +221,16 @@ function applyIdentityClaims(claims: IdentityClaims, signature: string): void {
analyticsContext = { ...analyticsContext, ...claims };
}

function emitIdentify(entries: Array<[keyof IdentityClaims, string]>, userId?: string): void {
function emitIdentify(claims: IdentityClaims): void {
const amplitudeIdentify = new Identify();
for (const [field, value] of entries) {
amplitudeIdentify.set(field, value);
for (const field of IDENTITY_FIELDS) {
const value = claims[field];
if (value) {
amplitudeIdentify.set(field, value);
}
}

const eventOptions = buildIdentifyEventOptions(userId);
const eventOptions = buildIdentifyEventOptions(claims.user_id);
void toSafeAnalyticsResult(identify(amplitudeIdentify, eventOptions), 'identify').promise;
void toSafeAnalyticsResult(_track('Identify Call', { source: SOURCE }, eventOptions), 'track:Identify Call').promise;
}
Expand Down
70 changes: 60 additions & 10 deletions test/service/analytics.svc.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ describe('analytics.svc', () => {
mockAuthTokenService.getStoredTokens.resolves({
accessToken: createAccessToken({
sub: 'user-1',
nes: { identity: 'nes-user-1' },
email: 'dev@herodevs.com',
company: 'HeroDevs',
role: 'Software Engineer',
Expand All @@ -138,7 +139,7 @@ describe('analytics.svc', () => {
expect(mockAmplitude.track.getCall(0).args[0]).toBe('Identify Call');

const metadata = mockAmplitude.identify.getCall(0).args[1];
expect(metadata.user_id).toBe('user-1');
expect(metadata.user_id).toBe('nes-user-1');
expect(typeof metadata.platform).toBe('string');
expect(typeof metadata.os_name).toBe('string');
expect(typeof metadata.os_version).toBe('string');
Expand All @@ -148,6 +149,7 @@ describe('analytics.svc', () => {
it('should use env CI token identity when stored token is unavailable', async () => {
mockConfig.ciTokenFromEnv = createAccessToken({
sub: 'env-user-1',
nes: { identity: 'nes-env-user-1' },
email: 'env@herodevs.com',
company: 'HeroDevs',
role: 'Platform Engineer',
Expand All @@ -159,8 +161,8 @@ describe('analytics.svc', () => {
expect(mockAmplitude.identify.calledOnce).toBe(true);
expect(mockAmplitude.track.calledOnce).toBe(true);
expect(mockAmplitude.track.getCall(0).args[0]).toBe('Identify Call');
expect(mockAmplitude.identify.getCall(0).args[1].user_id).toBe('env-user-1');
expect(mockAmplitude.track.getCall(0).args[2].user_id).toBe('env-user-1');
expect(mockAmplitude.identify.getCall(0).args[1].user_id).toBe('nes-env-user-1');
expect(mockAmplitude.track.getCall(0).args[2].user_id).toBe('nes-env-user-1');
});

it('should handle case when npm_package_version is undefined', async () => {
Expand Down Expand Up @@ -302,6 +304,7 @@ describe('analytics.svc', () => {
mockAuthTokenService.getStoredTokens.resolves({
accessToken: createAccessToken({
sub: 'user-123',
nes: { identity: 'nes-user-123' },
email: 'dev@herodevs.com',
company: 'HeroDevs',
role: 'Software Engineer',
Expand All @@ -313,7 +316,7 @@ describe('analytics.svc', () => {

expect(mockAmplitude.track.callCount).toBe(2);
const trackCall = mockAmplitude.track.getCall(1);
expect(trackCall.args[2].user_id).toBe('user-123');
expect(trackCall.args[2].user_id).toBe('nes-user-123');
});

it('should not throw when amplitude track throws synchronously', async () => {
Expand Down Expand Up @@ -351,6 +354,7 @@ describe('analytics.svc', () => {
mockAuthTokenService.getStoredTokens.resolves({
accessToken: createAccessToken({
sub: 'user-1',
nes: { identity: 'nes-user-1' },
email: 'dev@herodevs.com',
company: 'HeroDevs',
role: 'Software Engineer',
Expand All @@ -365,12 +369,30 @@ describe('analytics.svc', () => {
expect(identifyBuilder.set.calledWith('email', 'dev@herodevs.com')).toBe(true);
expect(identifyBuilder.set.calledWith('organization_name', 'HeroDevs')).toBe(true);
expect(identifyBuilder.set.calledWith('role', 'Software Engineer')).toBe(true);
expect(identifyBuilder.set.calledWith('user_id', 'user-1')).toBe(true);
expect(identifyBuilder.set.calledWith('user_id', 'nes-user-1')).toBe(true);

const identifyEventCall = mockAmplitude.track.getCall(0);
expect(identifyEventCall.args[0]).toBe('Identify Call');
expect(identifyEventCall.args[1]).toEqual({ source: 'cli' });
expect(identifyEventCall.args[2].user_id).toBe('user-1');
expect(identifyEventCall.args[2].user_id).toBe('nes-user-1');
});

it('should prefer nes.identity over sub when both are present', async () => {
const mod = await setupModule();
mockAuthTokenService.getStoredTokens.resolves({
accessToken: createAccessToken({
sub: 'keycloak-user-1',
nes: { identity: 'nes-user-1' },
email: 'dev@herodevs.com',
}),
});

await mod.refreshIdentityFromStoredToken();

expect(mockAmplitude.identify.calledOnce).toBe(true);
expect(mockAmplitude.track.calledOnce).toBe(true);
expect(mockAmplitude.identify.getCall(0).args[1].user_id).toBe('nes-user-1');
expect(mockAmplitude.track.getCall(0).args[2].user_id).toBe('nes-user-1');
});

it('should ignore non-canonical claim aliases for identity mapping', async () => {
Expand Down Expand Up @@ -401,9 +423,34 @@ describe('analytics.svc', () => {
expect(mockAmplitude.track.called).toBe(false);
});

it('should not fall back to sub when nes.identity is missing', async () => {
const mod = await setupModule();
mockAuthTokenService.getStoredTokens.resolves({
accessToken: createAccessToken({
sub: 'keycloak-user-1',
email: 'dev@herodevs.com',
company: 'HeroDevs',
role: 'Software Engineer',
}),
});

await mod.refreshIdentityFromStoredToken();

expect(mockAmplitude.identify.calledOnce).toBe(true);
expect(mockAmplitude.track.calledOnce).toBe(true);
expect(mockAmplitude.identify.getCall(0).args[1].user_id).toBeUndefined();
expect(mockAmplitude.track.getCall(0).args[2].user_id).toBeUndefined();
const identifyBuilder = mockAmplitude.Identify.getCall(0).returnValue as { set: sinon.SinonStub };
expect(identifyBuilder.set.calledWith('email', 'dev@herodevs.com')).toBe(true);
expect(identifyBuilder.set.calledWith('organization_name', 'HeroDevs')).toBe(true);
expect(identifyBuilder.set.calledWith('role', 'Software Engineer')).toBe(true);
expect(mockAmplitude.track.getCall(0).args[1]).toEqual({ source: 'cli' });
});

it('should fall back to env CI token when stored token payload is malformed', async () => {
mockConfig.ciTokenFromEnv = createAccessToken({
sub: 'env-fallback-user',
nes: { identity: 'nes-env-fallback-user' },
email: 'env-fallback@herodevs.com',
company: 'HeroDevs',
role: 'Engineer',
Expand All @@ -417,18 +464,20 @@ describe('analytics.svc', () => {

expect(mockAmplitude.identify.calledOnce).toBe(true);
expect(mockAmplitude.track.calledOnce).toBe(true);
expect(mockAmplitude.identify.getCall(0).args[1].user_id).toBe('env-fallback-user');
expect(mockAmplitude.identify.getCall(0).args[1].user_id).toBe('nes-env-fallback-user');
});

it('should prefer stored token identity over env CI token identity', async () => {
mockConfig.ciTokenFromEnv = createAccessToken({
sub: 'env-user',
nes: { identity: 'nes-env-user' },
email: 'env@herodevs.com',
});
const mod = await setupModule();
mockAuthTokenService.getStoredTokens.resolves({
accessToken: createAccessToken({
sub: 'stored-user',
nes: { identity: 'nes-stored-user' },
email: 'stored@herodevs.com',
company: 'HeroDevs',
}),
Expand All @@ -438,15 +487,16 @@ describe('analytics.svc', () => {

expect(mockAmplitude.identify.calledOnce).toBe(true);
expect(mockAmplitude.track.calledOnce).toBe(true);
expect(mockAmplitude.identify.getCall(0).args[1].user_id).toBe('stored-user');
expect(mockAmplitude.track.getCall(0).args[2].user_id).toBe('stored-user');
expect(mockAmplitude.identify.getCall(0).args[1].user_id).toBe('nes-stored-user');
expect(mockAmplitude.track.getCall(0).args[2].user_id).toBe('nes-stored-user');
});

it('should clear cached identity when no claims are available', async () => {
const mod = await setupModule();
mockAuthTokenService.getStoredTokens.resolves({
accessToken: createAccessToken({
sub: 'user-1',
nes: { identity: 'nes-user-1' },
email: 'dev@herodevs.com',
company: 'HeroDevs',
role: 'Software Engineer',
Expand All @@ -455,7 +505,7 @@ describe('analytics.svc', () => {

await mod.refreshIdentityFromStoredToken();
mod.track('authenticated-event');
expect(mockAmplitude.track.getCall(1).args[2].user_id).toBe('user-1');
expect(mockAmplitude.track.getCall(1).args[2].user_id).toBe('nes-user-1');

mockAuthTokenService.getStoredTokens.resolves(undefined);
await mod.refreshIdentityFromStoredToken();
Expand Down
Loading