Skip to content

Commit

Permalink
UBERF-7836: Fix github integeration (#6313)
Browse files Browse the repository at this point in the history
Signed-off-by: Andrey Sobolev <[email protected]>
  • Loading branch information
haiodo committed Aug 11, 2024
1 parent 24b08a4 commit 2209062
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 40 deletions.
1 change: 1 addition & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
54 changes: 38 additions & 16 deletions server/account/src/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 })
Expand Down Expand Up @@ -579,6 +579,21 @@ function getExtra (info: Account | AccountInfo | null, rec?: Record<string, any>

export const guestAccountEmail = '#[email protected]'

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
*/
Expand All @@ -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
Expand Down Expand Up @@ -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 })
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 })

Expand Down Expand Up @@ -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 })
Expand Down Expand Up @@ -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 })
Expand Down Expand Up @@ -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> = {
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 }))
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 }))
Expand Down Expand Up @@ -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 })
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions server/token/src/token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ export function generateToken (email: string, workspace: WorkspaceId, extra?: Re
/**
* @public
*/
export function decodeToken (token: string): Token {
const value = decode(token, getSecret(), false)
export function decodeToken (token: string, verify: boolean = true): Token {
const value = decode(token, getSecret(), !verify)
const { email, workspace, productId, ...extra } = value
return { email, workspace: getWorkspaceId(workspace, productId), extra }
}
4 changes: 3 additions & 1 deletion server/ws/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
3 changes: 2 additions & 1 deletion services/github/pod-github/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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" ]
92 changes: 85 additions & 7 deletions services/github/pod-github/src/platform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import core, {
Ref,
TxOperations
} from '@hcengineering/core'
import github, { GithubAuthentication, makeQuery } from '@hcengineering/github'
import { MongoClientReference, getMongoClient } from '@hcengineering/mongo'
import github, { GithubAuthentication, makeQuery, type GithubIntegration } from '@hcengineering/github'
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'
Expand Down Expand Up @@ -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()
Expand All @@ -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)
})
Expand All @@ -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({
installation_id: installationId
})
try {
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()
}
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
})
}
Expand All @@ -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)
Expand Down
Loading

0 comments on commit 2209062

Please sign in to comment.