Skip to content

Commit

Permalink
Gpl/970-multi-org-tests (#20436)
Browse files Browse the repository at this point in the history 
* fix tests for real

* [server] Create OrgService.createOrgOwnedUser, and use that across tests to fix the "can't join org" permission issues

* Update components/server/src/orgs/organization-service.ts

Co-authored-by: Filip Troníček <filip@gitpod.io>

---------

Co-authored-by: Filip Troníček <filip@gitpod.io>
  • Loading branch information
@geropl @filiptronicek
geropl and filiptronicek authored Dec 9, 2024
1 parent 2fa6e93 commit cf3a7e9
Show file tree
Hide file tree
Showing 10 changed files with 101 additions and 79 deletions.
11 changes: 5 additions & 6 deletions components/server/src/iam/iam-session-app.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import request from "supertest";

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

protected userServiceMock: Partial<UserService> = {
createUser: (params) => {
return { id: "id-new-user" } as any;
},
updateUser: (userId, update) => {
return {} as any;
},
Expand All @@ -67,8 +64,10 @@ class TestIamSessionApp {
listMembers: async (teamId: string): Promise<TeamMemberInfo[]> => {
return [];
},
async addOrUpdateMember(userId: string, teamId: string, memberId: string, role: TeamMemberRole): Promise<void> {
this.memberships.add(memberId);
async createOrgOwnedUser(params): Promise<User> {
const user = { id: "id-new-user" } as any as User;
this.memberships.add(user.id);
return user;
},
};

Expand Down
35 changes: 9 additions & 26 deletions components/server/src/iam/iam-session-app.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { reportJWTCookieIssued } from "../prometheus-metrics";
import { ApplicationError } from "@gitpod/gitpod-protocol/lib/messaging/error";
import { OrganizationService } from "../orgs/organization-service";
import { UserService } from "../user/user-service";
import { UserDB } from "@gitpod/gitpod-db/lib";
import { SYSTEM_USER, SYSTEM_USER_ID } from "../authorization/authorizer";
import { runWithSubjectId, runWithRequestContext } from "../util/request-context";

Expand All @@ -29,7 +28,6 @@ export class IamSessionApp {
@inject(UserService) private readonly userService: UserService,
@inject(OrganizationService) private readonly orgService: OrganizationService,
@inject(SessionHandler) private readonly session: SessionHandler,
@inject(UserDB) private readonly userDb: UserDB,
) {}

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

return this.userDb.transaction(async (_, ctx) => {
// Until we support SKIM (or any other means to sync accounts) we create new users here as a side-effect of the login
const user = await this.userService.createUser(
{
organizationId,
identity: { ...this.mapOIDCProfileToIdentity(payload), lastSigninTime: new Date().toISOString() },
userUpdate: (user) => {
user.fullName = claims.name;
user.name = claims.name;
user.avatarUrl = claims.picture;
},
},
ctx,
);

await this.orgService.addOrUpdateMember(
SYSTEM_USER_ID,
organizationId,
user.id,
"member",
{ flexibleRole: true },
ctx,
);
return user;
// Until we support SKIM (or any other means to sync accounts) we create new users here as a side-effect of the login
return this.orgService.createOrgOwnedUser({
organizationId,
identity: { ...this.mapOIDCProfileToIdentity(payload), lastSigninTime: new Date().toISOString() },
userUpdate: (user) => {
user.fullName = claims.name;
user.name = claims.name;
user.avatarUrl = claims.picture;
},
});
}
}
52 changes: 51 additions & 1 deletion components/server/src/orgs/organization-service.spec.db.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* See License.AGPL.txt in the project root for license information.
*/

import { BUILTIN_INSTLLATION_ADMIN_USER_ID, TypeORM } from "@gitpod/gitpod-db/lib";
import { BUILTIN_INSTLLATION_ADMIN_USER_ID, TypeORM, UserDB } from "@gitpod/gitpod-db/lib";
import { Organization, OrganizationSettings, TeamMemberRole, User } from "@gitpod/gitpod-protocol";
import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
Expand Down Expand Up @@ -473,4 +473,54 @@ describe("OrganizationService", async () => {
);
await assertUpdateSettings("should enable workspace sharing", { workspaceSharingDisabled: false }, {});
});

it("org-owned users can't create new organizations", async () => {
const userDB = container.get<UserDB>(UserDB);
const os = container.get(OrganizationService);

// create the owner (installation-level)
const owner = await userDB.newUser();

// create an org
const orgService = container.get(OrganizationService);
const myOrg = await orgService.createOrganization(owner.id, "my-org");

// create org-owned user
const member = await createOrgOwnedUser(os, myOrg.id);

await expectError(ErrorCodes.PERMISSION_DENIED, () => os.createOrganization(member.id, "member's crew"));
});

it("org-owned users can't join another org", async () => {
const userDB = container.get<UserDB>(UserDB);
const os = container.get(OrganizationService);

// create the owner (installation-level)
const owner = await userDB.newUser();

// create the orgs
const orgService = container.get(OrganizationService);
const myOrg = await orgService.createOrganization(owner.id, "my-org");
const anotherOrg = await orgService.createOrganization(owner.id, "another-org");

// create org-owned user
const member = await createOrgOwnedUser(os, myOrg.id);

const failingInvite = await orgService.getOrCreateInvite(owner.id, anotherOrg.id);
await expectError(ErrorCodes.PERMISSION_DENIED, () => os.joinOrganization(member.id, failingInvite.id));
});
});

