Opportunistic token refresh (for BitBucket Server) (#19715)

* [server] Introduce and use "reservedUntilDate" in TokenService * Fix for extending reservations, and a bit of cleanup * review comments
gitpod-io · May 13, 2024 · 435fa5e · 435fa5e
1 parent 9af4e75
commit 435fa5e
Show file tree

Hide file tree

Showing 12 changed files with 507 additions and 91 deletions.
diff --git a/components/gitpod-db/src/typeorm/entity/db-token-entry.ts b/components/gitpod-db/src/typeorm/entity/db-token-entry.ts
@@ -31,6 +31,12 @@ export class DBTokenEntry implements TokenEntry {
     })
     expiryDate?: string;
 
+    @Column({
+        default: "",
+        transformer: Transformer.MAP_EMPTY_STR_TO_UNDEFINED,
+    })
+    reservedUntilDate?: string;
+
     @Column()
     refreshable?: boolean;
 

diff --git a/components/gitpod-db/src/typeorm/migration/1715096375962-TokenReservedUntil.ts b/components/gitpod-db/src/typeorm/migration/1715096375962-TokenReservedUntil.ts
@@ -0,0 +1,27 @@
+/**
+ * Copyright (c) 2024 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 { MigrationInterface, QueryRunner } from "typeorm";
+import { columnExists } from "./helper/helper";
+
+const table = "d_b_token_entry";
+const newColumn = "reservedUntilDate";
+
+export class TokenReservedUntil1715096375962 implements MigrationInterface {
+    public async up(queryRunner: QueryRunner): Promise<void> {
+        if (!(await columnExists(queryRunner, table, newColumn))) {
+            await queryRunner.query(
+                `ALTER TABLE ${table} ADD COLUMN \`${newColumn}\` varchar(255) NOT NULL DEFAULT ''`,
+            );
+        }
+    }
+
+    public async down(queryRunner: QueryRunner): Promise<void> {
+        if (await columnExists(queryRunner, table, newColumn)) {
+            await queryRunner.query(`ALTER TABLE ${table} DROP COLUMN \`${newColumn}\``);
+        }
+    }
+}
diff --git a/components/gitpod-db/src/typeorm/user-db-impl.ts b/components/gitpod-db/src/typeorm/user-db-impl.ts
@@ -282,6 +282,7 @@ export class TypeORMUserDBImpl extends TransactionalDBImpl<UserDB> implements Us
             authId: identity.authId,
             token: token,
             expiryDate: token.expiryDate,
+            reservedUntilDate: token.reservedUntilDate,
             refreshable: !!token.refreshToken,
         };
         return await repo.save(entry);
@@ -324,7 +325,7 @@ export class TypeORMUserDBImpl extends TransactionalDBImpl<UserDB> implements Us
         }
     }
 
-    public async findTokenForIdentity(identity: Identity): Promise<Token | undefined> {
+    public async findTokenEntryForIdentity(identity: Identity): Promise<TokenEntry | undefined> {
         const tokenEntries = await this.findTokensForIdentity(identity);
         if (tokenEntries.length > 1) {
             // TODO(gpl) This line is very noisy thus we don't want it to be a warning. Still we need to keep track,
@@ -337,12 +338,18 @@ export class TypeORMUserDBImpl extends TransactionalDBImpl<UserDB> implements Us
         const latestTokenEntry = tokenEntries
             .sort((a, b) => `${a.token.updateDate}`.localeCompare(`${b.token.updateDate}`))
             .reverse()[0];
-        if (latestTokenEntry) {
-            if (latestTokenEntry.expiryDate !== latestTokenEntry.token.expiryDate) {
-                log.info(`Overriding 'expiryDate' of token to get refreshed on demand.`, { identity });
-            }
-            return { ...latestTokenEntry.token, expiryDate: latestTokenEntry.expiryDate };
+        if (!latestTokenEntry) {
+            return undefined;
         }
+        return {
+            ...latestTokenEntry,
+            token: {
+                // Take dates from the TokenEntry, as only those are being updated in the DB atm
+                ...latestTokenEntry.token,
+                expiryDate: latestTokenEntry.expiryDate,
+                reservedUntilDate: latestTokenEntry.reservedUntilDate,
+            },
+        };
     }
 
     public async findTokensForIdentity(identity: Identity): Promise<TokenEntry[]> {

diff --git a/components/gitpod-db/src/user-db.ts b/components/gitpod-db/src/user-db.ts
@@ -97,7 +97,7 @@ export interface UserDB extends OAuthUserRepository, OAuthTokenRepository, Trans
      * @param identity
      * @throws an error when there is more than one token
      */
-    findTokenForIdentity(identity: Identity): Promise<Token | undefined>;
+    findTokenEntryForIdentity(identity: Identity): Promise<TokenEntry | undefined>;
 
     /**
      *

diff --git a/components/gitpod-protocol/src/protocol.ts b/components/gitpod-protocol/src/protocol.ts
@@ -618,6 +618,7 @@ export interface Token {
     scopes: string[];
     updateDate?: string;
     expiryDate?: string;
+    reservedUntilDate?: string;
     idToken?: string;
     refreshToken?: string;
     username?: string;
@@ -628,6 +629,7 @@ export interface TokenEntry {
     authId: string;
     token: Token;
     expiryDate?: string;
+    reservedUntilDate?: string;
     refreshable?: boolean;
 }
 
@@ -1416,7 +1418,7 @@ export interface OAuth2Config {
 }
 
 export namespace AuthProviderEntry {
-    export type Type = "GitHub" | "GitLab" | string;
+    export type Type = "GitHub" | "GitLab" | "Bitbucket" | "BitbucketServer" | string;
     export type Status = "pending" | "verified";
     export type NewEntry = Pick<AuthProviderEntry, "ownerId" | "host" | "type"> & {
         clientId?: string;

diff --git a/components/server/src/auth/auth-provider.ts b/components/server/src/auth/auth-provider.ts
@@ -5,7 +5,7 @@
  */
 
 import express from "express";
-import { AuthProviderInfo, User, AuthProviderEntry } from "@gitpod/gitpod-protocol";
+import { AuthProviderInfo, User, AuthProviderEntry, Token } from "@gitpod/gitpod-protocol";
 
 import { UserEnvVarValue } from "@gitpod/gitpod-protocol";
 
@@ -89,7 +89,8 @@ export interface AuthProvider {
         state: string,
         scopes?: string[],
     ): void;
-    refreshToken?(user: User): Promise<void>;
+    refreshToken?(user: User, requestedLifetimeDate: Date): Promise<Token>;
+    requiresOpportunisticRefresh?(): boolean;
 }
 
 export interface AuthFlow {

diff --git a/components/server/src/auth/generic-auth-provider.ts b/components/server/src/auth/generic-auth-provider.ts
@@ -177,14 +177,15 @@ export abstract class GenericAuthProvider implements AuthProvider {
         );
     }
 
-    async refreshToken(user: User) {
+    async refreshToken(user: User, requestedLifetimeDate: Date): Promise<Token> {
         log.info(`(${this.strategyName}) Token to be refreshed.`, { userId: user.id });
         const { authProviderId } = this;
         const identity = User.getIdentity(user, authProviderId);
         if (!identity) {
             throw new Error(`Cannot find an identity for ${authProviderId}`);
         }
-        const token = await this.userDb.findTokenForIdentity(identity);
+        const tokenEntry = await this.userDb.findTokenEntryForIdentity(identity);
+        const token = tokenEntry?.token;
         if (!token) {
             throw new Error(`Cannot find any current token for ${authProviderId}`);
         }
@@ -217,20 +218,24 @@ export abstract class GenericAuthProvider implements AuthProvider {
             const expiryDate = tokenExpiresInSeconds
                 ? new Date(now.getTime() + tokenExpiresInSeconds * 1000).toISOString()
                 : undefined;
+            const reservedUntilDate = requestedLifetimeDate.toISOString();
             const newToken: Token = {
                 value: access_token,
                 username: this.tokenUsername,
                 scopes: token.scopes,
                 updateDate,
                 expiryDate,
+                reservedUntilDate,
                 refreshToken: refresh_token,
             };
-            await this.userDb.storeSingleToken(identity, newToken);
+            const newTokenEntry = await this.userDb.storeSingleToken(identity, newToken);
             log.info(`(${this.strategyName}) Token refreshed and updated.`, {
                 userId: user.id,
                 updateDate,
                 expiryDate,
+                reservedUntilDate,
             });
+            return newTokenEntry.token;
         } catch (error) {
             log.error({ userId: user.id }, `(${this.strategyName}) Failed to refresh token!`, {
                 error: new TrustedValue(error),
@@ -239,6 +244,10 @@ export abstract class GenericAuthProvider implements AuthProvider {
         }
     }
 
+    public requiresOpportunisticRefresh() {
+        return this.params.type === "BitbucketServer";
+    }
+
     protected cachedAuthCallbackPath: string | undefined = undefined;
     get authCallbackPath() {
         // This ends up being called quite often so we cache the URL constructor

diff --git a/components/server/src/prometheus-metrics.ts b/components/server/src/prometheus-metrics.ts
@@ -385,7 +385,7 @@ export function reportAuthorizerSubjectId(match: AuthorizerSubjectIdMatch) {
 export const scmTokenRefreshRequestsTotal = new prometheusClient.Counter({
     name: "gitpod_scm_token_refresh_requests_total",
     help: "Counter for the number of token refresh requests we issue against SCM systems",
-    labelNames: ["host", "result"],
+    labelNames: ["host", "result", "opportunisticRefresh"],
 });
 export type ScmTokenRefreshResult =
     | "success"
@@ -395,8 +395,13 @@ export type ScmTokenRefreshResult =
     | "no_token"
     | "not_refreshable"
     | "success_after_timeout";
-export function reportScmTokenRefreshRequest(host: string, result: ScmTokenRefreshResult) {
-    scmTokenRefreshRequestsTotal.labels(host, result).inc();
+export type OpportunisticRefresh = "true" | "false" | "reserved";
+export function reportScmTokenRefreshRequest(
+    host: string,
+    opportunisticRefresh: OpportunisticRefresh,
+    result: ScmTokenRefreshResult,
+) {
+    scmTokenRefreshRequestsTotal.labels(host, result, opportunisticRefresh).inc();
 }
 
 export const scmTokenRefreshLatencyHistogram = new prometheusClient.Histogram({

diff --git a/components/server/src/user/token-provider.ts b/components/server/src/user/token-provider.ts
@@ -12,7 +12,7 @@ export interface TokenProvider {
      * Returns a valid authentication token for the given host and user
      * @param user
      * @param host
-     * @param expiryThreshold the time in minutes which a token has to be valid for to be considered as valid
+     * @param requestedLifetimeMins the time in minutes which a token has to be valid for to be considered as valid
      */
-    getTokenForHost(user: User | string, host: string, expiryThreshold?: number): Promise<Token | undefined>;
+    getTokenForHost(user: User | string, host: string, requestedLifetimeMins?: number): Promise<Token | undefined>;
 }