diff --git a/components/dashboard/src/service/service-mock.ts b/components/dashboard/src/service/service-mock.ts index 87a5fb29872cc4..c743503e5fd8a1 100644 --- a/components/dashboard/src/service/service-mock.ts +++ b/components/dashboard/src/service/service-mock.ts @@ -192,7 +192,6 @@ const gitpodServiceMock = createServiceMock({ "clientId": "clientid-123", "clientSecret": "redacted" }, - "oauthRevision": "some-revision", "deleted": false }] }, diff --git a/components/gitpod-db/BUILD.yaml b/components/gitpod-db/BUILD.yaml index ac38f2035d4d7a..c5e1662721783a 100644 --- a/components/gitpod-db/BUILD.yaml +++ b/components/gitpod-db/BUILD.yaml @@ -16,8 +16,19 @@ packages: - name: migrations type: yarn srcs: - - "src/**/*.ts" - - package.json + - "src/typeorm/migration/**/*.ts" + - "src/typeorm/migrate-migrations-0_2_0.ts" + - "src/typeorm/entity/*.ts" + - "src/typeorm/ormconfig.ts" + - "src/typeorm/typeorm.ts" + - "src/typeorm/naming-strategy.ts" + - "src/typeorm/user-db-impl.ts" + - "src/typeorm/transformer.ts" + - "src/config.ts" + - "src/wait-for-db.ts" + - "src/migrate-migrations.ts" + - "src/user-db.ts" + - "package.json" deps: - components/gitpod-protocol:lib config: @@ -53,7 +64,6 @@ packages: - DB_PORT=23306 - DB_USER=root - DB_PASSWORD=test - - DB_ENCRYPTION_KEYS=[{"name":"general","version":1,"primary":true,"material":"5vRrp0H4oRgdkPnX1qQcS54Q0xggr6iyho42IQ1rO+c="}] ephemeral: true config: commands: diff --git a/components/gitpod-db/src/auth-provider-entry-db.ts b/components/gitpod-db/src/auth-provider-entry-db.ts index bb8b037037b30a..7caa5fe9707664 100644 --- a/components/gitpod-db/src/auth-provider-entry-db.ts +++ b/components/gitpod-db/src/auth-provider-entry-db.ts @@ -5,21 +5,15 @@ */ import { AuthProviderEntry as AuthProviderEntry } from "@gitpod/gitpod-protocol"; -import { createHash } from "crypto"; export const AuthProviderEntryDB = Symbol('AuthProviderEntryDB'); export interface AuthProviderEntryDB { - storeAuthProvider(ap: AuthProviderEntry, updateOAuthRevision: boolean): Promise; + storeAuthProvider(ap: AuthProviderEntry): Promise; delete(ap: AuthProviderEntry): Promise; - findAll(exceptOAuthRevisions?: string[]): Promise; - findAllHosts(): Promise; + findAll(): Promise; findByHost(host: string): Promise; findByUserId(userId: string): Promise; } - -export function hashOAuth(oauth: AuthProviderEntry["oauth"]): string { - return createHash('sha256').update(JSON.stringify(oauth)).digest('hex'); -} \ No newline at end of file diff --git a/components/gitpod-db/src/auth-provider-entry.spec.db.ts b/components/gitpod-db/src/auth-provider-entry.spec.db.ts deleted file mode 100644 index 3cef8be3702093..00000000000000 --- a/components/gitpod-db/src/auth-provider-entry.spec.db.ts +++ /dev/null @@ -1,94 +0,0 @@ -/** - * Copyright (c) 2022 Gitpod GmbH. All rights reserved. - * Licensed under the Gitpod Enterprise Source Code License, - * See License.enterprise.txt in the project root folder. - */ - -import * as chai from 'chai'; -import { suite, test, timeout } from 'mocha-typescript'; -import { testContainer } from './test-container'; -import { TypeORM } from './typeorm/typeorm'; -import { AuthProviderEntryDB } from '.'; -import { DBAuthProviderEntry } from './typeorm/entity/db-auth-provider-entry'; -import { DeepPartial } from '@gitpod/gitpod-protocol/lib/util/deep-partial'; -const expect = chai.expect; - -@suite @timeout(5000) -export class AuthProviderEntryDBSpec { - - typeORM = testContainer.get(TypeORM); - db = testContainer.get(AuthProviderEntryDB); - - async before() { - await this.clear(); - } - - async after() { - await this.clear(); - } - - protected async clear() { - const connection = await this.typeORM.getConnection(); - const manager = connection.manager; - await manager.clear(DBAuthProviderEntry); - } - - protected authProvider(ap: DeepPartial = {}): DBAuthProviderEntry { - const ownerId = "1234"; - const host = "github.com"; - return { - id: "0049b9d2-005f-43c2-a0ae-76377805d8b8", - host, - ownerId, - status: 'verified', - type: "GitHub", - oauthRevision: undefined, - deleted: false, - ...ap, - oauth: { - callBackUrl: "example.org/some/callback", - authorizationUrl: "example.org/some/auth", - settingsUrl: "example.org/settings", - configURL: "example.org/config", - clientId: "clientId", - clientSecret: "clientSecret", - tokenUrl: "example.org/get/token", - scope: "scope", - scopeSeparator: ",", - ...ap.oauth, - authorizationParams: {}, - }, - }; - } - - @test public async storeEmtpyOAuthRevision() { - const ap = this.authProvider(); - await this.db.storeAuthProvider(ap, false); - - const aap = await this.db.findByHost(ap.host); - expect(aap, "AuthProvider").to.deep.equal(ap); - } - - @test public async findAll() { - const ap1 = this.authProvider({ id: "1", oauthRevision: "rev1" }); - const ap2 = this.authProvider({ id: "2", oauthRevision: "rev2" }); - await this.db.storeAuthProvider(ap1, false); - await this.db.storeAuthProvider(ap2, false); - - const all = await this.db.findAll(); - expect(all, "findAll([])").to.deep.equal([ap1, ap2]); - expect(await this.db.findAll([ap1.oauthRevision!, ap2.oauthRevision!]), "findAll([ap1, ap2])").to.be.empty; - expect(await this.db.findAll([ap1.oauthRevision!]), "findAll([ap1])").to.deep.equal([ap2]); - } - - @test public async oauthRevision() { - const ap = this.authProvider({ id: "1" }); - await this.db.storeAuthProvider(ap, true); - - const loadedAp = await this.db.findByHost(ap.host); - expect(loadedAp, "findByHost()").to.deep.equal(ap); - expect(loadedAp?.oauthRevision, "findByHost()").to.equal("e05ea6fab8efcaba4b3246c2b2d3931af897c3bc2c1cf075c31614f0954f9840"); - } -} - -module.exports = AuthProviderEntryDBSpec diff --git a/components/gitpod-db/src/typeorm/auth-provider-entry-db-impl.ts b/components/gitpod-db/src/typeorm/auth-provider-entry-db-impl.ts index c043c5da1930f2..4eb1a099bf637f 100644 --- a/components/gitpod-db/src/typeorm/auth-provider-entry-db-impl.ts +++ b/components/gitpod-db/src/typeorm/auth-provider-entry-db-impl.ts @@ -11,7 +11,6 @@ import { AuthProviderEntry } from "@gitpod/gitpod-protocol"; import { AuthProviderEntryDB } from "../auth-provider-entry-db"; import { DBAuthProviderEntry } from "./entity/db-auth-provider-entry"; import { DBIdentity } from "./entity/db-identity"; -import { createHash } from "crypto"; @injectable() export class AuthProviderEntryDBImpl implements AuthProviderEntryDB { @@ -29,11 +28,8 @@ export class AuthProviderEntryDBImpl implements AuthProviderEntryDB { return (await this.getEntityManager()).getRepository(DBIdentity); } - async storeAuthProvider(ap: AuthProviderEntry, updateOAuthRevision: boolean): Promise { + async storeAuthProvider(ap: AuthProviderEntry): Promise { const repo = await this.getAuthProviderRepo(); - if (updateOAuthRevision) { - (ap.oauthRevision as any) = this.oauthContentHash(ap.oauth); - } return repo.save(ap); } @@ -49,27 +45,11 @@ export class AuthProviderEntryDBImpl implements AuthProviderEntryDB { await repo.update({ id }, { deleted: true }); } - async findAll(exceptOAuthRevisions: string[] = []): Promise { - exceptOAuthRevisions = exceptOAuthRevisions.filter(r => r !== ""); // never filter out '' which means "undefined" in the DB - - const repo = await this.getAuthProviderRepo(); - let query = repo.createQueryBuilder('auth_provider') - .where('auth_provider.deleted != true'); - if (exceptOAuthRevisions.length > 0) { - query = query.andWhere('auth_provider.oauthRevision NOT IN (:...exceptOAuthRevisions)', { exceptOAuthRevisions }); - } - return query.getMany(); - } - - async findAllHosts(): Promise { - const hostField: keyof DBAuthProviderEntry = "host"; - + async findAll(): Promise { const repo = await this.getAuthProviderRepo(); const query = repo.createQueryBuilder('auth_provider') - .select(hostField) .where('auth_provider.deleted != true'); - const result = (await query.execute()) as Pick[]; - return result.map(r => r.host); + return query.getMany(); } async findByHost(host: string): Promise { @@ -88,8 +68,4 @@ export class AuthProviderEntryDBImpl implements AuthProviderEntryDB { return query.getMany(); } - protected oauthContentHash(oauth: AuthProviderEntry["oauth"]): string { - const result = createHash('sha256').update(JSON.stringify(oauth)).digest('hex'); - return result; - } } diff --git a/components/gitpod-db/src/typeorm/entity/db-auth-provider-entry.ts b/components/gitpod-db/src/typeorm/entity/db-auth-provider-entry.ts index 43f20b84aaffa4..1cd535dc0c3667 100644 --- a/components/gitpod-db/src/typeorm/entity/db-auth-provider-entry.ts +++ b/components/gitpod-db/src/typeorm/entity/db-auth-provider-entry.ts @@ -4,7 +4,7 @@ * See License-AGPL.txt in the project root for license information. */ -import { PrimaryColumn, Column, Entity, Index } from "typeorm"; +import { PrimaryColumn, Column, Entity } from "typeorm"; import { TypeORM } from "../typeorm"; import { AuthProviderEntry, OAuth2Config } from "@gitpod/gitpod-protocol"; import { Transformer } from "../transformer"; @@ -37,13 +37,6 @@ export class DBAuthProviderEntry implements AuthProviderEntry { }) oauth: OAuth2Config; - @Index("ind_oauthRevision") - @Column({ - default: '', - transformer: Transformer.MAP_EMPTY_STR_TO_UNDEFINED, - }) - oauthRevision?: string; - @Column() deleted?: boolean; } diff --git a/components/gitpod-db/src/typeorm/migration/1643986994402-OAuthRevision.ts b/components/gitpod-db/src/typeorm/migration/1643986994402-OAuthRevision.ts deleted file mode 100644 index 0c8d057cd6276f..00000000000000 --- a/components/gitpod-db/src/typeorm/migration/1643986994402-OAuthRevision.ts +++ /dev/null @@ -1,52 +0,0 @@ -/** - * Copyright (c) 2021 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 { AuthProviderEntry } from "@gitpod/gitpod-protocol"; -import { MigrationInterface, QueryRunner } from "typeorm"; -import { dbContainerModule } from "../../container-module"; -import { columnExists, indexExists } from "./helper/helper"; -import { Container } from 'inversify'; -import { AuthProviderEntryDB } from "../../auth-provider-entry-db"; -import { UserDB } from "../../user-db"; - -const TABLE_NAME = "d_b_auth_provider_entry"; -const COLUMN_NAME: keyof AuthProviderEntry = "oauthRevision"; -const INDEX_NAME = "ind_oauthRevision"; - -export class OAuthRevision1643986994402 implements MigrationInterface { - - public async up(queryRunner: QueryRunner): Promise { - // create new column - if (!(await columnExists(queryRunner, TABLE_NAME, COLUMN_NAME))) { - await queryRunner.query(`ALTER TABLE ${TABLE_NAME} ADD COLUMN ${COLUMN_NAME} varchar(128) NOT NULL DEFAULT ''`); - } - - // create index on said column - if (!(await indexExists(queryRunner, TABLE_NAME, INDEX_NAME))) { - await queryRunner.query(`CREATE INDEX ${INDEX_NAME} ON ${TABLE_NAME} (${COLUMN_NAME})`); - } - - // to update all oauthRevisions we need to load all providers (to decrypt them) and - // write them back using the DB implementation (which does the calculation for us) - const container = new Container(); - container.load(dbContainerModule); - - container.get(UserDB); // initializes encryptionProvider as side effect - const db = container.get(AuthProviderEntryDB); - const allProviders = await db.findAll([]); - const writes: Promise[] = []; - for (const provider of allProviders) { - writes.push(db.storeAuthProvider(provider, true)); - } - await Promise.all(writes); - } - - public async down(queryRunner: QueryRunner): Promise { - await queryRunner.query(`ALTER TABLE ${TABLE_NAME} DROP INDEX ${INDEX_NAME}`); - await queryRunner.query(`ALTER TABLE ${TABLE_NAME} DROP COLUMN ${COLUMN_NAME}`); - } - -} diff --git a/components/gitpod-protocol/src/protocol.ts b/components/gitpod-protocol/src/protocol.ts index e01c3b60ae4cd5..2768b2955f2029 100644 --- a/components/gitpod-protocol/src/protocol.ts +++ b/components/gitpod-protocol/src/protocol.ts @@ -1171,8 +1171,6 @@ export interface AuthProviderEntry { readonly status: AuthProviderEntry.Status; readonly oauth: OAuth2Config; - /** A random string that is to change whenever oauth changes (enforced on DB level) */ - readonly oauthRevision?: string; } export interface OAuth2Config { diff --git a/components/server/src/auth/auth-provider-service.ts b/components/server/src/auth/auth-provider-service.ts index 826b138d9d8000..c6a799dc5ffc90 100644 --- a/components/server/src/auth/auth-provider-service.ts +++ b/components/server/src/auth/auth-provider-service.ts @@ -26,8 +26,8 @@ export class AuthProviderService { /** * Returns all auth providers. */ - async getAllAuthProviders(exceptOAuthRevisions: string[] = []): Promise { - const all = await this.authProviderDB.findAll(exceptOAuthRevisions); + async getAllAuthProviders(): Promise { + const all = await this.authProviderDB.findAll(); const transformed = all.map(this.toAuthProviderParams.bind(this)); // as a precaution, let's remove duplicates @@ -43,10 +43,6 @@ export class AuthProviderService { return Array.from(unique.values()); } - async getAllAuthProviderHosts(): Promise { - return this.authProviderDB.findAllHosts(); - } - protected toAuthProviderParams = (oap: AuthProviderEntry) => { ...oap, host: oap.host.toLowerCase(), @@ -86,14 +82,13 @@ export class AuthProviderService { } // update config on demand - const oauth = { - ...existing.oauth, - clientId: entry.clientId, - clientSecret: entry.clientSecret || existing.oauth.clientSecret, // FE may send empty ("") if not changed - }; authProvider = { ...existing, - oauth, + oauth: { + ...existing.oauth, + clientId: entry.clientId, + clientSecret: entry.clientSecret || existing.oauth.clientSecret, // FE may send empty ("") if not changed + }, status: "pending", } } else { @@ -103,7 +98,7 @@ export class AuthProviderService { } authProvider = this.initializeNewProvider(entry); } - return await this.authProviderDB.storeAuthProvider(authProvider as AuthProviderEntry, true); + return await this.authProviderDB.storeAuthProvider(authProvider as AuthProviderEntry); } protected initializeNewProvider(newEntry: AuthProviderEntry.NewEntry): AuthProviderEntry { const { host, type, clientId, clientSecret } = newEntry; @@ -111,17 +106,16 @@ export class AuthProviderService { if (!urls) { throw new Error("Unexpected service type."); } - const oauth: AuthProviderEntry["oauth"] = { - ...urls, - callBackUrl: this.callbackUrl(host), - clientId: clientId!, - clientSecret: clientSecret!, - }; - return { + return { ...newEntry, id: uuidv4(), type, - oauth, + oauth: { + ...urls, + callBackUrl: this.callbackUrl(host), + clientId, + clientSecret, + }, status: "pending", }; } @@ -142,7 +136,7 @@ export class AuthProviderService { ownerId: ownerId, status: "verified" }; - await this.authProviderDB.storeAuthProvider(ap, true); + await this.authProviderDB.storeAuthProvider(ap); } else { log.warn("Failed to find the AuthProviderEntry to be activated.", { params, id, ap }); } diff --git a/components/server/src/auth/host-context-provider-impl.ts b/components/server/src/auth/host-context-provider-impl.ts index 4bd22282a6a9ea..9dda0b9dbd1f4c 100644 --- a/components/server/src/auth/host-context-provider-impl.ts +++ b/components/server/src/auth/host-context-provider-impl.ts @@ -13,8 +13,6 @@ import { HostContextProvider, HostContextProviderFactory } from "./host-context- import { log } from '@gitpod/gitpod-protocol/lib/util/logging'; import { HostContainerMapping } from "./host-container-mapping"; import { RepositoryService } from "../repohost/repo-service"; -import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing"; -import { repeat } from "@gitpod/gitpod-protocol/lib/util/repeat"; @injectable() export class HostContextProviderImpl implements HostContextProvider { @@ -42,22 +40,21 @@ export class HostContextProviderImpl implements HostContextProvider { this.createFixedHosts(); try { - await this.updateDynamicHosts({ }); + await this.updateDynamicHosts(); } catch (error) { log.error(`Failed to update dynamic hosts.`, error); } // schedule periodic update of dynamic hosts - repeat(async () => { - const span = TraceContext.startSpan("updateDynamicHosts"); + const scheduler = () => setTimeout(async () => { try { - await this.updateDynamicHosts({span}); + await this.updateDynamicHosts(); } catch (error) { log.error(`Failed to update dynamic hosts.`, error); - } finally { - span.finish(); } + scheduler(); }, 1999); + scheduler(); this.initialized = true; } @@ -71,14 +68,11 @@ export class HostContextProviderImpl implements HostContextProvider { } } - protected async updateDynamicHosts(ctx: TraceContext) { - const knownOAuthRevisions = Array.from(this.dynamicHosts.entries()) - .map(([_, hostContext]) => hostContext.authProvider.params.oauthRevision) - .filter(rev => !!rev) as string[]; - const newAndUpdatedAuthProviders = await this.authProviderService.getAllAuthProviders(knownOAuthRevisions); - ctx.span?.setTag("updateDynamicHosts.newAndUpdatedAuthProviders", newAndUpdatedAuthProviders.length); + protected async updateDynamicHosts() { + const all = await this.authProviderService.getAllAuthProviders(); - for (const config of newAndUpdatedAuthProviders) { + const currentHosts = new Set(all.map(p => p.host.toLowerCase())); + for (const config of all) { const { host } = config; const existingContext = this.dynamicHosts.get(host); @@ -88,13 +82,9 @@ export class HostContextProviderImpl implements HostContextProvider { log.warn("Ignoring host update for dynamic Auth Provider: " + host, { config, existingConfig }); continue; } - if (existingConfig.status === config.status) { - if (!!config.oauthRevision && existingConfig.oauthRevision === config.oauthRevision) { - continue; - } - if (JSON.stringify(existingConfig.oauth) === JSON.stringify(config.oauth)) { - continue; - } + if (JSON.stringify(existingConfig.oauth) === JSON.stringify(config.oauth) + && existingConfig.status === config.status) { + continue; } log.debug("Updating existing dynamic Auth Provider: " + host, { config, existingConfig }); } else { @@ -111,8 +101,6 @@ export class HostContextProviderImpl implements HostContextProvider { } // remove obsolete entries - const currentHosts = new Set(await this.authProviderService.getAllAuthProviderHosts()) - ctx.span?.setTag("updateDynamicHosts.currentHostProviders", currentHosts.size); const tobeRemoved = [...this.dynamicHosts.keys()].filter(h => !currentHosts.has(h)); for (const host of tobeRemoved) { const hostContext = this.dynamicHosts.get(host);