diff --git a/chart/templates/server-deployment.yaml b/chart/templates/server-deployment.yaml index d5f48b3966940f..88dd1059b8cbfc 100644 --- a/chart/templates/server-deployment.yaml +++ b/chart/templates/server-deployment.yaml @@ -220,6 +220,8 @@ spec: key: apikey - name: GITPOD_GARBAGE_COLLECTION_DISABLED value: {{ $comp.garbageCollection.disabled | default "false" | quote }} + - name: OAUTH_SERVER_JWT_SECRET + value: {{ $comp.oauthServerJWTSecret | quote }} {{- if $comp.serverContainer.env }} {{ toYaml $comp.serverContainer.env | indent 8 }} {{- end }} diff --git a/chart/values.yaml b/chart/values.yaml index bfe17fd7074ce9..52a3416599251d 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -269,6 +269,8 @@ components: - "server-proxy-apikey-secret.yaml" - "auth-providers-configmap.yaml" sessionSecret: Important!Really-Change-This-Key! + oauthServerJWTSecret: | + {{ (randAlphaNum 20) | quote }} resources: cpu: "200m" github: diff --git a/components/dashboard/src/Login.tsx b/components/dashboard/src/Login.tsx index dee9e9895f6c99..e4e9812d6824bc 100644 --- a/components/dashboard/src/Login.tsx +++ b/components/dashboard/src/Login.tsx @@ -50,6 +50,23 @@ export function Login() { })(); }, []) + const authorizeSuccessful = async (payload?: string) => { + updateUser(); + // Check for a valid returnTo in payload + const safeReturnTo = getSafeURLRedirect(payload); + if (safeReturnTo) { + // ... and if it is, redirect to it + window.location.replace(safeReturnTo); + } + } + + const updateUser = async () => { + await getGitpodService().reconnect(); + const user = await getGitpodService().server.getLoggedInUser(); + setUser(user); + markLoggedIn(); + } + const openLogin = async (host: string) => { setErrorMessage(undefined); @@ -57,7 +74,7 @@ export function Login() { await openAuthorizeWindow({ login: true, host, - onSuccess: (payload?: string) => authorizeSuccessful(payload), + onSuccess: authorizeSuccessful, onError: (payload) => { let errorMessage: string; if (typeof payload === "string") { @@ -76,23 +93,6 @@ export function Login() { } } - const authorizeSuccessful = async (payload?: string) => { - updateUser(); - // Check for a valid returnTo in payload - const safeReturnTo = getSafeURLRedirect(payload); - if (safeReturnTo) { - // ... and if it is, redirect to it - window.location.replace(safeReturnTo); - } - } - - const updateUser = async () => { - await getGitpodService().reconnect(); - const user = await getGitpodService().server.getLoggedInUser(); - setUser(user); - markLoggedIn(); - } - return (
{showWelcome ?
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 398d4f12fe0c64..91b5c056d93f38 100644 --- a/components/gitpod-db/src/typeorm/auth-code-repository-db.ts +++ b/components/gitpod-db/src/typeorm/auth-code-repository-db.ts @@ -36,28 +36,21 @@ export class AuthCodeRepositoryDB implements OAuthAuthCodeRepository { authCodes = authCodes.filter(te => (new Date(te.expiresAt)).getTime() > Date.now()); log.info(`getByIdentifier post: ${JSON.stringify(authCodes)}`); const authCode = authCodes.length > 0 ? authCodes[0] : undefined; - return new Promise((resolve, reject) => { - if (authCode) { - log.info(`getByIdentifier found ${authCodeCode} ${JSON.stringify(authCode)}`); - resolve(authCode); - } else { - log.info(`getByIdentifier failed to find ${authCodeCode}`); - reject(`authentication code not found`); - } - }); + if (!authCode) { + throw new Error(`authentication code not found`); + } + return authCode; } public issueAuthCode(client: OAuthClient, user: OAuthUser | undefined, scopes: OAuthScope[]): OAuthAuthCode { const code = crypto.randomBytes(30).toString('hex'); log.info(`issueAuthCode: ${JSON.stringify(client)}, ${JSON.stringify(user)}, ${JSON.stringify(scopes)}, ${code}`); + // NOTE: caller (@jmondi/oauth2-server) is responsible for adding the remaining items, PKCE params, redirect URL, etc return { code: code, user, client, - redirectUri: "", - codeChallenge: undefined, - codeChallengeMethod: undefined, expiresAt: expiryInFuture.getEndDate(), - scopes: [], + scopes: scopes, }; } public async persist(authCode: OAuthAuthCode): Promise { diff --git a/components/gitpod-db/src/typeorm/user-db-impl.ts b/components/gitpod-db/src/typeorm/user-db-impl.ts index d79ca4ee4c6ee8..5df06a4df177b4 100644 --- a/components/gitpod-db/src/typeorm/user-db-impl.ts +++ b/components/gitpod-db/src/typeorm/user-db-impl.ts @@ -399,10 +399,7 @@ export class TypeORMUserDBImpl implements UserDB { } async persist(accessToken: OAuthToken): Promise { log.info(`persist access token ${JSON.stringify(accessToken)}`); - var scopes: string[] = []; - for (const scope of accessToken.scopes) { - scopes = scopes.concat(scope.name); - } + const scopes = accessToken.scopes.map((s) => s.name); // Does the token already exist? var dbToken: GitpodToken & { user: DBUser }; @@ -424,7 +421,7 @@ export class TypeORMUserDBImpl implements UserDB { } dbToken = { tokenHash, - name: `local-app`, + name: accessToken.client.id, type: GitpodTokenType.MACHINE_AUTH_TOKEN, user: user as DBUser, scopes: scopes, diff --git a/components/server/src/env.ts b/components/server/src/env.ts index f2da76b311ffed..959d4a569484fb 100644 --- a/components/server/src/env.ts +++ b/components/server/src/env.ts @@ -202,4 +202,5 @@ export class Env extends AbstractComponentEnv { readonly runDbDeleter: boolean = getEnvVar('RUN_DB_DELETER', 'false') === 'true'; + readonly oauthServerJWTSecret = getEnvVar("OAUTH_SERVER_JWT_SECRET") } diff --git a/components/server/src/oauth-server/db.ts b/components/server/src/oauth-server/db.ts index b15ad9ad2863e4..b4c202ab889db0 100644 --- a/components/server/src/oauth-server/db.ts +++ b/components/server/src/oauth-server/db.ts @@ -27,7 +27,7 @@ const getWorkspaceResourceScope: OAuthScope = { name: "resource:" + ScopedResour const getWorkspaceInstanceResourceScope: OAuthScope = { name: "resource:" + ScopedResourceGuard.marshalResourceScope({ kind: "workspaceInstance", subjectID: "*", operations: ["get"] }) }; // Clients -export const localAppClientID = 'gplctl-1.0'; +const localAppClientID = 'gplctl-1.0'; const localClient: OAuthClient = { id: localAppClientID, secret: `${localAppClientID}-secret`, diff --git a/components/server/src/oauth-server/oauth-authorization-server.ts b/components/server/src/oauth-server/oauth-authorization-server.ts index 33963c61babd02..60d0f8db03e6a1 100644 --- a/components/server/src/oauth-server/oauth-authorization-server.ts +++ b/components/server/src/oauth-server/oauth-authorization-server.ts @@ -14,9 +14,6 @@ import { const clientRepository = inMemoryClientRepository; const scopeRepository = inMemoryScopeRepository; -// TODO(rl) - get this from external secret -const jwtService = new JwtService("secret secret secret"); - class GitpodAuthorizationServer extends AuthorizationServer { enableGrantType(grantType: GrantIdentifier, accessTokenTTL?: DateInterval): void { log.info(`enableGrantType: ${grantType}:${JSON.stringify(accessTokenTTL)}`) @@ -43,14 +40,19 @@ class GitpodAuthorizationServer extends AuthorizationServer { } } -export function createAuthorizationServer(authCodeRepository: OAuthAuthCodeRepository, userRepository: OAuthUserRepository, tokenRepository: OAuthTokenRepository): GitpodAuthorizationServer { +export function createAuthorizationServer(authCodeRepository: OAuthAuthCodeRepository, userRepository: OAuthUserRepository, tokenRepository: OAuthTokenRepository, jwtSecret: string): GitpodAuthorizationServer { + log.info(`JWT:${jwtSecret}`) const authorizationServer = new GitpodAuthorizationServer( authCodeRepository, clientRepository, tokenRepository, scopeRepository, userRepository, - jwtService, + new JwtService(jwtSecret), + { + // Be explicit, communicate intent. Default is true but let's not assume that + requiresPKCE: true, + } ); authorizationServer.enableGrantType("authorization_code", new DateInterval('1d')); diff --git a/components/server/src/oauth-server/oauth-controller.ts b/components/server/src/oauth-server/oauth-controller.ts index 7cdac46588f616..fc3457e28e783a 100644 --- a/components/server/src/oauth-server/oauth-controller.ts +++ b/components/server/src/oauth-server/oauth-controller.ts @@ -13,7 +13,6 @@ import * as express from 'express'; import { inject, injectable } from "inversify"; import { Env } from "../env"; import { createAuthorizationServer } from './oauth-authorization-server'; -import { localAppClientID } from "./db"; @injectable() export class OAuthController { @@ -23,126 +22,131 @@ export class OAuthController { get oauthRouter(): express.Router { const router = express.Router(); - if (this.env.enableOAuthServer) { - const authorizationServer = createAuthorizationServer(this.authCodeRepositoryDb, this.userDb, this.userDb); - router.get("/oauth/authorize", async (req: express.Request, res: express.Response) => { - log.info(`AUTHORIZE: ${JSON.stringify(req.query)}`); + if (!this.env.enableOAuthServer) { + log.warn('OAuth server disabled!') + return router; + } - if (!req.isAuthenticated() || !User.is(req.user)) { - const redirectTarget = encodeURIComponent(`${this.env.hostUrl}api${req.originalUrl}`); - const redirectTo = `${this.env.hostUrl}login?returnTo=${redirectTarget}`; - log.info(`AUTH Redirecting to login: ${redirectTo}`); - res.redirect(redirectTo) - return - } + const authorizationServer = createAuthorizationServer(this.authCodeRepositoryDb, this.userDb, this.userDb, this.env.oauthServerJWTSecret); + router.get("/oauth/authorize", async (req: express.Request, res: express.Response) => { + log.info(`AUTHORIZE: ${JSON.stringify(req.query)}`); + + if (!req.isAuthenticated() || !User.is(req.user)) { + const redirectTarget = encodeURIComponent(`${this.env.hostUrl}api${req.originalUrl}`); + const redirectTo = `${this.env.hostUrl}login?returnTo=${redirectTarget}`; + log.info(`AUTH Redirecting to login: ${redirectTo}`); + res.redirect(redirectTo) + return + } - const user = req.user as User; - if (user.blocked) { - res.sendStatus(403); + const user = req.user as User; + if (user.blocked) { + res.sendStatus(403); + return; + } + + // Have they authorized the local-app? + const wasApproved = req.query['approved'] || ''; + log.info(`APPROVED?: ${wasApproved}`) + if (wasApproved === 'no') { + // Let the local app know they rejected the approval + const rt = req.query.redirect_uri; + if (!rt || !rt.startsWith("http://localhost:")) { + log.error(`/oauth/authorize: invalid returnTo URL: "${rt}"`) + res.sendStatus(400); return; } + res.redirect(`${rt}/?approved=no`); + return; + } - // Have they authorized the local-app? - const wasApproved = req.query['approved'] || ''; - log.info(`APPROVED?: ${wasApproved}`) - if (wasApproved === 'no') { - // Let the local app know they rejected the approval - const rt = req.query.redirect_uri; - if (!rt || !rt.startsWith("http://localhost:")) { - log.error(`/oauth/authorize: invalid returnTo URL: "${rt}"`) - res.sendStatus(400); - return; - } - res.redirect(`${rt}/?approved=no`); + const oauthClientsApproved = user?.additionalData?.oauthClientsApproved; + const clientID = req.query.client_id; + if (!clientID) { + res.sendStatus(400); + return; + } + if (!oauthClientsApproved || !oauthClientsApproved[clientID]) { + const client = await authorizationServer.getClientByIdentifier(clientID) + if (client) { + const redirectTarget = encodeURIComponent(`${this.env.hostUrl}api${req.originalUrl}`); + const redirectTo = `${this.env.hostUrl}oauth-approval?clientID=${client.id}&clientName=${client.name}&returnTo=${redirectTarget}`; + log.info(`AUTH Redirecting to approval: ${redirectTo}`); + res.redirect(redirectTo) + return; + } else { + log.error(`/oauth/authorize unknown client id: "${clientID}"`) + res.sendStatus(400); return; } + } - const oauthClientsApproved = user?.additionalData?.oauthClientsApproved; - const clientID = localAppClientID; - if (!oauthClientsApproved || !oauthClientsApproved[clientID]) { - const client = await authorizationServer.getClientByIdentifier(clientID) - if (client) { - const redirectTarget = encodeURIComponent(`${this.env.hostUrl}api${req.originalUrl}`); - const redirectTo = `${this.env.hostUrl}oauth-approval?clientID=${client.id}&clientName=${client.name}&returnTo=${redirectTarget}`; - log.info(`AUTH Redirecting to approval: ${redirectTo}`); - res.redirect(redirectTo) - return; - } else { - log.error(`/oauth/authorize unknown client id: "${clientID}"`) - res.sendStatus(400); - return; - } - } + const request = new OAuthRequest(req); - const request = new OAuthRequest(req); + try { + // Validate the HTTP request and return an AuthorizationRequest object. + const authRequest = await authorizationServer.validateAuthorizationRequest(request); - try { - // Validate the HTTP request and return an AuthorizationRequest object. - const authRequest = await authorizationServer.validateAuthorizationRequest(request); + // Once the user has logged in set the user on the AuthorizationRequest + authRequest.user = { id: user.id } + console.log(`user has logged in - setting the user on the AuthorizationRequest ${JSON.stringify(user)}, ${JSON.stringify(authRequest)}`); - // Once the user has logged in set the user on the AuthorizationRequest - authRequest.user = { id: user.id } - console.log(`user has logged in - setting the user on the AuthorizationRequest ${JSON.stringify(user)}, ${JSON.stringify(authRequest)}`); + // The user has approved the client so update the status + authRequest.isAuthorizationApproved = true; - // The user has approved the client so update the status - authRequest.isAuthorizationApproved = true; + // Return the HTTP redirect response + const oauthResponse = await authorizationServer.completeAuthorizationRequest(authRequest); + return handleResponse(req, res, oauthResponse); + } catch (e) { + handleError(e, res); + } + }); + + router.post("/oauth/token", async (req: express.Request, res: express.Response) => { + log.info(`TOKEN: ${JSON.stringify(req.body)}`); + const response = new OAuthResponse(res); + try { + const oauthResponse = await authorizationServer.respondToAccessTokenRequest(req, response); + return handleResponse(req, res, oauthResponse); + } catch (e) { + handleError(e, res); + return; + } + }); - // Return the HTTP redirect response - const oauthResponse = await authorizationServer.completeAuthorizationRequest(authRequest); - return handleResponse(req, res, oauthResponse); - } catch (e) { - handleError(e, res); - } - }); - - router.post("/oauth/token", async (req: express.Request, res: express.Response) => { - log.info(`TOKEN: ${JSON.stringify(req.body)}`); - const response = new OAuthResponse(res); - try { - const oauthResponse = await authorizationServer.respondToAccessTokenRequest(req, response); - return handleResponse(req, res, oauthResponse); - } catch (e) { - handleError(e, res); - return; - } - }); - - function handleError(e: Error | undefined, res: express.Response) { - // TODO(rl) clean up error handling - log.info('handleError', e ? e.message + '\n' + e.stack : 'no error'); - - 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) + function handleError(e: Error | undefined, res: express.Response) { + // TODO(rl) clean up error handling + log.info('handleError', e ? e.message + '\n' + e.stack : 'no error'); + + if (e instanceof OAuthException) { + res.status(e.status); res.send({ - err: e - }) + 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); + 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); } - - } else { - log.warn('OAuth server disabled!') } + return router; } } \ No newline at end of file diff --git a/components/server/tsconfig.json b/components/server/tsconfig.json index 71e4476f4e05d9..beef52bc439756 100644 --- a/components/server/tsconfig.json +++ b/components/server/tsconfig.json @@ -25,5 +25,5 @@ "include": [ "src", "ee/src" -, "../gitpod-db/src/typeorm/db-auth-code-repository.ts" ] + ] } \ No newline at end of file