Skip to content

Commit 4c6de84

Browse files
seanspeaksclaude
seanspeaks
and
claude
committed
fix: Support legacy "user" field for backward compatibility in credential identifiers
Added backward compatibility to accept both `identifiers.user` (legacy) and `identifiers.userId` (preferred) in credential repository operations. This fixes "userId required in identifiers" errors when API modules use the legacy field name. Changes: - Modified `upsertCredential` validation to accept user OR userId - Updated `_convertIdentifiersToWhere` to convert user to userId - userId takes precedence when both fields are present - Added comprehensive test coverage for backward compatibility This resolves credential creation failures in integrations that still use the legacy `user` field in their module definitions (e.g., Attio). Test Results: All 7 new tests passing 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]>
1 parent 330e50d commit 4c6de84

File tree

3 files changed

+308
-2
lines changed

3 files changed

+308
-2
lines changed

‎packages/core/credential/repositories/credential-repository-postgres.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ class CredentialRepositoryPostgres extends CredentialRepositoryInterface {
111111
if (!identifiers)
112112
throw new Error('identifiers required to upsert credential');
113113

114-
if (!identifiers.userId) {
114+
// Support both userId (preferred) and user (legacy) for backward compatibility
115+
if (!identifiers.userId && !identifiers.user) {
115116
throw new Error('userId required in identifiers');
116117
}
117118
if (!identifiers.externalId) {
@@ -154,7 +155,8 @@ class CredentialRepositoryPostgres extends CredentialRepositoryInterface {
154155

155156
const created = await this.prisma.credential.create({
156157
data: {
157-
userId: this._convertId(identifiers.userId),
158+
// Use userId from where clause (supports both userId and user fields)
159+
userId: where.userId,
158160
externalId,
159161
authIsValid: authIsValid,
160162
data: oauthData,
@@ -257,8 +259,11 @@ class CredentialRepositoryPostgres extends CredentialRepositoryInterface {
257259
const where = {};
258260

259261
if (identifiers.id) where.id = this._convertId(identifiers.id);
262+
// Support both userId (preferred) and user (legacy) for backward compatibility
260263
if (identifiers.userId)
261264
where.userId = this._convertId(identifiers.userId);
265+
else if (identifiers.user)
266+
where.userId = this._convertId(identifiers.user);
262267
if (identifiers.externalId) where.externalId = identifiers.externalId;
263268

264269
return where;

‎packages/core/credential/repositories/tests/credential-repository-user-userid-compatibility.test.js‎

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
jest.mock('../../../database/config', () => ({
2+
DB_TYPE: 'postgresql',
3+
getDatabaseType: jest.fn(() => 'postgresql'),
4+
PRISMA_LOG_LEVEL: 'error,warn',
5+
PRISMA_QUERY_LOGGING: false,
6+
}));
7+
8+
const { CredentialRepositoryPostgres } = require('../credential-repository-postgres');
9+
10+
describe('CredentialRepositoryPostgres - user/userId compatibility', () => {
11+
let repository;
12+
let mockPrisma;
13+
14+
beforeEach(() => {
15+
// Mock Prisma client
16+
mockPrisma = {
17+
credential: {
18+
findFirst: jest.fn(),
19+
create: jest.fn(),
20+
update: jest.fn(),
21+
},
22+
};
23+
24+
repository = new CredentialRepositoryPostgres();
25+
repository.prisma = mockPrisma;
26+
});
27+
28+
describe('upsertCredential with legacy "user" field', () => {
29+
it('should accept identifiers.user instead of identifiers.userId for backward compatibility', async () => {
30+
const credentialDetails = {
31+
identifiers: {
32+
user: '13', // Legacy field name
33+
externalId: 'workspace-123',
34+
},
35+
details: {
36+
access_token: 'token-abc',
37+
refresh_token: 'refresh-xyz',
38+
authIsValid: true,
39+
},
40+
};
41+
42+
mockPrisma.credential.findFirst.mockResolvedValue(null);
43+
mockPrisma.credential.create.mockResolvedValue({
44+
id: 1,
45+
userId: 13,
46+
externalId: 'workspace-123',
47+
data: {
48+
access_token: 'token-abc',
49+
refresh_token: 'refresh-xyz',
50+
},
51+
authIsValid: true,
52+
});
53+
54+
// Should not throw "userId required in identifiers" error
55+
await expect(
56+
repository.upsertCredential(credentialDetails)
57+
).resolves.not.toThrow();
58+
59+
// Should convert user to userId and create credential
60+
expect(mockPrisma.credential.create).toHaveBeenCalledWith(
61+
expect.objectContaining({
62+
data: expect.objectContaining({
63+
userId: 13, // Converted from string '13' to int
64+
externalId: 'workspace-123',
65+
}),
66+
})
67+
);
68+
});
69+
70+
it('should prefer userId over user when both are provided', async () => {
71+
const credentialDetails = {
72+
identifiers: {
73+
userId: '15', // Preferred field
74+
user: '13', // Legacy field (should be ignored)
75+
externalId: 'workspace-123',
76+
},
77+
details: {
78+
access_token: 'token-abc',
79+
authIsValid: true,
80+
},
81+
};
82+
83+
mockPrisma.credential.findFirst.mockResolvedValue(null);
84+
mockPrisma.credential.create.mockResolvedValue({
85+
id: 1,
86+
userId: 15,
87+
externalId: 'workspace-123',
88+
data: { access_token: 'token-abc' },
89+
authIsValid: true,
90+
});
91+
92+
await repository.upsertCredential(credentialDetails);
93+
94+
// Should use userId (15), not user (13)
95+
expect(mockPrisma.credential.create).toHaveBeenCalledWith(
96+
expect.objectContaining({
97+
data: expect.objectContaining({
98+
userId: 15, // Used userId field
99+
}),
100+
})
101+
);
102+
});
103+
104+
it('should throw error when neither userId nor user is provided', async () => {
105+
const credentialDetails = {
106+
identifiers: {
107+
externalId: 'workspace-123',
108+
// Missing both userId and user
109+
},
110+
details: {
111+
access_token: 'token-abc',
112+
authIsValid: true,
113+
},
114+
};
115+
116+
await expect(
117+
repository.upsertCredential(credentialDetails)
118+
).rejects.toThrow('userId required in identifiers');
119+
});
120+
121+
it('should update existing credential using user field in where clause', async () => {
122+
const credentialDetails = {
123+
identifiers: {
124+
user: '13', // Legacy field
125+
externalId: 'workspace-123',
126+
},
127+
details: {
128+
access_token: 'new-token',
129+
authIsValid: true,
130+
},
131+
};
132+
133+
const existingCredential = {
134+
id: 1,
135+
userId: 13,
136+
externalId: 'workspace-123',
137+
data: { access_token: 'old-token' },
138+
authIsValid: true,
139+
};
140+
141+
mockPrisma.credential.findFirst.mockResolvedValue(existingCredential);
142+
mockPrisma.credential.update.mockResolvedValue({
143+
...existingCredential,
144+
data: { access_token: 'new-token' },
145+
});
146+
147+
await repository.upsertCredential(credentialDetails);
148+
149+
// Should find existing credential using converted userId
150+
expect(mockPrisma.credential.findFirst).toHaveBeenCalledWith({
151+
where: expect.objectContaining({
152+
userId: 13, // Converted from user field
153+
externalId: 'workspace-123',
154+
}),
155+
});
156+
157+
// Should update the credential
158+
expect(mockPrisma.credential.update).toHaveBeenCalled();
159+
});
160+
});
161+
162+
describe('_convertIdentifiersToWhere compatibility', () => {
163+
it('should convert user field to userId in where clause', () => {
164+
const identifiers = {
165+
user: '13',
166+
externalId: 'workspace-123',
167+
};
168+
169+
const where = repository._convertIdentifiersToWhere(identifiers);
170+
171+
expect(where).toEqual({
172+
userId: 13, // Converted from user
173+
externalId: 'workspace-123',
174+
});
175+
});
176+
177+
it('should prioritize userId over user when both present', () => {
178+
const identifiers = {
179+
userId: '15',
180+
user: '13',
181+
externalId: 'workspace-123',
182+
};
183+
184+
const where = repository._convertIdentifiersToWhere(identifiers);
185+
186+
expect(where).toEqual({
187+
userId: 15, // Used userId, not user
188+
externalId: 'workspace-123',
189+
});
190+
});
191+
});
192+
});

‎packages/core/modules/tests/module-on-token-update.test.js‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
jest.mock('../../database/config', () => ({
2+
DB_TYPE: 'mongodb',
3+
getDatabaseType: jest.fn(() => 'mongodb'),
4+
PRISMA_LOG_LEVEL: 'error,warn',
5+
PRISMA_QUERY_LOGGING: false,
6+
}));
7+
8+
const { Module } = require('../module');
9+
10+
describe('Module.onTokenUpdate with organization userId', () => {
11+
let mockCredentialRepository;
12+
let mockApi;
13+
let mockDefinition;
14+
let module;
15+
16+
beforeEach(() => {
17+
// Mock credential repository
18+
mockCredentialRepository = {
19+
upsertCredential: jest.fn().mockResolvedValue({
20+
id: 'cred-123',
21+
userId: '13', // Organization user ID
22+
authIsValid: true,
23+
}),
24+
};
25+
26+
// Mock API instance
27+
mockApi = {
28+
access_token: 'test-access-token',
29+
refresh_token: 'test-refresh-token',
30+
DLGT_TOKEN_UPDATE: 'DLGT_TOKEN_UPDATE',
31+
};
32+
33+
// Mock module definition with required auth methods
34+
mockDefinition = {
35+
moduleName: 'testmodule',
36+
modelName: 'TestModule',
37+
API: class MockAPI {
38+
constructor() {}
39+
},
40+
requiredAuthMethods: {
41+
getToken: jest.fn(),
42+
getEntityDetails: jest.fn(),
43+
getCredentialDetails: jest.fn((api, userId) => {
44+
// This should return userId in identifiers
45+
return {
46+
identifiers: {
47+
userId: userId, // Should be passed through
48+
},
49+
details: {
50+
access_token: api.access_token,
51+
refresh_token: api.refresh_token,
52+
},
53+
};
54+
}),
55+
apiPropertiesToPersist: {
56+
credential: ['access_token', 'refresh_token'],
57+
entity: [],
58+
},
59+
testAuthRequest: jest.fn(),
60+
},
61+
};
62+
63+
// Create module instance with organization userId
64+
const entityObj = {
65+
id: 'entity-123',
66+
userId: '13', // Organization user ID
67+
};
68+
69+
module = new Module({
70+
definition: mockDefinition,
71+
userId: '13', // Organization user ID
72+
entity: entityObj,
73+
});
74+
75+
// Replace the credential repository with our mock
76+
module.credentialRepository = mockCredentialRepository;
77+
module.api = mockApi;
78+
});
79+
80+
it('should pass userId to credential repository when onTokenUpdate is called', async () => {
81+
await module.onTokenUpdate();
82+
83+
expect(mockCredentialRepository.upsertCredential).toHaveBeenCalledWith(
84+
expect.objectContaining({
85+
identifiers: expect.objectContaining({
86+
userId: '13', // Should include the organization userId
87+
}),
88+
details: expect.objectContaining({
89+
access_token: 'test-access-token',
90+
refresh_token: 'test-refresh-token',
91+
authIsValid: true,
92+
}),
93+
})
94+
);
95+
});
96+
97+
it('should not throw "userId required in identifiers" error', async () => {
98+
await expect(module.onTokenUpdate()).resolves.not.toThrow();
99+
});
100+
101+
it('should call getCredentialDetails with correct userId', async () => {
102+
await module.onTokenUpdate();
103+
104+
expect(mockDefinition.requiredAuthMethods.getCredentialDetails).toHaveBeenCalledWith(
105+
mockApi,
106+
'13' // Organization userId
107+
);
108+
});
109+
});

0 commit comments

Comments
 (0)