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

Support SNI via PFX #718

Merged
merged 1 commit into from
Jul 12, 2024
Merged

Support SNI via PFX #718

merged 1 commit into from
Jul 12, 2024

Conversation

rayluo
Copy link
Collaborator

@rayluo rayluo commented Jul 8, 2024

  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
Opt in via client_credential={
    "public_certificate": True,
    "private_key_pfx_path": "/path/to/cert.pfx",
}
@bgavrilMS
Copy link
Member

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
Collaborator 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
12 checks passed
@rayluo rayluo deleted the lab-sni branch July 12, 2024 07:34
@rayluo rayluo mentioned this pull request Jul 17, 2024
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.

2 participants