Skip to content

Commit ab06e0b

Browse files
AlexTugarevAlexTugarev
committed
wip: adding more tests
1 parent 0e19a2b commit ab06e0b

File tree

3 files changed

+135
-31
lines changed

3 files changed

+135
-31
lines changed

‎components/server/src/auth/auth-provider-service.spec.db.ts

Lines changed: 134 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66

77
import { TypeORM } from "@gitpod/gitpod-db/lib";
8-
import { Organization, User } from "@gitpod/gitpod-protocol";
8+
import { AuthProviderInfo, Organization, User } from "@gitpod/gitpod-protocol";
99
import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
1010
import * as chai from "chai";
1111
import { Container } from "inversify";
@@ -25,14 +25,15 @@ const expect = chai.expect;
2525

2626
describe("AuthProviderService", async () => {
2727
let service: AuthProviderService;
28+
let userService: UserService;
2829
let container: Container;
29-
let owner: User;
30+
let currentUser: User;
3031
let org: Organization;
3132

3233
const newEntry = () =>
3334
<AuthProviderEntry.NewEntry>{
3435
host: "github.com",
35-
ownerId: owner.id,
36+
ownerId: currentUser.id,
3637
type: "GitHub",
3738
clientId: "123",
3839
clientSecret: "secret-123",
@@ -50,7 +51,7 @@ describe("AuthProviderService", async () => {
5051
organizationId: undefined,
5152
type: "GitHub",
5253
status: "pending",
53-
ownerId: owner.id,
54+
ownerId: currentUser.id,
5455
};
5556
const expectedParams = () =>
5657
<Partial<AuthProviderParams>>{
@@ -64,7 +65,7 @@ describe("AuthProviderService", async () => {
6465
const newOrgEntry = () =>
6566
<AuthProviderEntry.NewOrgEntry>{
6667
host: "github.com",
67-
ownerId: owner.id,
68+
ownerId: currentUser.id,
6869
type: "GitHub",
6970
clientId: "123",
7071
clientSecret: "secret-123",
@@ -83,7 +84,7 @@ describe("AuthProviderService", async () => {
8384
organizationId: org.id,
8485
type: "GitHub",
8586
status: "pending",
86-
ownerId: owner.id,
87+
ownerId: currentUser.id,
8788
};
8889
const expectedOrgParams = () =>
8990
<Partial<AuthProviderParams>>{
@@ -94,22 +95,33 @@ describe("AuthProviderService", async () => {
9495
oauth: { ...expectedOrgEntry().oauth, clientSecret: "secret-123" },
9596
};
9697

98+
const addBuiltInProvider = (host: string = "github.com") => {
99+
const config = container.get<Config>(Config);
100+
config.builtinAuthProvidersConfigured = true;
101+
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
102+
config.authProviderConfigs.push((<Partial<AuthProviderParams>>{
103+
host,
104+
id: "Public-GitHub",
105+
verified: true,
106+
}) as any);
107+
};
108+
97109
beforeEach(async () => {
98110
container = createTestContainer();
99111
Experiments.configureTestingClient({
100112
centralizedPermissions: true,
101113
});
102114
service = container.get(AuthProviderService);
103-
const userService = container.get<UserService>(UserService);
104-
owner = await userService.createUser({
115+
userService = container.get<UserService>(UserService);
116+
currentUser = await userService.createUser({
105117
identity: {
106118
authId: "gh-user-1",
107119
authName: "user",
108120
authProviderId: "public-github",
109121
},
110122
});
111123
const os = container.get<OrganizationService>(OrganizationService);
112-
org = await os.createOrganization(owner.id, "myorg");
124+
org = await os.createOrganization(currentUser.id, "myorg");
113125
});
114126

115127
afterEach(async () => {
@@ -122,30 +134,25 @@ describe("AuthProviderService", async () => {
122134
const providersAtStart = await service.getAllAuthProviderParams();
123135
expect(providersAtStart).to.be.empty;
124136

125-
await service.createAuthProviderOfUser(owner.id, newEntry());
137+
await service.createAuthProviderOfUser(currentUser.id, newEntry());
126138

127139
const providers = await service.getAllAuthProviderParams();
128140
expect(providers).to.have.lengthOf(1);
129141
expect(providers[0]).to.deep.include(expectedParams());
130142
});
131143

132144
it("should fail in case of conflict with built-in provider", async () => {
133-
const config = container.get<Config>(Config);
134-
config.builtinAuthProvidersConfigured = true;
135-
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
136-
config.authProviderConfigs.push({
137-
host: "github.com",
138-
} as any);
145+
addBuiltInProvider();
139146

140147
const providersAtStart = await service.getAllAuthProviderParams();
141148
expect(providersAtStart).to.be.empty;
142149

143-
await expectError(ErrorCodes.CONFLICT, service.createAuthProviderOfUser(owner.id, newEntry()));
150+
await expectError(ErrorCodes.CONFLICT, service.createAuthProviderOfUser(currentUser.id, newEntry()));
144151
});
145152
it("should fail if host is not reachable", async () => {
146153
await expectError(
147154
ErrorCodes.BAD_REQUEST,
148-
service.createAuthProviderOfUser(owner.id, {
155+
service.createAuthProviderOfUser(currentUser.id, {
149156
...newEntry(),
150157
host: "please-dont-register-this-domain.com:666",
151158
}),
@@ -155,9 +162,9 @@ describe("AuthProviderService", async () => {
155162
const providersAtStart = await service.getAllAuthProviderParams();
156163
expect(providersAtStart).to.be.empty;
157164

158-
await service.createAuthProviderOfUser(owner.id, newEntry());
165+
await service.createAuthProviderOfUser(currentUser.id, newEntry());
159166

160-
await expectError(ErrorCodes.CONFLICT, service.createAuthProviderOfUser(owner.id, newEntry()));
167+
await expectError(ErrorCodes.CONFLICT, service.createAuthProviderOfUser(currentUser.id, newEntry()));
161168
});
162169
});
163170

@@ -166,7 +173,7 @@ describe("AuthProviderService", async () => {
166173
const providersAtStart = await service.getAllAuthProviderParams();
167174
expect(providersAtStart).to.be.empty;
168175

169-
await service.createOrgAuthProvider(owner.id, newOrgEntry());
176+
await service.createOrgAuthProvider(currentUser.id, newOrgEntry());
170177

171178
const providers = await service.getAllAuthProviderParams();
172179
expect(providers).to.have.lengthOf(1);
@@ -175,7 +182,7 @@ describe("AuthProviderService", async () => {
175182
it("should fail if host is not reachable", async () => {
176183
await expectError(
177184
ErrorCodes.BAD_REQUEST,
178-
service.createOrgAuthProvider(owner.id, {
185+
service.createOrgAuthProvider(currentUser.id, {
179186
...newOrgEntry(),
180187
host: "please-dont-register-this-domain.com:666",
181188
}),
@@ -185,21 +192,121 @@ describe("AuthProviderService", async () => {
185192
const providersAtStart = await service.getAllAuthProviderParams();
186193
expect(providersAtStart).to.be.empty;
187194

188-
await service.createOrgAuthProvider(owner.id, newOrgEntry());
195+
await service.createOrgAuthProvider(currentUser.id, newOrgEntry());
189196

190-
await expectError(ErrorCodes.CONFLICT, service.createAuthProviderOfUser(owner.id, newOrgEntry()));
197+
await expectError(ErrorCodes.CONFLICT, service.createAuthProviderOfUser(currentUser.id, newOrgEntry()));
191198
});
192199
});
193200
describe("getAuthProvider", async () => {
194201
it("should find org-level provider", async () => {
195202
const providersAtStart = await service.getAllAuthProviderParams();
196203
expect(providersAtStart).to.be.empty;
197204

198-
const created = await service.createOrgAuthProvider(owner.id, newOrgEntry());
205+
const created = await service.createOrgAuthProvider(currentUser.id, newOrgEntry());
199206

200-
const retrieved = await service.getAuthProvider(owner.id, created.id);
201-
console.log(JSON.stringify(retrieved));
207+
const retrieved = await service.getAuthProvider(currentUser.id, created.id);
202208
expect(retrieved).to.deep.include(expectedOrgEntry());
203209
});
210+
it("should find user-level provider", async () => {
211+
const providersAtStart = await service.getAllAuthProviderParams();
212+
expect(providersAtStart).to.be.empty;
213+
214+
const created = await service.createAuthProviderOfUser(currentUser.id, newEntry());
215+
216+
const retrieved = await service.getAuthProvider(currentUser.id, created.id);
217+
expect(retrieved).to.deep.include(expectedEntry());
218+
});
219+
it("should not find org-level provider for non-members", async () => {
220+
const providersAtStart = await service.getAllAuthProviderParams();
221+
expect(providersAtStart).to.be.empty;
222+
223+
const created = await service.createOrgAuthProvider(currentUser.id, newOrgEntry());
224+
225+
const nonMember = await userService.createUser({
226+
identity: {
227+
authId: "gh-user-2",
228+
authName: "user2",
229+
authProviderId: "public-github",
230+
},
231+
});
232+
233+
// expecting 404, as Orgs shall not be enumerable to non-members
234+
await expectError(ErrorCodes.NOT_FOUND, service.getAuthProvider(nonMember.id, created.id));
235+
});
236+
});
237+
238+
describe("getAuthProviderDescriptionsUnauthenticated", async () => {
239+
it("should find built-in provider", async () => {
240+
addBuiltInProvider();
241+
242+
const providers = await service.getAuthProviderDescriptionsUnauthenticated();
243+
expect(providers).to.has.lengthOf(1);
244+
expect(providers[0].authProviderId).to.be.equal("Public-GitHub");
245+
});
246+
it("should find only built-in providers but no user-level providers", async () => {
247+
addBuiltInProvider("localhost");
248+
249+
const created = await service.createAuthProviderOfUser(currentUser.id, newEntry());
250+
await service.markAsVerified({ userId: currentUser.id, id: created.id });
251+
252+
const providers = await service.getAuthProviderDescriptionsUnauthenticated();
253+
expect(providers).to.has.lengthOf(1);
254+
expect(providers[0].host).to.be.equal("localhost");
255+
});
256+
it("should find user-level providers if no built-in providers present", async () => {
257+
const created = await service.createAuthProviderOfUser(currentUser.id, newEntry());
258+
await service.markAsVerified({ userId: currentUser.id, id: created.id });
259+
260+
const providers = await service.getAuthProviderDescriptionsUnauthenticated();
261+
expect(providers).to.has.lengthOf(1);
262+
expect(providers[0]).to.deep.include(<Partial<AuthProviderInfo>>{
263+
authProviderId: created.id,
264+
authProviderType: created.type,
265+
host: created.host,
266+
});
267+
268+
const privateProperties: (keyof AuthProviderEntry)[] = ["oauth", "organizationId", "ownerId"];
269+
for (const privateProperty of privateProperties) {
270+
expect(providers[0]).to.not.haveOwnProperty(privateProperty);
271+
}
272+
});
273+
});
274+
275+
describe("getAuthProviderDescriptions", async () => {
276+
it("should find built-in provider", async () => {
277+
addBuiltInProvider();
278+
279+
const providers = await service.getAuthProviderDescriptions(currentUser);
280+
expect(providers).to.has.lengthOf(1);
281+
expect(providers[0].authProviderId).to.be.equal("Public-GitHub");
282+
});
283+
it("should find built-in providers and _own_ user-level providers", async () => {
284+
addBuiltInProvider("localhost");
285+
286+
const created = await service.createAuthProviderOfUser(currentUser.id, newEntry());
287+
await service.markAsVerified({ userId: currentUser.id, id: created.id });
288+
289+
const providers = await service.getAuthProviderDescriptions(currentUser);
290+
expect(providers).to.has.lengthOf(2);
291+
expect(providers[0].host).to.be.equal(created.host);
292+
expect(providers[1].host).to.be.equal("localhost");
293+
});
294+
it("should find user-level providers if no built-in providers present", async () => {
295+
const created = await service.createAuthProviderOfUser(currentUser.id, newEntry());
296+
await service.markAsVerified({ userId: currentUser.id, id: created.id });
297+
298+
const providers = await service.getAuthProviderDescriptions(currentUser);
299+
expect(providers).to.has.lengthOf(1);
300+
expect(providers[0]).to.deep.include(<Partial<AuthProviderInfo>>{
301+
authProviderId: created.id,
302+
authProviderType: created.type,
303+
host: created.host,
304+
organizationId: created.organizationId,
305+
ownerId: created.ownerId,
306+
});
307+
308+
const oauthProperty: keyof AuthProviderEntry = "oauth";
309+
expect(providers[0]).to.not.haveOwnProperty(oauthProperty);
310+
});
204311
});
205312
});

‎components/server/src/auth/auth-provider-service.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ export class AuthProviderService {
122122
result.push(toInfo(p));
123123
continue;
124124
}
125-
if (builtinAuthProvidersConfigured && !this.isBuiltIn(toInfo(p))) {
125+
if (builtinAuthProvidersConfigured && !this.isBuiltIn(p)) {
126126
continue;
127127
}
128128
if (this.isNotHidden(p) && this.isVerified(p)) {

‎components/server/src/auth/auth-provider.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,6 @@ export interface AuthProviderParams extends AuthProviderEntry {
2020
*/
2121
readonly verified: boolean;
2222

23-
/**
24-
* @deprecated unused
25-
*/
2623
readonly hiddenOnDashboard?: boolean;
2724

2825
/**

0 commit comments

Comments
 (0)