Skip to content

Commit 2cdf32e

Browse files
geroplgeropl
committed
[server] Create OrgService.createOrgOwnedUser, and use that across tests to fix the "can't join org" permission issues
1 parent 4284a09 commit 2cdf32e

9 files changed

+99
-78
lines changed

‎components/server/src/iam/iam-session-app.spec.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import request from "supertest";
1919

2020
import * as chai from "chai";
2121
import { OIDCCreateSessionPayload } from "./iam-oidc-create-session-payload";
22-
import { TeamMemberInfo, TeamMemberRole, User } from "@gitpod/gitpod-protocol";
22+
import { TeamMemberInfo, User } from "@gitpod/gitpod-protocol";
2323
import { OrganizationService } from "../orgs/organization-service";
2424
import { UserService } from "../user/user-service";
2525
import { TeamDB, UserDB } from "@gitpod/gitpod-db/lib";
@@ -40,9 +40,6 @@ class TestIamSessionApp {
4040
};
4141

4242
protected userServiceMock: Partial<UserService> = {
43-
createUser: (params) => {
44-
return { id: "id-new-user" } as any;
45-
},
4643
updateUser: (userId, update) => {
4744
return {} as any;
4845
},
@@ -67,8 +64,10 @@ class TestIamSessionApp {
6764
listMembers: async (teamId: string): Promise<TeamMemberInfo[]> => {
6865
return [];
6966
},
70-
async addOrUpdateMember(userId: string, teamId: string, memberId: string, role: TeamMemberRole): Promise<void> {
71-
this.memberships.add(memberId);
67+
async createOrgOwnedUser(params): Promise<User> {
68+
const user = { id: "id-new-user" } as any as User;
69+
this.memberships.add(user.id);
70+
return user;
7271
},
7372
};
7473

‎components/server/src/iam/iam-session-app.ts‎

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import { reportJWTCookieIssued } from "../prometheus-metrics";
1616
import { ApplicationError } from "@gitpod/gitpod-protocol/lib/messaging/error";
1717
import { OrganizationService } from "../orgs/organization-service";
1818
import { UserService } from "../user/user-service";
19-
import { UserDB } from "@gitpod/gitpod-db/lib";
2019
import { SYSTEM_USER, SYSTEM_USER_ID } from "../authorization/authorizer";
2120
import { runWithSubjectId, runWithRequestContext } from "../util/request-context";
2221

@@ -29,7 +28,6 @@ export class IamSessionApp {
2928
@inject(UserService) private readonly userService: UserService,
3029
@inject(OrganizationService) private readonly orgService: OrganizationService,
3130
@inject(SessionHandler) private readonly session: SessionHandler,
32-
@inject(UserDB) private readonly userDb: UserDB,
3331
) {}
3432

3533
public getMiddlewares() {
@@ -167,30 +165,15 @@ export class IamSessionApp {
167165
private async createNewOIDCUser(payload: OIDCCreateSessionPayload): Promise<User> {
168166
const { claims, organizationId } = payload;
169167

170-
return this.userDb.transaction(async (_, ctx) => {
171-
// Until we support SKIM (or any other means to sync accounts) we create new users here as a side-effect of the login
172-
const user = await this.userService.createUser(
173-
{
174-
organizationId,
175-
identity: { ...this.mapOIDCProfileToIdentity(payload), lastSigninTime: new Date().toISOString() },
176-
userUpdate: (user) => {
177-
user.fullName = claims.name;
178-
user.name = claims.name;
179-
user.avatarUrl = claims.picture;
180-
},
181-
},
182-
ctx,
183-
);
184-
185-
await this.orgService.addOrUpdateMember(
186-
SYSTEM_USER_ID,
187-
organizationId,
188-
user.id,
189-
"member",
190-
{ flexibleRole: true },
191-
ctx,
192-
);
193-
return user;
168+
// Until we support SKIM (or any other means to sync accounts) we create new users here as a side-effect of the login
169+
return this.orgService.createOrgOwnedUser({
170+
organizationId,
171+
identity: { ...this.mapOIDCProfileToIdentity(payload), lastSigninTime: new Date().toISOString() },
172+
userUpdate: (user) => {
173+
user.fullName = claims.name;
174+
user.name = claims.name;
175+
user.avatarUrl = claims.picture;
176+
},
194177
});
195178
}
196179
}

‎components/server/src/orgs/organization-service.spec.db.ts‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* See License.AGPL.txt in the project root for license information.
55
*/
66

7-
import { BUILTIN_INSTLLATION_ADMIN_USER_ID, TypeORM } from "@gitpod/gitpod-db/lib";
7+
import { BUILTIN_INSTLLATION_ADMIN_USER_ID, TypeORM, UserDB } from "@gitpod/gitpod-db/lib";
88
import { Organization, OrganizationSettings, TeamMemberRole, User } from "@gitpod/gitpod-protocol";
99
import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
1010
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
@@ -473,4 +473,54 @@ describe("OrganizationService", async () => {
473473
);
474474
await assertUpdateSettings("should enable workspace sharing", { workspaceSharingDisabled: false }, {});
475475
});
476+
477+
it("org-owned users can't create new organizations", async () => {
478+
const userDB = container.get<UserDB>(UserDB);
479+
const os = container.get(OrganizationService);
480+
481+
// create the owner (installation-level)
482+
const owner = await userDB.newUser();
483+
484+
// create an org
485+
const orgService = container.get(OrganizationService);
486+
const myOrg = await orgService.createOrganization(owner.id, "my-org");
487+
488+
// create org-owned user
489+
const member = await createOrgOwnedUser(os, myOrg.id);
490+
491+
await expectError(ErrorCodes.PERMISSION_DENIED, () => os.createOrganization(member.id, "member's crew"));
492+
});
493+
494+
it("org-owned users can't join another org", async () => {
495+
const userDB = container.get<UserDB>(UserDB);
496+
const os = container.get(OrganizationService);
497+
498+
// create the owner (installation-level)
499+
const owner = await userDB.newUser();
500+
501+
// create the orgs
502+
const orgService = container.get(OrganizationService);
503+
const myOrg = await orgService.createOrganization(owner.id, "my-org");
504+
const anotherOrg = await orgService.createOrganization(owner.id, "another-org");
505+
506+
// create org-owned user
507+
const member = await createOrgOwnedUser(os, myOrg.id);
508+
509+
const failingInvite = await orgService.getOrCreateInvite(owner.id, anotherOrg.id);
510+
await expectError(ErrorCodes.PERMISSION_DENIED, () => os.joinOrganization(member.id, failingInvite.id));
511+
});
476512
});
513+
514+
async function createOrgOwnedUser(os: OrganizationService, organizationId: string) {
515+
// create org-owned member
516+
return os.createOrgOwnedUser({
517+
organizationId,
518+
identity: {
519+
authId: "123",
520+
authProviderId: "https://accounts.google.com",
521+
authName: "member",
522+
lastSigninTime: new Date().toISOString(),
523+
},
524+
userUpdate: (user) => {},
525+
});
526+
}

‎components/server/src/orgs/organization-service.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
TeamMembershipInvite,
1414
WorkspaceTimeoutDuration,
1515
OrgMemberRole,
16+
User,
1617
} from "@gitpod/gitpod-protocol";
1718
import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
1819
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
@@ -33,7 +34,7 @@ import { StripeService } from "../billing/stripe-service";
3334
import { AttributionId } from "@gitpod/gitpod-protocol/lib/attribution";
3435
import { UsageService } from "./usage-service";
3536
import { CostCenter_BillingStrategy } from "@gitpod/gitpod-protocol/lib/usage";
36-
import { UserAuthentication } from "../user/user-authentication";
37+
import { CreateUserParams, UserAuthentication } from "../user/user-authentication";
3738

3839
@injectable()
3940
export class OrganizationService {
@@ -324,6 +325,25 @@ export class OrganizationService {
324325
return invite.teamId;
325326
}
326327

328+
/**
329+
* Conveniece method, analogue to UserService.createUser()
330+
*/
331+
public async createOrgOwnedUser(params: CreateUserParams & { organizationId: string }): Promise<User> {
332+
return this.userDB.transaction(async (_, ctx) => {
333+
const user = await this.userService.createUser(params, ctx);
334+
335+
await this.addOrUpdateMember(
336+
SYSTEM_USER_ID,
337+
params.organizationId,
338+
user.id,
339+
"member",
340+
{ flexibleRole: true },
341+
ctx,
342+
);
343+
return user;
344+
});
345+
}
346+
327347
/**
328348
* Add or update member to an organization, if there's no `owner` in the organization, target role will be owner
329349
*

‎components/server/src/user/env-var-service.spec.db.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,14 @@ import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-s
1919
import * as chai from "chai";
2020
import { Container } from "inversify";
2121
import "mocha";
22-
import { createTestContainer, withTestCtx } from "../test/service-testing-container-module";
22+
import { createTestContainer } from "../test/service-testing-container-module";
2323
import { resetDB } from "@gitpod/gitpod-db/lib/test/reset-db";
2424
import { OrganizationService } from "../orgs/organization-service";
2525
import { UserService } from "./user-service";
2626
import { expectError } from "../test/expect-utils";
2727
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
2828
import { EnvVarService } from "./env-var-service";
2929
import { ProjectsService } from "../projects/projects-service";
30-
import { SYSTEM_USER } from "../authorization/authorizer";
3130

3231
const expect = chai.expect;
3332

@@ -98,9 +97,8 @@ describe("EnvVarService", async () => {
9897

9998
const orgService = container.get<OrganizationService>(OrganizationService);
10099
org = await orgService.createOrganization(BUILTIN_INSTLLATION_ADMIN_USER_ID, "myOrg");
101-
const invite = await orgService.getOrCreateInvite(BUILTIN_INSTLLATION_ADMIN_USER_ID, org.id);
102100

103-
member = await userService.createUser({
101+
member = await orgService.createOrgOwnedUser({
104102
organizationId: org.id,
105103
identity: {
106104
authId: "foo",
@@ -109,7 +107,6 @@ describe("EnvVarService", async () => {
109107
primaryEmail: "[email protected]",
110108
},
111109
});
112-
await withTestCtx(SYSTEM_USER, () => orgService.joinOrganization(member.id, invite.id));
113110
stranger = await userService.createUser({
114111
identity: {
115112
authId: "foo2",

‎components/server/src/user/gitpod-token-service.spec.db.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,13 @@ import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-s
1010
import * as chai from "chai";
1111
import { Container } from "inversify";
1212
import "mocha";
13-
import { createTestContainer, withTestCtx } from "../test/service-testing-container-module";
13+
import { createTestContainer } from "../test/service-testing-container-module";
1414
import { resetDB } from "@gitpod/gitpod-db/lib/test/reset-db";
1515
import { OrganizationService } from "../orgs/organization-service";
1616
import { UserService } from "./user-service";
1717
import { expectError } from "../test/expect-utils";
1818
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
1919
import { GitpodTokenService } from "./gitpod-token-service";
20-
import { SYSTEM_USER } from "../authorization/authorizer";
2120

2221
const expect = chai.expect;
2322

@@ -35,10 +34,9 @@ describe("GitpodTokenService", async () => {
3534

3635
const orgService = container.get<OrganizationService>(OrganizationService);
3736
org = await orgService.createOrganization(BUILTIN_INSTLLATION_ADMIN_USER_ID, "myOrg");
38-
const invite = await orgService.getOrCreateInvite(BUILTIN_INSTLLATION_ADMIN_USER_ID, org.id);
3937

4038
const userService = container.get<UserService>(UserService);
41-
member = await userService.createUser({
39+
member = await orgService.createOrgOwnedUser({
4240
organizationId: org.id,
4341
identity: {
4442
authId: "foo",
@@ -47,7 +45,6 @@ describe("GitpodTokenService", async () => {
4745
primaryEmail: "[email protected]",
4846
},
4947
});
50-
await withTestCtx(SYSTEM_USER, () => orgService.joinOrganization(member.id, invite.id));
5148
stranger = await userService.createUser({
5249
identity: {
5350
authId: "foo2",

‎components/server/src/user/sshkey-service.spec.db.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,13 @@ import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-s
1010
import * as chai from "chai";
1111
import { Container } from "inversify";
1212
import "mocha";
13-
import { createTestContainer, withTestCtx } from "../test/service-testing-container-module";
13+
import { createTestContainer } from "../test/service-testing-container-module";
1414
import { resetDB } from "@gitpod/gitpod-db/lib/test/reset-db";
1515
import { SSHKeyService } from "./sshkey-service";
1616
import { OrganizationService } from "../orgs/organization-service";
1717
import { UserService } from "./user-service";
1818
import { expectError } from "../test/expect-utils";
1919
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
20-
import { SYSTEM_USER } from "../authorization/authorizer";
2120

2221
const expect = chai.expect;
2322

@@ -46,10 +45,9 @@ describe("SSHKeyService", async () => {
4645

4746
const orgService = container.get<OrganizationService>(OrganizationService);
4847
org = await orgService.createOrganization(BUILTIN_INSTLLATION_ADMIN_USER_ID, "myOrg");
49-
const invite = await orgService.getOrCreateInvite(BUILTIN_INSTLLATION_ADMIN_USER_ID, org.id);
5048

5149
const userService = container.get<UserService>(UserService);
52-
member = await userService.createUser({
50+
member = await orgService.createOrgOwnedUser({
5351
organizationId: org.id,
5452
identity: {
5553
authId: "foo",
@@ -58,7 +56,6 @@ describe("SSHKeyService", async () => {
5856
primaryEmail: "[email protected]",
5957
},
6058
});
61-
await withTestCtx(SYSTEM_USER, () => orgService.joinOrganization(member.id, invite.id));
6259
stranger = await userService.createUser({
6360
identity: {
6461
authId: "foo2",

‎components/server/src/user/token-service.spec.db.ts‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,10 @@ import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-s
88
import * as chai from "chai";
99
import "mocha";
1010
import { Container } from "inversify";
11-
import { createTestContainer, withTestCtx } from "../test/service-testing-container-module";
11+
import { createTestContainer } from "../test/service-testing-container-module";
1212
import { BUILTIN_INSTLLATION_ADMIN_USER_ID, TypeORM, UserDB } from "@gitpod/gitpod-db/lib";
1313
import { resetDB } from "@gitpod/gitpod-db/lib/test/reset-db";
1414
import { OrganizationService } from "../orgs/organization-service";
15-
import { SYSTEM_USER } from "../authorization/authorizer";
16-
import { UserService } from "./user-service";
1715
import { Organization, Token, User } from "@gitpod/gitpod-protocol";
1816
import { TokenService } from "./token-service";
1917
import { TokenProvider } from "./token-provider";
@@ -33,11 +31,9 @@ describe("TokenService", async () => {
3331

3432
let container: Container;
3533
let tokenService: TokenService;
36-
let userService: UserService;
3734
let userDB: UserDB;
3835
let orgService: OrganizationService;
3936
let org: Organization;
40-
let owner: User;
4137
let user: User;
4238

4339
let token: Token;
@@ -127,23 +123,10 @@ describe("TokenService", async () => {
127123

128124
tokenService = container.get<TokenService>(TokenService);
129125
userDB = container.get<UserDB>(UserDB);
130-
userService = container.get<UserService>(UserService);
131126
orgService = container.get<OrganizationService>(OrganizationService);
132127
org = await orgService.createOrganization(BUILTIN_INSTLLATION_ADMIN_USER_ID, "myOrg");
133-
const invite = await orgService.getOrCreateInvite(BUILTIN_INSTLLATION_ADMIN_USER_ID, org.id);
134-
// first not builtin user join an org will be an owner
135-
owner = await userService.createUser({
136-
organizationId: org.id,
137-
identity: {
138-
authId: "foo",
139-
authName: "bar",
140-
authProviderId: "github",
141-
primaryEmail: "[email protected]",
142-
},
143-
});
144-
await withTestCtx(SYSTEM_USER, () => orgService.joinOrganization(owner.id, invite.id));
145128

146-
user = await userService.createUser({
129+
user = await orgService.createOrgOwnedUser({
147130
organizationId: org.id,
148131
identity: {
149132
authId: githubUserAuthId,
@@ -159,7 +142,6 @@ describe("TokenService", async () => {
159142
primaryEmail: "[email protected]",
160143
});
161144
await userDB.storeUser(user);
162-
await withTestCtx(SYSTEM_USER, () => orgService.joinOrganization(user.id, invite.id));
163145

164146
// test data
165147
token = <Token>{

0 commit comments

Comments
 (0)