async function createOrgOwnedUser(os: OrganizationService, organizationId: string) {
// create org-owned member
return os.createOrgOwnedUser({
organizationId,
identity: {
authId: "123",
authProviderId: "https://accounts.google.com",
authName: "member",
lastSigninTime: new Date().toISOString(),
},
userUpdate: (user) => {},
});
}
2 changes: 1 addition & 1 deletion components/server/src/orgs/organization-service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe("OrganizationService", async () => {
} as any as InstallationService);
container.bind(StripeService).toConstantValue({} as any as StripeService);
container.bind(UsageService).toConstantValue({} as any as UsageService);
container.bind(UserAuthentication).toSelf().inSingletonScope();
container.bind(UserAuthentication).toConstantValue({} as any as UserAuthentication);
os = container.get(OrganizationService);
});

Expand Down
23 changes: 22 additions & 1 deletion components/server/src/orgs/organization-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
TeamMembershipInvite,
WorkspaceTimeoutDuration,
OrgMemberRole,
User,
} from "@gitpod/gitpod-protocol";
import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
Expand All @@ -33,7 +34,7 @@ import { StripeService } from "../billing/stripe-service";
import { AttributionId } from "@gitpod/gitpod-protocol/lib/attribution";
import { UsageService } from "./usage-service";
import { CostCenter_BillingStrategy } from "@gitpod/gitpod-protocol/lib/usage";
import { UserAuthentication } from "../user/user-authentication";
import { CreateUserParams, UserAuthentication } from "../user/user-authentication";

