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

[Tables] Fix Shared Key auth and update Readme #10988

Merged
merged 10 commits into from
Sep 4, 2020
Merged

[Tables] Fix Shared Key auth and update Readme #10988

merged 10 commits into from
Sep 4, 2020

Conversation

joheredi
Copy link
Member

@joheredi joheredi commented Sep 2, 2020

SharedKey auth was broken since the authentication pipeline was not being created correctly and also the construction of the Canonicalized resource was faulty.

Fix:

  • Explicitly pass the auth pipeline to the Client contructor
  • Switch to SharedKey Lite format which simplifies getting the canonicalized resource.
  • Moved getAccountConnectionString method to make sure we fail fast on browsers as it is not supported

I'm also including a couple of updates to the readme to fix small issues I noticed while running the samples

@joheredi joheredi requested review from xirzec and ellismg September 2, 2020 18:44
@joheredi joheredi requested a review from bterlson as a code owner September 2, 2020 18:44
@ghost ghost added the Tables label Sep 2, 2020
@joheredi joheredi linked an issue Sep 2, 2020 that may be closed by this pull request
Copy link
Member

@ellismg ellismg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I trust that this behaves as expected, but is it easy enough for us to write a recorded test that gives us some coverage when using an account key so we have at least some coverage in our matrix?

Copy link
Member

@ellismg ellismg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Thanks for adding the test. On small piece of feedback on one of the guards we have in a test, but it's not blocking.

@azure-pipelines
Copy link

Command 'js' is not supported by Azure Pipelines.

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.

@joheredi
Copy link
Member Author

joheredi commented Sep 4, 2020

/azp run js - tables - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@joheredi joheredi merged commit 5c7d58e into Azure:master Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tables] Requests fail when using TablesSharedKeyCredential
2 participants