diff --git a/ChangeLog.md b/ChangeLog.md index 0c8438bbe..d141abaaf 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -8,6 +8,7 @@ Blob: - Fixed issue of setting blob tag should not update Blob Etag and LastModified. (issue #2327) - Fix HTTP header parsing of `SubmitBatch()`. If a HTTP header has HTTP header delimiter (`:`) in its value, `SubmitBatch()` returns "400 One of the request inputs is not valid". For example, if `user-agent` header is `azsdk-cpp-storage-blobs/12.10.0-beta.1 (Darwin 23.1.0 arm64 Darwin Kernel Version 23.1.0: Mon Oct 9 21:28:12 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T8103)`, all `SubmitBatch()` requests are failed. +- Fixed issue of blob copying succeed without 'r' permission in source blob's SAS token credential. ## 2023.12 Version 3.29.0 diff --git a/src/blob/handlers/BlobHandler.ts b/src/blob/handlers/BlobHandler.ts index b9da01ae2..53eef62bc 100644 --- a/src/blob/handlers/BlobHandler.ts +++ b/src/blob/handlers/BlobHandler.ts @@ -122,39 +122,39 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { const response: Models.BlobGetPropertiesResponse = againstMetadata ? { - statusCode: 200, - metadata: res.metadata, - eTag: res.properties.etag, - requestId: context.contextId, - version: BLOB_API_VERSION, - date: context.startTime, - clientRequestId: options.requestId, - contentLength: res.properties.contentLength, - lastModified: res.properties.lastModified - } + statusCode: 200, + metadata: res.metadata, + eTag: res.properties.etag, + requestId: context.contextId, + version: BLOB_API_VERSION, + date: context.startTime, + clientRequestId: options.requestId, + contentLength: res.properties.contentLength, + lastModified: res.properties.lastModified + } : { - statusCode: 200, - metadata: res.metadata, - isIncrementalCopy: res.properties.incrementalCopy, - eTag: res.properties.etag, - requestId: context.contextId, - version: BLOB_API_VERSION, - date: context.startTime, - acceptRanges: "bytes", - blobCommittedBlockCount: - res.properties.blobType === Models.BlobType.AppendBlob - ? res.blobCommittedBlockCount - : undefined, - isServerEncrypted: true, - clientRequestId: options.requestId, - ...res.properties, - cacheControl: context.request!.getQuery("rscc") ?? res.properties.cacheControl, - contentDisposition: context.request!.getQuery("rscd") ?? res.properties.contentDisposition, - contentEncoding: context.request!.getQuery("rsce") ?? res.properties.contentEncoding, - contentLanguage: context.request!.getQuery("rscl") ?? res.properties.contentLanguage, - contentType: context.request!.getQuery("rsct") ?? res.properties.contentType, - tagCount: res.properties.tagCount, - }; + statusCode: 200, + metadata: res.metadata, + isIncrementalCopy: res.properties.incrementalCopy, + eTag: res.properties.etag, + requestId: context.contextId, + version: BLOB_API_VERSION, + date: context.startTime, + acceptRanges: "bytes", + blobCommittedBlockCount: + res.properties.blobType === Models.BlobType.AppendBlob + ? res.blobCommittedBlockCount + : undefined, + isServerEncrypted: true, + clientRequestId: options.requestId, + ...res.properties, + cacheControl: context.request!.getQuery("rscc") ?? res.properties.cacheControl, + contentDisposition: context.request!.getQuery("rscd") ?? res.properties.contentDisposition, + contentEncoding: context.request!.getQuery("rsce") ?? res.properties.contentEncoding, + contentLanguage: context.request!.getQuery("rscl") ?? res.properties.contentLanguage, + contentType: context.request!.getQuery("rsct") ?? res.properties.contentType, + tagCount: res.properties.tagCount, + }; return response; } @@ -329,12 +329,10 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { const metadata = convertRawHeadersToMetadata( blobCtx.request!.getRawHeaders() ); - - if (metadata != undefined) - { + + if (metadata != undefined) { Object.entries(metadata).forEach(([key, value]) => { - if (key.includes("-")) - { + if (key.includes("-")) { throw StorageErrorFactory.getInvalidMetadata(context.contextId!); } }); @@ -664,7 +662,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { throw StorageErrorFactory.getBlobNotFound(context.contextId!); } - if (sourceAccount !== blobCtx.account) { + const sig = url.searchParams.get("sig"); + if ((sourceAccount !== blobCtx.account) || (sig !== null)) { await this.validateCopySource(copySource, sourceAccount, context); } @@ -1103,7 +1102,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { tagCount: getBlobTagsCount(blob.blobTags), isServerEncrypted: true, clientRequestId: options.requestId, - creationTime:blob.properties.creationTime, + creationTime: blob.properties.creationTime, blobCommittedBlockCount: blob.properties.blobType === Models.BlobType.AppendBlob ? (blob.committedBlocksInOrder || []).length @@ -1176,9 +1175,9 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { contentLength <= 0 ? [] : this.rangesManager.fillZeroRanges(blob.pageRangesInOrder, { - start: rangeStart, - end: rangeEnd - }); + start: rangeStart, + end: rangeEnd + }); const bodyGetter = async () => { return this.extentStore.readExtents( @@ -1233,7 +1232,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { blobContentMD5: blob.properties.contentMD5, tagCount: getBlobTagsCount(blob.blobTags), isServerEncrypted: true, - creationTime:blob.properties.creationTime, + creationTime: blob.properties.creationTime, clientRequestId: options.requestId }; @@ -1243,7 +1242,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { public async query( options: Models.BlobQueryOptionalParams, context: Context - ): Promise{ + ): Promise { throw new NotImplementedError(context.contextId); } @@ -1266,13 +1265,13 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { ); const response: Models.BlobGetTagsResponse = { - statusCode: 200, - blobTagSet: tags === undefined ? []: tags.blobTagSet, - requestId: context.contextId, - version: BLOB_API_VERSION, - date: context.startTime, - clientRequestId: options.requestId, - }; + statusCode: 200, + blobTagSet: tags === undefined ? [] : tags.blobTagSet, + requestId: context.contextId, + version: BLOB_API_VERSION, + date: context.startTime, + clientRequestId: options.requestId, + }; return response; } @@ -1315,18 +1314,18 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { return response; } - private NewUriFromCopySource(copySource: string, context: Context): URL{ - try{ - return new URL(copySource) - } - catch - { - throw StorageErrorFactory.getInvalidHeaderValue( - context.contextId, - { - HeaderName: "x-ms-copy-source", - HeaderValue: copySource - }) - } + private NewUriFromCopySource(copySource: string, context: Context): URL { + try { + return new URL(copySource) + } + catch + { + throw StorageErrorFactory.getInvalidHeaderValue( + context.contextId, + { + HeaderName: "x-ms-copy-source", + HeaderValue: copySource + }) + } } } diff --git a/tests/blob/apis/blockblob.test.ts b/tests/blob/apis/blockblob.test.ts index d185ec5b2..8b230344c 100644 --- a/tests/blob/apis/blockblob.test.ts +++ b/tests/blob/apis/blockblob.test.ts @@ -1,7 +1,8 @@ import { StorageSharedKeyCredential, BlobServiceClient, - newPipeline + newPipeline, + BlobSASPermissions } from "@azure/storage-blob"; import assert = require("assert"); import crypto = require("crypto"); @@ -548,4 +549,45 @@ describe("BlockBlobAPIs", () => { body ); }); + + it("Start copy without required permission should fail @loki @sql", async () => { + const body: string = getUniqueName("randomstring"); + const expiryTime = new Date(); + expiryTime.setDate(expiryTime.getDate() + 1); + await blockBlobClient.upload(body, Buffer.byteLength(body)); + + const sourceURLWithoutPermission = await blockBlobClient.generateSasUrl({ + permissions: BlobSASPermissions.parse("w"), + expiresOn: expiryTime + }); + + const destBlobName: string = getUniqueName("destBlobName"); + const destBlobClient = containerClient.getBlockBlobClient(destBlobName); + + try { + await destBlobClient.beginCopyFromURL(sourceURLWithoutPermission); + assert.fail("Copy without required permision should fail"); + } + catch (ex) { + assert.deepStrictEqual(ex.statusCode, 403); + assert.ok(ex.message.startsWith("This request is not authorized to perform this operation using this permission.")); + assert.deepStrictEqual(ex.code, "CannotVerifyCopySource"); + } + + // Copy within the same account without SAS token should succeed. + const result = await (await destBlobClient.beginCopyFromURL(blockBlobClient.url)).pollUntilDone(); + assert.ok(result.copyId); + assert.strictEqual(result.errorCode, undefined); + + // Copy with 'r' permission should succeed. + const sourceURL = await blockBlobClient.generateSasUrl({ + permissions: BlobSASPermissions.parse("r"), + expiresOn: expiryTime + }); + + const resultWithPermission = await (await destBlobClient.beginCopyFromURL(sourceURL)).pollUntilDone(); + assert.ok(resultWithPermission.copyId); + assert.strictEqual(resultWithPermission.errorCode, undefined); + }); + }); diff --git a/tests/blob/sas.test.ts b/tests/blob/sas.test.ts index 3c9a4311e..f7b7d34d3 100644 --- a/tests/blob/sas.test.ts +++ b/tests/blob/sas.test.ts @@ -655,7 +655,8 @@ describe("Shared Access Signature (SAS) authentication", () => { ); const containerName = getUniqueName("con"); - const containerClient = serviceClientWithSAS.getContainerClient( + const containerClient = serviceClient.getContainerClient(containerName); + const containerClientWithSAS = serviceClientWithSAS.getContainerClient( containerName ); await containerClient.create(); @@ -663,7 +664,7 @@ describe("Shared Access Signature (SAS) authentication", () => { const blobName1 = getUniqueName("blob"); const blobName2 = getUniqueName("blob"); const blob1 = containerClient.getBlockBlobClient(blobName1); - const blob2 = containerClient.getBlockBlobClient(blobName2); + const blob2 = containerClientWithSAS.getBlockBlobClient(blobName2); await blob1.upload("hello", 5);