Skip to content

Commit e9a0932

Browse files
fix: correct git provider access check for existing deploys (#4570)
* fix: use canEditDeployGitSource for git provider access on existing deploys Replaces the simple userId ownership check with a new canEditDeployGitSource function that correctly handles all role/sharing scenarios. Owner always has access; admin and member only if they own the provider or it is shared with the org — being assigned via accessedGitProviders (enterprise) only grants permission to connect new deploys, not to edit the git source of existing ones. Adds 26 unit tests covering owner, admin, member (with/without enterprise license), shared providers, and the key regression case from issue #4469. * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 6b68fca commit e9a0932

4 files changed

Lines changed: 413 additions & 16 deletions

File tree

Lines changed: 369 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,369 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
import {
3+
canEditDeployGitSource,
4+
getAccessibleGitProviderIds,
5+
} from "@dokploy/server/services/git-provider";
6+
7+
const mockDb = vi.hoisted(() => ({
8+
query: {
9+
gitProvider: {
10+
findMany: vi.fn(),
11+
findFirst: vi.fn(),
12+
},
13+
member: {
14+
findFirst: vi.fn(),
15+
},
16+
},
17+
}));
18+
19+
vi.mock("@dokploy/server/db", () => ({ db: mockDb }));
20+
21+
const mockHasValidLicense = vi.hoisted(() => vi.fn());
22+
vi.mock("@dokploy/server/services/proprietary/license-key", () => ({
23+
hasValidLicense: mockHasValidLicense,
24+
}));
25+
26+
const ORG_ID = "org-1";
27+
const USER_OWNER = "user-owner";
28+
const USER_ADMIN = "user-admin";
29+
const USER_MEMBER = "user-member";
30+
const USER_MEMBER_2 = "user-member-2";
31+
32+
const providerOwned = {
33+
gitProviderId: "gp-owned",
34+
userId: USER_MEMBER,
35+
sharedWithOrganization: false,
36+
};
37+
const providerShared = {
38+
gitProviderId: "gp-shared",
39+
userId: USER_OWNER,
40+
sharedWithOrganization: true,
41+
};
42+
const providerPrivate = {
43+
gitProviderId: "gp-private",
44+
userId: USER_OWNER,
45+
sharedWithOrganization: false,
46+
};
47+
const providerOtherMember = {
48+
gitProviderId: "gp-other",
49+
userId: USER_MEMBER_2,
50+
sharedWithOrganization: false,
51+
};
52+
53+
const allProviders = [
54+
providerOwned,
55+
providerShared,
56+
providerPrivate,
57+
providerOtherMember,
58+
];
59+
60+
function session(userId: string) {
61+
return { userId, activeOrganizationId: ORG_ID };
62+
}
63+
64+
beforeEach(() => {
65+
vi.clearAllMocks();
66+
mockDb.query.gitProvider.findMany.mockResolvedValue(allProviders);
67+
mockHasValidLicense.mockResolvedValue(false);
68+
});
69+
70+
describe("getAccessibleGitProviderIds", () => {
71+
describe("owner", () => {
72+
beforeEach(() => {
73+
mockDb.query.member.findFirst.mockResolvedValue({
74+
role: "owner",
75+
accessedGitProviders: [],
76+
});
77+
});
78+
79+
it("returns all org providers", async () => {
80+
const ids = await getAccessibleGitProviderIds(session(USER_OWNER));
81+
expect(ids).toEqual(new Set(allProviders.map((p) => p.gitProviderId)));
82+
});
83+
84+
it("includes providers owned by other members", async () => {
85+
const ids = await getAccessibleGitProviderIds(session(USER_OWNER));
86+
expect(ids.has(providerOwned.gitProviderId)).toBe(true);
87+
expect(ids.has(providerOtherMember.gitProviderId)).toBe(true);
88+
});
89+
});
90+
91+
describe("admin", () => {
92+
beforeEach(() => {
93+
mockDb.query.member.findFirst.mockResolvedValue({
94+
role: "admin",
95+
accessedGitProviders: [],
96+
});
97+
});
98+
99+
it("returns all org providers", async () => {
100+
const ids = await getAccessibleGitProviderIds(session(USER_ADMIN));
101+
expect(ids).toEqual(new Set(allProviders.map((p) => p.gitProviderId)));
102+
});
103+
104+
it("includes providers owned by other members — fixes issue #4469", async () => {
105+
const ids = await getAccessibleGitProviderIds(session(USER_ADMIN));
106+
expect(ids.has(providerPrivate.gitProviderId)).toBe(true);
107+
expect(ids.has(providerOtherMember.gitProviderId)).toBe(true);
108+
});
109+
});
110+
111+
describe("member without enterprise license", () => {
112+
beforeEach(() => {
113+
mockDb.query.member.findFirst.mockResolvedValue({
114+
role: "member",
115+
accessedGitProviders: [providerPrivate.gitProviderId],
116+
});
117+
mockHasValidLicense.mockResolvedValue(false);
118+
});
119+
120+
it("can access their own provider", async () => {
121+
const ids = await getAccessibleGitProviderIds(session(USER_MEMBER));
122+
expect(ids.has(providerOwned.gitProviderId)).toBe(true);
123+
});
124+
125+
it("can access shared providers", async () => {
126+
const ids = await getAccessibleGitProviderIds(session(USER_MEMBER));
127+
expect(ids.has(providerShared.gitProviderId)).toBe(true);
128+
});
129+
130+
it("cannot access private providers of other users even if assigned (no license)", async () => {
131+
const ids = await getAccessibleGitProviderIds(session(USER_MEMBER));
132+
expect(ids.has(providerPrivate.gitProviderId)).toBe(false);
133+
});
134+
135+
it("cannot access providers of other members", async () => {
136+
const ids = await getAccessibleGitProviderIds(session(USER_MEMBER));
137+
expect(ids.has(providerOtherMember.gitProviderId)).toBe(false);
138+
});
139+
});
140+
141+
describe("member with enterprise license", () => {
142+
beforeEach(() => {
143+
mockHasValidLicense.mockResolvedValue(true);
144+
});
145+
146+
it("can access provider explicitly assigned to them", async () => {
147+
mockDb.query.member.findFirst.mockResolvedValue({
148+
role: "member",
149+
accessedGitProviders: [providerPrivate.gitProviderId],
150+
});
151+
const ids = await getAccessibleGitProviderIds(session(USER_MEMBER));
152+
expect(ids.has(providerPrivate.gitProviderId)).toBe(true);
153+
});
154+
155+
it("cannot access provider not assigned and not shared", async () => {
156+
mockDb.query.member.findFirst.mockResolvedValue({
157+
role: "member",
158+
accessedGitProviders: [],
159+
});
160+
const ids = await getAccessibleGitProviderIds(session(USER_MEMBER));
161+
expect(ids.has(providerPrivate.gitProviderId)).toBe(false);
162+
expect(ids.has(providerOtherMember.gitProviderId)).toBe(false);
163+
});
164+
165+
it("can access shared provider even without explicit assignment", async () => {
166+
mockDb.query.member.findFirst.mockResolvedValue({
167+
role: "member",
168+
accessedGitProviders: [],
169+
});
170+
const ids = await getAccessibleGitProviderIds(session(USER_MEMBER));
171+
expect(ids.has(providerShared.gitProviderId)).toBe(true);
172+
});
173+
174+
it("can access own provider regardless of assignments", async () => {
175+
mockDb.query.member.findFirst.mockResolvedValue({
176+
role: "member",
177+
accessedGitProviders: [],
178+
});
179+
const ids = await getAccessibleGitProviderIds(session(USER_MEMBER));
180+
expect(ids.has(providerOwned.gitProviderId)).toBe(true);
181+
});
182+
183+
it("cannot access provider of other member even with license but no assignment", async () => {
184+
mockDb.query.member.findFirst.mockResolvedValue({
185+
role: "member",
186+
accessedGitProviders: [],
187+
});
188+
const ids = await getAccessibleGitProviderIds(session(USER_MEMBER));
189+
expect(ids.has(providerOtherMember.gitProviderId)).toBe(false);
190+
});
191+
});
192+
193+
describe("member with no member record", () => {
194+
beforeEach(() => {
195+
mockDb.query.member.findFirst.mockResolvedValue(null);
196+
mockHasValidLicense.mockResolvedValue(true);
197+
});
198+
199+
it("only returns own providers and shared ones", async () => {
200+
const ids = await getAccessibleGitProviderIds(session(USER_MEMBER));
201+
expect(ids.has(providerOwned.gitProviderId)).toBe(true);
202+
expect(ids.has(providerShared.gitProviderId)).toBe(true);
203+
expect(ids.has(providerPrivate.gitProviderId)).toBe(false);
204+
});
205+
});
206+
207+
describe("enterprise license — member assigned to a provider they do not own", () => {
208+
// getAccessibleGitProviderIds still returns the provider (member can connect NEW deploys)
209+
it("member assigned to owner's private provider can USE the provider for new deploys", async () => {
210+
mockHasValidLicense.mockResolvedValue(true);
211+
mockDb.query.member.findFirst.mockResolvedValue({
212+
role: "member",
213+
accessedGitProviders: [providerPrivate.gitProviderId],
214+
});
215+
const ids = await getAccessibleGitProviderIds(session(USER_MEMBER));
216+
expect(ids.has(providerPrivate.gitProviderId)).toBe(true);
217+
});
218+
219+
it("member NOT assigned to owner's private provider cannot use it at all", async () => {
220+
mockHasValidLicense.mockResolvedValue(true);
221+
mockDb.query.member.findFirst.mockResolvedValue({
222+
role: "member",
223+
accessedGitProviders: [],
224+
});
225+
const ids = await getAccessibleGitProviderIds(session(USER_MEMBER));
226+
expect(ids.has(providerPrivate.gitProviderId)).toBe(false);
227+
});
228+
});
229+
230+
describe("empty org", () => {
231+
beforeEach(() => {
232+
mockDb.query.gitProvider.findMany.mockResolvedValue([]);
233+
mockDb.query.member.findFirst.mockResolvedValue({
234+
role: "admin",
235+
accessedGitProviders: [],
236+
});
237+
});
238+
239+
it("returns empty set when org has no providers", async () => {
240+
const ids = await getAccessibleGitProviderIds(session(USER_ADMIN));
241+
expect(ids.size).toBe(0);
242+
});
243+
});
244+
});
245+
246+
describe("canEditDeployGitSource", () => {
247+
beforeEach(() => {
248+
vi.clearAllMocks();
249+
mockHasValidLicense.mockResolvedValue(true);
250+
});
251+
252+
describe("owner", () => {
253+
it("can edit deploy using any provider", async () => {
254+
mockDb.query.member.findFirst.mockResolvedValue({ role: "owner" });
255+
const result = await canEditDeployGitSource(
256+
providerPrivate.gitProviderId,
257+
session(USER_OWNER),
258+
);
259+
expect(result).toBe(true);
260+
});
261+
});
262+
263+
describe("admin", () => {
264+
beforeEach(() => {
265+
mockDb.query.member.findFirst.mockResolvedValue({ role: "admin" });
266+
});
267+
268+
it("cannot edit deploy using owner's private provider (not shared)", async () => {
269+
mockDb.query.gitProvider.findFirst.mockResolvedValue({
270+
userId: USER_OWNER,
271+
sharedWithOrganization: false,
272+
});
273+
const result = await canEditDeployGitSource(
274+
providerPrivate.gitProviderId,
275+
session(USER_ADMIN),
276+
);
277+
expect(result).toBe(false);
278+
});
279+
280+
it("can edit deploy using a provider shared with the org", async () => {
281+
mockDb.query.gitProvider.findFirst.mockResolvedValue({
282+
userId: USER_OWNER,
283+
sharedWithOrganization: true,
284+
});
285+
const result = await canEditDeployGitSource(
286+
providerShared.gitProviderId,
287+
session(USER_ADMIN),
288+
);
289+
expect(result).toBe(true);
290+
});
291+
292+
it("can edit deploy using their own provider", async () => {
293+
mockDb.query.gitProvider.findFirst.mockResolvedValue({
294+
userId: USER_ADMIN,
295+
sharedWithOrganization: false,
296+
});
297+
const result = await canEditDeployGitSource(
298+
"gp-admin-owned",
299+
session(USER_ADMIN),
300+
);
301+
expect(result).toBe(true);
302+
});
303+
});
304+
305+
describe("member", () => {
306+
beforeEach(() => {
307+
mockDb.query.member.findFirst.mockResolvedValue({ role: "member" });
308+
});
309+
310+
it("can edit deploy using their own provider", async () => {
311+
mockDb.query.gitProvider.findFirst.mockResolvedValue({
312+
userId: USER_MEMBER,
313+
sharedWithOrganization: false,
314+
});
315+
const result = await canEditDeployGitSource(
316+
providerOwned.gitProviderId,
317+
session(USER_MEMBER),
318+
);
319+
expect(result).toBe(true);
320+
});
321+
322+
it("can edit deploy using a provider shared with the org", async () => {
323+
mockDb.query.gitProvider.findFirst.mockResolvedValue({
324+
userId: USER_OWNER,
325+
sharedWithOrganization: true,
326+
});
327+
const result = await canEditDeployGitSource(
328+
providerShared.gitProviderId,
329+
session(USER_MEMBER),
330+
);
331+
expect(result).toBe(true);
332+
});
333+
334+
it("cannot edit deploy using owner's private provider even with enterprise license and assignment", async () => {
335+
// This is the key case: enterprise, provider del owner, no compartido,
336+
// member tiene accessedGitProviders asignado — pero NO puede cambiar la branch del deploy del owner
337+
mockDb.query.gitProvider.findFirst.mockResolvedValue({
338+
userId: USER_OWNER,
339+
sharedWithOrganization: false,
340+
});
341+
const result = await canEditDeployGitSource(
342+
providerPrivate.gitProviderId,
343+
session(USER_MEMBER),
344+
);
345+
expect(result).toBe(false);
346+
});
347+
348+
it("cannot edit deploy using another member's private provider", async () => {
349+
mockDb.query.gitProvider.findFirst.mockResolvedValue({
350+
userId: USER_MEMBER_2,
351+
sharedWithOrganization: false,
352+
});
353+
const result = await canEditDeployGitSource(
354+
providerOtherMember.gitProviderId,
355+
session(USER_MEMBER),
356+
);
357+
expect(result).toBe(false);
358+
});
359+
360+
it("returns false if provider does not exist", async () => {
361+
mockDb.query.gitProvider.findFirst.mockResolvedValue(null);
362+
const result = await canEditDeployGitSource(
363+
"nonexistent-id",
364+
session(USER_MEMBER),
365+
);
366+
expect(result).toBe(false);
367+
});
368+
});
369+
});

0 commit comments

Comments
 (0)