From f635195c80e8c1a526161967a0e1383e176ed22c Mon Sep 17 00:00:00 2001 From: Petr Date: Tue, 10 Mar 2026 10:50:01 +0100 Subject: [PATCH] Add multi-domain support and full-email username generation - Support comma-separated domains in auth.allowed_domain config - Use full email as system username (user@domain.com -> user_domain_com) to avoid collisions with reserved names and across domains - Update both auth providers (google, email) for multi-domain display - Add tests for username generation and update email auth tests --- auth/email/provider.py | 20 ++++++---- auth/google/provider.py | 10 ++++- config/instance.yaml.example | 2 +- tests/test_email_auth.py | 14 +++++-- tests/test_username_generation.py | 51 ++++++++++++++++++++++++++ webapp/auth.py | 12 +++--- webapp/config.py | 8 +++- webapp/templates/login_magic_link.html | 6 +-- webapp/user_service.py | 24 +++++------- 9 files changed, 108 insertions(+), 39 deletions(-) create mode 100644 tests/test_username_generation.py diff --git a/auth/email/provider.py b/auth/email/provider.py index 1dae7e0..0d012a1 100644 --- a/auth/email/provider.py +++ b/auth/email/provider.py @@ -161,7 +161,7 @@ def login_email_form(): """Show email input form.""" return render_template( "login_magic_link.html", - allowed_domain=Config.ALLOWED_DOMAIN, + allowed_domains=Config.ALLOWED_DOMAINS, ) @@ -175,8 +175,9 @@ def send_magic_link(): return redirect(url_for("email_auth.login_email_form")) if not validate_email_domain(email): + domains_str = ", ".join(f"@{d}" for d in Config.ALLOWED_DOMAINS) flash( - f"Only @{Config.ALLOWED_DOMAIN} email addresses are allowed.", + f"Only {domains_str} email addresses are allowed.", "error", ) return redirect(url_for("email_auth.login_email_form")) @@ -248,21 +249,26 @@ class EmailAuthProvider(AuthProvider): return email_bp def get_login_button(self) -> dict: - domain = Config.ALLOWED_DOMAIN - subtitle = f'For @{domain} email addresses.' if domain else "" + domains = Config.ALLOWED_DOMAINS + if len(domains) > 1: + domain_str = ", ".join(f"@{d}" for d in domains) + elif domains: + domain_str = f"@{domains[0]}" + else: + domain_str = "" return { "text": "Sign in with Email", "url": "/login/email", "icon_html": _EMAIL_ICON_HTML, - "subtitle": subtitle, + "subtitle": f'For {domain_str} email addresses.' if domain_str else "", "order": 20, "css_class": "btn-email", "visible": True, } def is_available(self) -> bool: - """Available when allowed_domain is configured.""" - return bool(Config.ALLOWED_DOMAIN) + """Available when at least one allowed domain is configured.""" + return len(Config.ALLOWED_DOMAINS) > 0 def init_app(self, app) -> None: """No additional initialization needed.""" diff --git a/auth/google/provider.py b/auth/google/provider.py index d0e1d62..94a6918 100644 --- a/auth/google/provider.py +++ b/auth/google/provider.py @@ -59,8 +59,9 @@ def authorize(): # Validate domain if not validate_email_domain(email): logger.warning(f"Login attempt from non-allowed domain: {email}") + domains_str = ", ".join(f"@{d}" for d in Config.ALLOWED_DOMAINS) flash( - f"Only @{Config.ALLOWED_DOMAIN} email addresses are allowed.", "error" + f"Only {domains_str} email addresses are allowed.", "error" ) return redirect(url_for("auth.login")) @@ -93,11 +94,16 @@ class GoogleAuthProvider(AuthProvider): return google_bp def get_login_button(self) -> dict: + domains = Config.ALLOWED_DOMAINS + if len(domains) > 1: + domain_str = ", ".join(f"@{d}" for d in domains) + else: + domain_str = f"@{domains[0]}" if domains else "" return { "text": "Sign in with Google", "url": "/login/google", "icon_html": _GOOGLE_ICON_HTML, - "subtitle": f'For @{Config.ALLOWED_DOMAIN} email addresses.', + "subtitle": f'For {domain_str} email addresses.' if domain_str else "", "order": 10, "css_class": "btn-google", "visible": True, diff --git a/config/instance.yaml.example b/config/instance.yaml.example index acf4c90..49d97e7 100644 --- a/config/instance.yaml.example +++ b/config/instance.yaml.example @@ -36,7 +36,7 @@ deployment: # Email magic link auth works out of the box (no external service needed). # Google OAuth is optional - add credentials to enable it. auth: - allowed_domain: "" # Email domain for login (e.g., "acme.com") + allowed_domain: "" # Email domain(s) for login, comma-separated (e.g., "acme.com" or "acme.com, partner.org") webapp_secret_key: "${WEBAPP_SECRET_KEY}" # Optional: Google OAuth (if not set, only email magic link is available) google_client_id: "${GOOGLE_CLIENT_ID}" diff --git a/tests/test_email_auth.py b/tests/test_email_auth.py index af64098..cbd3a73 100644 --- a/tests/test_email_auth.py +++ b/tests/test_email_auth.py @@ -73,8 +73,7 @@ class TestEmailAuthProvider: assert provider.get_display_name() == "Email" def test_login_button_properties(self, monkeypatch): - # Reload config with allowed domain set - monkeypatch.setattr("webapp.config.Config.ALLOWED_DOMAIN", "acme.com") + monkeypatch.setattr("webapp.config.Config.ALLOWED_DOMAINS", ["acme.com"]) provider = EmailAuthProvider() button = provider.get_login_button() assert button["text"] == "Sign in with Email" @@ -84,12 +83,19 @@ class TestEmailAuthProvider: assert "btn-email" in button["css_class"] assert "acme.com" in button["subtitle"] + def test_login_button_multiple_domains(self, monkeypatch): + monkeypatch.setattr("webapp.config.Config.ALLOWED_DOMAINS", ["acme.com", "partner.org"]) + provider = EmailAuthProvider() + button = provider.get_login_button() + assert "acme.com" in button["subtitle"] + assert "partner.org" in button["subtitle"] + def test_provider_available_with_domain(self, monkeypatch): - monkeypatch.setattr("webapp.config.Config.ALLOWED_DOMAIN", "acme.com") + monkeypatch.setattr("webapp.config.Config.ALLOWED_DOMAINS", ["acme.com"]) provider = EmailAuthProvider() assert provider.is_available() is True def test_provider_unavailable_without_domain(self, monkeypatch): - monkeypatch.setattr("webapp.config.Config.ALLOWED_DOMAIN", "") + monkeypatch.setattr("webapp.config.Config.ALLOWED_DOMAINS", []) provider = EmailAuthProvider() assert provider.is_available() is False diff --git a/tests/test_username_generation.py b/tests/test_username_generation.py new file mode 100644 index 0000000..e9e7fe4 --- /dev/null +++ b/tests/test_username_generation.py @@ -0,0 +1,51 @@ +"""Tests for username generation from email addresses.""" + +import pytest + +from webapp.user_service import get_username_from_email, RESERVED_USERNAMES + + +class TestGetUsernameFromEmail: + """Test email-to-username conversion.""" + + def test_basic_email(self): + assert get_username_from_email("admin@test.com") == "admin_test_com" + + def test_email_with_dots(self): + assert get_username_from_email("john.doe@acme.com") == "john_doe_acme_com" + + def test_different_domains_produce_different_usernames(self): + """Same local part, different domains -> unique usernames.""" + u1 = get_username_from_email("pavel@test.com") + u2 = get_username_from_email("pavel@groupon.com") + u3 = get_username_from_email("pavel@keboola.com") + assert u1 == "pavel_test_com" + assert u2 == "pavel_groupon_com" + assert u3 == "pavel_keboola_com" + assert len({u1, u2, u3}) == 3 # all unique + + def test_email_normalized_to_lowercase(self): + assert get_username_from_email("Admin@Test.COM") == "admin_test_com" + + def test_empty_email(self): + assert get_username_from_email("") == "" + + def test_no_at_sign(self): + assert get_username_from_email("notanemail") == "" + + def test_none_email(self): + assert get_username_from_email(None) == "" + + def test_reserved_names_avoided(self): + """Usernames from emails should NOT collide with reserved names.""" + # admin@anything.com -> admin_anything_com (not 'admin') + username = get_username_from_email("admin@company.com") + assert username not in RESERVED_USERNAMES + assert username == "admin_company_com" + + def test_test_email_not_reserved(self): + username = get_username_from_email("test@company.com") + assert username not in RESERVED_USERNAMES + + def test_subdomain_email(self): + assert get_username_from_email("user@mail.acme.co.uk") == "user_mail_acme_co_uk" diff --git a/webapp/auth.py b/webapp/auth.py index 6b16e1b..b901987 100644 --- a/webapp/auth.py +++ b/webapp/auth.py @@ -66,23 +66,23 @@ def admin_required(f): def validate_email_domain(email: str) -> bool: - """Check if email belongs to allowed domain or whitelist. + """Check if email belongs to an allowed domain or whitelist. Allows access for: - 1. Configured allowed domain (for Google OAuth users) - 2. Whitelisted emails (for password auth external users) + 1. Any of the configured allowed domains (comma-separated in config) + 2. Whitelisted emails (for individually approved external users) """ if not email: return False email_lower = email.lower() - # Check whitelist first (for password auth users) + # Check whitelist first (individually approved emails) if email_lower in Config.ALLOWED_EMAILS: return True - # Check domain (for Google OAuth users) + # Check domain against all allowed domains domain = email_lower.split("@")[-1] - return domain == Config.ALLOWED_DOMAIN.lower() + return domain in Config.ALLOWED_DOMAINS @auth_bp.route("/login") diff --git a/webapp/config.py b/webapp/config.py index 24aea35..86e92db 100644 --- a/webapp/config.py +++ b/webapp/config.py @@ -44,8 +44,14 @@ class Config: GOOGLE_CLIENT_ID = os.environ.get("GOOGLE_CLIENT_ID", "") GOOGLE_CLIENT_SECRET = os.environ.get("GOOGLE_CLIENT_SECRET", "") - # Domain restriction for Google OAuth (loaded from instance config) + # Domain restriction for login (loaded from instance config) + # Supports single domain string or comma-separated list ALLOWED_DOMAIN = _get(_instance, "auth", "allowed_domain", default="") + ALLOWED_DOMAINS = [ + d.strip().lower() + for d in _get(_instance, "auth", "allowed_domain", default="").split(",") + if d.strip() + ] # Password authentication for external users (whitelisted emails) ALLOWED_EMAILS = [ diff --git a/webapp/templates/login_magic_link.html b/webapp/templates/login_magic_link.html index 1f0b7e3..2cf900f 100644 --- a/webapp/templates/login_magic_link.html +++ b/webapp/templates/login_magic_link.html @@ -27,7 +27,7 @@ - {% if allowed_domain %} + {% if allowed_domains %}

