Skip to content

auth: support service principal sn+issuer auth#7609

Merged
yugangw-msft merged 3 commits intoAzure:devfrom
yugangw-msft:sn
Oct 30, 2018
Merged

auth: support service principal sn+issuer auth#7609
yugangw-msft merged 3 commits intoAzure:devfrom
yugangw-msft:sn

Conversation

@yugangw-msft
Copy link
Contributor

@yugangw-msft yugangw-msft commented Oct 18, 2018

The goal is to support automatic certificate rolls. The tenant must be pre-configured with only accepting certain issuers such as AME.

Mark it as do not merge as we are waiting for a new release of adal to have the capacity. Once it happens, i will update the PR with test coverage.

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

@yugangw-msft
Copy link
Contributor Author

@tjprescott @williexu, can one of you help take a look?

Copy link
Member

Choose a reason for hiding this comment

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

This is duplicated. Should only be in 2.0.50.

Copy link
Member

Choose a reason for hiding this comment

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

What is "sn"?

Copy link
Contributor Author

@yugangw-msft yugangw-msft Oct 26, 2018

Choose a reason for hiding this comment

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

It means certificate subject name :). This abbreviation is used in the auth flow doc across, so i just use it. I know there is a risk it could be taken as serial number, so I used the full term in the command help.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me rename to --use-cert-sn-issuer to make it a bit more clear

Copy link
Member

Choose a reason for hiding this comment

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

typo: "applicable"

@yugangw-msft yugangw-msft force-pushed the sn branch 2 times, most recently from 1291127 to 69f00ae Compare October 26, 2018 17:32
if use_cert_sn_issuer and not service_principal:
raise CLIError("usage error: '--use-sn-issuer' is only applicable with a service principal")
if service_principal and not username:
raise CLIError('usage error: --service-principal --username NAME --password SECRET -t TENANT')
Copy link
Contributor

Choose a reason for hiding this comment

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

should we use the full option --tenant here?

2.0.50
++++++
* Fix issue where update commands using `--remove` and `--ids` fail after first update is applied to first resource in ids list.
* auth: support service principal sn+issuer auth
Copy link
Contributor

@adewaleo adewaleo Oct 29, 2018

Choose a reason for hiding this comment

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

Was the previous history change unnecessary to include in History.rst

Copy link
Member

Choose a reason for hiding this comment

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

The deleted history entry was part of 2.0.49.

@yugangw-msft yugangw-msft merged commit a52e250 into Azure:dev Oct 30, 2018
@yugangw-msft yugangw-msft deleted the sn branch October 30, 2018 03:04
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.

3 participants