Skip to content

Commit

Permalink
[server] move FGA calls into AuthProviderService
Browse files Browse the repository at this point in the history
* split internal upsert method `updateAuthProvider` into create and update
* refactor: move `getAuthProviders` logic from gitpod-server-impl to auth-provider-service
* adding db tests for auth provider server
* use redacted results in service
  • Loading branch information
AlexTugarev committed Nov 8, 2023
1 parent bf06755 commit 341a5bd
Show file tree
Hide file tree
Showing 12 changed files with 715 additions and 193 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ export class DBAuthProviderEntry implements AuthProviderEntry {
@Column()
ownerId: string;

@Column()
@Column({
...TypeORM.UUID_COLUMN_TYPE,
default: "",
transformer: Transformer.MAP_EMPTY_STR_TO_UNDEFINED,
})
organizationId?: string;

@Column("varchar")
Expand Down
27 changes: 19 additions & 8 deletions components/gitpod-protocol/src/gitpod-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,31 @@ export interface GitpodServer extends JsonRpcServer<GitpodClient>, AdminServer,
updateLoggedInUser(user: Partial<User>): Promise<User>;
sendPhoneNumberVerificationToken(phoneNumber: string): Promise<{ verificationId: string }>;
verifyPhoneNumberVerificationToken(phoneNumber: string, token: string, verificationId: string): Promise<boolean>;
getAuthProviders(): Promise<AuthProviderInfo[]>;
getOwnAuthProviders(): Promise<AuthProviderEntry[]>;
updateOwnAuthProvider(params: GitpodServer.UpdateOwnAuthProviderParams): Promise<AuthProviderEntry>;
deleteOwnAuthProvider(params: GitpodServer.DeleteOwnAuthProviderParams): Promise<void>;
getConfiguration(): Promise<Configuration>;
getToken(query: GitpodServer.GetTokenSearchOptions): Promise<Token | undefined>;
getGitpodTokenScopes(tokenHash: string): Promise<string[]>;
deleteAccount(): Promise<void>;
getClientRegion(): Promise<string | undefined>;

// Auth Provider API
getAuthProviders(): Promise<AuthProviderInfo[]>;
// user-level
getOwnAuthProviders(): Promise<AuthProviderEntry[]>;
updateOwnAuthProvider(params: GitpodServer.UpdateOwnAuthProviderParams): Promise<AuthProviderEntry>;
deleteOwnAuthProvider(params: GitpodServer.DeleteOwnAuthProviderParams): Promise<void>;
// org-level
createOrgAuthProvider(params: GitpodServer.CreateOrgAuthProviderParams): Promise<AuthProviderEntry>;
updateOrgAuthProvider(params: GitpodServer.UpdateOrgAuthProviderParams): Promise<AuthProviderEntry>;
getOrgAuthProviders(params: GitpodServer.GetOrgAuthProviderParams): Promise<AuthProviderEntry[]>;
deleteOrgAuthProvider(params: GitpodServer.DeleteOrgAuthProviderParams): Promise<void>;
// public-api compatibility
/** @deprecated used for public-api compatibility only */
getAuthProvider(id: string): Promise<AuthProviderEntry>;
/** @deprecated used for public-api compatibility only */
deleteAuthProvider(id: string): Promise<void>;
/** @deprecated used for public-api compatibility only */
updateAuthProvider(id: string, update: AuthProviderEntry.UpdateOAuth2Config): Promise<AuthProviderEntry>;

// Query/retrieve workspaces
getWorkspaces(options: GitpodServer.GetWorkspacesOptions): Promise<WorkspaceInfo[]>;
getWorkspaceOwner(workspaceId: string): Promise<UserInfo | undefined>;
Expand Down Expand Up @@ -167,10 +182,6 @@ export interface GitpodServer extends JsonRpcServer<GitpodClient>, AdminServer,
deleteTeam(teamId: string): Promise<void>;
getOrgSettings(orgId: string): Promise<OrganizationSettings>;
updateOrgSettings(teamId: string, settings: Partial<OrganizationSettings>): Promise<OrganizationSettings>;
createOrgAuthProvider(params: GitpodServer.CreateOrgAuthProviderParams): Promise<AuthProviderEntry>;
updateOrgAuthProvider(params: GitpodServer.UpdateOrgAuthProviderParams): Promise<AuthProviderEntry>;
getOrgAuthProviders(params: GitpodServer.GetOrgAuthProviderParams): Promise<AuthProviderEntry[]>;
deleteOrgAuthProvider(params: GitpodServer.DeleteOrgAuthProviderParams): Promise<void>;

getDefaultWorkspaceImage(params: GetDefaultWorkspaceImageParams): Promise<GetDefaultWorkspaceImageResult>;

Expand Down
2 changes: 1 addition & 1 deletion components/gitpod-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1529,7 +1529,6 @@ export interface AuthProviderInfo {
readonly ownerId?: string;
readonly organizationId?: string;
readonly verified: boolean;
readonly isReadonly?: boolean;
readonly hiddenOnDashboard?: boolean;
readonly disallowLogin?: boolean;
readonly icon?: string;
Expand Down Expand Up @@ -1588,6 +1587,7 @@ export namespace AuthProviderEntry {
clientSecret: string;
organizationId: string;
};
export type UpdateOAuth2Config = Pick<OAuth2Config, "clientId" | "clientSecret">;
export function redact(entry: AuthProviderEntry): AuthProviderEntry {
return {
...entry,
Expand Down
312 changes: 312 additions & 0 deletions components/server/src/auth/auth-provider-service.spec.db.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,312 @@
/**
* Copyright (c) 2023 Gitpod GmbH. All rights reserved.
* Licensed under the GNU Affero General Public License (AGPL).
* See License.AGPL.txt in the project root for license information.
*/

import { TypeORM } from "@gitpod/gitpod-db/lib";
import { AuthProviderInfo, Organization, User } from "@gitpod/gitpod-protocol";
import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
import * as chai from "chai";
import { Container } from "inversify";
import "mocha";
import { createTestContainer } from "../test/service-testing-container-module";
import { resetDB } from "@gitpod/gitpod-db/lib/test/reset-db";
import { UserService } from "../user/user-service";
import { AuthProviderService } from "./auth-provider-service";
import { Config } from "../config";
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
import { expectError } from "../test/expect-utils";
import { AuthProviderEntry } from "@gitpod/gitpod-protocol";
import { AuthProviderParams } from "./auth-provider";
import { OrganizationService } from "../orgs/organization-service";

const expect = chai.expect;

describe("AuthProviderService", async () => {
let service: AuthProviderService;
let userService: UserService;
let container: Container;
let currentUser: User;
let org: Organization;

const newEntry = () =>
<AuthProviderEntry.NewEntry>{
host: "github.com",
ownerId: currentUser.id,
type: "GitHub",
clientId: "123",
clientSecret: "secret-123",
};
const expectedEntry = () =>
<Partial<AuthProviderEntry>>{
host: "github.com",
oauth: {
authorizationUrl: "https://github.com/login/oauth/authorize",
callBackUrl: "https://gitpod.io/auth/callback",
clientId: "123",
clientSecret: "redacted",
tokenUrl: "https://github.com/login/oauth/access_token",
},
organizationId: undefined,
type: "GitHub",
status: "pending",
ownerId: currentUser.id,
};
const expectedParams = () =>
<Partial<AuthProviderParams>>{
builtin: false,
disallowLogin: false,
verified: false,
...expectedEntry(),
oauth: { ...expectedEntry().oauth, clientSecret: "secret-123" },
};

const newOrgEntry = () =>
<AuthProviderEntry.NewOrgEntry>{
host: "github.com",
ownerId: currentUser.id,
type: "GitHub",
clientId: "123",
clientSecret: "secret-123",
organizationId: org.id,
};
const expectedOrgEntry = () =>
<Partial<AuthProviderEntry>>{
host: "github.com",
oauth: {
authorizationUrl: "https://github.com/login/oauth/authorize",
callBackUrl: "https://gitpod.io/auth/callback",
clientId: "123",
clientSecret: "redacted",
tokenUrl: "https://github.com/login/oauth/access_token",
},
organizationId: org.id,
type: "GitHub",
status: "pending",
ownerId: currentUser.id,
};
const expectedOrgParams = () =>
<Partial<AuthProviderParams>>{
builtin: false,
disallowLogin: true,
verified: false,
...expectedOrgEntry(),
oauth: { ...expectedOrgEntry().oauth, clientSecret: "secret-123" },
};

const addBuiltInProvider = (host: string = "github.com") => {
const config = container.get<Config>(Config);
config.builtinAuthProvidersConfigured = true;
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
config.authProviderConfigs.push((<Partial<AuthProviderParams>>{
host,
id: "Public-GitHub",
verified: true,
}) as any);
};

beforeEach(async () => {
container = createTestContainer();
Experiments.configureTestingClient({
centralizedPermissions: true,
});
service = container.get(AuthProviderService);
userService = container.get<UserService>(UserService);
currentUser = await userService.createUser({
identity: {
authId: "gh-user-1",
authName: "user",
authProviderId: "public-github",
},
});
const os = container.get<OrganizationService>(OrganizationService);
org = await os.createOrganization(currentUser.id, "myorg");
});

afterEach(async () => {
// Clean-up database
await resetDB(container.get(TypeORM));
});

describe("createAuthProviderOfUser", async () => {
it("should create user-level provider", async () => {
const providersAtStart = await service.getAllAuthProviderParams();
expect(providersAtStart).to.be.empty;

await service.createAuthProviderOfUser(currentUser.id, newEntry());

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

it("should fail in case of conflict with built-in provider", async () => {
addBuiltInProvider();

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

await expectError(ErrorCodes.CONFLICT, service.createAuthProviderOfUser(currentUser.id, newEntry()));
});
it("should fail if host is not reachable", async () => {
await expectError(
ErrorCodes.BAD_REQUEST,
service.createAuthProviderOfUser(currentUser.id, {
...newEntry(),
host: "please-dont-register-this-domain.com:666",
}),
);
});
it("should fail if trying to register same host", async () => {
const providersAtStart = await service.getAllAuthProviderParams();
expect(providersAtStart).to.be.empty;

await service.createAuthProviderOfUser(currentUser.id, newEntry());

await expectError(ErrorCodes.CONFLICT, service.createAuthProviderOfUser(currentUser.id, newEntry()));
});
});

describe("createOrgAuthProvider", async () => {
it("should create org-level provider", async () => {
const providersAtStart = await service.getAllAuthProviderParams();
expect(providersAtStart).to.be.empty;

await service.createOrgAuthProvider(currentUser.id, newOrgEntry());

const providers = await service.getAllAuthProviderParams();
expect(providers).to.have.lengthOf(1);
expect(providers[0]).to.deep.include(expectedOrgParams());
});
it("should fail if host is not reachable", async () => {
await expectError(
ErrorCodes.BAD_REQUEST,
service.createOrgAuthProvider(currentUser.id, {
...newOrgEntry(),
host: "please-dont-register-this-domain.com:666",
}),
);
});
it("should fail if trying to register same host", async () => {
const providersAtStart = await service.getAllAuthProviderParams();
expect(providersAtStart).to.be.empty;

await service.createOrgAuthProvider(currentUser.id, newOrgEntry());

await expectError(ErrorCodes.CONFLICT, service.createAuthProviderOfUser(currentUser.id, newOrgEntry()));
});
});
describe("getAuthProvider", async () => {
it("should find org-level provider", async () => {
const providersAtStart = await service.getAllAuthProviderParams();
expect(providersAtStart).to.be.empty;

const created = await service.createOrgAuthProvider(currentUser.id, newOrgEntry());

const retrieved = await service.getAuthProvider(currentUser.id, created.id);
expect(retrieved).to.deep.include(expectedOrgEntry());
});
it("should find user-level provider", async () => {
const providersAtStart = await service.getAllAuthProviderParams();
expect(providersAtStart).to.be.empty;

const created = await service.createAuthProviderOfUser(currentUser.id, newEntry());

const retrieved = await service.getAuthProvider(currentUser.id, created.id);
expect(retrieved).to.deep.include(expectedEntry());
});
it("should not find org-level provider for non-members", async () => {
const providersAtStart = await service.getAllAuthProviderParams();
expect(providersAtStart).to.be.empty;

const created = await service.createOrgAuthProvider(currentUser.id, newOrgEntry());

const nonMember = await userService.createUser({
identity: {
authId: "gh-user-2",
authName: "user2",
authProviderId: "public-github",
},
});

// expecting 404, as Orgs shall not be enumerable to non-members
await expectError(ErrorCodes.NOT_FOUND, service.getAuthProvider(nonMember.id, created.id));
});
});

describe("getAuthProviderDescriptionsUnauthenticated", async () => {
it("should find built-in provider", async () => {
addBuiltInProvider();

const providers = await service.getAuthProviderDescriptionsUnauthenticated();
expect(providers).to.has.lengthOf(1);
expect(providers[0].authProviderId).to.be.equal("Public-GitHub");
});
it("should find only built-in providers but no user-level providers", async () => {
addBuiltInProvider("localhost");

const created = await service.createAuthProviderOfUser(currentUser.id, newEntry());
await service.markAsVerified({ userId: currentUser.id, id: created.id });

const providers = await service.getAuthProviderDescriptionsUnauthenticated();
expect(providers).to.has.lengthOf(1);
expect(providers[0].host).to.be.equal("localhost");
});
it("should find user-level providers if no built-in providers present", async () => {
const created = await service.createAuthProviderOfUser(currentUser.id, newEntry());
await service.markAsVerified({ userId: currentUser.id, id: created.id });

const providers = await service.getAuthProviderDescriptionsUnauthenticated();
expect(providers).to.has.lengthOf(1);
expect(providers[0]).to.deep.include(<Partial<AuthProviderInfo>>{
authProviderId: created.id,
authProviderType: created.type,
host: created.host,
});

const privateProperties: (keyof AuthProviderEntry)[] = ["oauth", "organizationId", "ownerId"];
for (const privateProperty of privateProperties) {
expect(providers[0]).to.not.haveOwnProperty(privateProperty);
}
});
});

describe("getAuthProviderDescriptions", async () => {
it("should find built-in provider", async () => {
addBuiltInProvider();

const providers = await service.getAuthProviderDescriptions(currentUser);
expect(providers).to.has.lengthOf(1);
expect(providers[0].authProviderId).to.be.equal("Public-GitHub");
});
it("should find built-in providers and _own_ user-level providers", async () => {
addBuiltInProvider("localhost");

const created = await service.createAuthProviderOfUser(currentUser.id, newEntry());
await service.markAsVerified({ userId: currentUser.id, id: created.id });

const providers = await service.getAuthProviderDescriptions(currentUser);
expect(providers).to.has.lengthOf(2);
expect(providers[0].host).to.be.equal(created.host);
expect(providers[1].host).to.be.equal("localhost");
});
it("should find user-level providers if no built-in providers present", async () => {
const created = await service.createAuthProviderOfUser(currentUser.id, newEntry());
await service.markAsVerified({ userId: currentUser.id, id: created.id });

const providers = await service.getAuthProviderDescriptions(currentUser);
expect(providers).to.has.lengthOf(1);
expect(providers[0]).to.deep.include(<Partial<AuthProviderInfo>>{
authProviderId: created.id,
authProviderType: created.type,
host: created.host,
organizationId: created.organizationId,
ownerId: created.ownerId,
});

const oauthProperty: keyof AuthProviderEntry = "oauth";
expect(providers[0]).to.not.haveOwnProperty(oauthProperty);
});
});
});
Loading

0 comments on commit 341a5bd

Please sign in to comment.