[Tables] Create clients from connectionString #10292

joheredi · 2020-07-27T01:52:27Z

Support 2 kinds of connection strings to instantiate TableClient and TableServiceClient

Account Connection String (Node.js Only)
SAS Connection String

Docs for Tables Shared Key Authorization: https://docs.microsoft.com/en-us/rest/api/storageservices/authorize-with-shared-key#table-service-shared-key-authorization

Note: Parts of the authorization using Shared Keys are similar to other Storage Services, however Tables has some specific requirements different from Blob, Queues, etc. It may be a good idea at some point to move common building blocks to a common/core package so that each service can compose their own Authorization reusing them. Filed issue #10291 to track this investigation

jeremymeng · 2020-07-27T17:49:29Z

/cc @HarshaNalluru who worked on this for storage.

sdk/tables/azure-tables/samples/javascript/package.json

ellismg · 2020-07-27T17:49:37Z

sdk/tables/azure-tables/src/SharedKeyCredential.ts

+import { SharedKeyCredentialPolicy } from "./SharedKeyCredentialPolicy";
+
+/**
+ * ONLY AVAILABLE IN NODE.JS RUNTIME.


This is only in Node.JS today because of the dependency on HMAC? We've been making a push in other libraries to be able to do this HMAC stuff in browser as well (I know Richard recently did some work in the Application Configuration SDK to support this). I'm looking at trying to get us a @azure/core-crypto library that will expose some of these primitives in a way that would work in node or browsers, so I'd like to know what all is keeping this node today, so we can expose the right set of primitives so we can have code like this work in the browser as well.

If we had support for doing SHA265 HMAC would that be sufficient to get us browser support here?

Yeah, I think that's the only thing holding us back in this case

I asked @XiaoningLiu before for other storage libraries. I remember there are more reasons than hmac support for not having connection strings/shared key credential for browsers. Maybe security related. @XiaoningLiu can you confirm?

Yes. It's security concern. SharedKey in browser session is almost public. Storage team recommends using SAS or OAuth in browser sessions.

Thanks @jeremymeng for bringing this up. @XiaoningLiu do you have a doc reference about the security issue, that I can include in the comments of this file to make sure people are aware why this is not supported in the Browser?

The SharedKeyCredential has complete access to your account. Putting it in the hands of clients means clients now have the power to do anything including delete all your data, and moreover, other (possibly untrusted 3rd party) code running in the application can exfiltrate the key trivially. Maybe a vague "for security reasons" is sufficient?

ellismg · 2020-07-27T17:54:59Z

sdk/tables/azure-tables/src/utils/connectionString.ts

+import { URL } from "./url";
+
+export interface ConnectionString {
+  kind: "AccountConnString" | "SASConnString";


Do these strings have some sort of well understood meaning? Would AccountConnectionString and SASConnectionString work better (or alternately since this is already on the ConnectionString interface, just Account and SAS)

I see that this doesn't end up getting exported from index.ts, so I'm less concerned about this.

This is only used to distinguish the connection strings internally, not for the users :)

ellismg · 2020-07-27T18:40:32Z

sdk/tables/azure-tables/tsconfig.json

@@ -2,7 +2,8 @@
  "extends": "../../../tsconfig.package",
  "compilerOptions": {
    "outDir": "./dist-esm",
-    "declarationDir": "./types"
+    "declarationDir": "./types",
+    "lib": ["dom", "es5", "es6", "es7", "esnext"]


Can you help me understand why this change happened here? I'm especially interested in the dom addition (and also I don't really understand why we would have libs for es5, es6, es7 and esnext. Shouldn't we pick a specific one (and have it be the oldest one we can use)?

dom as added to include DOM declarations and be able to work with URL in url.browser.ts. The others are not needed so I'm going to remove them.

dom as added to include DOM declarations and be able to work with URL in url.browser.ts.

Of course, that makes sense. Forgot about the special handing around URL.

mitchdenny · 2020-07-28T07:03:37Z

sdk/tables/ci.yml

@@ -14,8 +14,7 @@ trigger:
 pr:
  branches:
    include:
-      # TODO: re-enable master pr validation after tables tests are ready
-      #- master
+      - master


Unnecessary to disable/enable PR triggers in your fork, the trigger rules are actually controlled by the target branch anyway, so I suspect this didn't have an impact ;)

This did disable js - tables - ci in PR validation. See this PR #10207 which still has this commented out it doesn't trigger js - tables - ci

mahdiva · 2020-07-30T21:22:18Z

sdk/tables/azure-tables/samples/javascript/package.json

+  },
+  "homepage": "https://github.com/Azure/azure-sdk-for-js#readme",
+  "sideEffects": false,
+  "dependencies": {


Should we require @azure/tables in here and samples/typescript/package.json?

Yeah, I wasn't adding it because the package is not yet published. However it looks like we can add it here with a local reference. I'll do that 😄 once we publish we can start grabbing it from NPM

mahdiva · 2020-07-30T21:26:44Z

sdk/tables/azure-tables/samples/javascript/src/shardKey.js

@@ -0,0 +1,38 @@
+// Copyright (c) Microsoft Corporation.


I think there is a typo in the file name

Good catch 😄

Api Management - make /tenant endpoints ARM compliant in 2020-06-01-preview version (Azure#11549) * Adds base for updating Microsoft.ApiManagement from version stable/2019-12-01 to version 2020-06-01-preview * Updates readme * Updates API version in new specs and examples * Add support in API Management for Availability Zones (Azure#10284) * apim in azs * fix prettier check * PATCH should return 200 OK (Azure#10328) * add support for PATCH returning 200 OK * CI fixes prettier fix CI fixes part 2 * Password no longer a mandatory property when uploading Certificates * add missing x-ms-odata extension for filter support * +gatewayhostnameconfiguration protocol changes (Azure#10292) * [2020-06-01-preview] Update Oauth Server secrets Contract (Azure#10602) * Oauth server secrets contract * fix azureMonitor enum * API Management Service Deleted Services Resource (Azure#10607) * API Management Service Deleted Services Resource * Path fix * Lint + custom-words fixes * Location URI parameter for deletedservices Resource * GET for deletedservices by service name * Remove resourceGroupName from resource path * fixes * schema for purge operation * perttier applied * 204 response code added Co-authored-by: REDMOND\glfeokti <glfeokti@microsoft.com> * OperationNameFormat property added to Diagnostic contract (Azure#10641) * OperationNameFormat property added to Diagnostic contract * add azuremonitor to update contract Co-authored-by: REDMOND\glfeokti <glfeokti@microsoft.com> * [Microsoft.ApiManagement][2020-06-01-preview] Change Network Status response contract (Azure#10331) * Change Network Status response contract * Update examples for network status contract * ApiManagement - tenant/settings endpoints * ApiManagement - tenant/settings endpoints fix * ApiManagement - tenant/settings endpoints fix prettier * ApiManagement - tenant/settings endpoints fix 3 * ApiManagement - tenant/settings endpoints fix 4 * ApiManagement - tenant/settings endpoints fix 5 Co-authored-by: Samir Solanki <samirsolanki@outlook.com> Co-authored-by: maksimkim <maksim.kim@gmail.com> Co-authored-by: promoisha <feoktistovgg@gmail.com> Co-authored-by: REDMOND\glfeokti <glfeokti@microsoft.com> Co-authored-by: RupengLiu <rliu1211@terpmail.umd.edu> Co-authored-by: vfedonkin <vifedo@microsoft.com>

joheredi requested review from xirzec, jeremymeng, ellismg and mahdiva July 27, 2020 01:52

joheredi requested a review from bterlson as a code owner July 27, 2020 01:52

ghost added the Tables label Jul 27, 2020

joheredi requested a review from mitchdenny as a code owner July 27, 2020 01:54

joheredi requested a review from HarshaNalluru July 27, 2020 18:08

ellismg approved these changes Jul 27, 2020

View reviewed changes

mitchdenny reviewed Jul 28, 2020

View reviewed changes

HarshaNalluru approved these changes Jul 28, 2020

View reviewed changes

mahdiva reviewed Jul 30, 2020

View reviewed changes

joheredi added 11 commits July 30, 2020 15:45

Initial changes to support SharedKey auth

13ebff2

Add test for utility helpers

2b0da93

re-enable pipeline validation on PR

62216e7

Add javascript samples

c104cd4

update sample require

48cc25c

Address PR comments

abb5758

Add url to externalNodeBuiltins

e07674e

Support SharedKeyCredential

1257dc3

Rename ShareKeyCredential to TablesSharedKeyCredential

b341482

Address PR comments

5ff8aff

Fix file name TablesShredKeyCredential

a49df08

joheredi merged commit 5245f7e into Azure:master Jul 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Tables] Create clients from connectionString #10292

[Tables] Create clients from connectionString #10292

joheredi commented Jul 27, 2020

jeremymeng commented Jul 27, 2020

ellismg Jul 27, 2020

joheredi Jul 27, 2020

jeremymeng Jul 27, 2020

XiaoningLiu Jul 28, 2020

joheredi Jul 28, 2020

bterlson Jul 28, 2020

ellismg Jul 27, 2020

ellismg Jul 27, 2020

HarshaNalluru Jul 27, 2020

ellismg Jul 27, 2020

joheredi Jul 27, 2020 •

edited

ellismg Jul 27, 2020

mitchdenny Jul 28, 2020

joheredi Jul 28, 2020 •

edited

mahdiva Jul 30, 2020 •

edited

joheredi Jul 30, 2020

mahdiva Jul 30, 2020

joheredi Jul 30, 2020

[Tables] Create clients from connectionString #10292

[Tables] Create clients from connectionString #10292

Conversation

joheredi commented Jul 27, 2020

jeremymeng commented Jul 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joheredi Jul 27, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joheredi Jul 28, 2020 • edited

Choose a reason for hiding this comment

mahdiva Jul 30, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joheredi Jul 27, 2020 •

edited

joheredi Jul 28, 2020 •

edited

mahdiva Jul 30, 2020 •

edited