Skip to content

Support SNI via PFX#718

Merged
rayluo merged 1 commit intodevfrom
lab-sni
Jul 12, 2024
Merged

Support SNI via PFX#718
rayluo merged 1 commit intodevfrom
lab-sni

Conversation

@rayluo
Copy link
Contributor

@rayluo rayluo commented Jul 8, 2024 •
edited
Loading

  1. MSAL Python has long been supporting "Subject Name / Issuer authentication" (defined here https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/60) with this API:

    ConfidentialClientApplication("client_id", client_credential={
    "public_certificate": "--- BEGIN CERTIFICATE --- ... ",
    ...
}, ...)

    but MSAL Python did not use it in MSAL's own test environment.

  2. MSAL Python supports using client certificate in .pfx format since version 1.29, and started using it in the test automation.

  3. Recently, MSAL team's lab infrastructure requires "Subject Name / Issuer authentication" (defined here https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/60), otherwise the test automation will fail.

So, this PR adds SNI to the pfx code path, using this API.

ConfidentialClientApplication("client_id", client_credential={
    "public_certificate": True,  # The cert will be read from .pfx
    "private_key_pfx_path": "/path/to/cert.pfx",
}, ...)

This new API will be released in MSAL Python 1.30.0. Docs are staged here.

@rayluo rayluo requested a review from a team as a code owner July 8, 2024 08:44
@rayluo
Support SNI via PFX
bf44364 
Opt in via client_credential={
    "public_certificate": True,
    "private_key_pfx_path": "/path/to/cert.pfx",
}
@bgavrilMS
Copy link
Member

bgavrilMS commented Jul 9, 2024

All the other MSALs use the naming x5c or sendX5C. The name "public certificate" is difficult to understand, especially as it is a "bool".

I get that it's for consistency with the other API.

@rayluo
Copy link
Contributor Author

rayluo commented Jul 9, 2024

All the other MSALs use the naming x5c or sendX5C. The name "public certificate" is difficult to understand, especially as it is a "bool".

I get that it's for consistency with the other API.

The x5c is an overly shortened naming style only observed in JWT, which is exotic to people who are unfamiliar with JWT or OAuth2; MSAL shall ideally abstract those details away from app developer.

By definition, the x5c is a container that "... contains the X.509 public key certificate ...", so, public_certificate sounds about right. Conceptually, it also feels logical that "if you want eSTS to authenticate your app based on your public cert's subject name & issuer, then please provide that public cert"; that is better than "if you want ..., use param x5c even though you don't know whether x5c means a drone".

That being said, given that x5c is used on multiple MSALs, it probably won't harm if MSAL Python also introduces a boolean x5c=True as an alias for public_certificate=True. The latter is going to stay because we have to continue support the public_certificate="--- BEGIN CERTIFICATE --- ... PEM format" usage, so the MSAL Python community already knows and expects public_certificate.

@rayluo rayluo merged commit bf44364 into dev Jul 12, 2024
@rayluo rayluo deleted the lab-sni branch July 12, 2024 07:34
@rayluo rayluo mentioned this pull request Jul 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bgavrilMS bgavrilMS bgavrilMS approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rayluo @bgavrilMS