Skip to content

Commit 0288dd4

Browse files
authored
Merge pull request #238 from spoo-me/fix/current-user-email
fix(auth): propagate user email to CurrentUser for feature-flag email allowlists
2 parents ec2d747 + 25c20d3 commit 0288dd4

7 files changed

Lines changed: 212 additions & 9 deletions

File tree

dependencies/auth.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ class CurrentUser:
4141
email_verified: bool
4242
api_key_doc: ApiKeyDoc | None = field(default=None)
4343
amr: str = "pwd"
44+
# Lowercased user email — consumed by FeatureFlagService's ALLOWLIST
45+
# rollout (allowlist_emails). Populated from the "email" claim on the
46+
# JWT path and from the owning UserDoc on the API-key path. None for
47+
# access tokens minted before the claim existed; those users match by
48+
# user_id only until their next token refresh.
49+
email: str | None = field(default=None)
4450
# UserDoc.plan value (e.g. "FREE") — consumed by FeatureFlagService's
4551
# TIER rollout via getattr(user, "tier"). Populated from the DB on the
4652
# API-key path and from the (future) "plan" claim on the JWT path.
@@ -105,6 +111,9 @@ async def get_current_user(
105111
user_id=key.user_id,
106112
email_verified=email_verified,
107113
api_key_doc=key,
114+
# The owning UserDoc is already fetched above for
115+
# email_verified — no extra DB hit to carry the email.
116+
email=user.email.lower() if user and user.email else None,
108117
tier=user.plan.value if user and user.plan else None,
109118
)
110119