@injectable()
export class OrganizationService {
Expand Down Expand Up @@ -324,6 +325,26 @@ export class OrganizationService {
return invite.teamId;
}

/**
* Convenience method, analogue to UserService.createUser()
``
*/
public async createOrgOwnedUser(params: CreateUserParams & { organizationId: string }): Promise<User> {
return this.userDB.transaction(async (_, ctx) => {
const user = await this.userService.createUser(params, ctx);

await this.addOrUpdateMember(
SYSTEM_USER_ID,
params.organizationId,
user.id,
"member",
{ flexibleRole: true },
ctx,
);
return user;
});
}

/**
* Add or update member to an organization, if there's no `owner` in the organization, target role will be owner
*
Expand Down
7 changes: 2 additions & 5 deletions components/server/src/user/env-var-service.spec.db.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,14 @@ import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-s
import * as chai from "chai";
import { Container } from "inversify";
import "mocha";
import { createTestContainer, withTestCtx } from "../test/service-testing-container-module";
import { createTestContainer } from "../test/service-testing-container-module";
import { resetDB } from "@gitpod/gitpod-db/lib/test/reset-db";
import { OrganizationService } from "../orgs/organization-service";
import { UserService } from "./user-service";
import { expectError } from "../test/expect-utils";
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
import { EnvVarService } from "./env-var-service";
import { ProjectsService } from "../projects/projects-service";
import { SYSTEM_USER } from "../authorization/authorizer";

const expect = chai.expect;

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

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

member = await userService.createUser({
member = await orgService.createOrgOwnedUser({
organizationId: org.id,
identity: {
authId: "foo",
Expand All @@ -109,7 +107,6 @@ describe("EnvVarService", async () => {
primaryEmail: "yolo@yolo.com",
},
});
await withTestCtx(SYSTEM_USER, () => orgService.joinOrganization(member.id, invite.id));
stranger = await userService.createUser({
identity: {
authId: "foo2",
Expand Down
7 changes: 2 additions & 5 deletions components/server/src/user/gitpod-token-service.spec.db.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@ import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-s
import * as chai from "chai";
import { Container } from "inversify";
import "mocha";
import { createTestContainer, withTestCtx } from "../test/service-testing-container-module";
import { createTestContainer } from "../test/service-testing-container-module";
import { resetDB } from "@gitpod/gitpod-db/lib/test/reset-db";
import { OrganizationService } from "../orgs/organization-service";
import { UserService } from "./user-service";
import { expectError } from "../test/expect-utils";
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
import { GitpodTokenService } from "./gitpod-token-service";
import { SYSTEM_USER } from "../authorization/authorizer";

const expect = chai.expect;

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

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

const userService = container.get<UserService>(UserService);
member = await userService.createUser({
member = await orgService.createOrgOwnedUser({
organizationId: org.id,
identity: {
authId: "foo",
Expand All @@ -47,7 +45,6 @@ describe("GitpodTokenService", async () => {
primaryEmail: "yolo@yolo.com",
},
});
await withTestCtx(SYSTEM_USER, () => orgService.joinOrganization(member.id, invite.id));
stranger = await userService.createUser({
identity: {
authId: "foo2",
Expand Down
7 changes: 2 additions & 5 deletions components/server/src/user/sshkey-service.spec.db.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@ import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-s
import * as chai from "chai";
import { Container } from "inversify";
import "mocha";
import { createTestContainer, withTestCtx } from "../test/service-testing-container-module";
import { createTestContainer } from "../test/service-testing-container-module";
import { resetDB } from "@gitpod/gitpod-db/lib/test/reset-db";
import { SSHKeyService } from "./sshkey-service";
import { OrganizationService } from "../orgs/organization-service";
import { UserService } from "./user-service";
import { expectError } from "../test/expect-utils";
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
import { SYSTEM_USER } from "../authorization/authorizer";

const expect = chai.expect;

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

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

const userService = container.get<UserService>(UserService);
member = await userService.createUser({
member = await orgService.createOrgOwnedUser({
organizationId: org.id,
identity: {
authId: "foo",
Expand All @@ -58,7 +56,6 @@ describe("SSHKeyService", async () => {
primaryEmail: "yolo@yolo.com",
},
});
await withTestCtx(SYSTEM_USER, () => orgService.joinOrganization(member.id, invite.id));
stranger = await userService.createUser({
identity: {
authId: "foo2",
Expand Down
22 changes: 2 additions & 20 deletions components/server/src/user/token-service.spec.db.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@ import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-s
import * as chai from "chai";
import "mocha";
import { Container } from "inversify";
import { createTestContainer, withTestCtx } from "../test/service-testing-container-module";
import { createTestContainer } from "../test/service-testing-container-module";
import { BUILTIN_INSTLLATION_ADMIN_USER_ID, TypeORM, UserDB } from "@gitpod/gitpod-db/lib";
import { resetDB } from "@gitpod/gitpod-db/lib/test/reset-db";
import { OrganizationService } from "../orgs/organization-service";
import { SYSTEM_USER } from "../authorization/authorizer";
import { UserService } from "./user-service";
import { Organization, Token, User } from "@gitpod/gitpod-protocol";
import { TokenService } from "./token-service";
import { TokenProvider } from "./token-provider";
Expand All @@ -33,11 +31,9 @@ describe("TokenService", async () => {

let container: Container;
let tokenService: TokenService;
let userService: UserService;
let userDB: UserDB;
let orgService: OrganizationService;
let org: Organization;
let owner: User;
let user: User;

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

tokenService = container.get<TokenService>(TokenService);
userDB = container.get<UserDB>(UserDB);
userService = container.get<UserService>(UserService);
orgService = container.get<OrganizationService>(OrganizationService);
org = await orgService.createOrganization(BUILTIN_INSTLLATION_ADMIN_USER_ID, "myOrg");
const invite = await orgService.getOrCreateInvite(BUILTIN_INSTLLATION_ADMIN_USER_ID, org.id);
// first not builtin user join an org will be an owner
owner = await userService.createUser({
organizationId: org.id,
identity: {
authId: "foo",
authName: "bar",
authProviderId: "github",
primaryEmail: "yolo@yolo.com",
},
});
await withTestCtx(SYSTEM_USER, () => orgService.joinOrganization(owner.id, invite.id));

user = await userService.createUser({
user = await orgService.createOrgOwnedUser({
organizationId: org.id,
identity: {
authId: githubUserAuthId,
Expand All @@ -159,7 +142,6 @@ describe("TokenService", async () => {
primaryEmail: "yolo@yolo.com",
});
await userDB.storeUser(user);
await withTestCtx(SYSTEM_USER, () => orgService.joinOrganization(user.id, invite.id));

// test data
token = <Token>{
Expand Down
Loading

0 comments on commit cf3a7e9

Please sign in to comment.