From 6a5618d258bb2c8f6f19d4155acc98c69372d7e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Dubois?= Date: Mon, 27 Oct 2025 11:06:36 +0100 Subject: [PATCH 1/2] feat: remove deprecated methods related to redirect uris we are not using those methods anymore since 1.3.0 --- CHANGES.md | 4 +- src/pas/plugins/kimug/plugin/__init__.py | 1 - src/pas/plugins/kimug/utils.py | 57 ----------- tests/utils/test_utils.py | 121 ----------------------- 4 files changed, 3 insertions(+), 180 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 888e1f2..787c140 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,7 +1,9 @@ ## 1.3.2 (unreleased) -- Nothing changed yet. +- Remove deprecated methods related to redirect uris + We are not using those methods anymore since 1.3.0 + [remdub] ## 1.3.1 (2025-09-30) diff --git a/src/pas/plugins/kimug/plugin/__init__.py b/src/pas/plugins/kimug/plugin/__init__.py index 5391220..0d28b34 100644 --- a/src/pas/plugins/kimug/plugin/__init__.py +++ b/src/pas/plugins/kimug/plugin/__init__.py @@ -7,7 +7,6 @@ from zope.interface import implementer import jwt -import os # from jwt.algorithms import RSAAlgorithm diff --git a/src/pas/plugins/kimug/utils.py b/src/pas/plugins/kimug/utils.py index 795f51b..fe4753f 100644 --- a/src/pas/plugins/kimug/utils.py +++ b/src/pas/plugins/kimug/utils.py @@ -6,10 +6,8 @@ from zope.annotation.interfaces import IAnnotations from zope.component.hooks import setSite -import ast import logging import os -import re import requests import time import transaction @@ -18,61 +16,6 @@ logger = logging.getLogger("pas.plugins.kimug.utils") -def sanitize_redirect_uris(redirect_uris: tuple | list | str) -> tuple[str, ...]: - """Sanitize redirect_uris to ensure they are in the correct format.""" - if isinstance(redirect_uris, tuple): - # redirect_uris = ('http://url1', 'http://url2', 'http://url3') - return redirect_uris - elif isinstance(redirect_uris, list): - # redirect_uris = ['http://url1', 'http://url2', 'http://url3'] - return tuple(redirect_uris) - elif isinstance(redirect_uris, str): - pattern = r"\[((?:[^'\"[\],]+(?:, )?)+)\]" - if re.match(pattern, redirect_uris): - # redirect_uris = "[http://url1, http://url2, http://url3]" - redirect_uris = redirect_uris.strip("[]") - redirect_uris = redirect_uris.split(", ") - return tuple(redirect_uris) - else: - try: - # redirect_uris = "['http://url1', 'http://url2', 'http://url3']" - return tuple(ast.literal_eval(redirect_uris)) - except (ValueError, SyntaxError): - # redirect_uris is malformed - return () - - -def get_redirect_uris(current_redirect_uris: tuple[str, ...]) -> tuple[str, ...]: - """Get redirect_uris from environment variables.""" - website_hostname = os.environ.get("WEBSITE_HOSTNAME") - if website_hostname is not None: - website_hostname = f"https://{website_hostname}" - else: - website_hostname = "http://localhost:8080/Plone" - default_redirect_uri = f"{website_hostname}/acl_users/oidc/callback" - redirect_uris = os.environ.get( - "keycloak_redirect_uris", - f"({default_redirect_uri},)", - ) - redirect_uris = sanitize_redirect_uris(redirect_uris) - redirect_uris = current_redirect_uris + redirect_uris - if default_redirect_uri not in redirect_uris: - # the default redirect uri should always be present - redirect_uris = redirect_uris + (default_redirect_uri,) - redirect_uris = list(redirect_uris) - - # handle the case when we went to prod from preprod - # and the preprod uri is still in the redirect_uris - preprod_uri = "preprod.imio.be" - if preprod_uri not in default_redirect_uri: - for uri in redirect_uris: - if preprod_uri in uri: - redirect_uris.remove(uri) - # remove duplicates - redirect_uris = list(dict.fromkeys(redirect_uris)) - return tuple(redirect_uris) - - def get_redirect_uri() -> tuple[str, ...]: """Get the main redirect_uri from environment variables.""" website_hostname = os.environ.get("WEBSITE_HOSTNAME") diff --git a/tests/utils/test_utils.py b/tests/utils/test_utils.py index 983619d..d50562a 100644 --- a/tests/utils/test_utils.py +++ b/tests/utils/test_utils.py @@ -2,129 +2,8 @@ from plone import api from zope.annotation.interfaces import IAnnotations -import os - class TestUtils: - def test_sanitize_redirect_uris(self): - """Test sanitize_redirect_uris function.""" - - good_sanitized_uris = ( - "http://url1", - "http://url2", - "http://url3", - ) - - redirect_uris = "('http://url1', 'http://url2', 'http://url3')" - sanitized_uris = utils.sanitize_redirect_uris(redirect_uris) - assert sanitized_uris == good_sanitized_uris - - redirect_uris = '("http://url1", "http://url2", "http://url3")' - sanitized_uris = utils.sanitize_redirect_uris(redirect_uris) - assert sanitized_uris == good_sanitized_uris - - redirect_uris = "['http://url1', 'http://url2', 'http://url3']" - sanitized_uris = utils.sanitize_redirect_uris(redirect_uris) - assert sanitized_uris == good_sanitized_uris - - redirect_uris = '["http://url1", "http://url2", "http://url3"]' - sanitized_uris = utils.sanitize_redirect_uris(redirect_uris) - assert sanitized_uris == good_sanitized_uris - - redirect_uris = "[http://url1, http://url2, http://url3]" - sanitized_uris = utils.sanitize_redirect_uris(redirect_uris) - assert sanitized_uris == good_sanitized_uris - - redirect_uris = "something else" - sanitized_uris = utils.sanitize_redirect_uris(redirect_uris) - assert sanitized_uris == () - - def test_get_redirect_uris(self): - """Test get_redirect_uris function.""" - - current_redirect_uris = () - - # 1 : no values set on oidc settings - - # Test with no environment variable set - redirect_uris = utils.get_redirect_uris(current_redirect_uris) - assert redirect_uris == ("http://localhost:8080/Plone/acl_users/oidc/callback",) - - # set WEBSITE_HOSTNAME - os.environ["WEBSITE_HOSTNAME"] = "kimug.imio.be" - redirect_uris = utils.get_redirect_uris(current_redirect_uris) - assert redirect_uris == ("https://kimug.imio.be/acl_users/oidc/callback",) - - # set keycloak_redirect_uris - os.environ["keycloak_redirect_uris"] = "['http://url1', 'http://url2']" - redirect_uris = utils.get_redirect_uris(current_redirect_uris) - assert redirect_uris == ( - "http://url1", - "http://url2", - "https://kimug.imio.be/acl_users/oidc/callback", - ) - - # 2 : values set on oidc settings - - redirect_uris_from_oidc_settings = ( - "http://url1", - "http://url2", - "http://url3", - ) - - os.environ.pop("WEBSITE_HOSTNAME", None) - os.environ.pop("keycloak_redirect_uris", None) - - # Test with no environment variable set - redirect_uris = utils.get_redirect_uris(redirect_uris_from_oidc_settings) - assert redirect_uris == redirect_uris_from_oidc_settings + ( - "http://localhost:8080/Plone/acl_users/oidc/callback", - ) - # set WEBSITE_HOSTNAME - os.environ["WEBSITE_HOSTNAME"] = "kimug.imio.be" - redirect_uris = utils.get_redirect_uris(redirect_uris_from_oidc_settings) - assert redirect_uris == redirect_uris_from_oidc_settings + ( - "https://kimug.imio.be/acl_users/oidc/callback", - ) - # set keycloak_redirect_uris - os.environ["keycloak_redirect_uris"] = "['http://url4', 'http://url5']" - redirect_uris = utils.get_redirect_uris(redirect_uris_from_oidc_settings) - assert redirect_uris == redirect_uris_from_oidc_settings + ( - "http://url4", - "http://url5", - "https://kimug.imio.be/acl_users/oidc/callback", - ) - - # 3 : from preprod to prod - - os.environ["WEBSITE_HOSTNAME"] = "kimug.imio.be" - os.environ.pop("keycloak_redirect_uris", None) - redirect_uris_from_oidc_settings = ( - "https://kimug.preprod.imio.be/acl_users/oidc/callback", - ) - redirect_uris = utils.get_redirect_uris(redirect_uris_from_oidc_settings) - assert redirect_uris == ("https://kimug.imio.be/acl_users/oidc/callback",) - - os.environ["keycloak_redirect_uris"] = "[http://url1, http://url2]" - redirect_uris = utils.get_redirect_uris(redirect_uris_from_oidc_settings) - assert redirect_uris == ( - "http://url1", - "http://url2", - "https://kimug.imio.be/acl_users/oidc/callback", - ) - - # 4 : uris already in the oidc settings - - os.environ["WEBSITE_HOSTNAME"] = "kimug.imio.be" - os.environ[ - "keycloak_redirect_uris" - ] = "('https://kimug.imio.be/acl_users/oidc/callback',)" - redirect_uris_from_oidc_settings = ( - "https://kimug.imio.be/acl_users/oidc/callback", - ) - redirect_uris = utils.get_redirect_uris(redirect_uris_from_oidc_settings) - assert redirect_uris == ("https://kimug.imio.be/acl_users/oidc/callback",) - def test_toggle_authentication_plugins(self, portal): """Test toggle authentication plugins methods.""" From 07ed87554db43862f865268ae71bf69015ea4bad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Dubois?= Date: Mon, 27 Oct 2025 11:15:01 +0100 Subject: [PATCH 2/2] fix: tests --- tests/plugin/test_plugin.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/plugin/test_plugin.py b/tests/plugin/test_plugin.py index 1e36297..e672a69 100644 --- a/tests/plugin/test_plugin.py +++ b/tests/plugin/test_plugin.py @@ -71,8 +71,14 @@ def test_groups_roles(self, profile_last_version): role = plugin.getRolesForPrincipal(pas.getUserById("kimug")) roles = plugin.getRolesForPrincipal(pas.getUserById("kimug_with_groups")) assert role == ("Member",) - assert roles == ("Member", "Manager") + # assert roles == ("Member", "Manager") + assert roles == ( + "Member", + ) # https://github.com/IMIO/pas.plugins.kimug/commit/966d16cabd44379e12cfd580bff80e58a72f98bb os.environ["application_id"] = "delib" roles = plugin.getRolesForPrincipal(pas.getUserById("kimug")) - assert roles == ("Member", "Manager") + # assert roles == ("Member", "Manager") + assert roles == ( + "Member", + ) # https://github.com/IMIO/pas.plugins.kimug/commit/966d16cabd44379e12cfd580bff80e58a72f98bb del os.environ["application_id"]