@@ -131,11 +140,21 @@ async def get_current_user(
131140
user_id = ObjectId(claims["sub"])
132141
email_verified = bool(claims.get("email_verified", False))
133142
amr = claims.get("amr", ["pwd"])[0]
143+
# Tolerant read — access tokens minted before the "email" claim
144+
# existed simply carry email=None (never an error). Fixed on the
145+
# user's next token refresh.
146+
raw_email = claims.get("email")
147+
email = (
148+
raw_email.strip().lower()
149+
if isinstance(raw_email, str) and raw_email.strip()
150+
else None
151+
)
134152
structlog.contextvars.bind_contextvars(user_id=str(user_id), auth_method="jwt")
135153
return CurrentUser(
136154
user_id=user_id,
137155
email_verified=email_verified,
138156
amr=amr,
157+
email=email,
139158
# Not issued yet — the paid-plans launch adds the claim; TIER
140159
# flag rollouts become a pure data change at that point.
141160
tier=claims.get("plan"),

services/feature_flag_service.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,12 +204,12 @@ async def _lookup(self, name: str) -> FeatureFlagDoc | None:
204204

205205

206206
def _email_of(user: CurrentUser) -> str | None:
207-
"""Best-effort email extraction.
207+
"""Extract the user's lowercased email for allowlist matching.
208208
209-
``CurrentUser`` has ``user_id`` always but email is not on the dataclass
210-
today. When auth resolves a JWT or API key the user's email lives on the
211-
underlying ``UserDoc`` and isn't propagated here. For now allowlist by
212-
email is best-effort: if the email field is added later, this picks it
213-
up via ``getattr`` without code changes.
209+
``CurrentUser.email`` is populated on both auth paths: from the "email"
210+
JWT claim and from the owning ``UserDoc`` on the API-key path. It is
211+
``None`` only for access tokens minted before the claim existed (fixed
212+
on the next refresh) — those users still match by user_id. ``getattr``
213+
keeps this tolerant of CurrentUser-shaped stubs without the field.
214214
"""
215215
return getattr(user, "email", None)

services/token_factory.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ def generate_access_token(self, user: UserDoc, *, amr: str) -> str:
5858
iat — issued-at (UTC epoch seconds)
5959
exp — expiry (iat + access_token_ttl_seconds)
6060
amr — authentication method reference list, e.g. ["pwd"]
61+
email — lowercased email from the user document (consumed
62+
by feature-flag email allowlists via CurrentUser)
6163
email_verified — bool from the user document
6264
"""
6365
now = int(datetime.now(timezone.utc).timestamp())
@@ -68,6 +70,7 @@ def generate_access_token(self, user: UserDoc, *, amr: str) -> str:
6870
"iat": now,
6971
"exp": now + self._settings.access_token_ttl_seconds,
7072
"amr": [amr],
73+
"email": user.email.lower(),
7174
"email_verified": user.email_verified,
7275
}
7376
return pyjwt.encode(payload, self._signing_key(), algorithm=self._algorithm())

tests/integration/api_v1/conftest.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,13 @@ def _make_user(
5353
user_id: ObjectId | None = None,
5454
email_verified: bool = True,
5555
api_key_doc: ApiKeyDoc | None = None,
56+
email: str | None = None,
5657
) -> CurrentUser:
5758
return CurrentUser(
5859
user_id=user_id or ObjectId(),
5960
email_verified=email_verified,
6061
api_key_doc=api_key_doc,
62+
email=email,
6163
)
6264

6365

tests/unit/services/test_auth_service.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,26 @@ def test_access_token_has_correct_claims(self):
146146
assert payload["iss"] == "spoo.me"
147147
assert payload["aud"] == "spoo.me.api"
148148
assert payload["amr"] == ["pwd"]
149+
assert payload["email"] == "test@example.com"
149150
assert payload["email_verified"] is True
150151
assert "type" not in payload
151152
assert payload["exp"] - payload["iat"] == 900
152153

154+
def test_access_token_email_claim_is_lowercased(self):
155+
tf = make_token_factory()
156+
settings = make_jwt_settings()
157+
user = make_user_doc()
158+
user = user.model_copy(update={"email": "MixedCase@Example.COM"})
159+
token = tf.generate_access_token(user, amr="pwd")
160+
payload = pyjwt.decode(
161+
token,
162+
settings.jwt_secret,
163+
algorithms=["HS256"],
164+
audience=settings.jwt_audience,
165+
issuer=settings.jwt_issuer,
166+
)
167+
assert payload["email"] == "mixedcase@example.com"
168+
153169
def test_refresh_token_has_type_field(self):
154170
tf = make_token_factory()
155171
settings = make_jwt_settings()

tests/unit/services/test_feature_flag_service.py

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,88 @@ async def test_email_match_case_insensitive(self):
164164

165165
@pytest.mark.asyncio
166166
async def test_no_email_attribute_falls_back_to_user_id(self):
167-
# CurrentUser today doesn't carry email — service must not crash.
167+
# Stubs without an email attribute — service must not crash.
168168
flag = _flag(rollout_type=RolloutType.ALLOWLIST, allowlist_user_ids=[USER_A])
169169
service, _, _ = make_service(flag=flag)
170170
# _user() with no email omits the attribute; getattr returns None.
171171
assert await service.is_enabled("test_flag", _user(USER_A)) is True
172172

173+
@pytest.mark.asyncio
174+
async def test_email_only_allowlist_denies_stub_without_email_attribute(self):
175+
# Pins _email_of's getattr fallback: a user object with NO email
176+
# attribute at all (not CurrentUser with email=None) against an
177+
# email-ONLY allowlist → graceful deny, no AttributeError.
178+
flag = _flag(
179+
rollout_type=RolloutType.ALLOWLIST,
180+
allowlist_emails=["alice@example.com"],
181+
)
182+
service, _, _ = make_service(flag=flag)
183+
stub = _user(USER_A) # SimpleNamespace; email attribute omitted
184+
assert not hasattr(stub, "email")
185+
assert await service.is_enabled("test_flag", stub) is False
186+
187+
@pytest.mark.asyncio
188+
async def test_current_user_email_field_flows_through(self):
189+
"""CurrentUser.email (JWT "email" claim / UserDoc email) satisfies
190+
email-only allowlists — the real dataclass, not a stub."""
191+
from dependencies.auth import CurrentUser
192+
193+
flag = _flag(
194+
rollout_type=RolloutType.ALLOWLIST,
195+
allowlist_emails=["alice@example.com"],
196+
)
197+
service, _, _ = make_service(flag=flag)
198+
alice = CurrentUser(
199+
user_id=USER_A, email_verified=True, email="alice@example.com"
200+
)
201+
bob = CurrentUser(user_id=USER_B, email_verified=True, email="bob@example.com")
202+
assert await service.is_enabled("test_flag", alice) is True
203+
assert await service.is_enabled("test_flag", bob) is False
204+
205+
@pytest.mark.asyncio
206+
async def test_current_user_mixed_case_email_matches(self):
207+
# Doc validator lowercases allowlist entries; is_user_in_allowlist
208+
# lowercases the user's email — any casing on either side matches.
209+
from dependencies.auth import CurrentUser
210+
211+
flag = _flag(
212+
rollout_type=RolloutType.ALLOWLIST,
213+
allowlist_emails=["Alice@Example.COM"],
214+
)
215+
service, _, _ = make_service(flag=flag)
216+
alice = CurrentUser(
217+
user_id=USER_A, email_verified=True, email="ALICE@example.com"
218+
)
219+
assert await service.is_enabled("test_flag", alice) is True
220+
221+
@pytest.mark.asyncio
222+
async def test_current_user_none_email_denied_without_error(self):
223+
# Old access tokens (pre-"email" claim) resolve to email=None —
224+
# email-only allowlists deny them, and nothing raises.
225+
from dependencies.auth import CurrentUser
226+
227+
flag = _flag(
228+
rollout_type=RolloutType.ALLOWLIST,
229+
allowlist_emails=["alice@example.com"],
230+
)
231+
service, _, _ = make_service(flag=flag)
232+
old_token_user = CurrentUser(user_id=USER_A, email_verified=True)
233+
assert old_token_user.email is None
234+
assert await service.is_enabled("test_flag", old_token_user) is False
235+
236+
@pytest.mark.asyncio
237+
async def test_current_user_none_email_still_matches_by_user_id(self):
238+
from dependencies.auth import CurrentUser
239+
240+
flag = _flag(
241+
rollout_type=RolloutType.ALLOWLIST,
242+
allowlist_user_ids=[USER_A],
243+
allowlist_emails=["alice@example.com"],
244+
)
245+
service, _, _ = make_service(flag=flag)
246+
old_token_user = CurrentUser(user_id=USER_A, email_verified=True)
247+
assert await service.is_enabled("test_flag", old_token_user) is True
248+
173249

174250
# ── PERCENTAGE ───────────────────────────────────────────────────────────────
175251

tests/unit/test_auth_deps.py

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ def make_key_doc(revoked: bool = False, expires_at=None, scopes=None):
7474
)
7575

