Skip to content

Commit

Permalink
feat: support private S3 buckets (#9697)
Browse files Browse the repository at this point in the history
Signed-off-by: Matt Krick <matt.krick@gmail.com>
  • Loading branch information
mattkrick authored May 1, 2024
1 parent 29b6e81 commit db17c9d
Show file tree
Hide file tree
Showing 25 changed files with 633 additions and 892 deletions.
1 change: 0 additions & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ AI_EMBEDDER_WORKERS='1'
CDN_BASE_URL=''
# FILE_STORE_PROVIDER: local | s3 | gcs
FILE_STORE_PROVIDER='local'
# AWS_S3_BUCKET='BUCKET_NAME'
# GOOGLE_GCS_BUCKET='BUCKET_NAME'

# CHRONOS
Expand Down
1 change: 0 additions & 1 deletion docker/images/parabol-ubi/environments/pipeline
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ ATLASSIAN_CLIENT_ID=''
ATLASSIAN_CLIENT_SECRET=''
AWS_ACCESS_KEY_ID=''
AWS_REGION=''
AWS_S3_BUCKET=''
AWS_SECRET_ACCESS_KEY=''
CDN_BASE_URL=''
CI='true'
Expand Down
1 change: 0 additions & 1 deletion docs/s3.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ using a set of environment variables:

AWS_ACCESS_KEY_ID="XXXXXXXXXXXXXXXXXXXX"
AWS_REGION="us-east-1" # for example
AWS_S3_BUCKET="some-bucket-name"
AWS_SECRET_ACCESS_KEY="YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY"
CDN_BASE_URL="//some.url.com/instance" # (development|staging|production)

Expand Down
29 changes: 29 additions & 0 deletions packages/server/dataloader/customLoaderMakers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import Organization from '../database/types/Organization'
import OrganizationUser from '../database/types/OrganizationUser'
import {Reactable, ReactableEnum} from '../database/types/Reactable'
import Task, {TaskStatusEnum} from '../database/types/Task'
import getFileStoreManager from '../fileStorage/getFileStoreManager'
import isValid from '../graphql/isValid'
import {SAMLSource} from '../graphql/public/types/SAML'
import getKysely from '../postgres/getKysely'
Expand All @@ -29,6 +30,7 @@ import getMeetingTaskEstimates, {
} from '../postgres/queries/getMeetingTaskEstimates'
import {Team} from '../postgres/queries/getTeamsByIds'
import {AnyMeeting, MeetingTypeEnum} from '../postgres/types/Meeting'
import {Logger} from '../utils/Logger'
import getRedis from '../utils/getRedis'
import isUserVerified from '../utils/isUserVerified'
import NullableDataLoader from './NullableDataLoader'
Expand Down Expand Up @@ -837,3 +839,30 @@ export const isCompanyDomain = (parent: RootDataLoader) => {
}
)
}

export const fileStoreAsset = (parent: RootDataLoader) => {
return new DataLoader<string, string, string>(
async (urls) => {
// Our cloud saas has a public file store, so no need to make a presigned url
if (process.env.IS_ENTERPRISE !== 'true') return urls
const manager = getFileStoreManager()
const {baseUrl} = manager
const presignedUrls = await Promise.all(
urls.map(async (url) => {
// if the image is not hosted by us, ignore it
if (!url.startsWith(baseUrl)) return url
try {
return await manager.presignUrl(url)
} catch (e) {
Logger.log('Unable to presign url', url, e)
return url
}
})
)
return presignedUrls
},
{
...parent.dataLoaderOptions
}
)
}
3 changes: 3 additions & 0 deletions packages/server/fileStorage/FileStoreManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ import generateUID from '../generateUID'
export type FileAssetDir = 'store' | 'build'

export default abstract class FileStoreManager {
abstract baseUrl: string
abstract checkExists(fileName: string, assetDir?: FileAssetDir): Promise<boolean>
abstract prependPath(partialPath: string, assetDir?: FileAssetDir): string
abstract getPublicFileLocation(fullPath: string): string

protected abstract putFile(file: ArrayBufferLike, fullPath: string): Promise<string>
protected abstract putUserFile(file: ArrayBufferLike, partialPath: string): Promise<string>
abstract putBuildFile(file: ArrayBufferLike, partialPath: string): Promise<string>

abstract presignUrl(url: string): Promise<string>
async putUserAvatar(file: ArrayBufferLike, userId: string, ext: string, name?: string) {
const filename = name ?? generateUID()
// replace the first dot, if there is one, but not any other dots
Expand Down
6 changes: 5 additions & 1 deletion packages/server/fileStorage/GCSManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export default class GCSManager extends FileStoreManager {
private accessToken: string | undefined

// The CDN_BASE_URL without the env, e.g. storage.google.com/:bucket
private baseUrl: string
baseUrl: string
private cloudKey: CloudKey
constructor() {
super()
Expand Down Expand Up @@ -160,4 +160,8 @@ export default class GCSManager extends FileStoreManager {
const res = await fetch(url)
return res.status !== 404
}
async presignUrl(url: string): Promise<string> {
// not implemented yet!
return url
}
}
6 changes: 5 additions & 1 deletion packages/server/fileStorage/LocalFileStoreManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import makeAppURL from 'parabol-client/utils/makeAppURL'
import path from 'path'
import appOrigin from '../appOrigin'
import FileStoreManager from './FileStoreManager'
export default class LocalFileSystemManager extends FileStoreManager {
export default class LocalFileStoreManager extends FileStoreManager {
baseUrl = makeAppURL(appOrigin, 'self-hosted')
constructor() {
super()
const {PROTO, HOST} = process.env
Expand Down Expand Up @@ -47,4 +48,7 @@ export default class LocalFileSystemManager extends FileStoreManager {
return false
}
}
async presignUrl(url: string) {
return url
}
}
1 change: 0 additions & 1 deletion packages/server/fileStorage/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ After setting the `FILE_STORE_PROVIDER` env var, there are a couple other env va
- `AWS_ACCESS_KEY_ID="XXXXXXXXXXXXXXXXXXXX"`
- `AWS_REGION="some-region-1"`
- `AWS_SECRET_ACCESS_KEY="YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY"`
- `AWS_S3_BUCKET="some-bucket-name"`
- `CDN_BASE_URL="//some.url.com/instance"`

After these vars are set correctly, all should be good to go to begin uploading images to the configured S3 bucket.
Expand Down
34 changes: 22 additions & 12 deletions packages/server/fileStorage/S3FileStoreManager.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,23 @@
import {HeadObjectCommand, PutObjectCommand, S3Client} from '@aws-sdk/client-s3'
import {GetObjectCommand, HeadObjectCommand, PutObjectCommand, S3Client} from '@aws-sdk/client-s3'
import {getSignedUrl} from '@aws-sdk/s3-request-presigner'
import mime from 'mime-types'
import path from 'path'
import FileStoreManager, {FileAssetDir} from './FileStoreManager'

export default class S3Manager extends FileStoreManager {
// e.g. development, production
private envSubDir: string
// e.g. action-files.parabol.co. Usually matches CDN_BASE_URL for DNS reasons
private bucket: string

// e.g. https://action-files.parabol.co
private baseUrl: string
baseUrl: string
private s3: S3Client
constructor() {
super()
const {CDN_BASE_URL, AWS_S3_BUCKET, AWS_REGION} = process.env
const {CDN_BASE_URL, AWS_REGION, AWS_SECRET_ACCESS_KEY, AWS_ACCESS_KEY_ID} = process.env
if (!CDN_BASE_URL || CDN_BASE_URL === 'key_CDN_BASE_URL') {
throw new Error('CDN_BASE_URL ENV VAR NOT SET')
}

if (!AWS_S3_BUCKET) {
throw new Error('AWS_S3_BUCKET ENV VAR NOT SET')
}
const baseUrl = new URL(CDN_BASE_URL.replace(/^\/+/, 'https://'))
const {hostname, pathname} = baseUrl
if (!hostname || !pathname) {
Expand All @@ -33,9 +29,15 @@ export default class S3Manager extends FileStoreManager {
this.envSubDir = pathname.split('/').at(-1) as string

this.baseUrl = baseUrl.href.slice(0, baseUrl.href.lastIndexOf(this.envSubDir))

this.bucket = AWS_S3_BUCKET
// credentials are optional since the file store could be public & not need a key to write
const credentials =
AWS_ACCESS_KEY_ID && AWS_SECRET_ACCESS_KEY
? {accessKeyId: AWS_ACCESS_KEY_ID, secretAccessKey: AWS_SECRET_ACCESS_KEY}
: undefined
this.s3 = new S3Client({
credentials,
// The bucket is inferred from the CDN_BASE_URL
bucketEndpoint: true,
region: AWS_REGION
})
}
Expand All @@ -47,7 +49,7 @@ export default class S3Manager extends FileStoreManager {
protected async putFile(file: ArrayBufferLike, fullPath: string) {
const s3Params = {
Body: Buffer.from(file),
Bucket: this.bucket,
Bucket: this.baseUrl,
Key: fullPath,
ContentType: mime.lookup(fullPath) || 'application/octet-stream'
}
Expand All @@ -70,10 +72,18 @@ export default class S3Manager extends FileStoreManager {
async checkExists(key: string, assetDir?: FileAssetDir) {
const Key = this.prependPath(key, assetDir)
try {
await this.s3.send(new HeadObjectCommand({Bucket: this.bucket, Key}))
await this.s3.send(new HeadObjectCommand({Bucket: this.baseUrl, Key}))
} catch (e) {
if (e instanceof Error && e.name === 'NotFound') return false
}
return true
}

async presignUrl(url: string) {
// Important to decodeURI so `getSignedUrl` doesn't double encode e.g. local|123/avatars/123.jpg
const key = decodeURI(url.slice(this.baseUrl.length))
const command = new GetObjectCommand({Bucket: this.baseUrl, Key: key})
const encodedUri = await getSignedUrl(this.s3, command, {expiresIn: 604800})
return encodedUri
}
}
4 changes: 2 additions & 2 deletions packages/server/fileStorage/getFileStoreManager.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import FileStoreManager from './FileStoreManager'
import GCSManager from './GCSManager'
import LocalFileSystemManager from './LocalFileStoreManager'
import LocalFileStoreManager from './LocalFileStoreManager'
import S3Manager from './S3FileStoreManager'

let fileStoreManager: FileStoreManager
const managers = {
s3: S3Manager,
gcs: GCSManager,
local: LocalFileSystemManager
local: LocalFileStoreManager
}

type ManagersKey = keyof typeof managers
Expand Down
6 changes: 5 additions & 1 deletion packages/server/graphql/public/types/NotifyKudosReceived.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import {NotifyKudosReceivedResolvers} from '../resolverTypes'

const NotifyKudosReceived: NotifyKudosReceivedResolvers = {
__isTypeOf: ({type}) => type === 'KUDOS_RECEIVED',
emojiUnicode: ({emojiUnicode}) => emojiUnicode ?? '❤️'
emojiUnicode: ({emojiUnicode}) => emojiUnicode ?? '❤️',
picture: async ({picture}, _args, {dataLoader}) => {
if (!picture) return null
return dataLoader.get('fileStoreAsset').load(picture)
}
}

export default NotifyKudosReceived
4 changes: 4 additions & 0 deletions packages/server/graphql/public/types/NotifyMentioned.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ const NotifyMentioned: NotifyMentionedResolvers = {
return null
}
return retroReflection
},
senderPicture: async ({senderPicture}, _args, {dataLoader}) => {
if (!senderPicture) return null
return dataLoader.get('fileStoreAsset').load(senderPicture)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ const NotifyRequestToJoinOrg: NotifyRequestToJoinOrgResolvers = {
__isTypeOf: ({type}) => type === 'REQUEST_TO_JOIN_ORG',
domainJoinRequestId: ({domainJoinRequestId}) => {
return DomainJoinRequestId.join(domainJoinRequestId)
},
picture: async ({picture}, _args, {dataLoader}) => {
if (!picture) return null
return dataLoader.get('fileStoreAsset').load(picture)
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import {NotifyTeamsLimitExceededResolvers} from '../resolverTypes'

const NotifyTeamsLimitExceeded: NotifyTeamsLimitExceededResolvers = {
__isTypeOf: ({type}) => type === 'TEAMS_LIMIT_EXCEEDED'
__isTypeOf: ({type}) => type === 'TEAMS_LIMIT_EXCEEDED',
orgPicture: async ({orgPicture}, _args, {dataLoader}) => {
if (!orgPicture) return null
return dataLoader.get('fileStoreAsset').load(orgPicture)
}
}

export default NotifyTeamsLimitExceeded
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import {NotifyTeamsLimitReminderResolvers} from '../resolverTypes'

const NotifyTeamsLimitReminder: NotifyTeamsLimitReminderResolvers = {
__isTypeOf: ({type}) => type === 'TEAMS_LIMIT_REMINDER'
__isTypeOf: ({type}) => type === 'TEAMS_LIMIT_REMINDER',
orgPicture: async ({orgPicture}, _args, {dataLoader}) => {
if (!orgPicture) return null
return dataLoader.get('fileStoreAsset').load(orgPicture)
}
}

export default NotifyTeamsLimitReminder
4 changes: 4 additions & 0 deletions packages/server/graphql/public/types/Organization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ const Organization: OrganizationResolvers = {
if (!featureFlags) return {}
return Object.fromEntries(featureFlags.map((flag) => [flag as any, true]))
},
picture: async ({picture}, _args, {dataLoader}) => {
if (!picture) return null
return dataLoader.get('fileStoreAsset').load(picture)
},
tier: ({tier, trialStartDate}) => {
return getFeatureTier({tier, trialStartDate})
},
Expand Down
3 changes: 3 additions & 0 deletions packages/server/graphql/public/types/TeamMember.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ const TeamMember: TeamMemberResolvers = {
.get('organizationUsersByUserIdOrgId')
.load({userId, orgId: team.orgId})
return organizationUser?.role === 'ORG_ADMIN'
},
picture: async ({picture}, _args, {dataLoader}) => {
return dataLoader.get('fileStoreAsset').load(picture)
}
}

Expand Down
8 changes: 8 additions & 0 deletions packages/server/graphql/public/types/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,14 @@ const User: UserResolvers = {
urlObj.searchParams.append('RelayState', relayState)
return {url: urlObj.toString()}
},
picture: async ({picture}, _args, {dataLoader}) => {
return dataLoader.get('fileStoreAsset').load(picture)
},
rasterPicture: async ({picture}, _args, {dataLoader}) => {
const rasterPicture =
picture && picture.endsWith('.svg') ? picture.slice(0, -3) + 'png' : picture
return dataLoader.get('fileStoreAsset').load(rasterPicture)
},
tier: ({tier, trialStartDate}) => {
return getFeatureTier({tier, trialStartDate})
},
Expand Down
5 changes: 4 additions & 1 deletion packages/server/graphql/types/Invoice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ const Invoice = new GraphQLObjectType<any, GQLContext>({
},
picture: {
type: GraphQLURLType,
description: 'The picture of the organization'
description: 'The picture of the organization',
resolve: async ({picture}, _args, {dataLoader}) => {
return dataLoader.get('fileStoreAsset').load(picture)
}
},
startAt: {
type: new GraphQLNonNull(GraphQLISO8601Type),
Expand Down
5 changes: 0 additions & 5 deletions packages/server/graphql/types/Organization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import getActiveTeamCountByOrgIds from '../public/types/helpers/getActiveTeamCou
import {resolveForBillingLeaders} from '../resolvers'
import CreditCard from './CreditCard'
import GraphQLISO8601Type from './GraphQLISO8601Type'
import GraphQLURLType from './GraphQLURLType'
import OrgUserCount from './OrgUserCount'
import OrganizationUser, {OrganizationUserConnection} from './OrganizationUser'
import Team from './Team'
Expand Down Expand Up @@ -68,10 +67,6 @@ const Organization: GraphQLObjectType<any, GQLContext> = new GraphQLObjectType<a
type: new GraphQLNonNull(GraphQLString),
description: 'The name of the organization'
},
picture: {
type: GraphQLURLType,
description: 'The org avatar'
},
activeTeamCount: {
type: new GraphQLNonNull(GraphQLInt),
description: 'Number of teams with 3+ meetings (>1 attendee) that met within last 30 days',
Expand Down
5 changes: 0 additions & 5 deletions packages/server/graphql/types/TeamMember.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {default as sortRepoIntegrations} from '../queries/helpers/sortRepoIntegr
import {resolveTeam} from '../resolvers'
import GraphQLEmailType from './GraphQLEmailType'
import GraphQLISO8601Type from './GraphQLISO8601Type'
import GraphQLURLType from './GraphQLURLType'
import RepoIntegrationQueryPayload from './RepoIntegrationQueryPayload'
import {TaskConnection} from './Task'
import Team from './Team'
Expand Down Expand Up @@ -65,10 +64,6 @@ const TeamMember = new GraphQLObjectType<any, GQLContext>({
type: new GraphQLNonNull(GraphQLEmailType),
description: 'The user email'
},
picture: {
type: new GraphQLNonNull(GraphQLURLType),
description: 'url of user’s profile picture'
},
isSelf: {
type: new GraphQLNonNull(GraphQLBoolean),
description: 'true if this team member belongs to the user that queried it',
Expand Down
Loading

0 comments on commit db17c9d

Please sign in to comment.