Skip to content

add customKeyIdentifier to password credential#4507

Merged
lmazuel merged 3 commits intoAzure:masterfrom
yugangw-msft:keyDesc
Nov 27, 2018
Merged

add customKeyIdentifier to password credential#4507
lmazuel merged 3 commits intoAzure:masterfrom
yugangw-msft:keyDesc

Conversation

@yugangw-msft
Copy link
Contributor

//CC: @lmazuel. CLI users want this

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

  • I have reviewed the documentation for the workflow.
  • Validation tools were run on swagger spec(s) and have all been fixed in this PR.
  • The OpenAPI Hub was used for checking validation status and next steps.

@openapi-portal-comment
Copy link

If you're a MSFT employee, click this link
to view this PR's validation status on our new OpenAPI Hub spec management tool.

@AutorestCI
Copy link

AutorestCI commented Nov 20, 2018

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#3885

@AutorestCI
Copy link

AutorestCI commented Nov 20, 2018

Automation for azure-sdk-for-js

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-js#589

@AutorestCI
Copy link

AutorestCI commented Nov 20, 2018

Automation for azure-sdk-for-ruby

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-ruby#1958

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Nov 20, 2018

Automation for azure-sdk-for-go

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-go#3417

@AutorestCI
Copy link

AutorestCI commented Nov 20, 2018

Automation for azure-sdk-for-java

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-java#2338

@AutorestCI
Copy link

AutorestCI commented Nov 20, 2018

Automation for azure-sdk-for-node

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-node#4202

@yugangw-msft
Copy link
Contributor Author

@lmazuel, please review. If you are happy, please publish the python sdk

},
"customKeyIdentifier": {
"type":"string",
"format": "byte",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cert based id should be a plain string with the thumbprint as the value

Copy link
Member

Choose a reason for hiding this comment

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

byte – base64-encoded characters, for example, U3dhZ2dlciByb2Nrcw==

So it depends only if you have base64 string on the wire. If yes, the right value is "byte". If no, the right value is "string"

Copy link
Member

Choose a reason for hiding this comment

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

After check, this string is actually not byte on the wire and the fix is right.

},
"customKeyIdentifier": {
"type": "string",
"format": "byte",
Copy link
Member

Choose a reason for hiding this comment

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

byte – base64-encoded characters, for example, U3dhZ2dlciByb2Nrcw==

So it depends only if you have base64 string on the wire. If yes, the right value is "byte". If no, the right value is "string"

Copy link
Member

Choose a reason for hiding this comment

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

Checked on Fiddler, this one is definitley base64 (and then byte)

Copy link
Member

@lmazuel lmazuel left a comment

Choose a reason for hiding this comment

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

Double checked and tested with CLI

@lmazuel lmazuel merged commit 7bf2038 into Azure:master Nov 27, 2018
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.

5 participants