From 46c795b3b5b7fccc26d1fd1e7fd5b98c71e5b4b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Fri, 15 Sep 2023 17:24:05 +0200 Subject: [PATCH] refactor: only load oidc module if OIDC is configured --- canaille/__init__.py | 15 +- canaille/core/account.py | 2 +- canaille/core/templates/mails/admin.html | 34 ++-- canaille/templates/base.html | 2 +- tests/app/fixtures/themes/test/base.html | 14 +- tests/app/test_forms.py | 244 ---------------------- tests/core/test_account.py | 5 - tests/oidc/test_account.py | 20 ++ tests/oidc/test_forms.py | 248 +++++++++++++++++++++++ 9 files changed, 307 insertions(+), 277 deletions(-) create mode 100644 tests/oidc/test_account.py create mode 100644 tests/oidc/test_forms.py diff --git a/canaille/__init__.py b/canaille/__init__.py index 5add0d68..e4483b4e 100644 --- a/canaille/__init__.py +++ b/canaille/__init__.py @@ -65,12 +65,15 @@ def setup_jinja(app): def setup_blueprints(app): import canaille.core.blueprints - import canaille.oidc.blueprints app.url_map.strict_slashes = False app.register_blueprint(canaille.core.blueprints.bp) - app.register_blueprint(canaille.oidc.blueprints.bp) + + if "OIDC" in app.config: + import canaille.oidc.blueprints + + app.register_blueprint(canaille.oidc.blueprints.bp) def setup_flask(app): @@ -88,6 +91,7 @@ def global_processor(): return { "debug": app.debug or app.config.get("TESTING", False), "has_smtp": "SMTP" in app.config, + "has_oidc": "OIDC" in app.config, "has_password_recovery": app.config.get("ENABLE_PASSWORD_RECOVERY", True), "has_registration": app.config.get("ENABLE_REGISTRATION", False), "has_account_lockability": app.backend.get().has_account_lockability(), @@ -135,7 +139,6 @@ def setup_flask_converters(app): def create_app(config=None, validate=True, backend=None): - from .oidc.oauth import setup_oauth from .app.i18n import setup_i18n from .app.configuration import setup_config from .app.themes import setup_themer @@ -149,7 +152,6 @@ def create_app(config=None, validate=True, backend=None): try: setup_logging(app) setup_backend(app, backend) - setup_oauth(app) setup_flask_converters(app) setup_blueprints(app) setup_jinja(app) @@ -157,6 +159,11 @@ def create_app(config=None, validate=True, backend=None): setup_themer(app) setup_flask(app) + if "OIDC" in app.config: + from .oidc.oauth import setup_oauth + + setup_oauth(app) + except Exception as exc: # pragma: no cover if sentry_sdk: sentry_sdk.capture_exception(exc) diff --git a/canaille/core/account.py b/canaille/core/account.py index afd7a5a5..f6d4fd1c 100644 --- a/canaille/core/account.py +++ b/canaille/core/account.py @@ -67,7 +67,7 @@ def index(): if user.can_edit_self or user.can_manage_users: return redirect(url_for("core.account.profile_edition", edited_user=user)) - if user.can_use_oidc: + if "OIDC" in current_app.config and user.can_use_oidc: return redirect(url_for("oidc.consents.consents")) return redirect(url_for("core.account.about")) diff --git a/canaille/core/templates/mails/admin.html b/canaille/core/templates/mails/admin.html index a69c653e..f4aa80b2 100644 --- a/canaille/core/templates/mails/admin.html +++ b/canaille/core/templates/mails/admin.html @@ -10,22 +10,24 @@ {% trans %}Emails{% endtrans %} - - - {% trans %}Clients{% endtrans %} - - - - {% trans %}Add a client{% endtrans %} - - - - {% trans %}Tokens{% endtrans %} - - - - {% trans %}Codes{% endtrans %} - + {% if has_oidc %} + + + {% trans %}Clients{% endtrans %} + + + + {% trans %}Add a client{% endtrans %} + + + + {% trans %}Tokens{% endtrans %} + + + + {% trans %}Codes{% endtrans %} + + {% endif %} {% endblock %} {% block content %} diff --git a/canaille/templates/base.html b/canaille/templates/base.html index 32e1658d..5f1dd1e3 100644 --- a/canaille/templates/base.html +++ b/canaille/templates/base.html @@ -36,7 +36,7 @@ {% trans %}Profile{% endtrans %} {% endif %} - {% if user.can_use_oidc %} + {% if has_oidc and user.can_use_oidc %} diff --git a/tests/app/fixtures/themes/test/base.html b/tests/app/fixtures/themes/test/base.html index 50dbf81b..cf81d667 100644 --- a/tests/app/fixtures/themes/test/base.html +++ b/tests/app/fixtures/themes/test/base.html @@ -32,11 +32,13 @@ {% trans %}My profile{% endtrans %} - - - {% trans %}My consents{% endtrans %} - + {% if has_oidc and user.can_use_oidc %} + + + {% trans %}My consents{% endtrans %} + + {% endif %} {% if user.can_manage_users %} @@ -51,7 +53,7 @@ {% trans %}Groups{% endtrans %} {% endif %} - {% if user.can_manage_oidc %} + {% if has_oidc and user.can_manage_oidc %}
Admin diff --git a/tests/app/test_forms.py b/tests/app/test_forms.py index fabc4332..6fee185b 100644 --- a/tests/app/test_forms.py +++ b/tests/app/test_forms.py @@ -3,7 +3,6 @@ import pytest import wtforms from babel.dates import LOCALTZ -from canaille.app import models from canaille.app.forms import DateTimeUTCField from canaille.app.forms import phone_number from flask import current_app @@ -189,224 +188,6 @@ class Foobar: ) -def test_fieldlist_add(testclient, logged_admin): - assert not models.Client.query() - - res = testclient.get("/admin/client/add") - assert "redirect_uris-1" not in res.form.fields - - data = { - "client_name": "foobar", - "client_uri": "https://foo.bar", - "redirect_uris-0": "https://foo.bar/callback", - "grant_types": ["password", "authorization_code"], - "response_types": ["code", "token"], - "token_endpoint_auth_method": "none", - } - for k, v in data.items(): - res.form[k].force_value(v) - - res = res.form.submit(status=200, name="fieldlist_add", value="redirect_uris-0") - assert not models.Client.query() - - data["redirect_uris-1"] = "https://foo.bar/callback2" - for k, v in data.items(): - res.form[k].force_value(v) - - res = res.form.submit(status=302, name="action", value="edit") - res = res.follow(status=200) - - client_id = res.forms["readonly"]["client_id"].value - client = models.Client.get(client_id=client_id) - - assert client.redirect_uris == [ - "https://foo.bar/callback", - "https://foo.bar/callback2", - ] - - client.delete() - - -def test_fieldlist_delete(testclient, logged_admin): - assert not models.Client.query() - res = testclient.get("/admin/client/add") - - data = { - "client_name": "foobar", - "client_uri": "https://foo.bar", - "redirect_uris-0": "https://foo.bar/callback1", - "grant_types": ["password", "authorization_code"], - "response_types": ["code", "token"], - "token_endpoint_auth_method": "none", - } - for k, v in data.items(): - res.form[k].force_value(v) - res = res.form.submit(status=200, name="fieldlist_add", value="redirect_uris-0") - - res.form["redirect_uris-1"] = "https://foo.bar/callback2" - res = res.form.submit(status=200, name="fieldlist_remove", value="redirect_uris-1") - assert not models.Client.query() - assert "redirect_uris-1" not in res.form.fields - - res = res.form.submit(status=302, name="action", value="edit") - res = res.follow(status=200) - - client_id = res.forms["readonly"]["client_id"].value - client = models.Client.get(client_id=client_id) - - assert client.redirect_uris == [ - "https://foo.bar/callback1", - ] - - client.delete() - - -def test_fieldlist_add_invalid_field(testclient, logged_admin): - res = testclient.get("/admin/client/add") - data = { - "csrf_token": res.form["csrf_token"].value, - "client_name": "foobar", - "client_uri": "https://foo.bar", - "redirect_uris-0": "https://foo.bar/callback", - "grant_types": ["password", "authorization_code"], - "response_types": ["code", "token"], - "token_endpoint_auth_method": "none", - "fieldlist_add": "invalid", - } - testclient.post("/admin/client/add", data, status=400) - - -def test_fieldlist_delete_invalid_field(testclient, logged_admin): - assert not models.Client.query() - res = testclient.get("/admin/client/add") - - data = { - "csrf_token": res.form["csrf_token"].value, - "client_name": "foobar", - "client_uri": "https://foo.bar", - "redirect_uris-0": "https://foo.bar/callback1", - "redirect_uris-1": "https://foo.bar/callback2", - "grant_types": ["password", "authorization_code"], - "response_types": ["code", "token"], - "token_endpoint_auth_method": "none", - "fieldlist_remove": "invalid", - } - testclient.post("/admin/client/add", data, status=400) - - -def test_fieldlist_duplicate_value(testclient, logged_admin, client): - res = testclient.get("/admin/client/add") - data = { - "client_name": "foobar", - "client_uri": "https://foo.bar", - "redirect_uris-0": "https://foo.bar/samecallback", - "grant_types": ["password", "authorization_code"], - "response_types": ["code", "token"], - "token_endpoint_auth_method": "none", - } - for k, v in data.items(): - res.form[k].force_value(v) - res = res.form.submit(status=200, name="fieldlist_add", value="redirect_uris-0") - res.form["redirect_uris-1"] = "https://foo.bar/samecallback" - res = res.form.submit(status=200, name="action", value="edit") - res.mustcontain("This value is a duplicate") - - -def test_fieldlist_empty_value(testclient, logged_admin, client): - res = testclient.get("/admin/client/add") - data = { - "client_name": "foobar", - "client_uri": "https://foo.bar", - "redirect_uris-0": "https://foo.bar/samecallback", - "post_logout_redirect_uris-0": "https://foo.bar/callback1", - "grant_types": ["password", "authorization_code"], - "response_types": ["code", "token"], - "token_endpoint_auth_method": "none", - } - for k, v in data.items(): - res.form[k].force_value(v) - res = res.form.submit( - status=200, name="fieldlist_add", value="post_logout_redirect_uris-0" - ) - res.form.submit(status=302, name="action", value="edit") - client = models.Client.get() - client.delete() - - -def test_fieldlist_add_field_htmx(testclient, logged_admin): - res = testclient.get("/admin/client/add") - data = { - "csrf_token": res.form["csrf_token"].value, - "client_name": "foobar", - "client_uri": "https://foo.bar", - "redirect_uris-0": "https://foo.bar/callback", - "grant_types": ["password", "authorization_code"], - "response_types": ["code", "token"], - "token_endpoint_auth_method": "none", - "fieldlist_add": "redirect_uris-0", - } - response = testclient.post( - "/admin/client/add", - data, - headers={ - "HX-Request": "true", - "HX-Trigger-Name": "listfield_add", - }, - ) - assert 'name="redirect_uris-0' in response.text - assert 'name="redirect_uris-1' in response.text - - -def test_fieldlist_add_field_htmx_validation(testclient, logged_admin): - res = testclient.get("/admin/client/add") - data = { - "csrf_token": res.form["csrf_token"].value, - "client_name": "foobar", - "client_uri": "https://foo.bar", - "redirect_uris-0": "not-a-valid-uri", - "grant_types": ["password", "authorization_code"], - "response_types": ["code", "token"], - "token_endpoint_auth_method": "none", - "fieldlist_add": "redirect_uris-0", - } - response = testclient.post( - "/admin/client/add", - data, - headers={ - "HX-Request": "true", - "HX-Trigger-Name": "listfield_add", - }, - ) - assert 'name="redirect_uris-0' in response.text - assert 'name="redirect_uris-1' in response.text - assert "This is not a valid URL" in response.text - - -def test_fieldlist_remove_field_htmx(testclient, logged_admin): - res = testclient.get("/admin/client/add") - data = { - "csrf_token": res.form["csrf_token"].value, - "client_name": "foobar", - "client_uri": "https://foo.bar", - "redirect_uris-0": "https://foo.bar/callback1", - "redirect_uris-1": "https://foo.bar/callback2", - "grant_types": ["password", "authorization_code"], - "response_types": ["code", "token"], - "token_endpoint_auth_method": "none", - "fieldlist_remove": "redirect_uris-1", - } - response = testclient.post( - "/admin/client/add", - data, - headers={ - "HX-Request": "true", - "HX-Trigger-Name": "listfield_remove", - }, - ) - assert 'name="redirect_uris-0' in response.text - assert 'name="redirect_uris-1' not in response.text - - def test_fieldlist_add_readonly(testclient, logged_user, configuration): configuration["ACL"]["DEFAULT"]["WRITE"].remove("phone_numbers") configuration["ACL"]["DEFAULT"]["READ"].append("phone_numbers") @@ -445,31 +226,6 @@ def test_fieldlist_remove_readonly(testclient, logged_user, configuration): testclient.post("/profile/user", data, status=403) -def test_fieldlist_inline_validation(testclient, logged_admin): - res = testclient.get("/admin/client/add") - data = { - "csrf_token": res.form["csrf_token"].value, - "client_name": "foobar", - "client_uri": "https://foo.bar", - "redirect_uris-0": "invalid-url", - "redirect_uris-1": "https://foo.bar/callback2", - "grant_types": ["password", "authorization_code"], - "response_types": ["code", "token"], - "token_endpoint_auth_method": "none", - } - response = testclient.post( - "/admin/client/add", - data, - headers={ - "HX-Request": "true", - "HX-Trigger-Name": "redirect_uris-0", - }, - ) - assert 'name="redirect_uris-0' in response.text - assert 'name="redirect_uris-1' not in response.text - assert "This is not a valid URL" in response.text - - def test_inline_validation_invalid_field(testclient, logged_admin, user): res = testclient.get("/profile") testclient.post( diff --git a/tests/core/test_account.py b/tests/core/test_account.py index be24b2b6..e6119c5a 100644 --- a/tests/core/test_account.py +++ b/tests/core/test_account.py @@ -13,11 +13,6 @@ def test_index(testclient, user): res = testclient.get("/", status=302) assert res.location == "/profile/user" - testclient.app.config["ACL"]["DEFAULT"]["PERMISSIONS"] = ["use_oidc"] - g.user.reload() - res = testclient.get("/", status=302) - assert res.location == "/consent/" - testclient.app.config["ACL"]["DEFAULT"]["PERMISSIONS"] = [] g.user.reload() res = testclient.get("/", status=302) diff --git a/tests/oidc/test_account.py b/tests/oidc/test_account.py new file mode 100644 index 00000000..ced2a9ce --- /dev/null +++ b/tests/oidc/test_account.py @@ -0,0 +1,20 @@ +from flask import g + + +def test_index(testclient, user): + res = testclient.get("/", status=302) + assert res.location == "/login" + + g.user = user + res = testclient.get("/", status=302) + assert res.location == "/profile/user" + + testclient.app.config["ACL"]["DEFAULT"]["PERMISSIONS"] = ["use_oidc"] + g.user.reload() + res = testclient.get("/", status=302) + assert res.location == "/consent/" + + testclient.app.config["ACL"]["DEFAULT"]["PERMISSIONS"] = [] + g.user.reload() + res = testclient.get("/", status=302) + assert res.location == "/about" diff --git a/tests/oidc/test_forms.py b/tests/oidc/test_forms.py new file mode 100644 index 00000000..9791be2a --- /dev/null +++ b/tests/oidc/test_forms.py @@ -0,0 +1,248 @@ +from canaille.app import models + +# TODO: this test files tests generic features about forms, +# we should rewrite the tests without using specific OIDC +# forms. + + +def test_fieldlist_add(testclient, logged_admin): + assert not models.Client.query() + + res = testclient.get("/admin/client/add") + assert "redirect_uris-1" not in res.form.fields + + data = { + "client_name": "foobar", + "client_uri": "https://foo.bar", + "redirect_uris-0": "https://foo.bar/callback", + "grant_types": ["password", "authorization_code"], + "response_types": ["code", "token"], + "token_endpoint_auth_method": "none", + } + for k, v in data.items(): + res.form[k].force_value(v) + + res = res.form.submit(status=200, name="fieldlist_add", value="redirect_uris-0") + assert not models.Client.query() + + data["redirect_uris-1"] = "https://foo.bar/callback2" + for k, v in data.items(): + res.form[k].force_value(v) + + res = res.form.submit(status=302, name="action", value="edit") + res = res.follow(status=200) + + client_id = res.forms["readonly"]["client_id"].value + client = models.Client.get(client_id=client_id) + + assert client.redirect_uris == [ + "https://foo.bar/callback", + "https://foo.bar/callback2", + ] + + client.delete() + + +def test_fieldlist_delete(testclient, logged_admin): + assert not models.Client.query() + res = testclient.get("/admin/client/add") + + data = { + "client_name": "foobar", + "client_uri": "https://foo.bar", + "redirect_uris-0": "https://foo.bar/callback1", + "grant_types": ["password", "authorization_code"], + "response_types": ["code", "token"], + "token_endpoint_auth_method": "none", + } + for k, v in data.items(): + res.form[k].force_value(v) + res = res.form.submit(status=200, name="fieldlist_add", value="redirect_uris-0") + + res.form["redirect_uris-1"] = "https://foo.bar/callback2" + res = res.form.submit(status=200, name="fieldlist_remove", value="redirect_uris-1") + assert not models.Client.query() + assert "redirect_uris-1" not in res.form.fields + + res = res.form.submit(status=302, name="action", value="edit") + res = res.follow(status=200) + + client_id = res.forms["readonly"]["client_id"].value + client = models.Client.get(client_id=client_id) + + assert client.redirect_uris == [ + "https://foo.bar/callback1", + ] + + client.delete() + + +def test_fieldlist_add_invalid_field(testclient, logged_admin): + res = testclient.get("/admin/client/add") + data = { + "csrf_token": res.form["csrf_token"].value, + "client_name": "foobar", + "client_uri": "https://foo.bar", + "redirect_uris-0": "https://foo.bar/callback", + "grant_types": ["password", "authorization_code"], + "response_types": ["code", "token"], + "token_endpoint_auth_method": "none", + "fieldlist_add": "invalid", + } + testclient.post("/admin/client/add", data, status=400) + + +def test_fieldlist_delete_invalid_field(testclient, logged_admin): + assert not models.Client.query() + res = testclient.get("/admin/client/add") + + data = { + "csrf_token": res.form["csrf_token"].value, + "client_name": "foobar", + "client_uri": "https://foo.bar", + "redirect_uris-0": "https://foo.bar/callback1", + "redirect_uris-1": "https://foo.bar/callback2", + "grant_types": ["password", "authorization_code"], + "response_types": ["code", "token"], + "token_endpoint_auth_method": "none", + "fieldlist_remove": "invalid", + } + testclient.post("/admin/client/add", data, status=400) + + +def test_fieldlist_duplicate_value(testclient, logged_admin, client): + res = testclient.get("/admin/client/add") + data = { + "client_name": "foobar", + "client_uri": "https://foo.bar", + "redirect_uris-0": "https://foo.bar/samecallback", + "grant_types": ["password", "authorization_code"], + "response_types": ["code", "token"], + "token_endpoint_auth_method": "none", + } + for k, v in data.items(): + res.form[k].force_value(v) + res = res.form.submit(status=200, name="fieldlist_add", value="redirect_uris-0") + res.form["redirect_uris-1"] = "https://foo.bar/samecallback" + res = res.form.submit(status=200, name="action", value="edit") + res.mustcontain("This value is a duplicate") + + +def test_fieldlist_empty_value(testclient, logged_admin, client): + res = testclient.get("/admin/client/add") + data = { + "client_name": "foobar", + "client_uri": "https://foo.bar", + "redirect_uris-0": "https://foo.bar/samecallback", + "post_logout_redirect_uris-0": "https://foo.bar/callback1", + "grant_types": ["password", "authorization_code"], + "response_types": ["code", "token"], + "token_endpoint_auth_method": "none", + } + for k, v in data.items(): + res.form[k].force_value(v) + res = res.form.submit( + status=200, name="fieldlist_add", value="post_logout_redirect_uris-0" + ) + res.form.submit(status=302, name="action", value="edit") + client = models.Client.get() + client.delete() + + +def test_fieldlist_add_field_htmx(testclient, logged_admin): + res = testclient.get("/admin/client/add") + data = { + "csrf_token": res.form["csrf_token"].value, + "client_name": "foobar", + "client_uri": "https://foo.bar", + "redirect_uris-0": "https://foo.bar/callback", + "grant_types": ["password", "authorization_code"], + "response_types": ["code", "token"], + "token_endpoint_auth_method": "none", + "fieldlist_add": "redirect_uris-0", + } + response = testclient.post( + "/admin/client/add", + data, + headers={ + "HX-Request": "true", + "HX-Trigger-Name": "listfield_add", + }, + ) + assert 'name="redirect_uris-0' in response.text + assert 'name="redirect_uris-1' in response.text + + +def test_fieldlist_add_field_htmx_validation(testclient, logged_admin): + res = testclient.get("/admin/client/add") + data = { + "csrf_token": res.form["csrf_token"].value, + "client_name": "foobar", + "client_uri": "https://foo.bar", + "redirect_uris-0": "not-a-valid-uri", + "grant_types": ["password", "authorization_code"], + "response_types": ["code", "token"], + "token_endpoint_auth_method": "none", + "fieldlist_add": "redirect_uris-0", + } + response = testclient.post( + "/admin/client/add", + data, + headers={ + "HX-Request": "true", + "HX-Trigger-Name": "listfield_add", + }, + ) + assert 'name="redirect_uris-0' in response.text + assert 'name="redirect_uris-1' in response.text + assert "This is not a valid URL" in response.text + + +def test_fieldlist_remove_field_htmx(testclient, logged_admin): + res = testclient.get("/admin/client/add") + data = { + "csrf_token": res.form["csrf_token"].value, + "client_name": "foobar", + "client_uri": "https://foo.bar", + "redirect_uris-0": "https://foo.bar/callback1", + "redirect_uris-1": "https://foo.bar/callback2", + "grant_types": ["password", "authorization_code"], + "response_types": ["code", "token"], + "token_endpoint_auth_method": "none", + "fieldlist_remove": "redirect_uris-1", + } + response = testclient.post( + "/admin/client/add", + data, + headers={ + "HX-Request": "true", + "HX-Trigger-Name": "listfield_remove", + }, + ) + assert 'name="redirect_uris-0' in response.text + assert 'name="redirect_uris-1' not in response.text + + +def test_fieldlist_inline_validation(testclient, logged_admin): + res = testclient.get("/admin/client/add") + data = { + "csrf_token": res.form["csrf_token"].value, + "client_name": "foobar", + "client_uri": "https://foo.bar", + "redirect_uris-0": "invalid-url", + "redirect_uris-1": "https://foo.bar/callback2", + "grant_types": ["password", "authorization_code"], + "response_types": ["code", "token"], + "token_endpoint_auth_method": "none", + } + response = testclient.post( + "/admin/client/add", + data, + headers={ + "HX-Request": "true", + "HX-Trigger-Name": "redirect_uris-0", + }, + ) + assert 'name="redirect_uris-0' in response.text + assert 'name="redirect_uris-1' not in response.text + assert "This is not a valid URL" in response.text