Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow upload signed url to overwrite files #460

Merged
merged 1 commit into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/auth/jwt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
const JWT_ECC_ALGOS: jwt.Algorithm[] = ['ES256', 'ES384', 'ES512']
const JWT_ED_ALGOS: jwt.Algorithm[] = ['EdDSA'] as unknown as jwt.Algorithm[] // types for EdDSA not yet updated

interface jwtInterface {

Check warning on line 14 in src/auth/jwt.ts

View workflow job for this annotation

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

'jwtInterface' is defined but never used
sub?: string
role?: string
}
Expand All @@ -24,6 +24,7 @@

export type SignedUploadToken = {
owner: string | undefined
upsert: boolean
url: string
exp: number
}
Expand All @@ -47,7 +48,7 @@

// find the first key without a kid or with the matching kid and the "oct" type
const jwk = jwks.keys.find(
(key) => (!key.kid || key.kid === header.kid) && key.kty === 'oct' && (key as any).k

Check warning on line 51 in src/auth/jwt.ts

View workflow job for this annotation

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type
)

if (!jwk) {
Expand All @@ -55,7 +56,7 @@
return secret
}

return Buffer.from((jwk as any).k, 'base64')

Check warning on line 59 in src/auth/jwt.ts

View workflow job for this annotation

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type
}

// jwt is using an asymmetric algorithm
Expand Down Expand Up @@ -88,11 +89,11 @@
jwks: { keys: { kid?: string; kty: string }[] } | null
): jwt.GetPublicKeyOrSecret {
return (header: jwt.JwtHeader, callback: jwt.SigningKeyCallback) => {
let result: any = null

Check warning on line 92 in src/auth/jwt.ts

View workflow job for this annotation

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type

try {
result = findJWKFromHeader(header, secret, jwks)
} catch (e: any) {

Check warning on line 96 in src/auth/jwt.ts

View workflow job for this annotation

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type
callback(e)
return
}
Expand All @@ -108,9 +109,9 @@
const hasRSA = jwks.keys.find((key) => key.kty === 'RSA')
const hasECC = jwks.keys.find((key) => key.kty === 'EC')
const hasED = jwks.keys.find(
(key) => key.kty === 'OKP' && ((key as any).crv === 'Ed25519' || (key as any).crv === 'Ed448')

Check warning on line 112 in src/auth/jwt.ts

View workflow job for this annotation

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type

Check warning on line 112 in src/auth/jwt.ts

View workflow job for this annotation

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type
)
const hasHS = jwks.keys.find((key) => key.kty === 'oct' && (key as any).k)

Check warning on line 114 in src/auth/jwt.ts

View workflow job for this annotation

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type

algorithms = [
jwtAlgorithm as jwt.Algorithm,
Expand Down
14 changes: 13 additions & 1 deletion src/http/routes/object/getSignedUploadURL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ const getSignedUploadURLParamsSchema = {
required: ['bucketName', '*'],
} as const

const getSignedUploadURLHeadersSchema = {
type: 'object',
properties: {
'x-upsert': { type: 'string' },
authorization: { type: 'string' },
},
required: ['authorization'],
} as const

const successResponseSchema = {
type: 'object',
properties: {
Expand All @@ -29,6 +38,7 @@ const successResponseSchema = {
}
interface getSignedURLRequestInterface extends AuthenticatedRequest {
Params: FromSchema<typeof getSignedUploadURLParamsSchema>
Headers: FromSchema<typeof getSignedUploadURLHeadersSchema>
}

export default async function routes(fastify: FastifyInstance) {
Expand All @@ -54,7 +64,9 @@ export default async function routes(fastify: FastifyInstance) {

const signedUploadURL = await request.storage
.from(bucketName)
.signUploadObjectUrl(objectName, urlPath as string, uploadSignedUrlExpirationTime, owner)
.signUploadObjectUrl(objectName, urlPath as string, uploadSignedUrlExpirationTime, owner, {
upsert: request.headers['x-upsert'] === 'true',
})

return response.status(200).send({ url: signedUploadURL })
}
Expand Down
1 change: 1 addition & 0 deletions src/http/routes/object/uploadSignedObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export default async function routes(fastify: FastifyInstance) {
.uploadNewObject(request, {
owner,
objectName,
isUpsert: payload.upsert,
})

return response.status(objectMetadata?.httpStatusCode ?? 200).send({
Expand Down
27 changes: 18 additions & 9 deletions src/storage/object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -572,8 +572,15 @@ export class ObjectStorage {
* @param url
* @param expiresIn seconds
* @param owner
* @param options
*/
async signUploadObjectUrl(objectName: string, url: string, expiresIn: number, owner?: string) {
async signUploadObjectUrl(
objectName: string,
url: string,
expiresIn: number,
owner?: string,
options?: { upsert?: boolean }
) {
// check as super user if the object already exists
const found = await this.asSuperUser().findObject(objectName, 'id', {
dontErrorOnEmpty: true,
Expand All @@ -584,19 +591,21 @@ export class ObjectStorage {
}

// check if user has INSERT permissions
await this.db.testPermission((db) => {
return db.createObject({
bucket_id: this.bucketId,
name: objectName,
owner,
metadata: {},
})
await this.uploader.canUpload({
bucketId: this.bucketId,
objectName,
owner,
isUpsert: options?.upsert ?? false,
})

const urlParts = url.split('/')
const urlToSign = decodeURI(urlParts.splice(4).join('/'))
const { secret: jwtSecret } = await getJwtSecret(this.db.tenantId)
const token = await signJWT({ owner, url: urlToSign }, jwtSecret, expiresIn)
const token = await signJWT(
{ owner, url: urlToSign, upsert: Boolean(options?.upsert) },
jwtSecret,
expiresIn
)

return `/object/upload/sign/${urlToSign}?token=${token}`
}
Expand Down
69 changes: 68 additions & 1 deletion src/test/object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1544,6 +1544,73 @@ describe('testing uploading with generated signed upload URL', () => {
expect(response.statusCode).toBe(400)
expect(S3Backend.prototype.uploadObject).not.toHaveBeenCalled()
})

it('will allow overwriting a file when the generating a signed upload url with x-upsert:true', async () => {
function createUpload() {
const form = new FormData()
form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`))
return form
}

const BUCKET_ID = 'bucket2'
const OBJECT_NAME = 'signed/sadcat-upload-signed-2.png'
const urlToSign = `${BUCKET_ID}/${OBJECT_NAME}`
const owner = '317eadce-631a-4429-a0bb-f19a7a517b4a'

// Upload a file first
const resp = await app().inject({
method: 'POST',
url: `/object/${urlToSign}`,
payload: createUpload(),
headers: {
authorization: serviceKey,
},
})

expect(resp.statusCode).toBe(200)

const jwtToken = await signJWT({ owner, url: urlToSign, upsert: true }, jwtSecret, 100)
const response = await app().inject({
method: 'PUT',
url: `/object/upload/sign/${urlToSign}?token=${jwtToken}`,
payload: createUpload(),
})
expect(response.statusCode).toBe(200)
expect(S3Backend.prototype.uploadObject).toHaveBeenCalled()
})

it('will allow not be able overwriting a file when the generating a signed upload url without x-upsert header', async () => {
function createUpload() {
const form = new FormData()
form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`))
return form
}

const BUCKET_ID = 'bucket2'
const OBJECT_NAME = 'signed/sadcat-upload-signed-3.png'
const urlToSign = `${BUCKET_ID}/${OBJECT_NAME}`
const owner = '317eadce-631a-4429-a0bb-f19a7a517b4a'

// Upload a file first
const resp = await app().inject({
method: 'POST',
url: `/object/${urlToSign}`,
payload: createUpload(),
headers: {
authorization: serviceKey,
},
})

expect(resp.statusCode).toBe(200)

const jwtToken = await signJWT({ owner, url: urlToSign }, jwtSecret, 100)
const response = await app().inject({
method: 'PUT',
url: `/object/upload/sign/${urlToSign}?token=${jwtToken}`,
payload: createUpload(),
})
expect(response.statusCode).toBe(400)
})
})

/**
Expand Down Expand Up @@ -1864,7 +1931,7 @@ describe('testing list objects', () => {
})
expect(response.statusCode).toBe(200)
const responseJSON = JSON.parse(response.body)
expect(responseJSON).toHaveLength(5)
expect(responseJSON).toHaveLength(6)
const names = responseJSON.map((ele: any) => ele.name)
expect(names).toContain('curlimage.jpg')
expect(names).toContain('private')
Expand Down
Loading