AzureAD / microsoft-authentication-library-for-python

Microsoft Authentication Library (MSAL) for Python makes it easy to authenticate to Microsoft Entra ID. General docs are available here https://learn.microsoft.com/entra/msal/python/ Stable APIs are documented here https://msal-python.readthedocs.io. Questions can be asked on www.stackoverflow.com with tag "msal" + "python".
https://stackoverflow.com/questions/tagged/azure-ad-msal+python
Other
795 stars 200 forks source link

msal 1.28.1 throws ValueError when passing frozenset as scopes #710

Closed DerekCL closed 3 months ago

DerekCL commented 3 months ago

Describe the bug When using msal version 1.28.1 for azure b2c, passing a frozenset object as the scopes parameter to get_authorization_request_url results in a ValueError: API does not accept frozenset({'openid', 'offline_access', 'profile'}) value as user-provided scopes.

Specifically this error occurs because in

def get_authorization_request_url( self, scopes, # type: list[str] login_hint=None, # type: Optional[str] state=None, # Recommended by OAuth2 for CSRF protection redirect_uri=None, response_type="code", # Could be "token" if you use Implicit Grant prompt=None, nonce=None, domain_hint=None, # type: Optional[str] claims_challenge=None, **kwargs): """Constructs a URL for you to start a Authorization Code Grant.

    :param list[str] scopes: (Required)
        Scopes requested to access a protected API (a resource).
    :param str state: Recommended by OAuth2 for CSRF protection.
    :param str login_hint:
        Identifier of the user. Generally a User Principal Name (UPN).
    :param str redirect_uri:
        Address to return to upon receiving a response from the authority.
    :param str response_type:
        Default value is "code" for an OAuth2 Authorization Code grant.

        You could use other content such as "id_token" or "token",
        which would trigger an Implicit Grant, but that is
        `not recommended <https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-implicit-grant-flow#is-the-implicit-grant-suitable-for-my-app>`_.

    :param str prompt:
        By default, no prompt value will be sent, not even string ``"none"``.
        You will have to specify a value explicitly.
        Its valid values are the constants defined in
        :class:`Prompt <msal.Prompt>`.
    :param nonce:
        A cryptographically random value used to mitigate replay attacks. See also
        `OIDC specs <https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest>`_.
    :param domain_hint:
        Can be one of "consumers" or "organizations" or your tenant domain "contoso.com".
        If included, it will skip the email-based discovery process that user goes
        through on the sign-in page, leading to a slightly more streamlined user experience.
        More information on possible values available in
        `Auth Code Flow doc <https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-auth-code-flow#request-an-authorization-code>`_ and
        `domain_hint doc <https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-oapx/86fb452d-e34a-494e-ac61-e526e263b6d8>`_.
    :param claims_challenge:
         The claims_challenge parameter requests specific claims requested by the resource provider
         in the form of a claims_challenge directive in the www-authenticate header to be
         returned from the UserInfo Endpoint and/or in the ID Token and/or Access Token.
         It is a string of a JSON object which contains lists of claims being requested from these locations.

    :return: The authorization url as a string.
    """
    authority = kwargs.pop("authority", None)  # Historically we support this
    if authority:
        warnings.warn(
            "We haven't decided if this method will accept authority parameter")
    # The previous implementation is, it will use self.authority by default.
    # Multi-tenant app can use new authority on demand
    the_authority = Authority(
        authority,
        self.http_client,
        instance_discovery=self._instance_discovery,
        ) if authority else self.authority

    client = _ClientWithCcsRoutingInfo(
        {"authorization_endpoint": the_authority.authorization_endpoint},
        self.client_id,
        http_client=self.http_client)
    warnings.warn(
        "Change your get_authorization_request_url() "
        "to initiate_auth_code_flow()", DeprecationWarning)
    with warnings.catch_warnings(record=True):
        return client.build_auth_request_uri(
            response_type=response_type,
            redirect_uri=redirect_uri, state=state, login_hint=login_hint,
            prompt=prompt,
            scope=self._decorate_scope(scopes),
            nonce=nonce,
            domain_hint=domain_hint,
            claims=_merge_claims_challenge_and_capabilities(
                self._client_capabilities, claims_challenge),
            )

the _decorate_scope is changing the type to a frozen set

To Reproduce Steps to reproduce the behavior:

  1. Generate a app that uses client.get_authorization_request_url(scopes, redirect_uri=redirect_uri)
  2. Create a config.json file with the following content (replace placeholders with your actual values):

{ "authority": "https://login.microsoftonline.com/", "client_id": "", "client_secret": "", "scope": ["openid", "offline_access"] }

  1. Run the sample: python sample.py config.json

  2. Observe the ValueError being raised.

Expected behavior The sample should successfully acquire an authorization code and proceed with the authentication flow.

What you see instead 127.0.0.1 - - [14/Jun/2024 06:24:24] "GET /login HTTP/1.1" 500 - Traceback (most recent call last): File "/home/gitderekt/.cache/pypoetry/virtualenvs/woodstock-pfnYhH4M-py3.12/lib/python3.12/site-packages/flask/app.py", line 1488, in call return self.wsgi_app(environ, start_response) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/gitderekt/.cache/pypoetry/virtualenvs/woodstock-pfnYhH4M-py3.12/lib/python3.12/site-packages/opentelemetry/instrumentation/flask/init.py", line 359, in _wrapped_app result = wsgi_app(wrapped_app_environ, _start_response) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/gitderekt/.cache/pypoetry/virtualenvs/woodstock-pfnYhH4M-py3.12/lib/python3.12/site-packages/flask/app.py", line 1466, in wsgi_app response = self.handle_exception(e) ^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/gitderekt/.cache/pypoetry/virtualenvs/woodstock-pfnYhH4M-py3.12/lib/python3.12/site-packages/flask/app.py", line 1463, in wsgi_app response = self.full_dispatch_request() ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/gitderekt/.cache/pypoetry/virtualenvs/woodstock-pfnYhH4M-py3.12/lib/python3.12/site-packages/flask/app.py", line 872, in full_dispatch_request rv = self.handle_user_exception(e) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/gitderekt/.cache/pypoetry/virtualenvs/woodstock-pfnYhH4M-py3.12/lib/python3.12/site-packages/flask/app.py", line 870, in full_dispatch_request rv = self.dispatch_request() ^^^^^^^^^^^^^^^^^^^^^^^ File "/home/gitderekt/.cache/pypoetry/virtualenvs/woodstock-pfnYhH4M-py3.12/lib/python3.12/site-packages/flask/app.py", line 855, in dispatch_request return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args) # type: ignore[no-any-return] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/gitderekt/sss/Oma/app/routes/auth/auth.py", line 95, in login auth_url = get_auth_url(client, url_for("auth.auth_callback", _external=True)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/gitderekt/sss/Oma/app/routes/auth/auth.py", line 45, in get_auth_url return client.get_authorization_request_url(scopes, redirect_uri=redirect_uri) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/gitderekt/.cache/pypoetry/virtualenvs/woodstock-pfnYhH4M-py3.12/lib/python3.12/site-packages/msal/application.py", line 946, in get_authorization_request_url scope=self._decorate_scope(scopes), ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/gitderekt/.cache/pypoetry/virtualenvs/woodstock-pfnYhH4M-py3.12/lib/python3.12/site-packages/msal/application.py", line 626, in _decorate_scope raise ValueError( ValueError: API does not accept frozenset({'offline_access', 'openid', 'profile'}) value as user-provided scopes

The MSAL Python version you are using 1.28.1

Additional context Add any other context about the problem here.

rayluo commented 3 months ago

Thanks for bringing this to our attention. A few things here.

  1. Yes, the issue can be reproduced by simply running this one-liner in command line:

    python -c "from msal import PublicClientApplication as PCA; PCA('client_id').get_authorization_request_url(['openid', 'offline_access'])"

    and it can be reproduced in all different MSAL Python APIs and versions, because it is an intentional behavior.

  2. We could, and will, update our current confusing error message from ValueError: API does not accept frozenset({'offline_access', 'openid', 'profile'}) value as user-provided scopes to

    ValueError: You cannot use any scope value that is reserved. Your input: ['openid', 'offline_access'] The reserved list: ['openid', 'offline_access', 'profile']

    so, that is the root cause that you will need to adjust from your side.

  3. Besides, the function that you called, get_authorization_request_url(), has long be deprecated. There is a deprecation warning, guess you did not notice it, possibly because your app did not show warnings by default.

  4. Since you are using Flask, there is even a better recommendation for you. Try our Flask web app sample which was built upon (much) higher level API, and then you won't even run into the "what MSAL API to use", "what scope to use for merely sign-in" issue in the first place.

rayluo commented 3 months ago

Adjusted error message in https://github.com/AzureAD/microsoft-authentication-library-for-python/commit/da156fe360be6e8d2c855c401ff3b581d4ec6550