PR feedback

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 }}

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:

components/dashboard/src/Login.tsx

@@ -50,14 +50,31 @@ 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);
 
         try {
             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 (<div id="login-container" className="z-50 flex w-screen h-screen">
         {showWelcome ? <div id="feature-section" className="flex-grow bg-gray-100 dark:bg-gray-800 w-1/2 hidden lg:block">
             <div id="feature-section-column" className="flex max-w-xl h-full mx-auto pt-6">

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<OAuthAuthCode>((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<void> {

components/gitpod-db/src/typeorm/user-db-impl.ts

@@ -399,10 +399,7 @@ export class TypeORMUserDBImpl implements UserDB {
     }
     async persist(accessToken: OAuthToken): Promise<void> {
         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,

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")
 }

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`,

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'));