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
This commit is contained in:
parent
a8a9efeb60
commit
f635195c80
9 changed files with 108 additions and 39 deletions
|
|
@ -161,7 +161,7 @@ def login_email_form():
|
||||||
"""Show email input form."""
|
"""Show email input form."""
|
||||||
return render_template(
|
return render_template(
|
||||||
"login_magic_link.html",
|
"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"))
|
return redirect(url_for("email_auth.login_email_form"))
|
||||||
|
|
||||||
if not validate_email_domain(email):
|
if not validate_email_domain(email):
|
||||||
|
domains_str = ", ".join(f"@{d}" for d in Config.ALLOWED_DOMAINS)
|
||||||
flash(
|
flash(
|
||||||
f"Only @{Config.ALLOWED_DOMAIN} email addresses are allowed.",
|
f"Only {domains_str} email addresses are allowed.",
|
||||||
"error",
|
"error",
|
||||||
)
|
)
|
||||||
return redirect(url_for("email_auth.login_email_form"))
|
return redirect(url_for("email_auth.login_email_form"))
|
||||||
|
|
@ -248,21 +249,26 @@ class EmailAuthProvider(AuthProvider):
|
||||||
return email_bp
|
return email_bp
|
||||||
|
|
||||||
def get_login_button(self) -> dict:
|
def get_login_button(self) -> dict:
|
||||||
domain = Config.ALLOWED_DOMAIN
|
domains = Config.ALLOWED_DOMAINS
|
||||||
subtitle = f'For <strong>@{domain}</strong> email addresses.' if domain else ""
|
if len(domains) > 1:
|
||||||
|
domain_str = ", ".join(f"@{d}" for d in domains)
|
||||||
|
elif domains:
|
||||||
|
domain_str = f"@{domains[0]}"
|
||||||
|
else:
|
||||||
|
domain_str = ""
|
||||||
return {
|
return {
|
||||||
"text": "Sign in with Email",
|
"text": "Sign in with Email",
|
||||||
"url": "/login/email",
|
"url": "/login/email",
|
||||||
"icon_html": _EMAIL_ICON_HTML,
|
"icon_html": _EMAIL_ICON_HTML,
|
||||||
"subtitle": subtitle,
|
"subtitle": f'For <strong>{domain_str}</strong> email addresses.' if domain_str else "",
|
||||||
"order": 20,
|
"order": 20,
|
||||||
"css_class": "btn-email",
|
"css_class": "btn-email",
|
||||||
"visible": True,
|
"visible": True,
|
||||||
}
|
}
|
||||||
|
|
||||||
def is_available(self) -> bool:
|
def is_available(self) -> bool:
|
||||||
"""Available when allowed_domain is configured."""
|
"""Available when at least one allowed domain is configured."""
|
||||||
return bool(Config.ALLOWED_DOMAIN)
|
return len(Config.ALLOWED_DOMAINS) > 0
|
||||||
|
|
||||||
def init_app(self, app) -> None:
|
def init_app(self, app) -> None:
|
||||||
"""No additional initialization needed."""
|
"""No additional initialization needed."""
|
||||||
|
|
|
||||||
|
|
@ -59,8 +59,9 @@ def authorize():
|
||||||
# Validate domain
|
# Validate domain
|
||||||
if not validate_email_domain(email):
|
if not validate_email_domain(email):
|
||||||
logger.warning(f"Login attempt from non-allowed domain: {email}")
|
logger.warning(f"Login attempt from non-allowed domain: {email}")
|
||||||
|
domains_str = ", ".join(f"@{d}" for d in Config.ALLOWED_DOMAINS)
|
||||||
flash(
|
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"))
|
return redirect(url_for("auth.login"))
|
||||||
|
|
||||||
|
|
@ -93,11 +94,16 @@ class GoogleAuthProvider(AuthProvider):
|
||||||
return google_bp
|
return google_bp
|
||||||
|
|
||||||
def get_login_button(self) -> dict:
|
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 {
|
return {
|
||||||
"text": "Sign in with Google",
|
"text": "Sign in with Google",
|
||||||
"url": "/login/google",
|
"url": "/login/google",
|
||||||
"icon_html": _GOOGLE_ICON_HTML,
|
"icon_html": _GOOGLE_ICON_HTML,
|
||||||
"subtitle": f'For <strong>@{Config.ALLOWED_DOMAIN}</strong> email addresses.',
|
"subtitle": f'For <strong>{domain_str}</strong> email addresses.' if domain_str else "",
|
||||||
"order": 10,
|
"order": 10,
|
||||||
"css_class": "btn-google",
|
"css_class": "btn-google",
|
||||||
"visible": True,
|
"visible": True,
|
||||||
|
|
|
||||||
|
|
@ -36,7 +36,7 @@ deployment:
|
||||||
# Email magic link auth works out of the box (no external service needed).
|
# Email magic link auth works out of the box (no external service needed).
|
||||||
# Google OAuth is optional - add credentials to enable it.
|
# Google OAuth is optional - add credentials to enable it.
|
||||||
auth:
|
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}"
|
webapp_secret_key: "${WEBAPP_SECRET_KEY}"
|
||||||
# Optional: Google OAuth (if not set, only email magic link is available)
|
# Optional: Google OAuth (if not set, only email magic link is available)
|
||||||
google_client_id: "${GOOGLE_CLIENT_ID}"
|
google_client_id: "${GOOGLE_CLIENT_ID}"
|
||||||
|
|
|
||||||
|
|
@ -73,8 +73,7 @@ class TestEmailAuthProvider:
|
||||||
assert provider.get_display_name() == "Email"
|
assert provider.get_display_name() == "Email"
|
||||||
|
|
||||||
def test_login_button_properties(self, monkeypatch):
|
def test_login_button_properties(self, monkeypatch):
|
||||||
# Reload config with allowed domain set
|
monkeypatch.setattr("webapp.config.Config.ALLOWED_DOMAINS", ["acme.com"])
|
||||||
monkeypatch.setattr("webapp.config.Config.ALLOWED_DOMAIN", "acme.com")
|
|
||||||
provider = EmailAuthProvider()
|
provider = EmailAuthProvider()
|
||||||
button = provider.get_login_button()
|
button = provider.get_login_button()
|
||||||
assert button["text"] == "Sign in with Email"
|
assert button["text"] == "Sign in with Email"
|
||||||
|
|
@ -84,12 +83,19 @@ class TestEmailAuthProvider:
|
||||||
assert "btn-email" in button["css_class"]
|
assert "btn-email" in button["css_class"]
|
||||||
assert "acme.com" in button["subtitle"]
|
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):
|
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()
|
provider = EmailAuthProvider()
|
||||||
assert provider.is_available() is True
|
assert provider.is_available() is True
|
||||||
|
|
||||||
def test_provider_unavailable_without_domain(self, monkeypatch):
|
def test_provider_unavailable_without_domain(self, monkeypatch):
|
||||||
monkeypatch.setattr("webapp.config.Config.ALLOWED_DOMAIN", "")
|
monkeypatch.setattr("webapp.config.Config.ALLOWED_DOMAINS", [])
|
||||||
provider = EmailAuthProvider()
|
provider = EmailAuthProvider()
|
||||||
assert provider.is_available() is False
|
assert provider.is_available() is False
|
||||||
|
|
|
||||||
51
tests/test_username_generation.py
Normal file
51
tests/test_username_generation.py
Normal file
|
|
@ -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"
|
||||||
|
|
@ -66,23 +66,23 @@ def admin_required(f):
|
||||||
|
|
||||||
|
|
||||||
def validate_email_domain(email: str) -> bool:
|
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:
|
Allows access for:
|
||||||
1. Configured allowed domain (for Google OAuth users)
|
1. Any of the configured allowed domains (comma-separated in config)
|
||||||
2. Whitelisted emails (for password auth external users)
|
2. Whitelisted emails (for individually approved external users)
|
||||||
"""
|
"""
|
||||||
if not email:
|
if not email:
|
||||||
return False
|
return False
|
||||||
email_lower = email.lower()
|
email_lower = email.lower()
|
||||||
|
|
||||||
# Check whitelist first (for password auth users)
|
# Check whitelist first (individually approved emails)
|
||||||
if email_lower in Config.ALLOWED_EMAILS:
|
if email_lower in Config.ALLOWED_EMAILS:
|
||||||
return True
|
return True
|
||||||
|
|
||||||
# Check domain (for Google OAuth users)
|
# Check domain against all allowed domains
|
||||||
domain = email_lower.split("@")[-1]
|
domain = email_lower.split("@")[-1]
|
||||||
return domain == Config.ALLOWED_DOMAIN.lower()
|
return domain in Config.ALLOWED_DOMAINS
|
||||||
|
|
||||||
|
|
||||||
@auth_bp.route("/login")
|
@auth_bp.route("/login")
|
||||||
|
|
|
||||||
|
|
@ -44,8 +44,14 @@ class Config:
|
||||||
GOOGLE_CLIENT_ID = os.environ.get("GOOGLE_CLIENT_ID", "")
|
GOOGLE_CLIENT_ID = os.environ.get("GOOGLE_CLIENT_ID", "")
|
||||||
GOOGLE_CLIENT_SECRET = os.environ.get("GOOGLE_CLIENT_SECRET", "")
|
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_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)
|
# Password authentication for external users (whitelisted emails)
|
||||||
ALLOWED_EMAILS = [
|
ALLOWED_EMAILS = [
|
||||||
|
|
|
||||||
|
|
@ -27,7 +27,7 @@
|
||||||
<input type="email"
|
<input type="email"
|
||||||
id="email"
|
id="email"
|
||||||
name="email"
|
name="email"
|
||||||
placeholder="you@{{ allowed_domain or 'company.com' }}"
|
placeholder="you@{{ allowed_domains[0] if allowed_domains else 'company.com' }}"
|
||||||
required
|
required
|
||||||
autocomplete="email"
|
autocomplete="email"
|
||||||
autofocus
|
autofocus
|
||||||
|
|
@ -39,9 +39,9 @@
|
||||||
</button>
|
</button>
|
||||||
</form>
|
</form>
|
||||||
|
|
||||||
{% if allowed_domain %}
|
{% if allowed_domains %}
|
||||||
<p class="login-note">
|
<p class="login-note">
|
||||||
For <strong>@{{ allowed_domain }}</strong> email addresses.
|
For {% for d in allowed_domains %}<strong>@{{ d }}</strong>{% if not loop.last %}, {% endif %}{% endfor %} email addresses.
|
||||||
</p>
|
</p>
|
||||||
{% endif %}
|
{% endif %}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -40,27 +40,21 @@ RESERVED_USERNAMES = frozenset([
|
||||||
|
|
||||||
def get_username_from_email(email: str) -> str:
|
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)
|
Always uses the full email to avoid collisions:
|
||||||
For external emails: converts entire email to username (e.g., petr@simecek.org -> petr_simecek_org)
|
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:
|
if not email or "@" not in email:
|
||||||
return ""
|
return ""
|
||||||
|
|
||||||
email_lower = email.lower()
|
# Full email, normalized: replace @ and . with underscores
|
||||||
local_part, domain = email_lower.rsplit("@", 1)
|
safe_username = email.lower().replace("@", "_").replace(".", "_")
|
||||||
|
|
||||||
# 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(".", "_")
|
|
||||||
return safe_username
|
return safe_username
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue