From 545fb2d342ed75e0eb6d6f6c03bab4bad90cc945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Rohrlich?= Date: Wed, 9 Oct 2024 15:55:01 +0200 Subject: [PATCH 1/3] feat: change password events are logged in #177 --- canaille/core/endpoints/account.py | 4 ++++ demo/.gitignore | 1 + tests/core/test_profile_settings.py | 10 +++++++++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/canaille/core/endpoints/account.py b/canaille/core/endpoints/account.py index 637c99ce..f1e57569 100644 --- a/canaille/core/endpoints/account.py +++ b/canaille/core/endpoints/account.py @@ -744,6 +744,7 @@ def profile_settings(user, edited_user): def profile_settings_edit(editor, edited_user): menuitem = "profile" if editor.id == editor.id else "users" fields = editor.readable_fields | editor.writable_fields + request_ip = request.remote_addr or "unknown IP" available_fields = {"password", "groups", "user_name", "lock_date"} data = { @@ -781,6 +782,9 @@ def profile_settings_edit(editor, edited_user): and request.form["action"] == "edit-settings" ): Backend.instance.set_user_password(edited_user, form["password1"].data) + current_app.logger.info( + f'Changed password in settings for {edited_user.user_name} from {request_ip}' + ) Backend.instance.save(edited_user) flash(_("Profile updated successfully."), "success") diff --git a/demo/.gitignore b/demo/.gitignore index 694f1b64..48764a78 100644 --- a/demo/.gitignore +++ b/demo/.gitignore @@ -1,2 +1,3 @@ env *.pem +var/ \ No newline at end of file diff --git a/tests/core/test_profile_settings.py b/tests/core/test_profile_settings.py index cb802149..1d604c0b 100644 --- a/tests/core/test_profile_settings.py +++ b/tests/core/test_profile_settings.py @@ -1,4 +1,5 @@ import datetime +import logging from unittest import mock from flask import g @@ -118,7 +119,7 @@ def test_edition_without_groups( backend.save(logged_user) -def test_password_change(testclient, logged_user, backend): +def test_password_change(testclient, logged_user, backend, caplog): res = testclient.get("/profile/user/settings", status=200) res.form["password1"] = "new_password" @@ -136,6 +137,13 @@ def test_password_change(testclient, logged_user, backend): res = res.form.submit(name="action", value="edit-settings") assert ("success", "Profile updated successfully.") in res.flashes + + assert ( + "canaille", + logging.INFO, + "Changed password in settings for user from unknown IP", + ) in caplog.record_tuples + res = res.follow() backend.reload(logged_user) From 038e6c094ebc90b6d55890c27310d15fdabd4ecf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Rohrlich?= Date: Mon, 14 Oct 2024 14:04:39 +0200 Subject: [PATCH 2/3] feat: Added security logs for email update, forgotten password mail, token emission/refresh/revokation, new consent, consent revokation #177 --- CHANGES.rst | 4 ++++ canaille/app/__init__.py | 4 ++++ canaille/core/endpoints/account.py | 28 +++++++++++++++++++++- canaille/core/endpoints/auth.py | 25 +++++++++++++++---- canaille/oidc/endpoints/consents.py | 9 +++++++ canaille/oidc/endpoints/oauth.py | 24 ++++++++++++++++--- canaille/oidc/endpoints/tokens.py | 8 +++++++ doc/features.rst | 12 ++++++++++ tests/core/test_auth.py | 8 ++++--- tests/core/test_forgotten_password.py | 22 +++++++++++++++-- tests/core/test_profile_edition.py | 16 +++++++------ tests/core/test_profile_settings.py | 3 ++- tests/oidc/test_authorization_code_flow.py | 20 +++++++++++++--- tests/oidc/test_consent.py | 11 ++++++++- tests/oidc/test_refresh_token.py | 12 ++++++++-- tests/oidc/test_token_admin.py | 11 ++++++++- 16 files changed, 188 insertions(+), 29 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 862503d9..258cd41d 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,10 @@ [0.0.56] - Unreleased --------------------- +Added +^^^^^ +- New security events logs :issue:`177` + Changed ^^^^^^^ - Update to HTMX 2.0.3 :pr:`184` diff --git a/canaille/app/__init__.py b/canaille/app/__init__.py index dc1a3c32..c71af59e 100644 --- a/canaille/app/__init__.py +++ b/canaille/app/__init__.py @@ -66,3 +66,7 @@ class classproperty: def __get__(self, obj, owner): return self.f(owner) + + +def generate_security_log(message): + return "[SECURITY] " + message diff --git a/canaille/core/endpoints/account.py b/canaille/core/endpoints/account.py index f1e57569..5327c8c9 100644 --- a/canaille/core/endpoints/account.py +++ b/canaille/core/endpoints/account.py @@ -23,6 +23,7 @@ from werkzeug.datastructures import FileStorage from canaille.app import b64_to_obj from canaille.app import build_hash from canaille.app import default_fields +from canaille.app import generate_security_log from canaille.app import models from canaille.app import obj_to_b64 from canaille.app.flask import current_user @@ -575,6 +576,17 @@ def profile_edition_remove_email(user, edited_user, email): return True +def has_email_changed(old_emails): + emailstr = "emails-" + i = 0 + new_emails = set() + while request.form.get(emailstr + str(i)): + new_emails.add(request.form.get(emailstr + str(i))) + i += 1 + + return set(old_emails) != new_emails + + @bp.route("/profile/", methods=("GET", "POST")) @user_needed() def profile_edition(user, edited_user): @@ -583,6 +595,9 @@ def profile_edition(user, edited_user): ): abort(404) + is_email_modified = has_email_changed(user.emails) + + request_ip = request.remote_addr or "unknown IP" menuitem = "profile" if edited_user.id == user.id else "users" emails_readonly = ( current_app.features.has_email_confirmation and not user.can_manage_users @@ -611,7 +626,16 @@ def profile_edition(user, edited_user): return render_template("profile_edit.html", **render_context) profile_edition_main_form_validation(user, edited_user, profile_form) + + if is_email_modified: + current_app.logger.info( + generate_security_log( + f"Updated email for {edited_user.user_name} from {request_ip}" + ) + ) + flash(_("Profile updated successfully."), "success") + return redirect( url_for("core.account.profile_edition", edited_user=edited_user) ) @@ -783,7 +807,9 @@ def profile_settings_edit(editor, edited_user): ): Backend.instance.set_user_password(edited_user, form["password1"].data) current_app.logger.info( - f'Changed password in settings for {edited_user.user_name} from {request_ip}' + generate_security_log( + f"Changed password in settings for {edited_user.user_name} from {request_ip}" + ) ) Backend.instance.save(edited_user) diff --git a/canaille/core/endpoints/auth.py b/canaille/core/endpoints/auth.py index e716af1a..0190d33f 100644 --- a/canaille/core/endpoints/auth.py +++ b/canaille/core/endpoints/auth.py @@ -8,6 +8,7 @@ from flask import session from flask import url_for from canaille.app import build_hash +from canaille.app import generate_security_log from canaille.app.flask import current_user from canaille.app.flask import login_user from canaille.app.flask import logout_user @@ -96,7 +97,9 @@ def password(): if not success: logout_user() current_app.logger.info( - f'Failed login attempt for {session["attempt_login"]} from {request_ip}' + generate_security_log( + f'Failed login attempt for {session["attempt_login"]} from {request_ip}' + ) ) flash(message or _("Login failed, please check your information"), "error") return render_template( @@ -104,7 +107,9 @@ def password(): ) current_app.logger.info( - f'Succeed login attempt for {session["attempt_login"]} from {request_ip}' + generate_security_log( + f'Succeed login attempt for {session["attempt_login"]} from {request_ip}' + ) ) del session["attempt_login"] login_user(user) @@ -121,7 +126,9 @@ def logout(): if user: request_ip = request.remote_addr or "unknown IP" - current_app.logger.info(f"Logout {user.identifier} from {request_ip}") + current_app.logger.info( + generate_security_log(f"Logout {user.identifier} from {request_ip}") + ) flash( _( @@ -197,8 +204,16 @@ def forgotten(): ) return render_template("forgotten-password.html", form=form) - statuses = [send_password_reset_mail(user, email) for email in user.emails] - success = all(statuses) + request_ip = request.remote_addr or "unknown IP" + success = True + for email in user.emails: + if not send_password_reset_mail(user, email): + success = False + current_app.logger.info( + generate_security_log( + f"Sending a reset password mail to {email} for {user.user_name} from {request_ip}" + ) + ) if success: flash(success_message, "success") diff --git a/canaille/oidc/endpoints/consents.py b/canaille/oidc/endpoints/consents.py index 4610be60..5895316c 100644 --- a/canaille/oidc/endpoints/consents.py +++ b/canaille/oidc/endpoints/consents.py @@ -2,10 +2,13 @@ import datetime import uuid from flask import Blueprint +from flask import current_app from flask import flash from flask import redirect +from flask import request from flask import url_for +from canaille.app import generate_security_log from canaille.app import models from canaille.app.flask import user_needed from canaille.app.i18n import gettext as _ @@ -77,6 +80,12 @@ def revoke(user, consent): else: consent.revoke() + request_ip = request.remote_addr or "unknown IP" + current_app.logger.info( + generate_security_log( + f"Consent revoked for {user.user_name} in client {consent.client.client_name} from {request_ip}" + ) + ) flash(_("The access has been revoked"), "success") return redirect(url_for("oidc.consents.consents")) diff --git a/canaille/oidc/endpoints/oauth.py b/canaille/oidc/endpoints/oauth.py index 8ca2e8e9..4449bcf8 100644 --- a/canaille/oidc/endpoints/oauth.py +++ b/canaille/oidc/endpoints/oauth.py @@ -17,6 +17,7 @@ from flask import url_for from werkzeug.datastructures import CombinedMultiDict from canaille import csrf +from canaille.app import generate_security_log from canaille.app import models from canaille.app.flask import current_user from canaille.app.flask import logout_user @@ -178,6 +179,12 @@ def authorize_consent(client, user): issue_date=datetime.datetime.now(datetime.timezone.utc), ) Backend.instance.save(consent) + request_ip = request.remote_addr or "unknown IP" + current_app.logger.info( + generate_security_log( + f"New consent for {user.user_name} in client {consent.client.client_name} from {request_ip}" + ) + ) response = authorization.create_authorization_response(grant_user=grant_user) current_app.logger.debug("authorization endpoint response: %s", response.location) @@ -187,11 +194,22 @@ def authorize_consent(client, user): @bp.route("/token", methods=["POST"]) @csrf.exempt def issue_token(): - current_app.logger.debug( - "token endpoint request: POST: %s", request.form.to_dict(flat=False) - ) + request_params = request.form.to_dict(flat=False) + print(request_params["grant_type"]) + grant_type = request_params["grant_type"][0] + current_app.logger.debug("token endpoint request: POST: %s", request_params) response = authorization.create_token_response() current_app.logger.debug("token endpoint response: %s", response.json) + + if response.json.get("access_token"): + access_token = response.json["access_token"] + token = Backend.instance.get(models.Token, access_token=access_token) + request_ip = request.remote_addr or "unknown IP" + current_app.logger.info( + generate_security_log( + f"Issued {grant_type} token for {token.subject.user_name} in client {token.client.client_name} from {request_ip}" + ) + ) return response diff --git a/canaille/oidc/endpoints/tokens.py b/canaille/oidc/endpoints/tokens.py index e73951ca..e91a122b 100644 --- a/canaille/oidc/endpoints/tokens.py +++ b/canaille/oidc/endpoints/tokens.py @@ -2,9 +2,11 @@ import datetime from flask import Blueprint from flask import abort +from flask import current_app from flask import flash from flask import request +from canaille.app import generate_security_log from canaille.app import models from canaille.app.flask import permissions_needed from canaille.app.flask import render_htmx_template @@ -42,6 +44,12 @@ def view(user, token): elif request.form.get("action") == "revoke": token.revokation_date = datetime.datetime.now(datetime.timezone.utc) Backend.instance.save(token) + request_ip = request.remote_addr or "unknown IP" + current_app.logger.info( + generate_security_log( + f"Revoked token for {token.subject.user_name} in client {token.client.client_name} by {user.user_name} from {request_ip}" + ) + ) flash(_("The token has successfully been revoked."), "success") else: diff --git a/doc/features.rst b/doc/features.rst index d819e9f3..213e139a 100644 --- a/doc/features.rst +++ b/doc/features.rst @@ -263,6 +263,18 @@ Logging Canaille writes :attr:`logs ` for every important event happening, to help administrators understand what is going on and debug funky situations. +The following security events are logged with the tag [SECURITY] for easy retrieval : + +- Authentication attempt +- Password update +- Email update +- Forgotten password mail sent to user +- Token emission +- Token refresh +- Token revokation +- New consent given for client application +- Consent revokation + .. _feature_development: A tool for your development and tests diff --git a/tests/core/test_auth.py b/tests/core/test_auth.py index b44aaf35..107cd92b 100644 --- a/tests/core/test_auth.py +++ b/tests/core/test_auth.py @@ -1,6 +1,8 @@ import datetime import logging +from canaille.app import generate_security_log + def test_signin_and_out(testclient, user, caplog): with testclient.session_transaction() as session: @@ -25,7 +27,7 @@ def test_signin_and_out(testclient, user, caplog): assert ( "canaille", logging.INFO, - "Succeed login attempt for user from unknown IP", + generate_security_log("Succeed login attempt for user from unknown IP"), ) in caplog.record_tuples res = res.follow(status=302) res = res.follow(status=200) @@ -44,7 +46,7 @@ def test_signin_and_out(testclient, user, caplog): assert ( "canaille", logging.INFO, - "Logout user from unknown IP", + generate_security_log("Logout user from unknown IP"), ) in caplog.record_tuples res = res.follow(status=302) res = res.follow(status=200) @@ -81,7 +83,7 @@ def test_signin_wrong_password(testclient, user, caplog): assert ( "canaille", logging.INFO, - "Failed login attempt for user from unknown IP", + generate_security_log("Failed login attempt for user from unknown IP"), ) in caplog.record_tuples diff --git a/tests/core/test_forgotten_password.py b/tests/core/test_forgotten_password.py index 213e6f8d..aaec88c1 100644 --- a/tests/core/test_forgotten_password.py +++ b/tests/core/test_forgotten_password.py @@ -1,5 +1,8 @@ +import logging from unittest import mock +from canaille.app import generate_security_log + def test_password_forgotten_disabled(smtpd, testclient, user): testclient.app.config["CANAILLE"]["ENABLE_PASSWORD_RECOVERY"] = False @@ -11,7 +14,7 @@ def test_password_forgotten_disabled(smtpd, testclient, user): res.mustcontain(no="Forgotten password") -def test_password_forgotten(smtpd, testclient, user): +def test_password_forgotten(smtpd, testclient, user, caplog): res = testclient.get("/reset", status=200) res.form["login"] = "user" @@ -21,12 +24,19 @@ def test_password_forgotten(smtpd, testclient, user): "A password reset link has been sent at your email address. You should receive " "it within a few minutes.", ) in res.flashes + assert ( + "canaille", + logging.INFO, + generate_security_log( + "Sending a reset password mail to john@doe.com for user from unknown IP" + ), + ) in caplog.record_tuples res.mustcontain("Send again") assert len(smtpd.messages) == 1 -def test_password_forgotten_multiple_mails(smtpd, testclient, user, backend): +def test_password_forgotten_multiple_mails(smtpd, testclient, user, backend, caplog): user.emails = ["foo@bar.com", "foo@baz.com", "foo@foo.com"] backend.save(user) @@ -39,6 +49,14 @@ def test_password_forgotten_multiple_mails(smtpd, testclient, user, backend): "A password reset link has been sent at your email address. You should receive " "it within a few minutes.", ) in res.flashes + for email in user.emails: + assert ( + "canaille", + logging.INFO, + generate_security_log( + f"Sending a reset password mail to {email} for user from unknown IP" + ), + ) in caplog.record_tuples res.mustcontain("Send again") assert len(smtpd.messages) == 3 diff --git a/tests/core/test_profile_edition.py b/tests/core/test_profile_edition.py index d311bc52..9b0ef383 100644 --- a/tests/core/test_profile_edition.py +++ b/tests/core/test_profile_edition.py @@ -1,7 +1,10 @@ +import logging + import pytest from flask import g from webtest import Upload +from canaille.app import generate_security_log from canaille.core.populate import fake_users @@ -101,13 +104,7 @@ def test_edition_permission( testclient.get("/profile/user", status=200) -def test_edition( - testclient, - logged_user, - admin, - jpeg_photo, - backend, -): +def test_edition(testclient, logged_user, admin, jpeg_photo, backend, caplog): res = testclient.get("/profile/user", status=200) form = res.forms["baseform"] form["given_name"] = "given_name" @@ -131,6 +128,11 @@ def test_edition( assert res.flashes == [ ("success", "Le profil a été mis à jour avec succès.") ], res.text + assert ( + "canaille", + logging.INFO, + generate_security_log("Updated email for user from unknown IP"), + ) in caplog.record_tuples res = res.follow() backend.reload(logged_user) diff --git a/tests/core/test_profile_settings.py b/tests/core/test_profile_settings.py index 1d604c0b..27ca0ac4 100644 --- a/tests/core/test_profile_settings.py +++ b/tests/core/test_profile_settings.py @@ -4,6 +4,7 @@ from unittest import mock from flask import g +from canaille.app import generate_security_log from canaille.app import models @@ -141,7 +142,7 @@ def test_password_change(testclient, logged_user, backend, caplog): assert ( "canaille", logging.INFO, - "Changed password in settings for user from unknown IP", + generate_security_log("Changed password in settings for user from unknown IP"), ) in caplog.record_tuples res = res.follow() diff --git a/tests/oidc/test_authorization_code_flow.py b/tests/oidc/test_authorization_code_flow.py index c4a28a49..0be6a917 100644 --- a/tests/oidc/test_authorization_code_flow.py +++ b/tests/oidc/test_authorization_code_flow.py @@ -1,4 +1,5 @@ import datetime +import logging from urllib.parse import parse_qs from urllib.parse import urlsplit @@ -8,13 +9,14 @@ from authlib.oauth2.rfc7636 import create_s256_code_challenge from flask import g from werkzeug.security import gen_salt +from canaille.app import generate_security_log from canaille.app import models from . import client_credentials def test_nominal_case( - testclient, logged_user, client, keypair, trusted_client, backend + testclient, logged_user, client, keypair, trusted_client, backend, caplog ): assert not backend.query(models.Consent) @@ -28,8 +30,14 @@ def test_nominal_case( ), status=200, ) - res = res.form.submit(name="answer", value="accept", status=302) + assert ( + "canaille", + logging.INFO, + generate_security_log( + "New consent for user in client Some client from unknown IP" + ), + ) in caplog.record_tuples assert res.location.startswith(client.redirect_uris[0]) params = parse_qs(urlsplit(res.location).query) @@ -89,7 +97,13 @@ def test_nominal_case( assert claims["sub"] == logged_user.user_name assert claims["name"] == logged_user.formatted_name assert claims["aud"] == [client.client_id, trusted_client.client_id] - + assert ( + "canaille", + logging.INFO, + generate_security_log( + "Issued authorization_code token for user in client Some client from unknown IP" + ), + ) in caplog.record_tuples res = testclient.get( "/oauth/userinfo", headers={"Authorization": f"Bearer {access_token}"}, diff --git a/tests/oidc/test_consent.py b/tests/oidc/test_consent.py index f3d1cce1..eda35f25 100644 --- a/tests/oidc/test_consent.py +++ b/tests/oidc/test_consent.py @@ -1,6 +1,8 @@ +import logging from urllib.parse import parse_qs from urllib.parse import urlsplit +from canaille.app import generate_security_log from canaille.app import models from . import client_credentials @@ -10,7 +12,7 @@ def test_no_logged_no_access(testclient): testclient.get("/consent", status=403) -def test_revokation(testclient, client, consent, logged_user, token, backend): +def test_revokation(testclient, client, consent, logged_user, token, backend, caplog): res = testclient.get("/consent", status=200) res.mustcontain(client.client_name) res.mustcontain("Revoke access") @@ -20,6 +22,13 @@ def test_revokation(testclient, client, consent, logged_user, token, backend): res = testclient.get(f"/consent/revoke/{consent.consent_id}", status=302) assert ("success", "The access has been revoked") in res.flashes + assert ( + "canaille", + logging.INFO, + generate_security_log( + f"Consent revoked for {logged_user.user_name} in client {client.client_name} from unknown IP" + ), + ) in caplog.record_tuples res = res.follow(status=200) res.mustcontain(no="Revoke access") res.mustcontain("Restore access") diff --git a/tests/oidc/test_refresh_token.py b/tests/oidc/test_refresh_token.py index 6f693c0c..0a02090f 100644 --- a/tests/oidc/test_refresh_token.py +++ b/tests/oidc/test_refresh_token.py @@ -1,13 +1,15 @@ import datetime +import logging from urllib.parse import parse_qs from urllib.parse import urlsplit +from canaille.app import generate_security_log from canaille.app import models from . import client_credentials -def test_refresh_token(testclient, logged_user, client, backend): +def test_refresh_token(testclient, logged_user, client, backend, caplog): assert not backend.query(models.Consent) res = testclient.get( @@ -59,7 +61,13 @@ def test_refresh_token(testclient, logged_user, client, backend): new_token = backend.get(models.Token, access_token=access_token) assert new_token is not None assert old_token.access_token != new_token.access_token - + assert ( + "canaille", + logging.INFO, + generate_security_log( + "Issued refresh_token token for user in client Some client from unknown IP" + ), + ) in caplog.record_tuples backend.reload(old_token) assert old_token.revokation_date diff --git a/tests/oidc/test_token_admin.py b/tests/oidc/test_token_admin.py index f25e00bf..2c35e693 100644 --- a/tests/oidc/test_token_admin.py +++ b/tests/oidc/test_token_admin.py @@ -1,7 +1,9 @@ import datetime +import logging from werkzeug.security import gen_salt +from canaille.app import generate_security_log from canaille.app import models @@ -134,13 +136,20 @@ def test_revoke_bad_request(testclient, token, logged_admin): res = res.form.submit(name="action", value="invalid", status=400) -def test_revoke_token(testclient, token, logged_admin, backend): +def test_revoke_token(testclient, token, logged_admin, backend, caplog): assert not token.revoked res = testclient.get(f"/admin/token/{token.token_id}") res = res.form.submit(name="action", value="confirm-revoke") res = res.form.submit(name="action", value="revoke") assert ("success", "The token has successfully been revoked.") in res.flashes + assert ( + "canaille", + logging.INFO, + generate_security_log( + "Revoked token for user in client Some client by admin from unknown IP" + ), + ) in caplog.record_tuples backend.reload(token) assert token.revoked From 603eab0b3c93ca45c09fe2dc0648368e15066d3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Rohrlich?= Date: Mon, 21 Oct 2024 11:17:55 +0200 Subject: [PATCH 3/3] refactor : added proper security logging level and refactored change email logging --- canaille/__init__.py | 55 ++++++++++++++++++++++ canaille/app/__init__.py | 4 -- canaille/core/endpoints/account.py | 32 ++++--------- canaille/core/endpoints/auth.py | 23 +++------ canaille/oidc/endpoints/consents.py | 7 +-- canaille/oidc/endpoints/oauth.py | 14 ++---- canaille/oidc/endpoints/tokens.py | 7 +-- tests/core/test_auth.py | 14 +++--- tests/core/test_forgotten_password.py | 14 ++---- tests/core/test_profile_edition.py | 5 +- tests/core/test_profile_settings.py | 5 +- tests/oidc/test_authorization_code_flow.py | 13 ++--- tests/oidc/test_consent.py | 7 +-- tests/oidc/test_refresh_token.py | 7 +-- tests/oidc/test_token_admin.py | 7 +-- 15 files changed, 103 insertions(+), 111 deletions(-) diff --git a/canaille/__init__.py b/canaille/__init__.py index e4dc4021..5bf5cee5 100644 --- a/canaille/__init__.py +++ b/canaille/__init__.py @@ -1,4 +1,5 @@ import datetime +import logging import sys from logging.config import dictConfig from logging.config import fileConfig @@ -30,6 +31,11 @@ def setup_sentry(app): # pragma: no cover def setup_logging(app): conf = app.config["CANAILLE"]["LOGGING"] + + security_level_name = "SECURITY" + if not hasattr(logging, security_level_name): + addLoggingLevel(security_level_name, logging.INFO + 5) + if conf is None: log_level = "DEBUG" if app.debug else "INFO" dictConfig( @@ -157,3 +163,52 @@ def create_app( raise return app + + +def addLoggingLevel(levelName, levelNum, methodName=None): + """Comprehensively adds a new logging level to the `logging` module and the + currently configured logging class. + + `levelName` becomes an attribute of the `logging` module with the value + `levelNum`. `methodName` becomes a convenience method for both `logging` + itself and the class returned by `logging.getLoggerClass()` (usually just + `logging.Logger`). If `methodName` is not specified, `levelName.lower()` is + used. + + To avoid accidental clobberings of existing attributes, this method will + raise an `AttributeError` if the level name is already an attribute of the + `logging` module or if the method name is already present + + Example + ------- + >>> addLoggingLevel("TRACE", logging.DEBUG - 5) + >>> logging.getLogger(__name__).setLevel("TRACE") + >>> logging.getLogger(__name__).trace("that worked") + >>> logging.trace("so did this") + >>> logging.TRACE + 5 + """ + if not methodName: + methodName = levelName.lower() + + if hasattr(logging, levelName): + raise AttributeError(f"{levelName} already defined in logging module") + if hasattr(logging, methodName): + raise AttributeError(f"{methodName} already defined in logging module") + if hasattr(logging.getLoggerClass(), methodName): + raise AttributeError(f"{methodName} already defined in logger class") + + # This method was inspired by the answers to Stack Overflow post + # http://stackoverflow.com/q/2183233/2988730, especially + # http://stackoverflow.com/a/13638084/2988730 + def logForLevel(self, message, *args, **kwargs): + if self.isEnabledFor(levelNum): + self._log(levelNum, message, args, **kwargs) + + def logToRoot(message, *args, **kwargs): + logging.log(levelNum, message, *args, **kwargs) + + logging.addLevelName(levelNum, levelName) + setattr(logging, levelName, levelNum) + setattr(logging.getLoggerClass(), methodName, logForLevel) + setattr(logging, methodName, logToRoot) diff --git a/canaille/app/__init__.py b/canaille/app/__init__.py index c71af59e..dc1a3c32 100644 --- a/canaille/app/__init__.py +++ b/canaille/app/__init__.py @@ -66,7 +66,3 @@ class classproperty: def __get__(self, obj, owner): return self.f(owner) - - -def generate_security_log(message): - return "[SECURITY] " + message diff --git a/canaille/core/endpoints/account.py b/canaille/core/endpoints/account.py index 5327c8c9..d020f8e3 100644 --- a/canaille/core/endpoints/account.py +++ b/canaille/core/endpoints/account.py @@ -23,7 +23,6 @@ from werkzeug.datastructures import FileStorage from canaille.app import b64_to_obj from canaille.app import build_hash from canaille.app import default_fields -from canaille.app import generate_security_log from canaille.app import models from canaille.app import obj_to_b64 from canaille.app.flask import current_user @@ -576,17 +575,6 @@ def profile_edition_remove_email(user, edited_user, email): return True -def has_email_changed(old_emails): - emailstr = "emails-" - i = 0 - new_emails = set() - while request.form.get(emailstr + str(i)): - new_emails.add(request.form.get(emailstr + str(i))) - i += 1 - - return set(old_emails) != new_emails - - @bp.route("/profile/", methods=("GET", "POST")) @user_needed() def profile_edition(user, edited_user): @@ -595,8 +583,6 @@ def profile_edition(user, edited_user): ): abort(404) - is_email_modified = has_email_changed(user.emails) - request_ip = request.remote_addr or "unknown IP" menuitem = "profile" if edited_user.id == user.id else "users" emails_readonly = ( @@ -610,6 +596,10 @@ def profile_edition(user, edited_user): else None ) + has_email_changed = "emails" in profile_form and set( + profile_form["emails"].data + ) != set(user.emails) + render_context = { "menuitem": menuitem, "edited_user": edited_user, @@ -627,11 +617,9 @@ def profile_edition(user, edited_user): profile_edition_main_form_validation(user, edited_user, profile_form) - if is_email_modified: - current_app.logger.info( - generate_security_log( - f"Updated email for {edited_user.user_name} from {request_ip}" - ) + if has_email_changed: + current_app.logger.security( + f"Updated email for {edited_user.user_name} from {request_ip}" ) flash(_("Profile updated successfully."), "success") @@ -806,10 +794,8 @@ def profile_settings_edit(editor, edited_user): and request.form["action"] == "edit-settings" ): Backend.instance.set_user_password(edited_user, form["password1"].data) - current_app.logger.info( - generate_security_log( - f"Changed password in settings for {edited_user.user_name} from {request_ip}" - ) + current_app.logger.security( + f"Changed password in settings for {edited_user.user_name} from {request_ip}" ) Backend.instance.save(edited_user) diff --git a/canaille/core/endpoints/auth.py b/canaille/core/endpoints/auth.py index 0190d33f..9891755a 100644 --- a/canaille/core/endpoints/auth.py +++ b/canaille/core/endpoints/auth.py @@ -8,7 +8,6 @@ from flask import session from flask import url_for from canaille.app import build_hash -from canaille.app import generate_security_log from canaille.app.flask import current_user from canaille.app.flask import login_user from canaille.app.flask import logout_user @@ -96,20 +95,16 @@ def password(): request_ip = request.remote_addr or "unknown IP" if not success: logout_user() - current_app.logger.info( - generate_security_log( - f'Failed login attempt for {session["attempt_login"]} from {request_ip}' - ) + current_app.logger.security( + f'Failed login attempt for {session["attempt_login"]} from {request_ip}' ) flash(message or _("Login failed, please check your information"), "error") return render_template( "password.html", form=form, username=session["attempt_login"] ) - current_app.logger.info( - generate_security_log( - f'Succeed login attempt for {session["attempt_login"]} from {request_ip}' - ) + current_app.logger.security( + f'Succeed login attempt for {session["attempt_login"]} from {request_ip}' ) del session["attempt_login"] login_user(user) @@ -126,9 +121,7 @@ def logout(): if user: request_ip = request.remote_addr or "unknown IP" - current_app.logger.info( - generate_security_log(f"Logout {user.identifier} from {request_ip}") - ) + current_app.logger.security(f"Logout {user.identifier} from {request_ip}") flash( _( @@ -209,10 +202,8 @@ def forgotten(): for email in user.emails: if not send_password_reset_mail(user, email): success = False - current_app.logger.info( - generate_security_log( - f"Sending a reset password mail to {email} for {user.user_name} from {request_ip}" - ) + current_app.logger.security( + f"Sending a reset password mail to {email} for {user.user_name} from {request_ip}" ) if success: diff --git a/canaille/oidc/endpoints/consents.py b/canaille/oidc/endpoints/consents.py index 5895316c..a9976214 100644 --- a/canaille/oidc/endpoints/consents.py +++ b/canaille/oidc/endpoints/consents.py @@ -8,7 +8,6 @@ from flask import redirect from flask import request from flask import url_for -from canaille.app import generate_security_log from canaille.app import models from canaille.app.flask import user_needed from canaille.app.i18n import gettext as _ @@ -81,10 +80,8 @@ def revoke(user, consent): else: consent.revoke() request_ip = request.remote_addr or "unknown IP" - current_app.logger.info( - generate_security_log( - f"Consent revoked for {user.user_name} in client {consent.client.client_name} from {request_ip}" - ) + current_app.logger.security( + f"Consent revoked for {user.user_name} in client {consent.client.client_name} from {request_ip}" ) flash(_("The access has been revoked"), "success") diff --git a/canaille/oidc/endpoints/oauth.py b/canaille/oidc/endpoints/oauth.py index 4449bcf8..a4d63f5e 100644 --- a/canaille/oidc/endpoints/oauth.py +++ b/canaille/oidc/endpoints/oauth.py @@ -17,7 +17,6 @@ from flask import url_for from werkzeug.datastructures import CombinedMultiDict from canaille import csrf -from canaille.app import generate_security_log from canaille.app import models from canaille.app.flask import current_user from canaille.app.flask import logout_user @@ -180,10 +179,8 @@ def authorize_consent(client, user): ) Backend.instance.save(consent) request_ip = request.remote_addr or "unknown IP" - current_app.logger.info( - generate_security_log( - f"New consent for {user.user_name} in client {consent.client.client_name} from {request_ip}" - ) + current_app.logger.security( + f"New consent for {user.user_name} in client {consent.client.client_name} from {request_ip}" ) response = authorization.create_authorization_response(grant_user=grant_user) @@ -195,7 +192,6 @@ def authorize_consent(client, user): @csrf.exempt def issue_token(): request_params = request.form.to_dict(flat=False) - print(request_params["grant_type"]) grant_type = request_params["grant_type"][0] current_app.logger.debug("token endpoint request: POST: %s", request_params) response = authorization.create_token_response() @@ -205,10 +201,8 @@ def issue_token(): access_token = response.json["access_token"] token = Backend.instance.get(models.Token, access_token=access_token) request_ip = request.remote_addr or "unknown IP" - current_app.logger.info( - generate_security_log( - f"Issued {grant_type} token for {token.subject.user_name} in client {token.client.client_name} from {request_ip}" - ) + current_app.logger.security( + f"Issued {grant_type} token for {token.subject.user_name} in client {token.client.client_name} from {request_ip}" ) return response diff --git a/canaille/oidc/endpoints/tokens.py b/canaille/oidc/endpoints/tokens.py index e91a122b..543e4ace 100644 --- a/canaille/oidc/endpoints/tokens.py +++ b/canaille/oidc/endpoints/tokens.py @@ -6,7 +6,6 @@ from flask import current_app from flask import flash from flask import request -from canaille.app import generate_security_log from canaille.app import models from canaille.app.flask import permissions_needed from canaille.app.flask import render_htmx_template @@ -45,10 +44,8 @@ def view(user, token): token.revokation_date = datetime.datetime.now(datetime.timezone.utc) Backend.instance.save(token) request_ip = request.remote_addr or "unknown IP" - current_app.logger.info( - generate_security_log( - f"Revoked token for {token.subject.user_name} in client {token.client.client_name} by {user.user_name} from {request_ip}" - ) + current_app.logger.security( + f"Revoked token for {token.subject.user_name} in client {token.client.client_name} by {user.user_name} from {request_ip}" ) flash(_("The token has successfully been revoked."), "success") diff --git a/tests/core/test_auth.py b/tests/core/test_auth.py index 107cd92b..86e460a7 100644 --- a/tests/core/test_auth.py +++ b/tests/core/test_auth.py @@ -1,8 +1,6 @@ import datetime import logging -from canaille.app import generate_security_log - def test_signin_and_out(testclient, user, caplog): with testclient.session_transaction() as session: @@ -26,8 +24,8 @@ def test_signin_and_out(testclient, user, caplog): ) in res.flashes assert ( "canaille", - logging.INFO, - generate_security_log("Succeed login attempt for user from unknown IP"), + logging.SECURITY, + "Succeed login attempt for user from unknown IP", ) in caplog.record_tuples res = res.follow(status=302) res = res.follow(status=200) @@ -45,8 +43,8 @@ def test_signin_and_out(testclient, user, caplog): ) in res.flashes assert ( "canaille", - logging.INFO, - generate_security_log("Logout user from unknown IP"), + logging.SECURITY, + "Logout user from unknown IP", ) in caplog.record_tuples res = res.follow(status=302) res = res.follow(status=200) @@ -82,8 +80,8 @@ def test_signin_wrong_password(testclient, user, caplog): assert ("error", "Login failed, please check your information") in res.flashes assert ( "canaille", - logging.INFO, - generate_security_log("Failed login attempt for user from unknown IP"), + logging.SECURITY, + "Failed login attempt for user from unknown IP", ) in caplog.record_tuples diff --git a/tests/core/test_forgotten_password.py b/tests/core/test_forgotten_password.py index aaec88c1..fdb189ec 100644 --- a/tests/core/test_forgotten_password.py +++ b/tests/core/test_forgotten_password.py @@ -1,8 +1,6 @@ import logging from unittest import mock -from canaille.app import generate_security_log - def test_password_forgotten_disabled(smtpd, testclient, user): testclient.app.config["CANAILLE"]["ENABLE_PASSWORD_RECOVERY"] = False @@ -26,10 +24,8 @@ def test_password_forgotten(smtpd, testclient, user, caplog): ) in res.flashes assert ( "canaille", - logging.INFO, - generate_security_log( - "Sending a reset password mail to john@doe.com for user from unknown IP" - ), + logging.SECURITY, + "Sending a reset password mail to john@doe.com for user from unknown IP", ) in caplog.record_tuples res.mustcontain("Send again") @@ -52,10 +48,8 @@ def test_password_forgotten_multiple_mails(smtpd, testclient, user, backend, cap for email in user.emails: assert ( "canaille", - logging.INFO, - generate_security_log( - f"Sending a reset password mail to {email} for user from unknown IP" - ), + logging.SECURITY, + f"Sending a reset password mail to {email} for user from unknown IP", ) in caplog.record_tuples res.mustcontain("Send again") diff --git a/tests/core/test_profile_edition.py b/tests/core/test_profile_edition.py index 9b0ef383..5bc720ca 100644 --- a/tests/core/test_profile_edition.py +++ b/tests/core/test_profile_edition.py @@ -4,7 +4,6 @@ import pytest from flask import g from webtest import Upload -from canaille.app import generate_security_log from canaille.core.populate import fake_users @@ -130,8 +129,8 @@ def test_edition(testclient, logged_user, admin, jpeg_photo, backend, caplog): ], res.text assert ( "canaille", - logging.INFO, - generate_security_log("Updated email for user from unknown IP"), + logging.SECURITY, + "Updated email for user from unknown IP", ) in caplog.record_tuples res = res.follow() diff --git a/tests/core/test_profile_settings.py b/tests/core/test_profile_settings.py index 27ca0ac4..d106b026 100644 --- a/tests/core/test_profile_settings.py +++ b/tests/core/test_profile_settings.py @@ -4,7 +4,6 @@ from unittest import mock from flask import g -from canaille.app import generate_security_log from canaille.app import models @@ -141,8 +140,8 @@ def test_password_change(testclient, logged_user, backend, caplog): assert ( "canaille", - logging.INFO, - generate_security_log("Changed password in settings for user from unknown IP"), + logging.SECURITY, + "Changed password in settings for user from unknown IP", ) in caplog.record_tuples res = res.follow() diff --git a/tests/oidc/test_authorization_code_flow.py b/tests/oidc/test_authorization_code_flow.py index 0be6a917..8eda77b9 100644 --- a/tests/oidc/test_authorization_code_flow.py +++ b/tests/oidc/test_authorization_code_flow.py @@ -9,7 +9,6 @@ from authlib.oauth2.rfc7636 import create_s256_code_challenge from flask import g from werkzeug.security import gen_salt -from canaille.app import generate_security_log from canaille.app import models from . import client_credentials @@ -33,10 +32,8 @@ def test_nominal_case( res = res.form.submit(name="answer", value="accept", status=302) assert ( "canaille", - logging.INFO, - generate_security_log( - "New consent for user in client Some client from unknown IP" - ), + logging.SECURITY, + "New consent for user in client Some client from unknown IP", ) in caplog.record_tuples assert res.location.startswith(client.redirect_uris[0]) @@ -99,10 +96,8 @@ def test_nominal_case( assert claims["aud"] == [client.client_id, trusted_client.client_id] assert ( "canaille", - logging.INFO, - generate_security_log( - "Issued authorization_code token for user in client Some client from unknown IP" - ), + logging.SECURITY, + "Issued authorization_code token for user in client Some client from unknown IP", ) in caplog.record_tuples res = testclient.get( "/oauth/userinfo", diff --git a/tests/oidc/test_consent.py b/tests/oidc/test_consent.py index eda35f25..33336f31 100644 --- a/tests/oidc/test_consent.py +++ b/tests/oidc/test_consent.py @@ -2,7 +2,6 @@ import logging from urllib.parse import parse_qs from urllib.parse import urlsplit -from canaille.app import generate_security_log from canaille.app import models from . import client_credentials @@ -24,10 +23,8 @@ def test_revokation(testclient, client, consent, logged_user, token, backend, ca assert ("success", "The access has been revoked") in res.flashes assert ( "canaille", - logging.INFO, - generate_security_log( - f"Consent revoked for {logged_user.user_name} in client {client.client_name} from unknown IP" - ), + logging.SECURITY, + f"Consent revoked for {logged_user.user_name} in client {client.client_name} from unknown IP", ) in caplog.record_tuples res = res.follow(status=200) res.mustcontain(no="Revoke access") diff --git a/tests/oidc/test_refresh_token.py b/tests/oidc/test_refresh_token.py index 0a02090f..be9f99dc 100644 --- a/tests/oidc/test_refresh_token.py +++ b/tests/oidc/test_refresh_token.py @@ -3,7 +3,6 @@ import logging from urllib.parse import parse_qs from urllib.parse import urlsplit -from canaille.app import generate_security_log from canaille.app import models from . import client_credentials @@ -63,10 +62,8 @@ def test_refresh_token(testclient, logged_user, client, backend, caplog): assert old_token.access_token != new_token.access_token assert ( "canaille", - logging.INFO, - generate_security_log( - "Issued refresh_token token for user in client Some client from unknown IP" - ), + logging.SECURITY, + "Issued refresh_token token for user in client Some client from unknown IP", ) in caplog.record_tuples backend.reload(old_token) assert old_token.revokation_date diff --git a/tests/oidc/test_token_admin.py b/tests/oidc/test_token_admin.py index 2c35e693..10834a8c 100644 --- a/tests/oidc/test_token_admin.py +++ b/tests/oidc/test_token_admin.py @@ -3,7 +3,6 @@ import logging from werkzeug.security import gen_salt -from canaille.app import generate_security_log from canaille.app import models @@ -145,10 +144,8 @@ def test_revoke_token(testclient, token, logged_admin, backend, caplog): assert ("success", "The token has successfully been revoked.") in res.flashes assert ( "canaille", - logging.INFO, - generate_security_log( - "Revoked token for user in client Some client by admin from unknown IP" - ), + logging.SECURITY, + "Revoked token for user in client Some client by admin from unknown IP", ) in caplog.record_tuples backend.reload(token)