Merge tag '0.9.2' into microsoft-main

Author: rayluo

identity/django.py

@@ -1,7 +1,7 @@
 from functools import partial, wraps
 import logging
 import os
-from typing import List  # Needed in Python 3.7 & 3.8
+from typing import List, Optional  # Needed in Python 3.7 & 3.8
 from urllib.parse import urlparse
 
 from django.shortcuts import redirect, render
@@ -50,9 +50,9 @@ def __init__(self, *args, **kwargs):
     def login(
         self,
         request,
-        next_link:str = None,  # Obtain the next_link from the app developer,
+        next_link: Optional[str] = None,  # Obtain the next_link from the app developer,
             # NOT from query string which could become an open redirect vulnerability.
-        scopes: List[str]=None,
+        scopes: Optional[List[str]] = None,
     ):
         # The login view.
         # App developer could redirect to the login page from inside a view,
@@ -117,7 +117,7 @@ def login_required(  # Named after Django's login_required
         function=None,
         /,  # Requires Python 3.8+
         *,
-        scopes: List[str]=None,
+        scopes: Optional[List[str]] = None,
     ):
         """A decorator that ensures the user to be logged in,
         and optinoally also have consented to a list of scopes.

identity/flask.py

@@ -62,20 +62,28 @@ def build_app():
         self._session = session  # Not available during class definition
         super(Auth, self).__init__(app, *args, **kwargs)
 
-    def _render_auth_error(self, *, error, error_description=None):
+    def _render_auth_error(  # type: ignore[override]
+        self, *, error, error_description=None,
+    ) -> str:
         return render_template(
             f"{self._endpoint_prefix}/auth_error.html",
             error=error,
             error_description=error_description,
             reset_password_url=self._get_reset_password_url(),
             )
 
-    def login(self, *, next_link: str=None, scopes: List[str]=None):
+    def login(
+        self,
+        *,
+        next_link: Optional[str] = None,
+        scopes: Optional[List[str]] = None,
+    ) -> str:
         config_error = self._get_configuration_error()
         if config_error:
             return self._render_auth_error(
                 error="configuration_error", error_description=config_error)
-        log_in_result = self._auth.log_in(
+        assert self._auth, "_auth should have been initialized"  # And mypy needs this
+        log_in_result: dict = self._auth.log_in(
             scopes=scopes,  # Have user consent to scopes (if any) during log-in
             redirect_uri=self._redirect_uri,
             prompt="select_account",  # Optional. More values defined in  https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
@@ -106,7 +114,7 @@ def login_required(  # Named after Django's login_required
         function=None,
         /,  # Requires Python 3.8+
         *,
-        scopes: List[str]=None,
+        scopes: Optional[List[str]] = None,
     ):
         """A decorator that ensures the user to be logged in,
         and optinoally also have consented to a list of scopes.

identity/pallet.py

@@ -3,17 +3,17 @@
 from inspect import iscoroutinefunction
 import logging
 import os
-from typing import List  # Needed in Python 3.7 & 3.8
+from typing import List, Optional  # Needed in Python 3.7 & 3.8
 from urllib.parse import urlparse
-from .web import WebFrameworkAuth
+from .web import WebFrameworkAuth, Auth
 
 
 logger = logging.getLogger(__name__)
 
 
 class PalletAuth(WebFrameworkAuth):  # A common base class for Flask and Quart
     _endpoint_prefix = "identity"  # A convention to match the template's folder name
-    _auth = None  # None means not initialized yet
+    _auth: Optional[Auth] = None  # None means not initialized yet
 
     def __init__(self, app, *args, **kwargs):
         if not (
@@ -64,14 +64,14 @@ def __getattribute__(self, name):
 
     def logout(self):
         return self.__class__._redirect(  # self._redirect(...) won't work
-            self._auth.log_out(self._request.host_url))
+            self._auth.log_out(self._request.url_root))
 
     def login_required(  # Named after Django's login_required
         self,
         function=None,
         /,  # Requires Python 3.8+
         *,
-        scopes: List[str]=None,
+        scopes: Optional[List[str]] = None,
     ):
         # With or without brackets. Inspired by https://stackoverflow.com/a/39335652/728675
 
@@ -92,7 +92,7 @@ async def wrapper(*args, **kwargs):
                 if context:
                     return await function(*args, context=context, **kwargs)
                 # Save an http 302 by calling self.login(request) instead of redirect(self.login)
-                return await self.login(next_link=self._request.path, scopes=scopes)
+                return await self.login(next_link=self._request.url, scopes=scopes)
         else:  # For Flask
             @wraps(function)
             def wrapper(*args, **kwargs):
@@ -101,9 +101,6 @@ def wrapper(*args, **kwargs):
                 if context:
                     return function(*args, context=context, **kwargs)
                 # Save an http 302 by calling self.login(request) instead of redirect(self.login)