- For @{{ allowed_domain }} email addresses. + For {% for d in allowed_domains %}@{{ d }}{% if not loop.last %}, {% endif %}{% endfor %} email addresses.

{% endif %} diff --git a/webapp/user_service.py b/webapp/user_service.py index 3276a78..4a1d4b2 100644 --- a/webapp/user_service.py +++ b/webapp/user_service.py @@ -40,27 +40,21 @@ RESERVED_USERNAMES = frozenset([ def get_username_from_email(email: str) -> str: """ - Extract username from email address. + Convert email address to a unique system username. - For allowed domain emails: takes the part before @ (e.g., john.doe@example.com -> john.doe) - For external emails: converts entire email to username (e.g., petr@simecek.org -> petr_simecek_org) + Always uses the full email to avoid collisions: + admin@test.com -> admin_test_com + pavel@groupon.com -> pavel_groupon_com + john.doe@acme.com -> john_doe_acme_com - This prevents username collisions between internal and external users. + This ensures uniqueness across multiple domains and avoids + collisions with reserved system usernames like 'admin', 'test', etc. """ if not email or "@" not in email: return "" - email_lower = email.lower() - local_part, domain = email_lower.rsplit("@", 1) - - # Internal domain users: just use local part - from .config import Config - if domain == Config.ALLOWED_DOMAIN: - return local_part - - # External users: convert entire email to safe username - # petr@simecek.org -> petr_simecek_org - safe_username = email_lower.replace("@", "_").replace(".", "_") + # Full email, normalized: replace @ and . with underscores + safe_username = email.lower().replace("@", "_").replace(".", "_") return safe_username