feat(auth)!: remove the public access-request endpoint
POST /auth/request-access had no caller left: the login page's form was gone, and registration and a refused Google sign-in already land in the same admin approval. A third public write path with no protection of its own (owner decision, E34). Its one guard moves to where every path passes: upsert_pending_invite no longer opens a pending row for an address the env already lets in. The registration path lacked it — registering b.ob@gmail.com beside an env-granted bob@gmail.com opened a pending row, and is_allowed answers from the invite rows once any exist, so the real owner was locked out. The tests that drove the endpoint now drive registration or the shared function.
This commit is contained in:
+11
-42
@@ -29,6 +29,7 @@ from app.auth import (
|
||||
WRITE_SCOPES,
|
||||
current_user,
|
||||
env_admin_canonicals,
|
||||
env_allowed_canonicals,
|
||||
find_user_by_email,
|
||||
get_or_create_demo_user,
|
||||
google_enabled,
|
||||
@@ -72,7 +73,6 @@ _reset_limiter = RateLimiter(max_events=5, window_seconds=300)
|
||||
# Separate from _reset_limiter on purpose: opening a link must not eat the budget for REQUESTING one.
|
||||
_reset_check_limiter = RateLimiter(max_events=10, window_seconds=300)
|
||||
_verify_resend_limiter = RateLimiter(max_events=5, window_seconds=300)
|
||||
_request_access_limiter = RateLimiter(max_events=5, window_seconds=300)
|
||||
_suspend_email_limiter = RateLimiter(max_events=1, window_seconds=3600)
|
||||
|
||||
|
||||
@@ -196,9 +196,9 @@ def upsert_pending_invite(db: Session, email: str, lang: str | None = None) -> I
|
||||
not reopen a decision that has already been made for that mailbox. The row is still STORED under
|
||||
the address as typed, because that is where mail has to go.
|
||||
|
||||
`lang` is the language of the surface the request came from — the sign-in page, or the locale
|
||||
Google reported for a refused sign-in. The latest one wins: someone who asks again from a
|
||||
Hungarian page is telling us something newer than last time. Anything unsupported is ignored
|
||||
`lang` is the language the request came in — the locale Google reported for a refused sign-in.
|
||||
The latest one wins: someone who asks again under another locale is telling us something newer
|
||||
than last time. Anything unsupported is ignored
|
||||
rather than stored, so a forged body cannot put junk in the column.
|
||||
|
||||
The commit contract is unchanged: this still commits ONLY when it writes something. Callers rely
|
||||
@@ -215,6 +215,12 @@ def upsert_pending_invite(db: Session, email: str, lang: str | None = None) -> I
|
||||
.scalars()
|
||||
.all()
|
||||
)
|
||||
# An address the env already lets in has nothing to ask for, and a pending row for it would be
|
||||
# worse than nothing: `is_allowed` answers from the invite rows once any exist, so a stranger
|
||||
# registering a sibling spelling (`b.ob@` beside an env-granted `bob@`) would lock the real
|
||||
# owner out of a grant no admin could see.
|
||||
if not rows and canon in env_allowed_canonicals():
|
||||
return InviteRequest(None, False)
|
||||
# An approved row wins, so a denied duplicate can't mask an approval. Then a row that is not
|
||||
# DENIED, preferring the exact spelling — a denied sibling must never be picked while a pending
|
||||
# row exists, or the reopen branch below would fire and put one person in the admin's queue
|
||||
@@ -628,43 +634,6 @@ async def logout(request: Request):
|
||||
return JSONResponse({"ok": True, "switched": False})
|
||||
|
||||
|
||||
class AccessRequestIn(EmailIn):
|
||||
# The language of the page this request was made from. Same reasoning as registration's `lang`
|
||||
# (and the same shape): it is the ONLY moment this preference exists anywhere the server can
|
||||
# see, since the requester has no account to carry it, and it decides what language the approval
|
||||
# or denial mail is written in.
|
||||
lang: str | None = None
|
||||
|
||||
|
||||
@router.post("/request-access")
|
||||
async def request_access(
|
||||
body: AccessRequestIn,
|
||||
request: Request,
|
||||
background: BackgroundTasks,
|
||||
db: Session = Depends(get_db),
|
||||
) -> dict:
|
||||
"""Public: ask for access. Idempotent — repeat requests for the same email don't
|
||||
duplicate or re-spam admins (upsert answers `is_new=False` for an already-pending row).
|
||||
|
||||
Answers a uniform ``"pending"`` for EVERY outcome — already-allowed, newly-pending,
|
||||
already-pending, and rate-limited alike. The old ``"approved"`` reply for an allow-listed
|
||||
address made this a public enumeration oracle for the allow-list (which the rest of the module
|
||||
is careful to avoid, cf. demo_login); an already-allowed user can still just sign in with Google.
|
||||
Rate-limited per client IP like register/login/reset, and a block returns "pending" too so the
|
||||
endpoint can't be probed for throttling either."""
|
||||
email = (body.email or "").strip().lower()
|
||||
if not valid_email(email):
|
||||
raise refused(400, "invalid_email")
|
||||
if not _request_access_limiter.allow(client_key(request)):
|
||||
return {"ok": True, "status": "pending"}
|
||||
if is_allowed(db, email):
|
||||
return {"ok": True, "status": "pending"} # already allowed — don't reveal it
|
||||
req = upsert_pending_invite(db, email, body.lang)
|
||||
if req.is_new:
|
||||
notify_admins_of_request(db, background, email)
|
||||
return {"ok": True, "status": "pending"}
|
||||
|
||||
|
||||
@router.post("/demo")
|
||||
def demo_login(body: EmailIn, request: Request, db: Session = Depends(get_db)) -> dict:
|
||||
"""Hidden demo entry: a whitelisted email logs straight into the shared demo account,
|
||||
@@ -918,7 +887,7 @@ def _register_account(email: str, password: str, lang: str | None = None) -> Non
|
||||
db.commit()
|
||||
if email_ok:
|
||||
send_verification_link(db, user)
|
||||
# Fired unconditionally, unlike the other two callers which gate on a fresh invite row: for
|
||||
# Fired unconditionally, unlike the Google callback, which gates on a fresh invite row: for
|
||||
# an address the admin already approved, nothing lands in the Access-requests queue, so this
|
||||
# is the only place the account's existence is announced at all. Delivery is a separate
|
||||
# matter — with no working SMTP `_send` drops it, which is why the pre-approved registrant
|
||||
|
||||
Reference in New Issue
Block a user