diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7ffd94e0..360454d9 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -58,7 +58,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 with: - python-version: "3.9" + python-version: "3.11" - name: update pip run: | python -m pip install -U pip @@ -73,7 +73,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 with: - python-version: "3.9" + python-version: "3.11" - name: update pip run: | python -m pip install -U pip @@ -87,7 +87,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 with: - python-version: "3.9" + python-version: "3.11" - name: update pip run: | python -m pip install -U pip diff --git a/CHANGES.rst b/CHANGES.rst index f50ff53b..68a56115 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -36,9 +36,10 @@ Fixes redirect location. - (:pr:`908`) Improve CSRF documentation and testing. Fix bug where a CSRF failure could return an HTML page even if the request was JSON. -- (:pr:`xxx`) Chore - stop setting all config as attributes. init_app(\*\*kwargs) can only +- (:pr:`911`) Chore - stop setting all config as attributes. init_app(\*\*kwargs) can only set forms, flags, and utility classes. -- (:pr:`xxx`) Remove deprecation of AUTO_LOGIN_AFTER_CONFIRM - it has a reasonable use case. +- (:pr:`911`) Remove deprecation of AUTO_LOGIN_AFTER_CONFIRM - it has a reasonable use case. +- (:issue:`912`) Improve oauth debugging support. Handle next propagation in a more general way. Notes ++++++ @@ -57,7 +58,8 @@ Backwards Compatibility Concerns - Passing in an AnonymousUser class as part of Security initialization has been removed. - The never-public method _get_unauthorized_response has been removed. - Social-Oauth - a new configuration variable :py:data:`SECURITY_POST_OAUTH_LOGIN_VIEW` was introduced - and it replaces :py:data:`SECURITY_POST_LOGIN_VIEW` in the oauthresponse logic. + and it replaces :py:data:`SECURITY_POST_LOGIN_VIEW` in the oauthresponse logic when + :py:data:`SECURITY_REDIRECT_BEHAVIOR` == `"spa"`. - Two-Factor setup. Prior to this release when setting up "SMS" the `/tf-setup` endpoint could be POSTed to w/o a phone number, and then another POST could be made to set the phone number. This has always been confusing and added complexity to the code. Now, if "SMS" is selected, the phone number diff --git a/docs/api.rst b/docs/api.rst index bf19c007..1c38f22e 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -164,7 +164,17 @@ Utils .. autoclass:: flask_security.SmsSenderFactory :members: createSender +.. py:class:: OauthCbType[oauth: OAuth, token: t.Any] + + This callback is called when the oauth + redirect happens. It must take the response from the provider and return + a tuple of - which will be used + to look up the user in the datastore. + .. autoclass:: flask_security.OAuthGlue + :members: register_provider, register_provider_ext + +.. autoclass:: flask_security.FsOAuthProvider :members: diff --git a/docs/conf.py b/docs/conf.py index 3fc61898..441c4d40 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -101,6 +101,7 @@ ("py:class", "AuthenticatorSelectionCriteria"), ("py:class", "UserVerificationRequirement"), ("py:class", "OAuth"), + ("py:class", "OAuthError"), ("py:class", "authlib.integrations.flask_client.OAuth"), ("py:class", "t.Type"), ("py:class", "t.Callable"), @@ -109,6 +110,9 @@ ] autodoc_typehints = "description" # autodoc_mock_imports = ["flask_sqlalchemy"] +autodoc_type_aliases = { + "CbType": "oauth_provider.CbType", +} intersphinx_mapping = { "python": ("https://docs.python.org/3/", None), @@ -121,6 +125,7 @@ "flask_sqlalchemy": ("https://flask-sqlalchemy.palletsprojects.com/", None), "flask_login": ("https://flask-login.readthedocs.io/en/latest/", None), "passlib": ("https://passlib.readthedocs.io/en/stable", None), + "authlib": ("https://docs.authlib.org/en/latest/", None), } # -- Options for HTML output --------------------------------------------- diff --git a/docs/features.rst b/docs/features.rst index 92c174d8..ec89c38c 100644 --- a/docs/features.rst +++ b/docs/features.rst @@ -281,7 +281,7 @@ views and features (such as two-factor authentication). Flask-Security is shippe with support for github and google - others can be added by the application (see `loginpass`_ for many examples). -See :py:class:`flask_security.OAuthGlue` +See :py:class:`flask_security.OAuthGlue` and :py:class:`flask_security.FsOAuthProvider` Please note - this is for authentication only, and the authenticating user must already be a registered user in your application. Once authenticated, all further @@ -292,6 +292,10 @@ for details. Note in particular, that you must setup and provide provider specif information - and most importantly - XX_CLIENT_ID and XX_CLIENT_SECRET should be specified as environment variables. +We have seen issues with some providers when `SESSION_COOKIE_SAMESITE` = "strict". +The handshake (sometimes just the first time when the user is being asked to accept your application) +fails due to the session cookie not getting sent as part of the redirect. + A very simple example of configuring social auth with Flask-Security is available in the `examples` directory. diff --git a/examples/oauth/server/app.py b/examples/oauth/server/app.py index fcf322e6..6304dd05 100644 --- a/examples/oauth/server/app.py +++ b/examples/oauth/server/app.py @@ -1,5 +1,5 @@ """ -Copyright 2020-2023 by J. Christopher Wagner (jwag). All rights reserved. +Copyright 2020-2024 by J. Christopher Wagner (jwag). All rights reserved. :license: MIT, see LICENSE for more details. A simple example of utilizing Flask-Security's oauth glue layer. @@ -15,7 +15,7 @@ This example is designed for a browser based client. This example uses github as the oauth provider. Before this example will work: -1) on github register a new oauth application and grap the CLIENT_ID and CLIENT_SECRET. +1) on github register a new oauth application and grab the CLIENT_ID and CLIENT_SECRET. These must be passed in as env variables: "GITHUB_CLIENT_ID" and "GITHUB_CLIENT_SECRET". See: https://docs.authlib.org/en/latest/client/flask.html# for details. @@ -30,10 +30,11 @@ import os -from flask import Flask, flash, render_template_string +from flask import Flask, flash, render_template_string, redirect from flask_security import ( Security, auth_required, + FsOAuthProvider, MailUtil, ) from flask_wtf import CSRFProtect @@ -41,6 +42,22 @@ from models import db, user_datastore +# This is my test tenant on Azure +class AzureProvider(FsOAuthProvider): + def authlib_config(self): + return { + "api_base_url": "https://graph.microsoft.com/", + "server_metadata_url": "https://login.microsoftonline.com/" + "8ad9bf45-2e93-4043-8f46-a6420c8e9d68/v2.0/" + ".well-known/openid-configuration", + "client_kwargs": {"scope": "openid email profile"}, + } + + def fetch_identity_cb(self, oauth, token): + profile = token["userinfo"] + return "email", profile["email"] + + def _find_bool(v): if str(v).lower() in ["true"]: return True @@ -73,6 +90,7 @@ def create_app(): app.config["SECURITY_TOTP_SECRETS"] = { "1": "TjQ9Qa31VOrfEzuPy4VHQWPCTmRzCnFzMKLxXYiZu9B" } + # app.config["SESSION_COOKIE_SAMESITE"] = "strict" # As of Flask-SQLAlchemy 2.4.0 it is easy to pass in options directly to the # underlying engine. This option makes sure that DB connections from the pool @@ -109,13 +127,22 @@ def create_app(): # Setup Flask-Security db.init_app(app) - Security(app, user_datastore, mail_util_cls=FlashMailUtil) + security = Security(app, user_datastore, mail_util_cls=FlashMailUtil) + # If configured - setup Azure - which works slightly differently - + # in particular w.r.t. the redirect URL having to match the ENTIRE + # url including query params (such as next). + if app.config.get("AZURE_CLIENT_SECRET"): + security.oauthglue.register_provider_ext(AzureProvider("azure")) @app.route("/home") @auth_required() def home(): return render_template_string("Hello {{ current_user.email }}") + @app.route("/") + def root(): + return redirect("/home") + return app diff --git a/flask_security/__init__.py b/flask_security/__init__.py index 02eda528..94330907 100644 --- a/flask_security/__init__.py +++ b/flask_security/__init__.py @@ -60,6 +60,7 @@ ) from .mail_util import MailUtil from .oauth_glue import OAuthGlue +from .oauth_provider import FsOAuthProvider from .password_util import PasswordUtil from .phone_util import PhoneUtil from .recovery_codes import ( diff --git a/flask_security/core.py b/flask_security/core.py index 2613c1e5..76ea80c6 100644 --- a/flask_security/core.py +++ b/flask_security/core.py @@ -418,7 +418,8 @@ ), "OAUTH_HANDSHAKE_ERROR": ( _( - "An error occurred while communicating with the Oauth provider. " + "An error occurred while communicating with the Oauth provider:" + " (%(exerror)s - %(exdesc)s). " "Please try again." ), "error", @@ -1076,7 +1077,8 @@ class Security: and signin. Defaults to :class:`WebauthnUtil` :param mf_recovery_codes_util_cls: Class for generating, checking, encrypting and decrypting recovery codes. Defaults to :class:`MfRecoveryCodesUtil` - :param oauth: An instance of authlib.integrations.flask_client.OAuth + :param oauth: An instance of authlib.integrations.flask_client.OAuth. If not set, + Flask-Security will create one. .. tip:: Be sure that all your configuration values have been set PRIOR to diff --git a/flask_security/oauth_glue.py b/flask_security/oauth_glue.py index 823fe33d..4f343965 100644 --- a/flask_security/oauth_glue.py +++ b/flask_security/oauth_glue.py @@ -9,33 +9,38 @@ """ +from __future__ import annotations + import typing as t -from typing import Any try: from authlib.integrations.flask_client import OAuth from authlib.integrations.base_client.errors import ( - MismatchingStateError, OAuthError, ) except ImportError: # pragma: no cover pass -from flask import abort, after_this_request, redirect, request +from flask import abort, after_this_request, redirect, request, session from flask_login import current_user from .decorators import unauth_csrf +from .oauth_provider import ( + OauthCbType, + FsOAuthProvider, + GoogleFsOauthProvider, + GitHubFsOauthProvider, +) from .proxies import _security from .utils import ( config_value as cv, do_flash, login_user, get_message, - get_post_login_redirect, + get_post_action_redirect, get_url, is_user_authenticated, json_error_response, - propagate_next, slash_url_suffix, url_for_security, view_commit, @@ -45,40 +50,9 @@ import flask from flask.typing import ResponseValue -CbType = t.Callable[["OAuth", str], t.Tuple[str, t.Any]] - - -GITHUB = dict( - access_token_url="https://github.com/login/oauth/access_token", - access_token_params=None, - authorize_url="https://github.com/login/oauth/authorize", - authorize_params=None, - api_base_url="https://api.github.com/", - client_kwargs={"scope": "user:email"}, -) - - -def github_fetch_identity(oauth: "OAuth", token: str) -> t.Tuple[str, t.Any]: - resp = oauth.github.get("user", token=token) - profile = resp.json() - return "email", profile["email"] - - -GOOGLE = dict( - server_metadata_url="https://accounts.google.com/.well-known/openid-configuration", - client_kwargs={"scope": "openid email profile"}, -) - - -def google_fetch_identity( - oauth: "OAuth", token: t.Any -) -> t.Tuple[str, t.Any]: # pragma no cover - profile = token["userinfo"] - return "email", profile["email"] - @unauth_csrf() -def oauthstart(name: str) -> "ResponseValue": +def oauthstart(name: str) -> ResponseValue: """View to start an oauth authentication. Name is a pre-registered oauth provider. TODO: remember me? @@ -99,45 +73,47 @@ def oauthstart(name: str) -> "ResponseValue": else: return redirect(get_url(cv("POST_LOGIN_VIEW"))) # we never want to return here or to the redirect location. - values = dict(next=request.args.get("next", "/")) - return _security.oauthglue.get_redirect(name, **values) + # Some providers match on entire redirect url - so we don't want to + # store next there. Use session. + session.pop("fs_oauth_next", None) + if request.args.get("next"): + session["fs_oauth_next"] = request.args.get("next") + return _security.oauthglue.get_redirect(name) -def oauthresponse(name: str) -> "ResponseValue": +def oauthresponse(name: str) -> ResponseValue: """ Callback from oauth provider - response is provider specific Since this is a callback from oauth provider - there is no form, - however the ?next= parameter is returned as we specified in oauthstart + We may have stored the original 'next' in the session N.B. all responses MUST be redirects. """ assert _security.oauthglue - oauth_provider = _security.oauthglue.oauth_provider(name) - if not oauth_provider: - # this shouldn't really be able to happen + authlib_provider = _security.oauthglue.authlib_provider(name) + oauth_provider = _security.oauthglue.providers.get(name) + if not authlib_provider or not oauth_provider: + # this should only happen with purposeful bad API call abort(404) # TODO - redirect... to where? # This parses the Flask Request try: - token = oauth_provider.authorize_access_token() - except (MismatchingStateError, OAuthError): + token = authlib_provider.authorize_access_token() + except OAuthError as e: """One known way this can happen is if the session cookie 'samesite' is set to 'strict' and e.g. the first time the user goes to github and has to authorize this app and then redirect - the session cookie isn't sent - and by default that is where the state is kept. """ - # N.B. flashing doesn't seem to work - probably for same reason as - # the original failure... - m, c = get_message("OAUTH_HANDSHAKE_ERROR") - if cv("REDIRECT_BEHAVIOR") == "spa": - return redirect(get_url(cv("LOGIN_ERROR_VIEW"), qparams={c: m})) - do_flash(m, c) - return redirect(url_for_security("login")) + return oauth_provider.oauth_response_failure(e) - field_name, value = _security.oauthglue._oauth_response(name, token) + field_name, value = oauth_provider.fetch_identity_cb( + _security.oauthglue.oauth_app, token + ) user = _security.datastore.find_user(**{field_name: value}) if user: after_this_request(view_commit) + next_loc = session.pop("fs_oauth_next", None) response = _security.two_factor_plugins.tf_enter( - user, False, "oauth", next_loc=propagate_next(request.url, None) + user, False, "oauth", next_loc=next_loc ) if response: return response @@ -149,7 +125,10 @@ def oauthresponse(name: str) -> "ResponseValue": cv("POST_OAUTH_LOGIN_VIEW"), qparams=user.get_redirect_qparams() ) ) - return redirect(get_post_login_redirect()) + redirect_url = get_post_action_redirect( + "SECURITY_POST_LOGIN_VIEW", dict(next=next_loc) + ) + return redirect(redirect_url) # Seems ok to show identity - the only identity it could be is the callers # so seems no way this can be used to enumerate registered users. m, c = get_message("IDENTITY_NOT_REGISTERED", id=value) @@ -167,26 +146,30 @@ class OAuthGlue: There are some builtin providers which can be used or not - configured via :py:data:`SECURITY_OAUTH_BUILTIN_PROVIDERS`. Any other provider can be registered - using app.security.oauthglue.register_provider(). + using :py:meth:`register_provider_ext`. See `Flask OAuth Client `_ .. versionadded:: 5.1.0 + + .. versionchanged:: 5.4.0 + Added register_provider_ext which allows applications more control to + manage new providers (such as extended error handling). """ - def __init__(self, app: "flask.Flask", oauthapp: t.Optional["OAuth"] = None): + def __init__(self, app: flask.Flask, oauthapp: OAuth | None = None): if not oauthapp: oauthapp = OAuth(app) self.oauth = oauthapp - self.providers: t.Dict[str, t.Dict[str, CbType]] = dict() + self.providers: t.Dict[str, FsOAuthProvider] = dict() if cv("OAUTH_BUILTIN_PROVIDERS", app=app): for provider in cv("OAUTH_BUILTIN_PROVIDERS", app=app): if provider == "github": - self.register_provider("github", GITHUB, github_fetch_identity) + self.register_provider_ext(GitHubFsOauthProvider("github")) elif provider == "google": - self.register_provider("google", GOOGLE, google_fetch_identity) + self.register_provider_ext(GoogleFsOauthProvider("google")) - def _create_blueprint(self, app: "flask.Flask", bp: "flask.Blueprint") -> None: + def _create_blueprint(self, app: flask.Flask, bp: flask.Blueprint) -> None: # Routes for each type of oauth provider start_url = cv("OAUTH_START_URL", app=app) response_url = cv("OAUTH_RESPONSE_URL", app=app) @@ -201,14 +184,14 @@ def _create_blueprint(self, app: "flask.Flask", bp: "flask.Blueprint") -> None: endpoint="oauthresponse", )(oauthresponse) - def get_redirect(self, name: str, **values: t.Any) -> "ResponseValue": - oauth_provider = self.oauth_provider(name) - if not oauth_provider: + def get_redirect(self, name: str, **values: t.Any) -> ResponseValue: + authlib_provider = self.authlib_provider(name) + if not authlib_provider: return abort(404) start_uri = url_for_security( "oauthresponse", name=name, _external=True, **values ) - redirect_url = oauth_provider.authorize_redirect(start_uri) + redirect_url = authlib_provider.authorize_redirect(start_uri) return redirect_url @property @@ -219,14 +202,14 @@ def provider_names(self): def oauth_app(self): return self.oauth - def oauth_provider(self, name): + def authlib_provider(self, name): return getattr(self.oauth, name, None) def register_provider( self, name: str, - registration_info: t.Optional[t.Dict[str, t.Any]], - fetch_identity_cb: CbType, + registration_info: dict[str, t.Any] | None, + fetch_identity_cb: OauthCbType, ) -> None: """Add a provider to the list. @@ -243,10 +226,26 @@ def register_provider( application. If you register directly with OAuth make sure to use the same `name`. + .. deprecated:: 5.4.0 + Use :py:meth:`register_provider_ext` instead. + """ - self.providers[name] = dict(cb=fetch_identity_cb) - if registration_info: - self.oauth.register(name, **registration_info) + pcls = FsOAuthProvider( + name, + registration_info=registration_info, + fetch_identity_cb=fetch_identity_cb, + ) + self.register_provider_ext(pcls) - def _oauth_response(self, name: str, token: str) -> t.Tuple[str, Any]: - return self.providers[name]["cb"](self.oauth, token) + def register_provider_ext(self, provider: FsOAuthProvider) -> None: + """Register a provider via an instance of subclass. + This is the new way - to provide more control for applications + + The authlib provider can be registered here (by calling Oauth) + or already be done by the application. + If you register directly with OAuth make sure to use + the same `name` when instantiating the class. + """ + self.providers[provider.name] = provider + if not self.authlib_provider(provider.name): + self.oauth.register(provider.name, **provider.authlib_config()) diff --git a/flask_security/oauth_provider.py b/flask_security/oauth_provider.py new file mode 100644 index 00000000..be5e8179 --- /dev/null +++ b/flask_security/oauth_provider.py @@ -0,0 +1,130 @@ +""" + flask_security.oauth_provider + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + Class and methods to create providers for oauth_glue. + Example providers for github and google. + + :copyright: (c) 2024-2024 by J. Christopher Wagner (jwag). + :license: MIT, see LICENSE for more details. + +""" + +from __future__ import annotations +import collections.abc as cabc +import sys + +try: + from authlib.integrations.flask_client import OAuth + from authlib.integrations.base_client.errors import ( + OAuthError, + ) +except ImportError: # pragma: no cover + pass + +import typing as t + +from flask import redirect + +from .utils import ( + config_value as cv, + do_flash, + get_message, + get_url, + url_for_security, +) + +if t.TYPE_CHECKING: # pragma: no cover + from flask.typing import ResponseValue + +if sys.version_info >= (3, 9): + OauthCbType = cabc.Callable[["OAuth", t.Any], tuple[str, t.Any]] +else: + OauthCbType = t.Callable[["OAuth", t.Any], t.Tuple[str, t.Any]] + + +class FsOAuthProvider: + """ + Subclass this or instantiate to add new oauth providers. + + Subclassing allows for customizing additional aspects of the oauth flow + in particular - a custom error path for oauth flow state mismatches and + other errors thrown by authlib. + + Call security.oauthglue.register_provider_ext(myproviderclass("myprovider")) + + :param name: a name for provider - must match what was passed if this + is already registered with Oauth. + :param registration_info: This dict is passed directly to Oauth as + part of registration - not needed if provider already registered with + Oauth + :param fetch_identity_cb: Call back from response to oauth flow. + """ + + def __init__( + self, + name: str, + registration_info: dict[str, t.Any] | None = None, + fetch_identity_cb: OauthCbType | None = None, + ): + self.name = name + self._registration_info = registration_info or {} + self._fetch_identity_cb = fetch_identity_cb + + def authlib_config(self) -> dict[str, t.Any]: + """Return dict with authlib configuration. + This is called as part of provider registration.""" + return self._registration_info + + def fetch_identity_cb(self, oauth: OAuth, token: t.Any) -> t.Tuple[str, t.Any]: + """This callback is called when the oauth + redirect happens. It must take the response from the provider and return + a tuple of - which will be used + to look up the user in the datastore.""" + if not self._fetch_identity_cb: # pragma no cover + raise NotImplementedError + return self._fetch_identity_cb(oauth, token) + + def oauth_response_failure(self, e: OAuthError) -> ResponseValue: + """Called if authlib authorize_access_token throws an error. + + N.B. flashing doesn't seem to work in some cases - if the session + cookie has samesite='strict' and it is the first registration. + """ + m, c = get_message( + "OAUTH_HANDSHAKE_ERROR", exerror=e.error, exdesc=e.description + ) + if cv("REDIRECT_BEHAVIOR") == "spa": + return redirect(get_url(cv("LOGIN_ERROR_VIEW"), qparams={c: m})) + do_flash(m, c) + return redirect(url_for_security("login")) + + +class GitHubFsOauthProvider(FsOAuthProvider): + def authlib_config(self): + return dict( + access_token_url="https://github.com/login/oauth/access_token", + access_token_params=None, + authorize_url="https://github.com/login/oauth/authorize", + authorize_params=None, + api_base_url="https://api.github.com/", + client_kwargs={"scope": "user:email"}, + ) + + def fetch_identity_cb(self, oauth, token): + resp = oauth.github.get("user", token=token) + profile = resp.json() + return "email", profile["email"] + + +class GoogleFsOauthProvider(FsOAuthProvider): + def authlib_config(self): + return dict( + server_metadata_url="https://accounts.google.com/" + ".well-known/openid-configuration", + client_kwargs={"scope": "openid email profile"}, + ) + + def fetch_identity_cb(self, oauth, token): # pragma no cover + profile = token["userinfo"] + return "email", profile["email"] diff --git a/flask_security/utils.py b/flask_security/utils.py index cb83933e..03d079bc 100644 --- a/flask_security/utils.py +++ b/flask_security/utils.py @@ -590,33 +590,35 @@ def validate_redirect_url(url: str) -> bool: return True -def get_post_action_redirect(config_key: str) -> str: +def get_post_action_redirect( + config_key: str, next_loc: FlaskForm | MultiDict | dict | None +) -> str: """ There is a security angle here - the result of this method is sent to Flask::redirect() - and we need to be sure that it can't be interpreted as a user-input external URL - that would mean we would have an 'open-redirect' vulnerability. """ - rurl = propagate_next(find_redirect(config_key), request.form) + rurl = propagate_next(find_redirect(config_key), next_loc) (scheme, netloc, path, query, fragment) = urlsplit(rurl) safe_url = urlunsplit((scheme, netloc, quote(path), query, fragment)) return safe_url def get_post_login_redirect() -> str: - return get_post_action_redirect("SECURITY_POST_LOGIN_VIEW") + return get_post_action_redirect("SECURITY_POST_LOGIN_VIEW", request.form) def get_post_register_redirect() -> str: - return get_post_action_redirect("SECURITY_POST_REGISTER_VIEW") + return get_post_action_redirect("SECURITY_POST_REGISTER_VIEW", request.form) def get_post_logout_redirect() -> str: - return get_post_action_redirect("SECURITY_POST_LOGOUT_VIEW") + return get_post_action_redirect("SECURITY_POST_LOGOUT_VIEW", request.form) def get_post_verify_redirect() -> str: - return get_post_action_redirect("SECURITY_POST_VERIFY_VIEW") + return get_post_action_redirect("SECURITY_POST_VERIFY_VIEW", request.form) def find_redirect(key: str) -> str: @@ -632,15 +634,14 @@ def find_redirect(key: str) -> str: return rv -def propagate_next( - fallback_url: str, form: t.Optional[t.Union[FlaskForm, MultiDict]] -) -> str: +def propagate_next(fallback_url: str, form: FlaskForm | MultiDict | dict | None) -> str: """Compute appropriate redirect URL The application can add a 'next' query parameter or have 'next' as a form field. If either exist, make sure they are valid (not pointing to external location) If neither, return the fallback_url - Can be passed either request.form (which is really a MultiDict OR a real form. + Can be passed either request.form + (which is really a MultiDict OR a real form OR a dict with a 'next' key). """ form_next = None if form and isinstance(form, FlaskForm): diff --git a/tests/test_oauthglue.py b/tests/test_oauthglue.py index c151e4bc..30cde9c5 100644 --- a/tests/test_oauthglue.py +++ b/tests/test_oauthglue.py @@ -4,7 +4,7 @@ Oauth glue tests - oauthglue is a very thin shim between FS and authlib - :copyright: (c) 2022-2023 by J. Christopher Wagner (jwag). + :copyright: (c) 2022-2024 by J. Christopher Wagner (jwag). :license: MIT, see LICENSE for more details. """ @@ -15,12 +15,14 @@ from flask import redirect from flask_wtf import CSRFProtect +from flask_security import FsOAuthProvider from tests.test_utils import ( authenticate, check_location, get_csrf_token, get_form_action, get_form_input, + get_session, init_app_with_options, is_authenticated, logout, @@ -168,7 +170,14 @@ def test_bad_api(app, sqlalchemy_datastore, get_message): oauth_app.github.set_exception(MismatchingStateError) response = client.get("/login/oauthresponse/github", follow_redirects=True) assert response.status_code == 200 - assert get_message("OAUTH_HANDSHAKE_ERROR") in response.data + assert ( + get_message( + "OAUTH_HANDSHAKE_ERROR", + exerror="mismatching_state", + exdesc="CSRF Warning! State not equal in request and response.", + ) + in response.data + ) @pytest.mark.settings(oauth_enable=True) @@ -267,7 +276,12 @@ def test_spa(app, sqlalchemy_datastore, get_message): split = urlsplit(response.location) assert "/login-error" == split.path qparams = dict(parse_qsl(split.query)) - assert qparams["error"] == get_message("OAUTH_HANDSHAKE_ERROR").decode() + msg = get_message( + "OAUTH_HANDSHAKE_ERROR", + exerror="mismatching_state", + exdesc="CSRF Warning! State not equal in request and response.", + ) + assert qparams["error"] == msg.decode() @pytest.mark.settings(oauth_enable=True, post_login_view="/post-login") @@ -288,3 +302,68 @@ def test_already_auth(app, sqlalchemy_datastore, get_message): response = client.post("/login/oauthstart/github", follow_redirects=False) assert response.status_code == 302 check_location(app, response.location, "/post-login") + + +@pytest.mark.settings(oauth_enable=True, post_login_view="/post-login") +def test_simple_next(app, sqlalchemy_datastore, get_message): + # For oauth we stash 'next' in the session since we can't really + # send it all around the oauth providers. + init_app_with_options( + app, sqlalchemy_datastore, **{"security_args": {"oauth": MockOAuth()}} + ) + client = app.test_client() + response = client.get("/profile", follow_redirects=True) + github_url = get_form_action(response, 1) + + response = client.post(github_url, follow_redirects=False) + assert "/whatever" in response.location + session = get_session(response) + assert "fs_oauth_next" in session + + response = client.get("/login/oauthresponse/github", follow_redirects=False) + assert response.status_code == 302 + assert check_location(app, response.location, "/profile") + session = get_session(response) + assert "fs_oauth_next" not in session + + +@pytest.mark.settings(oauth_enable=True, post_login_view="/post_login") +def test_provider_class(app, sqlalchemy_datastore, get_message): + from authlib.integrations.base_client.errors import MismatchingStateError + + class MyOauthProvider(FsOAuthProvider): + def fetch_identity_cb(self, oauth, token): + resp = oauth.myoauth.get("user", token=token) + profile = resp.json() + return "email", profile["email"] + + def oauth_response_failure(self, e): + return redirect("/uh-oh") + + init_app_with_options( + app, sqlalchemy_datastore, **{"security_args": {"oauth": MockOAuth()}} + ) + # Have to register with Oauthglue. + app.security.oauthglue.register_provider_ext(MyOauthProvider("myoauth")) + + client = app.test_client() + response = client.get("/login") + myoauth_url = get_form_action(response, 2) + + response = client.post(myoauth_url, follow_redirects=False) + assert "/whatever" in response.location + + # test error - and that our handler is called + oauth_app = app.security.oauthglue.oauth_app + oauth_app.myoauth.set_exception(MismatchingStateError) + response = client.get("/login/oauthresponse/myoauth", follow_redirects=False) + assert response.status_code == 302 + assert check_location(app, response.location, "/uh-oh") + + # now log in successfully + oauth_app.myoauth.set_exception(None) + + response = client.get("/login/oauthresponse/myoauth", follow_redirects=False) + assert response.status_code == 302 + assert check_location(app, response.location, "/post_login") + assert is_authenticated(client, get_message) diff --git a/tox.ini b/tox.ini index e042e81f..6513c296 100644 --- a/tox.ini +++ b/tox.ini @@ -52,7 +52,7 @@ commands = pytest --basetemp={envtmpdir} {posargs:tests} [testenv:async] -basepython = python3.10 +basepython = python3.11 deps = -r requirements/tests.txt commands = @@ -87,6 +87,7 @@ commands = pytest --basetemp={envtmpdir} {posargs:tests} [testenv:style] +basepython = python3.11 deps = pre-commit skip_install = true commands = @@ -94,11 +95,13 @@ commands = pre-commit run --all-files --show-diff-on-failure [testenv:docs] +basepython = python3.11 deps = -r requirements/docs.txt commands = sphinx-build -W -b html -d {envtmpdir}/doctrees docs {envtmpdir}/html [testenv:coverage] +basepython = python3.11 deps = -r requirements/tests.txt commands = @@ -116,6 +119,7 @@ commands = pytest --basetemp={envtmpdir} {posargs} [testenv:makedist] +basepython = python3.11 deps = -r requirements/tests.txt build @@ -128,6 +132,7 @@ commands = check-wheel-contents dist [testenv:mypy] +basepython = python3.11 deps = -r requirements/tests.txt types-setuptools