-                return self.login(
-                    next_link=self._request.path,  # https://flask.palletsprojects.com/en/3.0.x/api/#flask.Request.path
-                    scopes=scopes,
-                    )
+                return self.login(next_link=self._request.url, scopes=scopes)
         return wrapper
 

identity/quart.py

@@ -70,11 +70,17 @@ async def _render_auth_error(self, *, error, error_description=None):
             reset_password_url=self._get_reset_password_url(),
             )
 
-    async def login(self, *, next_link: str=None, scopes: List[str]=None):
+    async def login(
+        self,
+        *,
+        next_link: Optional[str] = None,
+        scopes: Optional[List[str]] = None,
+    ):
         config_error = self._get_configuration_error()
         if config_error:
             return await self._render_auth_error(
                 error="configuration_error", error_description=config_error)
+        assert self._auth, "_auth should have been initialized"  # And mypy needs this
         log_in_result = self._auth.log_in(
             scopes=scopes,  # Have user consent to scopes (if any) during log-in
             redirect_uri=self._redirect_uri,
@@ -106,7 +112,7 @@ def login_required(  # Named after Django's login_required
         function=None,
         /,  # Requires Python 3.8+
         *,
-        scopes: List[str]=None,
+        scopes: Optional[List[str]] = None,
     ):
         """A decorator that ensures the user to be logged in,
         and optinoally also have consented to a list of scopes.

identity/version.py

@@ -1 +1 @@
-__version__ = "0.9.1"  # Note: Perhaps update ReadTheDocs and README.md too?
+__version__ = "0.9.2"  # Note: Perhaps update ReadTheDocs and README.md too?

identity/web.py

@@ -2,7 +2,7 @@
 import functools
 import logging
 import time
-from typing import List  # Needed in Python 3.7 & 3.8
+from typing import List, Optional  # Needed in Python 3.7 & 3.8
 
 import requests
 import msal
@@ -97,9 +97,14 @@ def _save_user_into_session(self, id_token_claims):
         self._session[self._USER] = id_token_claims
 
     def log_in(
-        self, scopes=None, redirect_uri=None, state=None, prompt=None,
-        next_link=None,
-    ):
+        self,
+        *,
+        scopes: Optional[List[str]] = None,
+        redirect_uri: Optional[str] = None,
+        state: Optional[str] = None,
+        prompt: Optional[str] = None,
+        next_link: Optional[str] = None,
+    ) -> dict:
         """This is the first leg of the authentication/authorization.
 
         :param list scopes:
@@ -162,7 +167,7 @@ def log_in(
                 "user_code": flow["user_code"],
                 }
 
-    def complete_log_in(self, auth_response=None):
+    def complete_log_in(self, auth_response: Optional[dict] = None) -> dict:
         """This is the second leg of the authentication/authorization.
 
         It is used inside your redirect_uri controller.
@@ -356,15 +361,15 @@ def __init__(
         client_id: str,
         *,
         client_credential=None,
-        oidc_authority: str=None,
-        authority: str=None,
-        redirect_uri: str=None,
+        oidc_authority: Optional[str] = None,
+        authority: Optional[str] = None,
+        redirect_uri: Optional[str] = None,
         # We end up accepting Microsoft Entra ID B2C parameters rather than generic urls
         # because it is troublesome to build those urls in settings.py or templates
-        b2c_tenant_name: str=None,
-        b2c_signup_signin_user_flow: str=None,
-        b2c_edit_profile_user_flow: str=None,
-        b2c_reset_password_user_flow: str=None,
+        b2c_tenant_name: Optional[str] = None,
+        b2c_signup_signin_user_flow: Optional[str] = None,
+        b2c_edit_profile_user_flow: Optional[str] = None,
+        b2c_reset_password_user_flow: Optional[str] = None,
     ):
         """Create an identity helper for a web application.
 
@@ -419,8 +424,9 @@ def __init__(
         self._client_id = client_id
         self._client_credential = client_credential
         self._redirect_uri = redirect_uri
-        self._http_cache = {}  # All subsequent Auth instances will share this
+        self._http_cache: dict = {}  # All subsequent Auth instances will share this
 
+        self._authority: Optional[str] = None  # It makes mypy happy
         # Note: We do not use overload, because we want to allow the caller to
         # have only one code path that relay in all the optional parameters.
         if b2c_tenant_name and b2c_signup_signin_user_flow:
@@ -467,7 +473,7 @@ def _get_configuration_error(self):
 (2.3) the B2C_TENANT_NAME and SIGNUPSIGNIN_USER_FLOW pair?
 """
 
-    def _build_auth(self, session):
+    def _build_auth(self, session) -> Auth:
         return Auth(
             session=session,
             oidc_authority=self._oidc_authority,
@@ -523,7 +529,9 @@ def _get_reset_password_url(self):
             )["auth_uri"] if self._reset_password_auth and self._redirect_uri else None
 
     @abstractmethod
-    def _render_auth_error(error, *, error_description=None):
+    def _render_auth_error(
+        error, *, error_description=None,
+    ):  # Return value could be a str, or a framework-specific Response object
         # The default auth_error.html template may or may not escape.
         # If a web framework does not escape it by default, a subclass shall escape it.
         pass

setup.cfg

@@ -64,7 +64,7 @@ django =
 flask =
     flask
     # If Flask-Session is not maintained in future, Flask-Session2 should work as well
-    Flask-Session>=0.3.2,<0.6
+    Flask-Session >= 0.3.2, !=0.7.0, <0.9
 quart =
     quart
     # Note that Quart-session is a no-op by default.

tests/test_flask.py

@@ -1,34 +1,60 @@
+import shutil
 from unittest.mock import patch, Mock
 
+import pytest
 from flask import Flask
 
 from identity.flask import Auth
 
 
-def test_logout():
+@pytest.fixture()
+def app():  # https://flask.palletsprojects.com/en/3.0.x/testing/
     app = Flask(__name__)
-    app.config["SESSION_TYPE"] = "filesystem"  # Required for Flask-session,
-        # see also https://stackoverflow.com/questions/26080872
-    auth = Auth(app, client_id="fake")
-    with app.test_request_context("/", method="GET"):
-        assert auth._request.host_url in auth.logout().get_data(as_text=True), (
-            "The host_url should be in the logout URL. There was a bug in 0.9.0.")
+    app.config.update({
+        "APPLICATION_ROOT": "/app_root",  # Mimicking app with explicit root
+        "SESSION_TYPE": "filesystem",  # Required for Flask-session,
+            # see also https://stackoverflow.com/questions/26080872
+    })
+    yield app
+    shutil.rmtree("flask_session")  # clean up
+
+@pytest.fixture()
+def auth(app):
+    return Auth(
+        app,
+        client_id="fake",
+        redirect_uri="http://localhost:5000/redirect",  # To use auth code flow
+        oidc_authority="https://example.com/foo",
+    )
+
+def test_logout(app, auth):
+    with patch.object(auth._auth, "_get_oidc_config", new=Mock(return_value={
+        "end_session_endpoint": "https://example.com/end_session",
+    })):
+        with app.test_request_context("/", method="GET"):
+            homepage = "http://localhost/app_root"
+            assert homepage in auth.logout().get_data(as_text=True), (
+                "The homepage should be in the logout URL. There was a bug in 0.9.0.")
 
 @patch("msal.authority.tenant_discovery", new=Mock(return_value={
     "authorization_endpoint": "https://example.com/placeholder",
     "token_endpoint": "https://example.com/placeholder",
     }))
-def test_login_should_locate_its_template():
-    app = Flask(__name__)
-    app.config["SESSION_TYPE"] = "filesystem"  # Required for Flask-session,
-        # see also https://stackoverflow.com/questions/26080872
-    client_id = str(hash(app))
-    auth = Auth(
-        app,
-        client_id=client_id,
-        redirect_uri="http://localhost:5000/redirect",  # To use auth code flow
-        oidc_authority="https://example.com/foo",
-        )
-    with app.test_request_context("/", method="GET"):
-        assert client_id in auth.login()  # Proper template output contains client_id
+def test_login(app, auth):
+
+    @app.route("/path")
+    @auth.login_required
+    def dummy_view():
+        return "content visible after login"
+
+    with app.test_request_context("/path", method="GET"):
+        should_find_template = "login() should have template to render"
+        assert auth._client_id in auth.login(), should_find_template
+        with app.test_client() as client:
+            result = client.get("/path?foo=bar")
+            assert auth._client_id in str(result.data), should_find_template
+            from flask import session  # It is different than auth._auth._session
+            assert session.get("_auth_flow", {}).get("identity.web.next_link") == (
+                "http://localhost/app_root/path?foo=bar"  # The full url
+                ), "Next path should honor APPLICATION_ROOT"
 

tox.ini

@@ -1,4 +1,7 @@
 [tox]
+# So the following name can be referenced in other sections such as testenv:type
+x_project_name = identity
+
 env_list =
     py3
 minversion = 4.21.2
@@ -12,4 +15,13 @@ deps =
     -r requirements.txt
 commands =
     pip list
-    pytest {tty:--color=yes} -o asyncio_default_fixture_loop_scope=function {posargs}
+    pytest {tty:--color=yes} -o asyncio_default_fixture_loop_scope=function -rA {posargs}
+
+[testenv:type]
+deps =
+    -r requirements.txt
+    mypy
+commands =
+    mypy --install-types --non-interactive
+    mypy {[tox]x_project_name}
+