From 2bdf2c7bea47bb4027ab8230a0230cef68c82158 Mon Sep 17 00:00:00 2001 From: Wombosvideo Date: Sun, 9 Feb 2025 13:35:30 +0100 Subject: [PATCH 1/3] Replace deprecated logger.warn calls with logger.warning for consistency Some files were not included due to currently being refactored --- core/admin/mailu/api/common.py | 2 +- core/admin/mailu/internal/nginx.py | 4 ++-- core/admin/mailu/internal/views/auth.py | 2 +- core/admin/mailu/limiter.py | 20 ++++++++++---------- core/base/libs/socrate/socrate/system.py | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/core/admin/mailu/api/common.py b/core/admin/mailu/api/common.py index 6dc75a88..429d806c 100644 --- a/core/admin/mailu/api/common.py +++ b/core/admin/mailu/api/common.py @@ -27,7 +27,7 @@ def api_token_authorization(func): abort(401, 'A valid Authorization header is mandatory') if len(v1.api_token) < 4 or not hmac.compare_digest(request.headers.get('Authorization').removeprefix('Bearer '), v1.api_token): utils.limiter.rate_limit_ip(client_ip) - flask.current_app.logger.warn(f'Invalid API token provided by {client_ip}.') + flask.current_app.logger.warning(f'Invalid API token provided by {client_ip}.') abort(403, 'Invalid API token') flask.current_app.logger.info(f'Valid API token provided by {client_ip}.') return func(*args, **kwds) diff --git a/core/admin/mailu/internal/nginx.py b/core/admin/mailu/internal/nginx.py index b9cbe879..ccbccb4e 100644 --- a/core/admin/mailu/internal/nginx.py +++ b/core/admin/mailu/internal/nginx.py @@ -102,13 +102,13 @@ def handle_authentication(headers): password = urllib.parse.unquote(headers["Auth-Pass"]) ip = urllib.parse.unquote(headers["Client-Ip"]) except: - app.logger.warn(f'Received undecodable user/password from front: {headers.get("Auth-User", "")!r}') + app.logger.warning(f'Received undecodable user/password from front: {headers.get("Auth-User", "")!r}') else: try: user = models.User.query.get(user_email) if '@' in user_email else None except sqlalchemy.exc.StatementError as exc: exc = str(exc).split('\n', 1)[0] - app.logger.warn(f'Invalid user {user_email!r}: {exc}') + app.logger.warning(f'Invalid user {user_email!r}: {exc}') else: is_valid_user = user is not None ip = urllib.parse.unquote(headers["Client-Ip"]) diff --git a/core/admin/mailu/internal/views/auth.py b/core/admin/mailu/internal/views/auth.py index c74bcc9e..c619366f 100644 --- a/core/admin/mailu/internal/views/auth.py +++ b/core/admin/mailu/internal/views/auth.py @@ -103,7 +103,7 @@ def basic_authentication(): user = models.User.query.get(user_email) if '@' in user_email else None except sqlalchemy.exc.StatementError as exc: exc = str(exc).split('\n', 1)[0] - app.logger.warn(f'Invalid user {user_email!r}: {exc}') + app.logger.warning(f'Invalid user {user_email!r}: {exc}') else: if user is not None and nginx.check_credentials(user, password.decode('utf-8'), client_ip, "web", flask.request.headers.get('X-Real-Port', None), user_email): response = flask.Response() diff --git a/core/admin/mailu/limiter.py b/core/admin/mailu/limiter.py index 6fc078c1..e22b39a2 100644 --- a/core/admin/mailu/limiter.py +++ b/core/admin/mailu/limiter.py @@ -49,7 +49,7 @@ class LimitWraperFactory(object): client_network = utils.extract_network_from_ip(ip) is_rate_limited = self.is_subject_to_rate_limits(ip) and not limiter.test(client_network) if is_rate_limited: - app.logger.warn(f'Authentication attempt from {ip} has been rate-limited.') + app.logger.warning(f'Authentication attempt from {ip} has been rate-limited.') return is_rate_limited def rate_limit_ip(self, ip, username=None): @@ -65,7 +65,7 @@ class LimitWraperFactory(object): limiter = self.get_limiter(app.config["AUTH_RATELIMIT_USER"], 'auth-user') is_rate_limited = self.is_subject_to_rate_limits(ip) and not limiter.test(device_cookie if device_cookie_name == username else username) if is_rate_limited: - app.logger.warn(f'Authentication attempt from {ip} for {username} has been rate-limited.') + app.logger.warning(f'Authentication attempt from {ip} for {username} has been rate-limited.') return is_rate_limited def rate_limit_user(self, username, ip, device_cookie=None, device_cookie_name=None, password=''): @@ -78,10 +78,10 @@ class LimitWraperFactory(object): limiter.hit(device_cookie if device_cookie_name == username else username) self.rate_limit_ip(ip, username) - """ Device cookies as described on: - https://owasp.org/www-community/Slow_Down_Online_Guessing_Attacks_with_Device_Cookies - """ - def parse_device_cookie(self, cookie): + def parse_device_cookie(self, cookie: str): + """ Device cookies as described on: + https://owasp.org/www-community/Slow_Down_Online_Guessing_Attacks_with_Device_Cookies + """ try: login, nonce, _ = cookie.split('$') if hmac.compare_digest(cookie, self.device_cookie(login, nonce)): @@ -90,11 +90,11 @@ class LimitWraperFactory(object): pass return None, None - """ Device cookies don't require strong crypto: - 72bits of nonce, 96bits of signature is more than enough - and these values avoid padding in most cases - """ def device_cookie(self, username, nonce=None): + """ Device cookies don't require strong crypto: + 72bits of nonce, 96bits of signature is more than enough + and these values avoid padding in most cases + """ if not nonce: nonce = secrets.token_urlsafe(9) sig = str(base64.urlsafe_b64encode(hmac.new(app.device_cookie_key, bytearray(f'device_cookie|{username}|{nonce}', 'utf-8'), 'sha256').digest()[20:]), 'utf-8') diff --git a/core/base/libs/socrate/socrate/system.py b/core/base/libs/socrate/socrate/system.py index d92caf0c..38e4b7ba 100644 --- a/core/base/libs/socrate/socrate/system.py +++ b/core/base/libs/socrate/socrate/system.py @@ -19,7 +19,7 @@ def resolve_hostname(hostname): try: return sorted(socket.getaddrinfo(hostname, None, socket.AF_UNSPEC, socket.SOCK_STREAM, 0, socket.AI_PASSIVE), key=lambda s:s[0])[0][4][0] except Exception as e: - log.warn("Unable to lookup '%s': %s",hostname,e) + log.warning("Unable to lookup '%s': %s",hostname,e) raise e def _coerce_value(value): From 0ed1e9c8c8fb6f569485fbcd0a381a65eb342be8 Mon Sep 17 00:00:00 2001 From: Wombosvideo Date: Sun, 9 Feb 2025 20:43:38 +0100 Subject: [PATCH 2/3] Refactor OIDC client and imports for improved organization - Move OIDC client to a separate module - Organize imports --- core/admin/mailu/oidc.py | 310 ++++++++++++++++++++++++++++++++++++++ core/admin/mailu/utils.py | 211 ++------------------------ 2 files changed, 324 insertions(+), 197 deletions(-) create mode 100644 core/admin/mailu/oidc.py diff --git a/core/admin/mailu/oidc.py b/core/admin/mailu/oidc.py new file mode 100644 index 00000000..d596bad5 --- /dev/null +++ b/core/admin/mailu/oidc.py @@ -0,0 +1,310 @@ +"""OIDC Client""" + +from time import time +from typing import Optional +import flask + +# [OIDC] Import the OIDC related modules +from oic.oic import Client +from oic.extension.client import Client as ExtensionClient +from oic.utils.authn.client import CLIENT_AUTHN_METHOD +from oic.utils.settings import OicClientSettings +from oic import rndstr +from oic.exception import MessageException, NotForMe +from oic.oauth2.message import ( + ROPCAccessTokenRequest, + AccessTokenResponse, + ErrorResponse, +) +from oic.oic.message import ( + AuthorizationResponse, + RegistrationResponse, + EndSessionRequest, + BackChannelLogoutRequest, + OpenIDSchema, + UserInfoErrorResponse, +) +from oic.oauth2.grant import Token + + +# [OIDC] Client class +class OicClient: + """OpenID Connect Client""" + + app: Optional[flask.Flask] = None + client: Optional[Client] = None + extension_client: Optional[ExtensionClient] = None + registration_response: Optional[RegistrationResponse] = None + enable_change_password_redirect: bool = True + change_password_url: Optional[str] = None + redirect_url: Optional[str] = None + + def init_app(self, app: flask.Flask): + """Initialize OIDC client""" + + self.app = app + + settings = OicClientSettings(verify_ssl=app.config["OIDC_VERIFY_SSL"]) + + self.enable_change_password_redirect = ( + app.config["OIDC_CHANGE_PASSWORD_REDIRECT_ENABLED"] or False + ) + + self.client = Client(client_authn_method=CLIENT_AUTHN_METHOD, settings=settings) + self.client.provider_config(app.config["OIDC_PROVIDER_INFO_URL"]) + + self.extension_client = ExtensionClient( + client_authn_method=CLIENT_AUTHN_METHOD, settings=settings + ) + self.extension_client.provider_config(app.config["OIDC_PROVIDER_INFO_URL"]) + + self.change_password_url = app.config["OIDC_CHANGE_PASSWORD_REDIRECT_URL"] or ( + self.client.issuer + "/.well-known/change-password" + ) + self.redirect_url = app.config["OIDC_REDIRECT_URL"] or ( + "https://" + app.config["HOSTNAME"] + ) + + client_reg = RegistrationResponse( + client_id=app.config["OIDC_CLIENT_ID"], + client_secret=app.config["OIDC_CLIENT_SECRET"], + redirect_uris=[f"{self.redirect_url}/sso/login"], + ) + self.client.store_registration_info(client_reg) + self.extension_client.store_registration_info(client_reg) + + def get_redirect_url(self) -> Optional[str]: + """Get the redirect URL""" + if not self.is_enabled(): + return None + + flask.session["state"] = rndstr() + flask.session["nonce"] = rndstr() + + args = { + "client_id": self.client.client_id, + "response_type": ["code"], + "scope": ["openid", "email"], + "nonce": flask.session["nonce"], + "redirect_uri": self.redirect_url + "/sso/login", + "state": flask.session["state"], + } + + auth_req = self.client.construct_AuthorizationRequest(request_args=args) + login_url = auth_req.request(self.client.authorization_endpoint) + return login_url + + def _get_authorization_code(self, query: str) -> Optional[AuthorizationResponse]: + """ + Get the authorization response and check if it is valid + + :param query: The query string + :return: The authorization code if valid, None otherwise + """ + + auth_response = self.client.parse_response( + AuthorizationResponse, info=query, sformat="urlencoded" + ) + + if not isinstance(auth_response, AuthorizationResponse): + # If this line is reached, auth_response is an ErrorResponse + # TODO: Decide what to do with the error response + + self.app.logger.debug(f"[OIDC] Error response in authorization: {auth_response}") + return None + + if "state" not in flask.session: + self.app.logger.warning("[OIDC] No state in session") + return None + + if flask.session["state"] != auth_response["state"]: + self.app.logger.warning( + f"[OIDC] State mismatch: expected {flask.session['state']}, got {auth_response['state']}" + ) + return None + + return auth_response["code"] + + def _get_id_and_access_tokens(self, auth_response_code: str): + """ + Get the id and access tokens + + :param auth_response_code: The authorization response code + :return: The token response if valid, None otherwise + """ + + token_response = self.client.do_access_token_request( + state=flask.session["state"], + request_args={"code": auth_response_code}, + authn_method="client_secret_basic", + ) + + if not isinstance(token_response, AccessTokenResponse): + self.app.logger.warning( + f"[OIDC] No access token or invalid response: {token_response}" + ) + return None + + if "id_token" not in token_response: + self.app.logger.warning("[OIDC] No id token in response") + return None + + if token_response["id_token"]["nonce"] != flask.session["nonce"]: + self.app.logger.warning("[OIDC] Nonce mismatch") + return None + + if "access_token" not in token_response: + self.app.logger.warning("[OIDC] No access token or invalid response") + return None + + return token_response + + def exchange_code( + self, query: str + ) -> tuple[str, str, str, AccessTokenResponse] | tuple[None, None, None, None]: + """Exchange the code for the token""" + + auth_response_code = self._get_authorization_code(query) + if not auth_response_code: + return None, None, None, None + + token_response = self._get_id_and_access_tokens(auth_response_code) + if not token_response: + return None, None, None, None + + user_info_response = self.get_user_info(token_response) + if not isinstance(user_info_response, OpenIDSchema): + # If this line is reached, user_info_response is an ErrorResponse + # TODO: Decide what to do with the error response + + self.app.logger.debug("[OIDC] Error response in user info") + return None, None, None, None + + return ( + user_info_response["email"], + user_info_response["sub"], + token_response["id_token"], + token_response, + ) + + def get_token(self, username: str, password: str) -> Optional[AccessTokenResponse]: + """Get the token from the username and password""" + + args = { + "username": username, + "password": password, + "client_id": self.extension_client.client_id, + "client_secret": self.extension_client.client_secret, + "grant_type": "password", + } + url, body, ht_args, _ = self.extension_client.request_info( + ROPCAccessTokenRequest, request_args=args, method="POST" + ) + response = self.extension_client.request_and_return( + url, AccessTokenResponse, "POST", body, "json", "", ht_args + ) + if isinstance(response, AccessTokenResponse): + return response + + # If this line is reached, response is an ErrorResponse + # TODO: Decide what to do with the error response + + return None + + def get_user_info( + self, token: AccessTokenResponse + ) -> OpenIDSchema | UserInfoErrorResponse | ErrorResponse: + """Get user info from the token""" + return self.client.do_user_info_request(access_token=token["access_token"]) + + def check_validity( + self, token: AccessTokenResponse + ) -> Optional[AccessTokenResponse]: + """Check if the token is still valid""" + + # Assume the token is valid if it has an expiration time and it has not expired + if "exp" in token["id_token"] and token["id_token"]["exp"] > time(): + return token + return self.refresh_token(token) + + def refresh_token( + self, token: AccessTokenResponse + ) -> Optional[AccessTokenResponse]: + """Refresh the token""" + try: + args = {"refresh_token": token["refresh_token"]} + response = self.client.do_access_token_refresh( + request_args=args, token=Token(token) + ) + if isinstance(response, AccessTokenResponse): + return response + + # If this line is reached, response is an ErrorResponse + # TODO: Decide what to do with the error response + except Exception as e: + flask.current_app.logger.error(f"Error refreshing token: {e}") + return None + + def logout(self, id_token: str) -> str: + """ + Logout the user + + :param id_token: The id token to be used for logout + :return: The logout URL + """ + + state = rndstr() + flask.session["state"] = state + + args = { + "state": state, + "id_token_hint": id_token, + "post_logout_redirect_uri": self.redirect_url + "/sso/logout", + "client_id": self.client.client_id, + } + + request = self.client.construct_EndSessionRequest(request_args=args) + # TODO: Check if identical: uri = request.request(self.client.end_session_endpoint) + uri, _, _, _ = self.client.uri_and_body(EndSessionRequest, request, "GET", args) + return uri + + def backchannel_logout(self, body): + """ + Backchannel logout. See https://openid.net/specs/openid-connect-backchannel-1_0.html + """ + + # TODO: Finish backchannel logout implementation + req = BackChannelLogoutRequest().from_dict(body) + + kwargs = { + "aud": self.client.client_id, + "iss": self.client.issuer, + "keyjar": self.client.keyjar, + } + + try: + req.verify(**kwargs) + except (MessageException, ValueError, NotForMe) as err: + self.app.logger.error(err) + return None + + sub = req["logout_token"]["sub"] + + if sub is not None and sub != "": + return sub + + return True + + def is_enabled(self): + """Check if OIDC is enabled""" + + if self.app is not None and self.app.config["OIDC_ENABLED"]: + return True + return False + + def change_password(self) -> Optional[str]: + """Get the URL for changing password if the redirect is enabled""" + + if self.enable_change_password_redirect: + return self.change_password_url + return None diff --git a/core/admin/mailu/utils.py b/core/admin/mailu/utils.py index 0d7321ee..59c839f9 100644 --- a/core/admin/mailu/utils.py +++ b/core/admin/mailu/utils.py @@ -1,10 +1,14 @@ """ Mailu admin app utilities """ -try: - import cPickle as pickle -except ImportError: - import pickle +from datetime import datetime, timedelta +import hmac +import ipaddress +from multiprocessing import Value +import pickle +import secrets +import string +import time import dns.resolver import dns.exception @@ -13,38 +17,20 @@ import dns.rdtypes import dns.rdatatype import dns.rdataclass -import hmac -import secrets -import string -import time - -from multiprocessing import Value -from mailu import limiter -from flask import current_app as app - import flask +from flask import current_app as app +from flask.sessions import SessionMixin, SessionInterface import flask_login import flask_migrate import flask_babel -import ipaddress + import redis -from datetime import datetime, timedelta -from flask.sessions import SessionMixin, SessionInterface from itsdangerous.encoding import want_bytes from werkzeug.datastructures import CallbackDict from werkzeug.middleware.proxy_fix import ProxyFix -# [OIDC] Import the OIDC related modules -from oic.oic import Client -from oic.extension.client import Client as ExtensionClient -from oic.utils.authn.client import CLIENT_AUTHN_METHOD -from oic.utils.settings import OicClientSettings -from oic import rndstr -from oic.exception import MessageException, NotForMe -from oic.oauth2.message import ROPCAccessTokenRequest, AccessTokenResponse, ErrorResponse -from oic.oic.message import AuthorizationResponse, RegistrationResponse, EndSessionRequest, BackChannelLogoutRequest -from oic.oauth2.grant import Token +from mailu import limiter, oidc # Login configuration login = flask_login.LoginManager() @@ -137,177 +123,8 @@ class PrefixMiddleware(object): proxy = PrefixMiddleware() -# [OIDC] Client class -class OicClient: - "Redirects user to OpenID Provider if configured" - - def __init__(self): - self.app = None - self.client = None - self.extension_client = None - self.registration_response = None - self.change_password_redirect_enabled = True, - self.change_password_url = None - - def init_app(self, app): - self.app = app - - settings = OicClientSettings() - - settings.verify_ssl = app.config['OIDC_VERIFY_SSL'] - - self.change_password_redirect_enabled = app.config['OIDC_CHANGE_PASSWORD_REDIRECT_ENABLED'] - - self.client = Client(client_authn_method=CLIENT_AUTHN_METHOD,settings=settings) - self.client.provider_config(app.config['OIDC_PROVIDER_INFO_URL']) - - self.extension_client = ExtensionClient(client_authn_method=CLIENT_AUTHN_METHOD,settings=settings) - self.extension_client.provider_config(app.config['OIDC_PROVIDER_INFO_URL']) - self.change_password_url = app.config['OIDC_CHANGE_PASSWORD_REDIRECT_URL'] or (self.client.issuer + '/.well-known/change-password') - self.redirect_url = app.config['OIDC_REDIRECT_URL'] or ("https://" + self.app.config['HOSTNAME']) - info = {"client_id": app.config['OIDC_CLIENT_ID'], "client_secret": app.config['OIDC_CLIENT_SECRET'], "redirect_uris": [ self.redirect_url + "/sso/login" ]} - client_reg = RegistrationResponse(**info) - self.client.store_registration_info(client_reg) - self.extension_client.store_registration_info(client_reg) - - def get_redirect_url(self): - if not self.is_enabled(): - return None - flask.session["state"] = rndstr() - flask.session["nonce"] = rndstr() - args = { - "client_id": self.client.client_id, - "response_type": ["code"], - "scope": ["openid", "email"], - "nonce": flask.session["nonce"], - "redirect_uri": self.redirect_url + "/sso/login", - "state": flask.session["state"] - } - - auth_req = self.client.construct_AuthorizationRequest(request_args=args) - login_url = auth_req.request(self.client.authorization_endpoint) - return login_url - - def exchange_code(self, query): - aresp = self.client.parse_response(AuthorizationResponse, info=query, sformat="urlencoded") - - if not isinstance(aresp, AuthorizationResponse): - flask.current_app.logger.warning('[OIDC] No authorization response or invalid response') - return None, None, None, None - - has_state = "state" in flask.session - is_state_valid = "state" in aresp and aresp["state"] == flask.session["state"] if has_state else None - - if not ("state" in flask.session and aresp["state"] == flask.session["state"]): - flask.current_app.logger.warning(f'[OIDC] has_state: {has_state}, is_state_valid: {is_state_valid}') - return None, None, None, None - args = { - "code": aresp["code"] - } - response = self.client.do_access_token_request(state=aresp["state"], - request_args=args, - authn_method="client_secret_basic") - - if not isinstance(response, AccessTokenResponse): - flask.current_app.logger.warning(f'[OIDC] No access token or invalid response: {response}') - return None, None, None, None - - has_id_token = "id_token" in response - has_nonce = "nonce" in flask.session - is_id_token_valid = "nonce" in response["id_token"] and response["id_token"]["nonce"] == flask.session["nonce"] if has_id_token and has_nonce else None - - if "id_token" not in response or response["id_token"]["nonce"] != flask.session["nonce"]: - flask.current_app.logger.warning(f'[OIDC] has_id_token: {has_id_token}, has_nonce: {has_nonce}, is_id_token_valid: {is_id_token_valid}') - return None, None, None, None - if 'access_token' not in response or not isinstance(response, AccessTokenResponse): - flask.current_app.logger.warning('[OIDC] No access token or invalid response') - return None, None, None, None - user_response = self.client.do_user_info_request( - access_token=response['access_token']) - return user_response['email'], user_response['sub'], response["id_token"], response - - - def get_token(self, username, password): - args = { - "username": username, - "password": password, - "client_id": self.extension_client.client_id, - "client_secret": self.extension_client.client_secret, - "grant_type": "password" - } - url, body, ht_args, csi = self.extension_client.request_info(ROPCAccessTokenRequest, - request_args=args, method="POST") - response = self.extension_client.request_and_return(url, AccessTokenResponse, "POST", body, "json", "", ht_args) - if isinstance(response, AccessTokenResponse): - return response - return None - - - def get_user_info(self, token): - return self.client.do_user_info_request( - access_token=token['access_token']) - - def check_validity(self, token): - if 'exp' in token['id_token'] and token['id_token']['exp'] > time.time(): - return token - else: - return self.refresh_token(token) - - def refresh_token(self, token): - try: - args = { - "refresh_token": token['refresh_token'] - } - response = self.client.do_access_token_refresh(request_args=args, token=Token(token)) - if isinstance(response, AccessTokenResponse): - return response - else: - return None - except Exception as e: - print(e) - return None - - def logout(self, id_token): - state = rndstr() - flask.session['state'] = state - - args = { - "state": state, - "id_token_hint": id_token, - "post_logout_redirect_uri": self.redirect_url + "/sso/logout", - "client_id": self.client.client_id - } - - request = self.client.construct_EndSessionRequest(request_args=args) - uri, body, h_args, cis = self.client.uri_and_body(EndSessionRequest, method="GET", request_args=args, cis=request) - return uri - - def backchannel_logout(self, body): - # TODO: Finish backchannel logout implementation - req = BackChannelLogoutRequest().from_dict(body) - - kwargs = {"aud": self.client.client_id, "iss": self.client.issuer, "keyjar": self.client.keyjar} - - try: - req.verify(**kwargs) - except (MessageException, ValueError, NotForMe) as err: - self.app.logger.error(err) - return False - - sub = req["logout_token"]["sub"] - - if sub is not None and sub != '': - MailuSessionExtension.prune_sessions(None, None, self.app, sub) - - return True - - def is_enabled(self): - return self.app is not None and self.app.config['OIDC_ENABLED'] - - def change_password(self): - return self.change_password_url if self.change_password_redirect_enabled else None - -oic_client = OicClient() +# OIDC client +oic_client = oidc.OicClient() # Data migrate migrate = flask_migrate.Migrate() From 61f75849dadc2385ef59bb2ec4d7076ffb78eac7 Mon Sep 17 00:00:00 2001 From: Wombosvideo Date: Sun, 9 Feb 2025 20:46:06 +0100 Subject: [PATCH 3/3] Add external link icon for OIDC users to password update link in sidebar --- core/admin/mailu/ui/templates/sidebar.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/admin/mailu/ui/templates/sidebar.html b/core/admin/mailu/ui/templates/sidebar.html index 089b7cfe..f52a07c2 100644 --- a/core/admin/mailu/ui/templates/sidebar.html +++ b/core/admin/mailu/ui/templates/sidebar.html @@ -24,7 +24,7 @@ {%- endif %}