From b6004f3336c029e181729315d09500d84022c1a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20M=C3=BChlbachler-Pietrzykowski?= Date: Thu, 2 May 2024 16:17:10 +0200 Subject: [PATCH] feat: use oidc well-known url (#2077) --- app/auth/views/oidc.py | 32 +++--- app/config.py | 4 +- example.env | 4 +- tests/auth/test_oidc.py | 248 ++++++++++++++++++++++++++++++++-------- tests/test.env | 4 +- 5 files changed, 219 insertions(+), 73 deletions(-) diff --git a/app/auth/views/oidc.py b/app/auth/views/oidc.py index 12c4e491..bbda5435 100644 --- a/app/auth/views/oidc.py +++ b/app/auth/views/oidc.py @@ -1,14 +1,13 @@ from flask import request, session, redirect, flash, url_for from requests_oauthlib import OAuth2Session +import requests + from app import config from app.auth.base import auth_bp from app.auth.views.login_utils import after_login from app.config import ( URL, - OIDC_AUTHORIZATION_URL, - OIDC_USER_INFO_URL, - OIDC_TOKEN_URL, OIDC_SCOPES, OIDC_NAME_FIELD, ) @@ -16,14 +15,15 @@ from app.db import Session from app.email_utils import send_welcome_email from app.log import LOG from app.models import User, SocialAuth -from app.utils import encode_url, sanitize_email, sanitize_next_url +from app.utils import sanitize_email, sanitize_next_url # need to set explicitly redirect_uri instead of leaving the lib to pre-fill redirect_uri # when served behind nginx, the redirect_uri is localhost... and not the real url -_redirect_uri = URL + "/auth/oidc/callback" +redirect_uri = URL + "/auth/oidc/callback" SESSION_STATE_KEY = "oauth_state" +SESSION_NEXT_KEY = "oauth_redirect_next" @auth_bp.route("/oidc/login") @@ -32,18 +32,17 @@ def oidc_login(): return redirect(url_for("auth.login")) next_url = sanitize_next_url(request.args.get("next")) - if next_url: - redirect_uri = _redirect_uri + "?next=" + encode_url(next_url) - else: - redirect_uri = _redirect_uri + + auth_url = requests.get(config.OIDC_WELL_KNOWN_URL).json()["authorization_endpoint"] oidc = OAuth2Session( config.OIDC_CLIENT_ID, scope=[OIDC_SCOPES], redirect_uri=redirect_uri ) - authorization_url, state = oidc.authorization_url(OIDC_AUTHORIZATION_URL) + authorization_url, state = oidc.authorization_url(auth_url) # State is used to prevent CSRF, keep this for later. session[SESSION_STATE_KEY] = state + session[SESSION_NEXT_KEY] = next_url return redirect(authorization_url) @@ -60,19 +59,23 @@ def oidc_callback(): flash("Please use another sign in method then", "warning") return redirect("/") + oidc_configuration = requests.get(config.OIDC_WELL_KNOWN_URL).json() + user_info_url = oidc_configuration["userinfo_endpoint"] + token_url = oidc_configuration["token_endpoint"] + oidc = OAuth2Session( config.OIDC_CLIENT_ID, state=session[SESSION_STATE_KEY], scope=[OIDC_SCOPES], - redirect_uri=_redirect_uri, + redirect_uri=redirect_uri, ) oidc.fetch_token( - OIDC_TOKEN_URL, + token_url, client_secret=config.OIDC_CLIENT_SECRET, authorization_response=request.url, ) - oidc_user_data = oidc.get(OIDC_USER_INFO_URL) + oidc_user_data = oidc.get(user_info_url) if oidc_user_data.status_code != 200: LOG.e( f"cannot get oidc user data {oidc_user_data.status_code} {oidc_user_data.text}" @@ -111,7 +114,8 @@ def oidc_callback(): Session.commit() # The activation link contains the original page, for ex authorize page - next_url = sanitize_next_url(request.args.get("next")) if request.args else None + next_url = session[SESSION_NEXT_KEY] + session[SESSION_NEXT_KEY] = None return after_login(user, next_url) diff --git a/app/config.py b/app/config.py index 7a59e91b..f22cdcde 100644 --- a/app/config.py +++ b/app/config.py @@ -245,9 +245,7 @@ FACEBOOK_CLIENT_ID = os.environ.get("FACEBOOK_CLIENT_ID") FACEBOOK_CLIENT_SECRET = os.environ.get("FACEBOOK_CLIENT_SECRET") CONNECT_WITH_OIDC_ICON = os.environ.get("CONNECT_WITH_OIDC_ICON") -OIDC_AUTHORIZATION_URL = os.environ.get("OIDC_AUTHORIZATION_URL") -OIDC_USER_INFO_URL = os.environ.get("OIDC_USER_INFO_URL") -OIDC_TOKEN_URL = os.environ.get("OIDC_TOKEN_URL") +OIDC_WELL_KNOWN_URL = os.environ.get("OIDC_WELL_KNOWN_URL") OIDC_CLIENT_ID = os.environ.get("OIDC_CLIENT_ID") OIDC_CLIENT_SECRET = os.environ.get("OIDC_CLIENT_SECRET") OIDC_SCOPES = os.environ.get("OIDC_SCOPES") diff --git a/example.env b/example.env index 4ee09519..45716f82 100644 --- a/example.env +++ b/example.env @@ -118,9 +118,7 @@ WORDS_FILE_PATH=local_data/test_words.txt # Login with OIDC # CONNECT_WITH_OIDC_ICON=fa-github -# OIDC_AUTHORIZATION_URL=to_fill -# OIDC_USER_INFO_URL=to_fill -# OIDC_TOKEN_URL=to_fill +# OIDC_WELL_KNOWN_URL=to_fill # OIDC_SCOPES=openid email profile # OIDC_NAME_FIELD=name # OIDC_CLIENT_ID=to_fill diff --git a/tests/auth/test_oidc.py b/tests/auth/test_oidc.py index e35bb5e4..5d22e673 100644 --- a/tests/auth/test_oidc.py +++ b/tests/auth/test_oidc.py @@ -1,5 +1,5 @@ from app import config -from flask import url_for +from flask import url_for, session from urllib.parse import parse_qs from urllib3.util import parse_url from app.auth.views.oidc import create_user @@ -10,7 +10,21 @@ from app.models import User from app.config import URL, OIDC_CLIENT_ID -def test_oidc_login(flask_client): +mock_well_known_response = { + "authorization_endpoint": "http://localhost:7777/authorization-endpoint", + "userinfo_endpoint": "http://localhost:7777/userinfo-endpoint", + "token_endpoint": "http://localhost:7777/token-endpoint", +} + + +@patch("requests.get") +def test_oidc_login(mock_get, flask_client): + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = None + + mock_get.return_value.json.return_value = mock_well_known_response + r = flask_client.get( url_for("auth.oidc_login"), follow_redirects=False, @@ -28,8 +42,41 @@ def test_oidc_login(flask_client): assert expected_redirect_url == query["redirect_uri"][0] -def test_oidc_login_no_client_id(flask_client): +@patch("requests.get") +def test_oidc_login_next_url(mock_get, flask_client): + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = None + + mock_get.return_value.json.return_value = mock_well_known_response + + with flask_client: + r = flask_client.get( + url_for("auth.oidc_login", next="/dashboard/settings/"), + follow_redirects=False, + ) + location = r.headers.get("Location") + assert location is not None + + parsed = parse_url(location) + query = parse_qs(parsed.query) + + expected_redirect_url = f"{URL}/auth/oidc/callback" + + assert "code" == query["response_type"][0] + assert OIDC_CLIENT_ID == query["client_id"][0] + assert expected_redirect_url == query["redirect_uri"][0] + assert session["oauth_redirect_next"] == "/dashboard/settings/" + + +@patch("requests.get") +def test_oidc_login_no_client_id(mock_get, flask_client): config.OIDC_CLIENT_ID = None + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = None + + mock_get.return_value.json.return_value = mock_well_known_response r = flask_client.get( url_for("auth.oidc_login"), @@ -47,8 +94,14 @@ def test_oidc_login_no_client_id(flask_client): config.OIDC_CLIENT_ID = "to_fill" -def test_oidc_login_no_client_secret(flask_client): +@patch("requests.get") +def test_oidc_login_no_client_secret(mock_get, flask_client): config.OIDC_CLIENT_SECRET = None + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = None + + mock_get.return_value.json.return_value = mock_well_known_response r = flask_client.get( url_for("auth.oidc_login"), @@ -66,9 +119,14 @@ def test_oidc_login_no_client_secret(flask_client): config.OIDC_CLIENT_SECRET = "to_fill" -def test_oidc_callback_no_oauth_state(flask_client): - with flask_client.session_transaction() as session: - session["oauth_state"] = None +@patch("requests.get") +def test_oidc_callback_no_oauth_state(mock_get, flask_client): + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = None + sess["oauth_state"] = None + + mock_get.return_value.json.return_value = mock_well_known_response r = flask_client.get( url_for("auth.oidc_callback"), @@ -78,11 +136,16 @@ def test_oidc_callback_no_oauth_state(flask_client): assert location is None -def test_oidc_callback_no_client_id(flask_client): - with flask_client.session_transaction() as session: - session["oauth_state"] = "state" +@patch("requests.get") +def test_oidc_callback_no_client_id(mock_get, flask_client): + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = None + sess["oauth_state"] = "state" config.OIDC_CLIENT_ID = None + mock_get.return_value.json.return_value = mock_well_known_response + r = flask_client.get( url_for("auth.oidc_callback"), follow_redirects=False, @@ -97,15 +160,20 @@ def test_oidc_callback_no_client_id(flask_client): assert expected_redirect_url == parsed.path config.OIDC_CLIENT_ID = "to_fill" - with flask_client.session_transaction() as session: - session["oauth_state"] = None + with flask_client.session_transaction() as sess: + sess["oauth_state"] = None -def test_oidc_callback_no_client_secret(flask_client): - with flask_client.session_transaction() as session: - session["oauth_state"] = "state" +@patch("requests.get") +def test_oidc_callback_no_client_secret(mock_get, flask_client): + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = None + sess["oauth_state"] = "state" config.OIDC_CLIENT_SECRET = None + mock_get.return_value.json.return_value = mock_well_known_response + r = flask_client.get( url_for("auth.oidc_callback"), follow_redirects=False, @@ -120,16 +188,23 @@ def test_oidc_callback_no_client_secret(flask_client): assert expected_redirect_url == parsed.path config.OIDC_CLIENT_SECRET = "to_fill" - with flask_client.session_transaction() as session: - session["oauth_state"] = None + with flask_client.session_transaction() as sess: + sess["oauth_state"] = None +@patch("requests.get") @patch("requests_oauthlib.OAuth2Session.fetch_token") @patch("requests_oauthlib.OAuth2Session.get") -def test_oidc_callback_invalid_user(mock_get, mock_fetch_token, flask_client): - mock_get.return_value = MockResponse(400, {}) - with flask_client.session_transaction() as session: - session["oauth_state"] = "state" +def test_oidc_callback_invalid_user( + mock_oauth_get, mock_fetch_token, mock_get, flask_client +): + mock_oauth_get.return_value = MockResponse(400, {}) + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = None + sess["oauth_state"] = "state" + + mock_get.return_value.json.return_value = mock_well_known_response r = flask_client.get( url_for("auth.oidc_callback"), @@ -143,18 +218,25 @@ def test_oidc_callback_invalid_user(mock_get, mock_fetch_token, flask_client): expected_redirect_url = "/auth/login" assert expected_redirect_url == parsed.path - assert mock_get.called + assert mock_oauth_get.called - with flask_client.session_transaction() as session: - session["oauth_state"] = None + with flask_client.session_transaction() as sess: + sess["oauth_state"] = None +@patch("requests.get") @patch("requests_oauthlib.OAuth2Session.fetch_token") @patch("requests_oauthlib.OAuth2Session.get") -def test_oidc_callback_no_email(mock_get, mock_fetch_token, flask_client): - mock_get.return_value = MockResponse(200, {}) - with flask_client.session_transaction() as session: - session["oauth_state"] = "state" +def test_oidc_callback_no_email( + mock_oauth_get, mock_fetch_token, mock_get, flask_client +): + mock_oauth_get.return_value = MockResponse(200, {}) + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = None + sess["oauth_state"] = "state" + + mock_get.return_value.json.return_value = mock_well_known_response r = flask_client.get( url_for("auth.oidc_callback"), @@ -168,20 +250,27 @@ def test_oidc_callback_no_email(mock_get, mock_fetch_token, flask_client): expected_redirect_url = "/auth/login" assert expected_redirect_url == parsed.path - assert mock_get.called + assert mock_oauth_get.called with flask_client.session_transaction() as session: session["oauth_state"] = None +@patch("requests.get") @patch("requests_oauthlib.OAuth2Session.fetch_token") @patch("requests_oauthlib.OAuth2Session.get") -def test_oidc_callback_disabled_registration(mock_get, mock_fetch_token, flask_client): +def test_oidc_callback_disabled_registration( + mock_oauth_get, mock_fetch_token, mock_get, flask_client +): config.DISABLE_REGISTRATION = True email = random_string() - mock_get.return_value = MockResponse(200, {"email": email}) - with flask_client.session_transaction() as session: - session["oauth_state"] = "state" + mock_oauth_get.return_value = MockResponse(200, {"email": email}) + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = None + sess["oauth_state"] = "state" + + mock_get.return_value.json.return_value = mock_well_known_response r = flask_client.get( url_for("auth.oidc_callback"), @@ -195,26 +284,33 @@ def test_oidc_callback_disabled_registration(mock_get, mock_fetch_token, flask_c expected_redirect_url = "/auth/register" assert expected_redirect_url == parsed.path - assert mock_get.called + assert mock_oauth_get.called config.DISABLE_REGISTRATION = False - with flask_client.session_transaction() as session: - session["oauth_state"] = None + with flask_client.session_transaction() as sess: + sess["oauth_state"] = None +@patch("requests.get") @patch("requests_oauthlib.OAuth2Session.fetch_token") @patch("requests_oauthlib.OAuth2Session.get") -def test_oidc_callback_registration(mock_get, mock_fetch_token, flask_client): +def test_oidc_callback_registration( + mock_oauth_get, mock_fetch_token, mock_get, flask_client +): email = random_string() - mock_get.return_value = MockResponse( + mock_oauth_get.return_value = MockResponse( 200, { "email": email, config.OIDC_NAME_FIELD: "name", }, ) - with flask_client.session_transaction() as session: - session["oauth_state"] = "state" + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = None + sess["oauth_state"] = "state" + + mock_get.return_value.json.return_value = mock_well_known_response user = User.get_by(email=email) assert user is None @@ -231,28 +327,33 @@ def test_oidc_callback_registration(mock_get, mock_fetch_token, flask_client): expected_redirect_url = "/dashboard/" assert expected_redirect_url == parsed.path - assert mock_get.called + assert mock_oauth_get.called user = User.get_by(email=email) assert user is not None assert user.email == email - with flask_client.session_transaction() as session: - session["oauth_state"] = None + with flask_client.session_transaction() as sess: + sess["oauth_state"] = None +@patch("requests.get") @patch("requests_oauthlib.OAuth2Session.fetch_token") @patch("requests_oauthlib.OAuth2Session.get") -def test_oidc_callback_login(mock_get, mock_fetch_token, flask_client): +def test_oidc_callback_login(mock_oauth_get, mock_fetch_token, mock_get, flask_client): email = random_string() - mock_get.return_value = MockResponse( + mock_oauth_get.return_value = MockResponse( 200, { "email": email, }, ) - with flask_client.session_transaction() as session: - session["oauth_state"] = "state" + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = None + sess["oauth_state"] = "state" + + mock_get.return_value.json.return_value = mock_well_known_response user = User.create( email=email, @@ -275,10 +376,57 @@ def test_oidc_callback_login(mock_get, mock_fetch_token, flask_client): expected_redirect_url = "/dashboard/" assert expected_redirect_url == parsed.path - assert mock_get.called + assert mock_oauth_get.called - with flask_client.session_transaction() as session: - session["oauth_state"] = None + with flask_client.session_transaction() as sess: + sess["oauth_state"] = None + + +@patch("requests.get") +@patch("requests_oauthlib.OAuth2Session.fetch_token") +@patch("requests_oauthlib.OAuth2Session.get") +def test_oidc_callback_login_with_next_url( + mock_oauth_get, mock_fetch_token, mock_get, flask_client +): + email = random_string() + mock_oauth_get.return_value = MockResponse( + 200, + { + "email": email, + }, + ) + config.OIDC_WELL_KNOWN_URL = "http://localhost:7777/well-known-url" + with flask_client.session_transaction() as sess: + sess["oauth_redirect_next"] = "/dashboard/settings/" + sess["oauth_state"] = "state" + + mock_get.return_value.json.return_value = mock_well_known_response + + user = User.create( + email=email, + name="name", + password="", + activated=True, + ) + user = User.get_by(email=email) + assert user is not None + + r = flask_client.get( + url_for("auth.oidc_callback"), + follow_redirects=False, + ) + location = r.headers.get("Location") + assert location is not None + + parsed = parse_url(location) + + expected_redirect_url = "/dashboard/settings/" + + assert expected_redirect_url == parsed.path + assert mock_oauth_get.called + + with flask_client.session_transaction() as sess: + sess["oauth_state"] = None def test_create_user(): diff --git a/tests/test.env b/tests/test.env index 49941bee..0c492223 100644 --- a/tests/test.env +++ b/tests/test.env @@ -51,9 +51,7 @@ FACEBOOK_CLIENT_SECRET=to_fill # Login with OIDC CONNECT_WITH_OIDC_ICON=fa-github -OIDC_AUTHORIZATION_URL=to_fill -OIDC_USER_INFO_URL=to_fill -OIDC_TOKEN_URL=to_fill +OIDC_WELL_KNOWN_URL=to_fill OIDC_SCOPES=openid email profile OIDC_NAME_FIELD=name OIDC_CLIENT_ID=to_fill