7676

77-
def make_jwt_token(token_type: str = "access", ttl_seconds: int = 900):
77+
def make_jwt_token(
78+
token_type: str = "access",
79+
ttl_seconds: int = 900,
80+
email: str | None = None,
81+
):
7882
now = datetime.now(timezone.utc)
7983
payload = {
8084
"sub": str(USER_OID),
@@ -85,6 +89,8 @@ def make_jwt_token(token_type: str = "access", ttl_seconds: int = 900):
8589
"iat": now,
8690
"exp": now + timedelta(seconds=ttl_seconds),
8791
}
92+
if email is not None:
93+
payload["email"] = email
8894
return pyjwt.encode(payload, JWT_SECRET, algorithm="HS256")
8995

9096

@@ -102,7 +108,7 @@ async def test_no_auth_returns_none(self):
102108
@pytest.mark.asyncio
103109
async def test_api_key_valid_returns_current_user(self):
104110
key_doc = make_key_doc()
105-
user_mock = MagicMock(email_verified=True)
111+
user_mock = MagicMock(email_verified=True, email="Owner@Example.com")
106112

107113
with (
108114
patch("dependencies.auth.get_settings", return_value=make_settings()),
@@ -119,6 +125,8 @@ async def test_api_key_valid_returns_current_user(self):
119125
assert result.user_id == USER_OID
120126
assert result.api_key_doc == key_doc
121127
assert result.email_verified is True
128+
# Email comes from the owning UserDoc (already fetched), lowercased.
129+
assert result.email == "owner@example.com"
122130

123131
@pytest.mark.asyncio
124132
async def test_api_key_revoked_returns_none(self):
@@ -192,6 +200,85 @@ async def test_jwt_valid_returns_current_user(self):
192200
assert result.email_verified is True
193201
assert result.api_key_doc is None
194202

203+
@pytest.mark.asyncio
204+
async def test_jwt_email_claim_populates_current_user_lowercased(self):
205+
token = make_jwt_token(email="Alice@Example.COM")
206+
req = make_request(auth_header=f"Bearer {token}")
207+
208+
with patch("dependencies.auth.get_settings", return_value=make_settings()):
209+
result = await get_current_user(req, db=MagicMock())
210+
211+
assert result is not None
212+
assert result.email == "alice@example.com"
213+
214+
@pytest.mark.asyncio
215+
async def test_jwt_without_email_claim_yields_none_email(self):
216+
# Old access tokens minted before the "email" claim existed must
217+
# still authenticate — email is simply None, never an error.
218+
token = make_jwt_token()
219+
req = make_request(auth_header=f"Bearer {token}")
220+
221+
with patch("dependencies.auth.get_settings", return_value=make_settings()):
222+
result = await get_current_user(req, db=MagicMock())
223+
224+
assert result is not None
225+
assert result.email is None
226+
227+
@pytest.mark.asyncio
228+
async def test_jwt_blank_email_claim_yields_none_email(self):
229+
token = make_jwt_token(email=" ")
230+
req = make_request(auth_header=f"Bearer {token}")
231+
232+
with patch("dependencies.auth.get_settings", return_value=make_settings()):
233+
result = await get_current_user(req, db=MagicMock())
234+
235+
assert result is not None
236+
assert result.email is None
237+
238+
@pytest.mark.asyncio
239+
@pytest.mark.parametrize("bad_email", [42, ["alice@example.com"]])
240+
async def test_jwt_non_string_email_claim_yields_none_email(self, bad_email):
241+
# A token whose "email" claim is not a string (int, list, …) must
242+
# still authenticate — the claim parses to None, never an error.
243+
token = make_jwt_token(email=bad_email)
244+
req = make_request(auth_header=f"Bearer {token}")
245+
246+
with patch("dependencies.auth.get_settings", return_value=make_settings()):
247+
result = await get_current_user(req, db=MagicMock())
248+
249+
assert result is not None
250+
assert result.user_id == USER_OID
251+
assert result.email is None
252+
253+
@pytest.mark.asyncio
254+
async def test_token_factory_round_trip_populates_email(self):
255+
# Mint with the real TokenFactory → resolve via get_current_user:
256+
# the email claim survives the round trip and is lowercased.
257+
from schemas.models.user import UserDoc
258+
from services.token_factory import TokenFactory
259+
260+
user_doc = UserDoc.from_mongo(
261+
{
262+
"_id": USER_OID,
263+
"email": "Round.Trip@Example.COM",
264+
"email_verified": True,
265+
"user_name": "Round Trip",
266+
"created_at": datetime.now(timezone.utc),
267+
"updated_at": datetime.now(timezone.utc),
268+
}
269+
)
270+
token = TokenFactory(make_jwt_settings()).generate_access_token(
271+
user_doc, amr="pwd"
272+
)
273+
req = make_request(auth_header=f"Bearer {token}")
274+
275+
with patch("dependencies.auth.get_settings", return_value=make_settings()):
276+
result = await get_current_user(req, db=MagicMock())
277+
278+
assert result is not None
279+
assert result.user_id == USER_OID
280+
assert result.email == "round.trip@example.com"
281+
195282
@pytest.mark.asyncio
196283
async def test_jwt_refresh_token_rejected(self):
197284
token = make_jwt_token(token_type="refresh")

0 commit comments

Comments
 (0)