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

Add support for sending X5C when using a service principal with certificate for authentication #1666

Merged
merged 5 commits into from
Jul 19, 2024

Conversation

spongemike2
Copy link

When using a service principal with certificate authentication, every time the certificate is renewed, the new certificate needs to be uploaded to the service principal's AAD app registration in order for authentication to continue to work.

However, a technology called "X5C" has made this unnecessary by allowing any certificate, with a specific subject, issued by a known, trusted, predetermined CA, to be used.

For this to work, the AAD app registration's manifest needs to be updated to reflect the subject name, and during authentication, the request for "X5C" authentication needs to be sent along with the certificate's signature.

This change enables that to take place.

Copy link

@spongyryno spongyryno left a comment

Choose a reason for hiding this comment

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

Should version be removed?

Copy link
Collaborator

@mjcheetham mjcheetham left a comment

Choose a reason for hiding this comment

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

Thank you for this addition! Just a few things please:

  • address the comment about parsing the config values
  • drop the VERSION file change
  • please can you also add docs for the GCM_AZREPOS_SP_CERT_SEND_X5C and credential.azreposServicePrincipalCertificateSendX5C settings to the corresponding markdown docs in /docs!

😄

VERSION Outdated Show resolved Hide resolved
src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs Outdated Show resolved Hide resolved
copiden

This comment was marked as spam.

Copy link
Collaborator

@mjcheetham mjcheetham left a comment

Choose a reason for hiding this comment

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

Great stuff! Thank you!

@mjcheetham mjcheetham merged commit 1d06bd5 into git-ecosystem:main Jul 19, 2024
8 checks passed
@spongemike2 spongemike2 deleted the mlyons branch September 4, 2024 20:06
@mjcheetham mjcheetham mentioned this pull request Sep 30, 2024
mjcheetham pushed a commit that referenced this pull request Sep 30, 2024
**Changes:**

- Drop no longer needed workflows (#1659)
- Documentation fixes (#1664, #1697)
- Configurable GPG store path via Git config (#1698)
- Fix Visual Studio build problems and update dependencies (#1711)
- Support sending X5C with certificate auth (#1666)
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.

4 participants