UBERF-7836: Fix github integeration (#6313)

Signed-off-by: Andrey Sobolev <haiodo@gmail.com>

.vscode/launch.json

@@ -244,6 +244,7 @@
         "MINIO_ENDPOINT": "localhost",
         "MINIO_ACCESS_KEY": "minioadmin",
         "MINIO_SECRET_KEY": "minioadmin",
+        "PLATFORM_OPERATION_LOGGING": "true",
         "FRONT_URL": "http://localhost:8080",
         "PORT": "3500"
       },

server/account/src/operations.ts

@@ -65,7 +65,7 @@ import {
   registerStringLoaders
 } from '@hcengineering/server-pipeline'
 import { buildStorageFromConfig, storageConfigFromEnv } from '@hcengineering/server-storage'
-import { decodeToken, generateToken } from '@hcengineering/server-token'
+import { decodeToken as decodeTokenRaw, generateToken, type Token } from '@hcengineering/server-token'
 import toolPlugin, {
   connect,
   initializeWorkspace,
@@ -398,7 +398,7 @@ export async function getAccountInfoByToken (
   let email: string = ''
   let workspace: WorkspaceId
   try {
-    ;({ email, workspace } = decodeToken(token))
+    ;({ email, workspace } = decodeToken(ctx, token))
   } catch (err: any) {
     Analytics.handleError(err)
     ctx.error('Invalid token', { token })
@@ -579,6 +579,21 @@ function getExtra (info: Account | AccountInfo | null, rec?: Record<string, any>
 
 export const guestAccountEmail = '#guest@hc.engineering'
 
+function decodeToken (ctx: MeasureContext, token: string): Token {
+  // eslint-disable-next-line no-useless-catch
+  try {
+    return decodeTokenRaw(token)
+  } catch (err: any) {
+    try {
+      // Ok we have error, but we need to log a proper message
+      ctx.warn('failed to verify token', { ...decodeTokenRaw(token, false) })
+    } catch (err2: any) {
+      // Ignore
+    }
+    throw err
+  }
+}
+
 /**
  * @public
  */
@@ -592,7 +607,7 @@ export async function selectWorkspace (
   kind: 'external' | 'internal',
   allowAdmin: boolean = true
 ): Promise<WorkspaceLoginInfo> {
-  const decodedToken = decodeToken(token)
+  const decodedToken = decodeToken(ctx, token)
   const email = cleanEmail(decodedToken.email)
 
   const endpointKind = kind === 'external' ? EndpointKind.External : EndpointKind.Internal
@@ -776,7 +791,7 @@ export async function confirm (
   branding: Branding | null,
   token: string
 ): Promise<LoginInfo> {
-  const decode = decodeToken(token)
+  const decode = decodeToken(ctx, token)
   const _email = decode.extra?.confirm
   if (_email === undefined) {
     ctx.error('confirm email invalid', { token: decode })
@@ -1010,7 +1025,7 @@ export async function listWorkspaces (
   branding: Branding | null,
   token: string
 ): Promise<WorkspaceInfo[]> {
-  decodeToken(token) // Just verify token is valid
+  decodeToken(ctx, token) // Just verify token is valid
   return (await db.collection<Workspace>(WORKSPACE_COLLECTION).find(withProductId(productId, {})).toArray())
     .map((it) => ({ ...it, productId }))
     .filter((it) => it.disabled !== true)
@@ -1447,7 +1462,7 @@ export const createUserWorkspace =
       token: string,
       workspaceName: string
     ): Promise<LoginInfo> => {
-      const { email } = decodeToken(token)
+      const { email } = decodeToken(ctx, token)
 
       ctx.info('Creating workspace', { workspaceName, email })
 
@@ -1567,7 +1582,7 @@ export async function getInviteLink (
   role?: AccountRole,
   personId?: Ref<Person>
 ): Promise<ObjectId> {
-  const { workspace, email } = decodeToken(token)
+  const { workspace, email } = decodeToken(ctx, token)
   const wsPromise = await getWorkspaceById(db, productId, workspace.name)
   if (wsPromise === null) {
     ctx.error('workspace not found', { workspace, email })
@@ -1620,7 +1635,7 @@ export async function getUserWorkspaces (
   branding: Branding | null,
   token: string
 ): Promise<ClientWorkspaceInfo[]> {
-  const { email } = decodeToken(token)
+  const { email } = decodeToken(ctx, token)
   const account = await getAccount(db, email)
   if (account === null) {
     ctx.error('account not found', { email })
@@ -1648,7 +1663,7 @@ export async function getWorkspaceInfo (
   token: string,
   _updateLastVisit: boolean = false
 ): Promise<ClientWorkspaceInfo> {
-  const { email, workspace, extra } = decodeToken(token)
+  const { email, workspace, extra } = decodeToken(ctx, token)
   const guest = extra?.guest === 'true'
   let account: Pick<Account, 'admin' | 'workspaces'> | Account | null = null
   const query: Filter<Workspace> = {
@@ -1772,7 +1787,7 @@ export async function createMissingEmployee (
   branding: Branding | null,
   token: string
 ): Promise<void> {
-  const { email } = decodeToken(token)
+  const { email } = decodeToken(ctx, token)
   const wsInfo = await getWorkspaceInfo(ctx, db, productId, branding, token)
   const account = await getAccount(db, email)
 
@@ -2020,7 +2035,7 @@ export async function changePassword (
   oldPassword: string,
   password: string
 ): Promise<void> {
-  const { email } = decodeToken(token)
+  const { email } = decodeToken(ctx, token)
   const account = await getAccountInfo(ctx, db, productId, branding, email, oldPassword)
 
   const salt = randomBytes(32)
@@ -2121,7 +2136,7 @@ export async function restorePassword (
   token: string,
   password: string
 ): Promise<LoginInfo> {
-  const decode = decodeToken(token)
+  const decode = decodeToken(ctx, token)
   const email = decode.extra?.restore
   if (email === undefined) {
     throw new PlatformError(new Status(Severity.ERROR, platform.status.AccountNotFound, { account: email }))
@@ -2180,7 +2195,7 @@ export async function checkJoin (
   token: string,
   inviteId: ObjectId
 ): Promise<WorkspaceLoginInfo> {
-  const { email } = decodeToken(token)
+  const { email } = decodeToken(ctx, token)
   const invite = await getInvite(db, inviteId)
   const workspace = await checkInvite(ctx, invite, email)
   const ws = await getWorkspaceById(db, productId, workspace.name)
@@ -2283,7 +2298,7 @@ export async function leaveWorkspace (
   token: string,
   email: string
 ): Promise<void> {
-  const tokenData = decodeToken(token)
+  const tokenData = decodeToken(ctx, token)
 
   const currentAccount = await getAccount(db, tokenData.email)
   if (currentAccount === null) {
@@ -2324,7 +2339,7 @@ export async function sendInvite (
   personId?: Ref<Person>,
   role?: AccountRole
 ): Promise<void> {
-  const tokenData = decodeToken(token)
+  const tokenData = decodeToken(ctx, token)
   const currentAccount = await getAccount(db, tokenData.email)
   if (currentAccount === null) {
     throw new PlatformError(new Status(Severity.ERROR, platform.status.AccountNotFound, { account: tokenData.email }))
@@ -2449,6 +2464,13 @@ function wrap (
           err instanceof PlatformError
             ? err.status
             : new Status(Severity.ERROR, platform.status.InternalServerError, {})
+
+        if (((err.message as string) ?? '') === 'Signature verification failed') {
+          // Let's send un authorized
+          return {
+            error: new Status(Severity.ERROR, platform.status.Unauthorized, {})
+          }
+        }
         if (status.code === platform.status.InternalServerError) {
           Analytics.handleError(err)
           ctx.error('error', { status, err })
@@ -2617,7 +2639,7 @@ export async function changeUsername (
   first: string,
   last: string
 ): Promise<void> {
-  const { email } = decodeToken(token)
+  const { email } = decodeToken(ctx, token)
   const account = await getAccount(db, email)
 
   if (account == null) {

server/token/src/token.ts

@@ -26,8 +26,8 @@ export function generateToken (email: string, workspace: WorkspaceId, extra?: Re
 /**
  * @public
  */
-export function decodeToken (token: string): Token {
+export function decodeToken (token: string, verify: boolean = true): Token {
-  const value = decode(token, getSecret(), false)
+  const value = decode(token, getSecret(), !verify)
   const { email, workspace, productId, ...extra } = value
   return { email, workspace: getWorkspaceId(workspace, productId), extra }
 }

server/ws/src/server.ts

@@ -316,7 +316,9 @@ class TSessionManager implements SessionManager {
     ) {
       ctx.warn('model version mismatch', {
         version: this.modelVersion,
-        workspaceVersion: versionToString(workspaceInfo.version)
+        workspaceVersion: versionToString(workspaceInfo.version),
+        workspace: workspaceInfo.workspaceId,
+        workspaceUrl: workspaceInfo.workspaceUrl
       })
       // Version mismatch, return upgrading.
       return { upgrade: true, upgradeInfo: workspaceInfo.upgrade }

services/github/pod-github/Dockerfile

@@ -5,6 +5,7 @@ WORKDIR /usr/src/app
 
 RUN npm install --ignore-scripts=false --verbose bufferutil utf-8-validate @mongodb-js/zstd snappy --unsafe-perm
 COPY bundle/bundle.js ./
+COPY bundle/bundle.js.map ./
 
 RUN apt-get update
 RUN apt-get install libjemalloc2
@@ -13,4 +14,4 @@ ENV LD_PRELOAD=libjemalloc.so.2
 ENV MALLOC_CONF=dirty_decay_ms:1000,narenas:2,background_thread:true
 
 EXPOSE 3078
-CMD [ "node", "--inspect", "--async-stack-traces", "bundle.js" ]
+CMD [ "node", "--inspect", "--async-stack-traces", "--enable-source-maps", "bundle.js" ]

services/github/pod-github/src/platform.ts

@@ -15,8 +15,8 @@ import core, {
   Ref,
   TxOperations
 } from '@hcengineering/core'
-import github, { GithubAuthentication, makeQuery } from '@hcengineering/github'
+import github, { GithubAuthentication, makeQuery, type GithubIntegration } from '@hcengineering/github'
-import { MongoClientReference, getMongoClient } from '@hcengineering/mongo'
+import { getMongoClient, MongoClientReference } from '@hcengineering/mongo'
 import { setMetadata } from '@hcengineering/platform'
 import { buildStorageFromConfig, storageConfigFromEnv } from '@hcengineering/server-storage'
 import serverToken, { generateToken } from '@hcengineering/server-token'
@@ -199,9 +199,39 @@ export class PlatformWorker {
     installationId: number,
     accountId: Ref<Account>
   ): Promise<void> {
-    if (this.integrations.find((it) => it.installationId === installationId) != null) {
+    const oldInstallation = this.integrations.find((it) => it.installationId === installationId)
+    if (oldInstallation != null) {
+      ctx.info('update integration', { workspace, installationId, accountId })
       // What to do with installation in different workspace?
       // Let's remove it and sync to new one.
+      if (oldInstallation.workspace !== workspace) {
+        //
+        const oldWorkspace = oldInstallation.workspace
+
+        await this.integrationCollection.updateOne(
+          { installationId: oldInstallation.installationId },
+          { $set: { workspace } }
+        )
+        oldInstallation.workspace = workspace
+
+        const oldWorker = this.clients.get(oldWorkspace) as GithubWorker
+        if (oldWorker !== undefined) {
+          await this.removeInstallationFromWorkspace(oldWorker.client, installationId)
+          await oldWorker.reloadRepositories(installationId)
+        } else {
+          let client: Client | undefined
+          try {
+            client = await createPlatformClient(oldWorkspace, config.ProductID, 30000)
+            await this.removeInstallationFromWorkspace(oldWorker, installationId)
+            await client.close()
+          } catch (err: any) {
+            ctx.error('failed to remove old installation from workspace', { workspace: oldWorkspace, installationId })
+          }
+        }
+      }
+
+      await this.updateInstallation(installationId)
+
       const worker = this.clients.get(workspace) as GithubWorker
       await worker?.reloadRepositories(installationId)
       worker?.triggerUpdate()
@@ -214,7 +244,9 @@ export class PlatformWorker {
       installationId,
       accountId
     }
-    await ctx.withLog('add integration', { workspace, installationId, accountId }, async (ctx) => {
+    ctx.info('add integration', { workspace, installationId, accountId })
+
+    await ctx.with('add integration', { workspace, installationId, accountId }, async (ctx) => {
       await this.integrationCollection.insertOne(record)
       this.integrations.push(record)
     })
@@ -228,12 +260,43 @@ export class PlatformWorker {
     this.triggerCheckWorkspaces()
   }
 
+  private async removeInstallationFromWorkspace (client: Client, installationId: number): Promise<void> {
+    const wsIntegerations = await client.findAll(github.class.GithubIntegration, { installationId })
+
+    for (const intValue of wsIntegerations) {
+      const ops = new TxOperations(client, core.account.System)
+      await ops.remove<GithubIntegration>(intValue)
+    }
+  }
+
   async removeInstallation (ctx: MeasureContext, workspace: string, installationId: number): Promise<void> {
     const installation = this.installations.get(installationId)
     if (installation !== undefined) {
-      await installation.octokit.rest.apps.deleteInstallation({
+      try {
-        installation_id: installationId
+        await installation.octokit.rest.apps.deleteInstallation({
-      })
+          installation_id: installationId
+        })
+      } catch (err: any) {
+        if (err.status !== 404) {
+          // Already deleted.
+          ctx.error('error from github api', { error: err })
+        }
+        await this.handleInstallationEventDelete(installationId)
+      }
+      // Let's check if workspace somehow still have installation and remove it
+      const worker = this.clients.get(workspace) as GithubWorker
+      if (worker !== undefined) {
+        await GithubWorker.checkIntegrations(worker.client, this.installations)
+      } else {
+        let client: Client | undefined
+        try {
+          client = await createPlatformClient(workspace, config.ProductID, 30000)
+          await GithubWorker.checkIntegrations(client, this.installations)
+          await client.close()
+        } catch (err: any) {
+          ctx.error('failed to clean installation from workspace', { workspace, installationId })
+        }
+      }
     }
     this.triggerCheckWorkspaces()
   }
@@ -578,6 +641,12 @@ export class PlatformWorker {
     const rateLimiter = new RateLimiter(5)
     let errors = 0
     let idx = 0
+    const connecting = new Map<string, number>()
+    const connectingInfo = setInterval(() => {
+      for (const [c, d] of connecting.entries()) {
+        this.ctx.info('connecting to workspace', { workspace: c, time: Date.now() - d })
+      }
+    }, 5000)
     for (const workspace of workspaces) {
       const widx = ++idx
       if (this.clients.has(workspace)) {
@@ -607,8 +676,14 @@ export class PlatformWorker {
             errors++
             return
           }
+          if (workspaceInfo?.disabled === true) {
+            this.ctx.error('Workspace is disabled workspaceId', { workspace })
+            return
+          }
           const branding = Object.values(this.brandingMap).find((b) => b.key === workspaceInfo?.branding) ?? null
           const workerCtx = this.ctx.newChild('worker', { workspace: workspaceInfo.workspace }, {})
+
+          connecting.set(workspaceInfo.workspace, Date.now())
           workerCtx.info('************************* Register worker ************************* ', {
             workspaceId: workspaceInfo.workspaceId,
             workspace: workspaceInfo.workspace,
@@ -658,6 +733,8 @@ export class PlatformWorker {
           this.ctx.info("Couldn't create WS worker", { workspace, error: e })
           console.error(e)
           errors++
+        } finally {
+          connecting.delete(workspaceInfo.workspace)
         }
       })
     }
@@ -667,6 +744,7 @@ export class PlatformWorker {
       Analytics.handleError(e)
       errors++
     }
+    clearInterval(connectingInfo)
     // Close deleted workspaces
     for (const deleted of Array.from(toDelete.keys())) {
       const ws = this.clients.get(deleted)

services/github/pod-github/src/server.ts

@@ -73,13 +73,12 @@ export async function start (ctx: MeasureContext, brandingMap: BrandingMap): Pro
 
   // eslint-disable-next-line @typescript-eslint/no-misused-promises
   app.post('/api/v1/installation', async (req, res) => {
+    const payloadData: {
+      installationId: number
+      accountId: Ref<Account>
+      token: string
+    } = req.body
     try {
-      const payloadData: {
-        installationId: number
-        accountId: Ref<Account>
-        token: string
-      } = req.body
-
       const decodedToken = decodeToken(payloadData.token)
       ctx.info('/api/v1/installation', {
         email: decodedToken.email,
@@ -87,6 +86,10 @@ export async function start (ctx: MeasureContext, brandingMap: BrandingMap): Pro
         body: req.body
       })
 
+      ctx.info('map-installation', {
+        workspace: decodedToken.workspace.name,
+        installationid: payloadData.installationId
+      })
       await ctx.withLog('map-installation', {}, async (ctx) => {
         await worker.mapInstallation(
           ctx,
@@ -99,6 +102,12 @@ export async function start (ctx: MeasureContext, brandingMap: BrandingMap): Pro
       res.json({})
     } catch (err: any) {
       Analytics.handleError(err)
+      const tok = decodeToken(payloadData.token, false)
+      ctx.error('failed to map-installation', {
+        workspace: tok.workspace?.name,
+        installationid: payloadData.installationId,
+        email: tok?.email
+      })
       res.status(401)
       res.json({ error: err.message })
     }
@@ -121,7 +130,12 @@ export async function start (ctx: MeasureContext, brandingMap: BrandingMap): Pro
       } = JSON.parse(atob(payloadData.state))
 
       const decodedToken = decodeToken(decodedData.token)
-
+      ctx.info('request github access-token', {
+        workspace: decodedToken.workspace.name,
+        accountId: payloadData.accountId,
+        code: payloadData.code,
+        state: payloadData.state
+      })
       await ctx.withLog('request-github-access-token', {}, async (ctx) => {
         await worker.requestGithubAccessToken({
           workspace: decodedToken.workspace.name,
@@ -154,7 +168,11 @@ export async function start (ctx: MeasureContext, brandingMap: BrandingMap): Pro
         body: req.body
       })
 
-      await ctx.withLog('map-installation', {}, async (ctx) => {
+      ctx.info('remove-installation', {
+        workspace: decodedToken.workspace.name,
+        installationId: payloadData.installationId
+      })
+      await ctx.withLog('remove-installation', {}, async (ctx) => {
         await worker.removeInstallation(ctx, decodedToken.workspace.name, payloadData.installationId)
       })
       res.status(200)

services/github/pod-github/src/worker.ts

@@ -624,7 +624,7 @@ export class GithubWorker implements IntegrationManager {
     return statuses.filter((it) => allowedTypes.has(it._id))
   }
 
-  async init (): Promise<boolean> {
+  async init (): Promise<void> {
     this.registerNotifyHandler()
 
     await this.queryAccounts()
@@ -692,7 +692,6 @@ export class GithubWorker implements IntegrationManager {
     this.triggerRequests = 1
     this.updateRequests = 1
     this.syncPromise = this.syncAndWait()
-    return true
   }
 
   projects: GithubProject[] = []
@@ -1504,9 +1503,8 @@ export class GithubWorker implements IntegrationManager {
         branding
       )
       ctx.info('Init worker', { workspace: workspace.workspaceUrl, workspaceId: workspace.workspaceName })
-      if (await worker.init()) {
+      void worker.init()
-        return worker
+      return worker
-      }
     } catch (err: any) {
       ctx.error('timeout during to connect', { workspace, error: err })
       await client?.close()