Skip to content

Alpha support to Elliptic Curve keys to JsonWebKey classes#3539

Closed
msfcolombo wants to merge 5 commits intoAzure:psSdkJson6from
msfcolombo:ECC
Closed

Alpha support to Elliptic Curve keys to JsonWebKey classes#3539
msfcolombo wants to merge 5 commits intoAzure:psSdkJson6from
msfcolombo:ECC

Conversation

@msfcolombo
Copy link
Copy Markdown

Alpha support to Elliptic Curve keys to JsonWebKey classes

This PR adds alpha support for Elliptic Curve keys the Microsoft.Azure.KeyVault.WebKey namespace. This is intended to allow customers and partners accessing the Elliptic Curve features of Azure Key Vault in preview mode.


This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

SDK Generation Guidelines

  • If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
  • The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as the commitid of your swagger spec or link to the swagger spec, used to generate the code.
  • The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK.

@msftclas
Copy link
Copy Markdown

msftclas commented Aug 1, 2017

@msfcolombo,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@shahabhijeet
Copy link
Copy Markdown
Contributor

@azuresdkci test this please

Copy link
Copy Markdown
Contributor

@shahabhijeet shahabhijeet left a comment

Choose a reason for hiding this comment

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

@msfcolombo please have a signoff from include Rich Randal or Dragos

var ecdsa_jwk = jwk.ToECDsa(usePrivateKey);

Assert.Equal(ecdsa_jwk.KeySize, ecdsa.KeySize);
#if NET452
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@msfcolombo please use #FullNetFx moniker for Full Desktop version of .NET

@@ -1,11 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<Import Project="$([MSBuild]::GetPathOfFileAbove('AzSdk.props'))" />
<Import Project="$([MSBuild]::GetPathOfFileAbove('AzSdk.reference.props'))" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@msfcolombo any reason to start taking dependency on common clientRuntime binaries. Only management plane has taken dependency on them?

Copy link
Copy Markdown
Author

@msfcolombo msfcolombo Aug 4, 2017

Choose a reason for hiding this comment

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

We intend to provide WebKey to more environments. It's a low-level component. The use of AzSdk.reference.props is to insure compatibility and avoid proj code duplication.

Copy link
Copy Markdown
Contributor

@herveyw-msft herveyw-msft left a comment

Choose a reason for hiding this comment

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

Approved.

Copy link
Copy Markdown
Contributor

@dragav dragav left a comment

Choose a reason for hiding this comment

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

Approved.

@msfcolombo
Copy link
Copy Markdown
Author

@shahabhijeet I did the change you requested, and commented over usage of AzSdk.reference.props. I got sign-off from Hervey and Dragos. Is there something else before merging?

@shahabhijeet
Copy link
Copy Markdown
Contributor

@azuresdkci test this please

@msfcolombo
Copy link
Copy Markdown
Author

@shahabhijeet All requested changes were performed. Thanks for the push into keeping a low number of dependencies, and let us know if there is anything preventing merge.

@msfcolombo
Copy link
Copy Markdown
Author

Changes here are now included on #3577.

@msfcolombo msfcolombo closed this Aug 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants