diff --git a/components/gitpod-db/package.json b/components/gitpod-db/package.json index 086768d0e5bc14..219ef46769f6fa 100644 --- a/components/gitpod-db/package.json +++ b/components/gitpod-db/package.json @@ -22,7 +22,7 @@ ], "dependencies": { "@gitpod/gitpod-protocol": "0.1.5", - "@jmondi/oauth2-server": "^1.1.0", + "@jmondi/oauth2-server": "^2.1.0", "mysql": "^2.15.0", "reflect-metadata": "^0.1.10", "the-big-username-blacklist": "^1.5.2", diff --git a/components/gitpod-db/src/typeorm/auth-code-repository-db.ts b/components/gitpod-db/src/typeorm/auth-code-repository-db.ts index 7d2336d0a3246b..9c232a9c1cfe57 100644 --- a/components/gitpod-db/src/typeorm/auth-code-repository-db.ts +++ b/components/gitpod-db/src/typeorm/auth-code-repository-db.ts @@ -27,7 +27,7 @@ export class AuthCodeRepositoryDB implements OAuthAuthCodeRepository { return (await this.getEntityManager()).getRepository(DBOAuthAuthCodeEntry); } - public async getByIdentifier(authCodeCode: string): Promise { + public async getByIdentifier(authCodeCode: string): Promise { const authCodeRepo = await this.getOauthAuthCodeRepo(); let authCodes = await authCodeRepo.find({ code: authCodeCode }); authCodes = authCodes.filter(te => (new Date(te.expiresAt)).getTime() > Date.now()); @@ -48,7 +48,7 @@ export class AuthCodeRepositoryDB implements OAuthAuthCodeRepository { scopes: scopes, }; } - public async persist(authCode: OAuthAuthCode): Promise { + public async persist(authCode: DBOAuthAuthCodeEntry): Promise { const authCodeRepo = await this.getOauthAuthCodeRepo(); authCodeRepo.save(authCode); } diff --git a/components/gitpod-db/src/typeorm/entity/db-oauth-auth-code.ts b/components/gitpod-db/src/typeorm/entity/db-oauth-auth-code.ts index 2b91d35d1006a2..8a1b6319d51e79 100644 --- a/components/gitpod-db/src/typeorm/entity/db-oauth-auth-code.ts +++ b/components/gitpod-db/src/typeorm/entity/db-oauth-auth-code.ts @@ -4,7 +4,7 @@ * See License-AGPL.txt in the project root for license information. */ -import { OAuthAuthCode, OAuthClient, OAuthScope } from "@jmondi/oauth2-server"; +import { CodeChallengeMethod, OAuthAuthCode, OAuthClient, OAuthScope } from "@jmondi/oauth2-server"; import { Column, Entity, JoinColumn, ManyToOne, PrimaryGeneratedColumn } from "typeorm"; import { Transformer } from "../transformer"; import { DBUser } from "./db-user"; @@ -38,7 +38,7 @@ export class DBOAuthAuthCodeEntry implements OAuthAuthCode { type: "varchar", length: 10, }) - codeChallengeMethod: string + codeChallengeMethod: CodeChallengeMethod @Column({ type: 'timestamp', diff --git a/components/gitpod-db/src/typeorm/user-db-impl.ts b/components/gitpod-db/src/typeorm/user-db-impl.ts index 85dc3135e88810..290b2041f85335 100644 --- a/components/gitpod-db/src/typeorm/user-db-impl.ts +++ b/components/gitpod-db/src/typeorm/user-db-impl.ts @@ -419,7 +419,7 @@ export class TypeORMUserDBImpl implements UserDB { } else { var user: MaybeUser; if (accessToken.user) { - user = await this.findUserById(accessToken.user.id) + user = await this.findUserById(String(accessToken.user.id)) } dbToken = { tokenHash, diff --git a/components/server/package.json b/components/server/package.json index df4357171c409c..91b576d569308b 100644 --- a/components/server/package.json +++ b/components/server/package.json @@ -39,7 +39,7 @@ "@gitpod/ws-manager": "0.1.5", "@google-cloud/storage": "^5.6.0", "@improbable-eng/grpc-web-node-http-transport": "^0.14.0", - "@jmondi/oauth2-server": "^1.1.0", + "@jmondi/oauth2-server": "^2.1.0", "@octokit/rest": "18.5.6", "@probot/get-private-key": "1.1.0", "amqplib": "^0.5.2", diff --git a/components/server/src/oauth-server/db.ts b/components/server/src/oauth-server/db.ts index 4eed0cc74f97f4..a241c0ac54c125 100644 --- a/components/server/src/oauth-server/db.ts +++ b/components/server/src/oauth-server/db.ts @@ -17,7 +17,7 @@ export interface InMemory { } // Scopes -const scopes: OAuthScope[] = [ +const localAppScopes: OAuthScope[] = [ { name: "function:getGitpodTokenScopes" }, { name: "function:getWorkspace" }, { name: "function:getWorkspaces" }, @@ -35,16 +35,39 @@ const localClient: OAuthClient = { // NOTE: these need to be kept in sync with the port range in the local app redirectUris: Array.from({ length: 10 }, (_, i) => 'http://127.0.0.1:' + (63110 + i)), allowedGrants: ['authorization_code'], - scopes, + scopes: localAppScopes, } +function createVSCodeClient(protocol: 'vscode' | 'vscode-insiders'): OAuthClient { + return { + id: protocol + '-' + 'gitpod', + name: `VS Code${protocol === 'vscode-insiders' ? ' Insiders' : ''}: Gitpod`, + redirectUris: [protocol + '://gitpod.gitpod-desktop/complete-gitpod-auth'], + allowedGrants: ['authorization_code'], + scopes: [ + { name: "function:getGitpodTokenScopes" }, + { name: "function:getLoggedInUser" }, + { name: "function:accessCodeSyncStorage" }, + { name: "resource:default" } + ], + } +} + +const vscode = createVSCodeClient('vscode'); +const vscodeInsiders = createVSCodeClient('vscode-insiders'); + export const inMemoryDatabase: InMemory = { clients: { [localClient.id]: localClient, + [vscode.id]: vscode, + [vscodeInsiders.id]: vscodeInsiders }, tokens: {}, scopes: {}, }; -for (const scope of scopes) { - inMemoryDatabase.scopes[scope.name] = scope; +for (const clientId in inMemoryDatabase.clients) { + const client = inMemoryDatabase.clients[clientId]; + for (const scope of client.scopes) { + inMemoryDatabase.scopes[scope.name] = scope; + } } diff --git a/components/server/src/oauth-server/oauth-controller.ts b/components/server/src/oauth-server/oauth-controller.ts index a5d7d2a4b2cde4..f82b2364b09b3a 100644 --- a/components/server/src/oauth-server/oauth-controller.ts +++ b/components/server/src/oauth-server/oauth-controller.ts @@ -8,9 +8,11 @@ import { AuthCodeRepositoryDB } from '@gitpod/gitpod-db/lib/typeorm/auth-code-re import { UserDB } from '@gitpod/gitpod-db/lib/user-db'; import { User } from "@gitpod/gitpod-protocol"; import { log } from '@gitpod/gitpod-protocol/lib/util/logging'; -import { OAuthException, OAuthRequest, OAuthResponse } from "@jmondi/oauth2-server"; +import { OAuthRequest, OAuthResponse } from "@jmondi/oauth2-server"; +import { handleExpressResponse, handleExpressError } from "@jmondi/oauth2-server/dist/adapters/express" import * as express from 'express'; import { inject, injectable } from "inversify"; +import { URL } from 'url'; import { Config } from '../config'; import { clientRepository, createAuthorizationServer } from './oauth-authorization-server'; @@ -40,7 +42,7 @@ export class OAuthController { } private async hasApproval(user: User, clientID: string, req: express.Request, res: express.Response): Promise { - // Have they just authorized, or not, the local-app? + // Have they just authorized, or not, registered clients? const wasApproved = req.query['approved'] || ''; if (wasApproved === 'no') { const additionalData = user?.additionalData; @@ -49,14 +51,25 @@ export class OAuthController { await this.userDb.updateUserPartial(user); } - // Let the local app know they rejected the approval - const rt = req.query.redirect_uri; - if (!rt || !rt.startsWith("http://127.0.0.1:")) { - log.error(`/oauth/authorize: invalid returnTo URL: "${rt}"`) + // Let the client know they rejected the approval + const client = await clientRepository.getByIdentifier(clientID); + if (client) { + const normalizedRedirectUri = new URL(req.query.redirect_uri); + normalizedRedirectUri.search = ''; + + if (!client.redirectUris.some(u => new URL(u).toString() === normalizedRedirectUri.toString())) { + log.error(`/oauth/authorize: invalid returnTo URL: "${req.query.redirect_uri}"`) + res.sendStatus(400); + return false; + } + } else { + log.error(`/oauth/authorize unknown client id: "${clientID}"`) res.sendStatus(400); return false; } - res.redirect(`${rt}/?approved=no`); + const redirectUri = new URL(req.query.redirect_uri); + redirectUri.searchParams.append('approved', 'no'); + res.redirect(redirectUri.toString()); return false; } else if (wasApproved == 'yes') { const additionalData = user.additionalData = user.additionalData || {}; @@ -123,9 +136,9 @@ export class OAuthController { // Return the HTTP redirect response const oauthResponse = await authorizationServer.completeAuthorizationRequest(authRequest); - return handleResponse(req, res, oauthResponse); + return handleExpressResponse(res, oauthResponse); } catch (e) { - handleError(e, res); + handleExpressError(e, res); } }); @@ -133,43 +146,13 @@ export class OAuthController { const response = new OAuthResponse(res); try { const oauthResponse = await authorizationServer.respondToAccessTokenRequest(req, response); - return handleResponse(req, res, oauthResponse); + return handleExpressResponse(res, oauthResponse); } catch (e) { - handleError(e, res); + handleExpressError(e, res); return; } }); - function handleError(e: Error | undefined, res: express.Response) { - if (e instanceof OAuthException) { - res.status(e.status); - res.send({ - status: e.status, - message: e.message, - stack: e.stack, - }); - return; - } - // Generic error - res.status(500) - res.send({ - err: e - }) - } - - function handleResponse(req: express.Request, res: express.Response, response: OAuthResponse) { - if (response.status === 302) { - if (!response.headers.location) { - throw new Error("missing redirect location"); - } - res.set(response.headers); - res.redirect(response.headers.location); - } else { - res.set(response.headers); - res.status(response.status).send(response.body); - } - } - return router; } } \ No newline at end of file diff --git a/yarn.lock b/yarn.lock index 2ac19904f70344..fa8c5cd6a242f8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3286,10 +3286,10 @@ "@types/yargs" "^15.0.0" chalk "^4.0.0" -"@jmondi/oauth2-server@^1.1.0": - version "1.1.0" - resolved "https://registry.yarnpkg.com/@jmondi/oauth2-server/-/oauth2-server-1.1.0.tgz#37014c4aceaee9b4559df224a4d3a743e66e5929" - integrity sha512-7UuliIJVnn3ISVEQ/CeRdKdY6gQb+RbOAlCZysdWytcvWNF+5Xb32ARbjOnzzLRzlh5ZxAKC5Zta0TZSKeXchg== +"@jmondi/oauth2-server@^2.1.0": + version "2.1.0" + resolved "https://registry.yarnpkg.com/@jmondi/oauth2-server/-/oauth2-server-2.1.0.tgz#ffa10dd8b9c5c8b480824bf0ecf104d5d00ec4a7" + integrity sha512-R6zxiKCC0MyAk3M9rV8gM0bDqvXNZgiDLTriefkfsZIXZoVw52W2X8usf5Y8qSwnmdP4u3ijXbb2fSUAcSbDdA== dependencies: jsonwebtoken "^8.5.1" ms "^2.1.3" @@ -4665,7 +4665,7 @@ "@types/history" "*" "@types/react" "*" -"@types/react@*", "@types/react@^17.0.0": +"@types/react@*", "@types/react@17.0.0", "@types/react@^17.0.0": version "17.0.0" resolved "https://registry.yarnpkg.com/@types/react/-/react-17.0.0.tgz#5af3eb7fad2807092f0046a1302b7823e27919b8" integrity sha512-aj/L7RIMsRlWML3YB6KZiXB3fV2t41+5RBGYF8z+tAKU43Px8C3cYUZsDvf1/+Bm4FK21QWBrDutu8ZJ/70qOw== @@ -14859,7 +14859,7 @@ mz@^2.4.0: object-assign "^4.0.1" thenify-all "^1.0.0" -nan@^2.12.1, nan@^2.13.2, nan@^2.9.2: +nan@2.14.1, nan@^2.12.1, nan@^2.13.2, nan@^2.14.0, nan@^2.9.2: version "2.14.1" resolved "https://registry.yarnpkg.com/nan/-/nan-2.14.1.tgz#d7be34dfa3105b91494c3147089315eff8874b01" integrity sha512-isWHgVjnFjh2x2yuJ/tj3JbwoHu3UC2dX5G/88Cm24yB6YopVgxvBObDY7n5xW6ExmFhJpSEQqFPvq9zaXc8Jw== @@ -15406,6 +15406,13 @@ onetime@^5.1.2: dependencies: mimic-fn "^2.1.0" +oniguruma@7.2.1: + version "7.2.1" + resolved "https://registry.yarnpkg.com/oniguruma/-/oniguruma-7.2.1.tgz#51775834f7819b6e31aa878706aa7f65ad16b07f" + integrity sha512-WPS/e1uzhswPtJSe+Zls/kAj27+lEqZjCmRSjnYk/Z4L2Mu+lJC2JWtkZhPJe4kZeTQfz7ClcLyXlI4J68MG2w== + dependencies: + nan "^2.14.0" + open@^7.0.2: version "7.4.2" resolved "https://registry.yarnpkg.com/open/-/open-7.4.2.tgz#b8147e26dcf3e426316c730089fd71edd29c2321" @@ -17540,7 +17547,7 @@ react-dev-utils@^11.0.3: strip-ansi "6.0.0" text-table "0.2.0" -react-dom@^17.0.1: +react-dom@17.0.1, react-dom@^17.0.1: version "17.0.1" resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-17.0.1.tgz#1de2560474ec9f0e334285662ede52dbc5426fc6" integrity sha512-6eV150oJZ9U2t9svnsspTMrWNyHc6chX0KzDeAOXftRa8bNeOKTTfCJ7KorIwenkHd2xqVTBTCZd79yk/lx/Ug== @@ -17669,7 +17676,7 @@ react-scripts@^4.0.3: optionalDependencies: fsevents "^2.1.3" -react@^17.0.1: +react@17.0.1, react@^17.0.1: version "17.0.1" resolved "https://registry.yarnpkg.com/react/-/react-17.0.1.tgz#6e0600416bd57574e3f86d92edba3d9008726127" integrity sha512-lG9c9UuMHdcAexXtigOZLX8exLWkW0Ku29qPRU8uhF2R9BN96dLCt0psvzPLlHc5OWkgymP3qwTRgbnw5BKx3w==