July 2020 - Aur-dev - lists.archlinux.org

[PATCH] Redirect to referer after SSO login
by Frédéric Mangano-Tarumi 02 Aug '20

02 Aug '20

Introduce a `redirect` query argument to SSO login endpoints so that users are redirected to the page they were originally on when they clicked the Login link. --- aurweb/routers/sso.py | 23 +++++++++++++++++------ web/html/login.php | 18 ++++++++++++------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/aurweb/routers/sso.py b/aurweb/routers/sso.py index 2e4fbacc..4ef31b0a 100644 --- a/aurweb/routers/sso.py +++ b/aurweb/routers/sso.py @@ -30,16 +30,21 @@ oauth.register( @router.get("/sso/login") -async def login(request: Request): +async def login(request: Request, redirect: str = None): """ Redirect the user to the SSO provider’s login page. We specify prompt=login to force the user to input their credentials even if they’re already logged on the SSO. This is less practical, but given AUR has the potential to impact many users, better safe than sorry. + + The `redirect` argument is a query parameter specifying the post-login + redirect URL. """ - redirect_uri = aurweb.config.get("options", "aur_location") + "/sso/authenticate" - return await oauth.sso.authorize_redirect(request, redirect_uri, prompt="login") + authenticate_url = aurweb.config.get("options", "aur_location") + "/sso/authenticate" + if redirect: + authenticate_url = authenticate_url + "?" + urlencode([("redirect", redirect)]) + return await oauth.sso.authorize_redirect(request, authenticate_url, prompt="login") def is_account_suspended(conn, user_id): @@ -82,8 +87,15 @@ def is_ip_banned(conn, ip): return result.fetchone() is not None +def is_aur_url(url): + aur_location = aurweb.config.get("options", "aur_location") + if not aur_location.endswith("/"): + aur_location = aur_location + "/" + return url.startswith(aur_location) + + @router.get("/sso/authenticate") -async def authenticate(request: Request, conn=Depends(aurweb.db.connect)): +async def authenticate(request: Request, redirect: str = None, conn=Depends(aurweb.db.connect)): """ Receive an OpenID Connect ID token, validate it, then process it to create an new AUR session. @@ -118,8 +130,7 @@ async def authenticate(request: Request, conn=Depends(aurweb.db.connect)): return "Sorry, we don’t seem to know you Sir " + sub elif len(aur_accounts) == 1: sid = open_session(request, conn, aur_accounts[0][Users.c.ID]) - response = RedirectResponse("/") - # TODO redirect to the referrer + response = RedirectResponse(redirect if redirect and is_aur_url(redirect) else "/") response.set_cookie(key="AURSID", value=sid, httponly=True, secure=request.url.scheme == "https") if "id_token" in token: diff --git a/web/html/login.php b/web/html/login.php index 3a146f60..3f3d66cc 100644 --- a/web/html/login.php +++ b/web/html/login.php @@ -9,6 +9,10 @@ if (!$disable_http_login || (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'])) { $login_error = $login['error']; } +$referer = in_request('referer'); +if ($referer === '') + $referer = $_SERVER['HTTP_REFERER']; + html_header('AUR ' . __("Login")); ?> <div id="dev-login" class="box"> @@ -40,13 +44,15 @@ html_header('AUR ' . __("Login")); <p> <input type="submit" class="button" value="<?php print __("Login"); ?>" /> <a href="<?= get_uri('/passreset/') ?>">[<?= __('Forgot Password') ?>]</a> - <?php if (config_get('sso', 'openid_configuration')): ?> - <a href="<?= get_uri('/sso/login') ?>">[<?= __('Login through SSO') ?>]</a> + <?php if (config_get('sso', 'openid_configuration')): + $sso_login_url = get_uri('/sso/login'); + if (isset($referer)) + $sso_login_url .= '?redirect=' . urlencode($referer); + ?> + <a href="<?= htmlspecialchars($sso_login_url, ENT_QUOTES) ?>">[<?= __('Login through SSO') ?>]</a> <?php endif; ?> - <?php if (in_request('referer') !== ""): ?> - <input id="id_referer" type="hidden" name="referer" value="<?= htmlspecialchars(in_request('referer'), ENT_QUOTES) ?>" /> - <?php elseif (isset($_SERVER['HTTP_REFERER'])): ?> - <input id="id_referer" type="hidden" name="referer" value="<?= htmlspecialchars($_SERVER['HTTP_REFERER'], ENT_QUOTES) ?>" /> + <?php if (isset($referer)): ?> + <input id="id_referer" type="hidden" name="referer" value="<?= htmlspecialchars($referer, ENT_QUOTES) ?>" /> <?php endif; ?> </p> </fieldset> -- 2.27.0

2 1

[PATCH] Remove the per-user session limit
by Frédéric Mangano-Tarumi 02 Aug '20

02 Aug '20

This feature was originally introduced by f961ffd9c7f2d3d51d3e3b060990a4fef9e56c1b as a fix for FS#12898 <https://bugs.archlinux.org/task/12898>. As of today, it is broken because of the `q.SessionID IS NULL` condition in the WHERE clause, which can’t be true because SessionID is not nullable. As a consequence, the session limit was not applied. The fact the absence of the session limit hasn’t caused any issue so far, and hadn’t even been noticed, suggests the feature is unneeded. --- aurweb/routers/sso.py | 2 +- conf/config.defaults | 1 - web/lib/acctfuncs.inc.php | 17 ----------------- 3 files changed, 1 insertion(+), 19 deletions(-) diff --git a/aurweb/routers/sso.py b/aurweb/routers/sso.py index 2e4fbacc..73c884a4 100644 --- a/aurweb/routers/sso.py +++ b/aurweb/routers/sso.py @@ -56,7 +56,7 @@ def open_session(request, conn, user_id): raise HTTPException(status_code=403, detail=_('Account suspended')) # TODO This is a terrible message because it could imply the attempt at # logging in just caused the suspension. - # TODO apply [options] max_sessions_per_user + sid = uuid.uuid4().hex conn.execute(Sessions.insert().values( UsersID=user_id, diff --git a/conf/config.defaults b/conf/config.defaults index 49259754..98e033b7 100644 --- a/conf/config.defaults +++ b/conf/config.defaults @@ -13,7 +13,6 @@ passwd_min_len = 8 default_lang = en default_timezone = UTC sql_debug = 0 -max_sessions_per_user = 8 login_timeout = 7200 persistent_cookie_timeout = 2592000 max_filesize_uncompressed = 8388608 diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index ebabb840..bc603d3b 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -596,23 +596,6 @@ function try_login() { /* Generate a session ID and store it. */ while (!$logged_in && $num_tries < 5) { - $session_limit = config_get_int('options', 'max_sessions_per_user'); - if ($session_limit) { - /* - * Delete all user sessions except the - * last ($session_limit - 1). - */ - $q = "DELETE s.* FROM Sessions s "; - $q.= "LEFT JOIN (SELECT SessionID FROM Sessions "; - $q.= "WHERE UsersId = " . $userID . " "; - $q.= "ORDER BY LastUpdateTS DESC "; - $q.= "LIMIT " . ($session_limit - 1) . ") q "; - $q.= "ON s.SessionID = q.SessionID "; - $q.= "WHERE s.UsersId = " . $userID . " "; - $q.= "AND q.SessionID IS NULL;"; - $dbh->query($q); - } - $new_sid = new_sid(); $q = "INSERT INTO Sessions (UsersID, SessionID, LastUpdateTS)" ." VALUES (" . $userID . ", '" . $new_sid . "', " . strval(time()) . ")"; -- 2.27.0

2 1

[PATCH 1/3] Update last login information on SSO login
by Frédéric Mangano-Tarumi 29 Jul '20

29 Jul '20

--- aurweb/routers/sso.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/aurweb/routers/sso.py b/aurweb/routers/sso.py index 7b9c67c8..817adadb 100644 --- a/aurweb/routers/sso.py +++ b/aurweb/routers/sso.py @@ -63,7 +63,13 @@ def open_session(request, conn, user_id): SessionID=sid, LastUpdateTS=time.time(), )) - # TODO update Users.LastLogin and Users.LastLoginIPAddress + + # Update user’s last login information. + conn.execute(Users.update() + .where(Users.c.ID == user_id) + .values(LastLogin=int(time.time()), + LastLoginIPAddress=request.client.host)) + return sid -- 2.27.0

2 1

[PATCH 3/3] Guard OAuth exceptions to provide better messages
by Frédéric Mangano-Tarumi 28 Jul '20

28 Jul '20

--- aurweb/routers/sso.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/aurweb/routers/sso.py b/aurweb/routers/sso.py index 817adadb..2e4fbacc 100644 --- a/aurweb/routers/sso.py +++ b/aurweb/routers/sso.py @@ -5,7 +5,7 @@ from urllib.parse import urlencode import fastapi -from authlib.integrations.starlette_client import OAuth +from authlib.integrations.starlette_client import OAuth, OAuthError from fastapi import Depends, HTTPException from fastapi.responses import RedirectResponse from sqlalchemy.sql import select @@ -95,8 +95,18 @@ async def authenticate(request: Request, conn=Depends(aurweb.db.connect)): detail=_('The login form is currently disabled for your IP address, ' 'probably due to sustained spam attacks. Sorry for the ' 'inconvenience.')) - token = await oauth.sso.authorize_access_token(request) - user = await oauth.sso.parse_id_token(request, token) + + try: + token = await oauth.sso.authorize_access_token(request) + user = await oauth.sso.parse_id_token(request, token) + except OAuthError: + # Here, most OAuth errors should be caused by forged or expired tokens. + # Let’s give attackers as little information as possible. + _ = get_translator_for_request(request) + raise HTTPException( + status_code=400, + detail=_('Bad OAuth token. Please retry logging in from the start.')) + sub = user.get("sub") # this is the SSO account ID in JWT terminology if not sub: _ = get_translator_for_request(request) -- 2.27.0

1 0

[PATCH 2/3] HTML error pages for FastAPI
by Frédéric Mangano-Tarumi 28 Jul '20

28 Jul '20

--- aurweb/asgi.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/aurweb/asgi.py b/aurweb/asgi.py index 60c7ade7..9293ed77 100644 --- a/aurweb/asgi.py +++ b/aurweb/asgi.py @@ -1,4 +1,7 @@ -from fastapi import FastAPI +import http + +from fastapi import FastAPI, HTTPException +from fastapi.responses import HTMLResponse from starlette.middleware.sessions import SessionMiddleware import aurweb.config @@ -14,3 +17,14 @@ if not session_secret: app.add_middleware(SessionMiddleware, secret_key=session_secret) app.include_router(sso.router) + + +(a)app.exception_handler(HTTPException) +async def http_exception_handler(request, exc): + """ + Dirty HTML error page to replace the default JSON error responses. + In the future this should use a proper Arch-themed HTML template. + """ + phrase = http.HTTPStatus(exc.status_code).phrase + return HTMLResponse(f"<h1>{exc.status_code} {phrase}</h1><p>{exc.detail}</p>", + status_code=exc.status_code) -- 2.27.0

1 0

[PATCH 2/2] Stop redirecting stderr with proc_open
by Frédéric Mangano-Tarumi 27 Jul '20

27 Jul '20

Error outputs were piped to a temporary buffer that wasn’t read by anyone, making debugging hard because errors were completely silenced. By not explicitly redirecting stderr on proc_open, the subprocess inherits its parent stderr. --- web/lib/acctfuncs.inc.php | 2 -- web/lib/pkgbasefuncs.inc.php | 2 -- 2 files changed, 4 deletions(-) diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index d238c0e0..ebabb840 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -1349,7 +1349,6 @@ function notify($params) { $descspec = array( 0 => array('pipe', 'r'), 1 => array('pipe', 'w'), - 2 => array('pipe', 'w') ); $p = proc_open($cmd, $descspec, $pipes); @@ -1360,7 +1359,6 @@ function notify($params) { fclose($pipes[0]); fclose($pipes[1]); - fclose($pipes[2]); return proc_close($p); } diff --git a/web/lib/pkgbasefuncs.inc.php b/web/lib/pkgbasefuncs.inc.php index a4925891..22b882b4 100644 --- a/web/lib/pkgbasefuncs.inc.php +++ b/web/lib/pkgbasefuncs.inc.php @@ -96,7 +96,6 @@ function render_comment($id) { $descspec = array( 0 => array('pipe', 'r'), 1 => array('pipe', 'w'), - 2 => array('pipe', 'w') ); $p = proc_open($cmd, $descspec, $pipes); @@ -107,7 +106,6 @@ function render_comment($id) { fclose($pipes[0]); fclose($pipes[1]); - fclose($pipes[2]); return proc_close($p); } -- 2.27.0

2 1

[PATCH 1/2] aurweb.spawn: Support stdout redirections to non-tty
by Frédéric Mangano-Tarumi 27 Jul '20

27 Jul '20

Only ttys have a terminal size. If we can’t obtain it, we’ll just use 80 as a sane default. --- aurweb/spawn.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/aurweb/spawn.py b/aurweb/spawn.py index 5da8587e..46d534d9 100644 --- a/aurweb/spawn.py +++ b/aurweb/spawn.py @@ -87,12 +87,16 @@ def start(): return atexit.register(stop) + try: + terminal_width = os.get_terminal_size().columns + except OSError: + terminal_width = 80 print("{ruler}\n" "Spawing PHP and FastAPI, then nginx as a reverse proxy.\n" "Check out {aur_location}\n" "Hit ^C to terminate everything.\n" "{ruler}" - .format(ruler=("-" * os.get_terminal_size().columns), + .format(ruler=("-" * terminal_width), aur_location=aurweb.config.get('options', 'aur_location'))) # PHP -- 2.27.0

2 1

[PATCH] Remove PackageNotifications when User is suspended
by Kevin Morris 25 Jul '20

25 Jul '20

This path will only trigger when suspending a user through an account page; if the database is managed manually or through another script, PackageNotifications will also need manual modifications if intended. This change was written as a solution to https://bugs.archlinux.org/task/65554. Signed-off-by: Kevin Morris <kevr.gtalk(a)gmail.com> --- web/lib/acctfuncs.inc.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index d238c0e0..fb7cca8c 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -403,6 +403,10 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$BE="",$H="",$P=" $q.= " WHERE ID = ".intval($UID); $result = $dbh->exec($q); + // Clean up PackageNotifications for $UID as well. + $q = "DELETE FROM PackageNotifications WHERE UserID = ".intval($UID); + $result = $dbh->exec($q); + if (isset($ssh_keys) && count($ssh_keys) > 0) { $ssh_key_result = account_set_ssh_keys($UID, $ssh_keys, $ssh_fingerprints); } else { -- 2.20.1

3 13

[PATCH 1/3] SSO: Port IP ban checking
by Frédéric Mangano-Tarumi 25 Jul '20

25 Jul '20

--- aurweb/routers/sso.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/aurweb/routers/sso.py b/aurweb/routers/sso.py index 04ecdca6..efd4462c 100644 --- a/aurweb/routers/sso.py +++ b/aurweb/routers/sso.py @@ -14,7 +14,7 @@ from starlette.requests import Request import aurweb.config import aurweb.db -from aurweb.schema import Sessions, Users +from aurweb.schema import Bans, Sessions, Users router = fastapi.APIRouter() @@ -57,13 +57,28 @@ def open_session(conn, user_id): return sid +def is_ip_banned(conn, ip): + """ + Check if an IP is banned. `ip` is a string and may be an IPv4 as well as an + IPv6, depending on the server’s configuration. + """ + result = conn.execute(Bans.select().where(Bans.c.IPAddress == ip)) + return result.fetchone() is not None + + @router.get("/sso/authenticate") async def authenticate(request: Request, conn=Depends(aurweb.db.connect)): """ Receive an OpenID Connect ID token, validate it, then process it to create an new AUR session. """ - # TODO check for banned IPs + # TODO Handle translations + if is_ip_banned(conn, request.client.host): + raise HTTPException( + status_code=403, + detail='The login form is currently disabled for your IP address, ' + 'probably due to sustained spam attacks. Sorry for the ' + 'inconvenience.') token = await oauth.sso.authorize_access_token(request) user = await oauth.sso.parse_id_token(request, token) sub = user.get("sub") # this is the SSO account ID in JWT terminology -- 2.27.0

2 1

[PATCH 3/3] Build a translation facility for FastAPI
by Frédéric Mangano-Tarumi 20 Jul '20

20 Jul '20

--- aurweb/l10n.py | 22 ++++++++++++++++++++++ aurweb/routers/sso.py | 20 +++++++++++--------- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/aurweb/l10n.py b/aurweb/l10n.py index 51b56abb..a476ecd8 100644 --- a/aurweb/l10n.py +++ b/aurweb/l10n.py @@ -16,3 +16,25 @@ class Translator: self._localedir, languages=[lang]) return self._translator[lang].gettext(s) + + +def get_translator_for_request(request): + """ + Determine the preferred language from a FastAPI request object and build a + translator function for it. + + Example: + ```python + _ = get_translator_for_request(request) + print(_("Hello")) + ``` + """ + lang = request.cookies.get("AURLANG") + if lang is None: + lang = aurweb.config.get("options", "default_lang") + translator = Translator() + + def translate(message): + return translator.translate(message, lang) + + return translate diff --git a/aurweb/routers/sso.py b/aurweb/routers/sso.py index 3e3b743d..7b9c67c8 100644 --- a/aurweb/routers/sso.py +++ b/aurweb/routers/sso.py @@ -14,6 +14,7 @@ from starlette.requests import Request import aurweb.config import aurweb.db +from aurweb.l10n import get_translator_for_request from aurweb.schema import Bans, Sessions, Users router = fastapi.APIRouter() @@ -46,13 +47,13 @@ def is_account_suspended(conn, user_id): return row is not None and bool(row[0]) -def open_session(conn, user_id): +def open_session(request, conn, user_id): """ Create a new user session into the database. Return its SID. """ - # TODO Handle translations. if is_account_suspended(conn, user_id): - raise HTTPException(status_code=403, detail='Account suspended') + _ = get_translator_for_request(request) + raise HTTPException(status_code=403, detail=_('Account suspended')) # TODO This is a terrible message because it could imply the attempt at # logging in just caused the suspension. # TODO apply [options] max_sessions_per_user @@ -81,25 +82,26 @@ async def authenticate(request: Request, conn=Depends(aurweb.db.connect)): Receive an OpenID Connect ID token, validate it, then process it to create an new AUR session. """ - # TODO Handle translations if is_ip_banned(conn, request.client.host): + _ = get_translator_for_request(request) raise HTTPException( status_code=403, - detail='The login form is currently disabled for your IP address, ' - 'probably due to sustained spam attacks. Sorry for the ' - 'inconvenience.') + detail=_('The login form is currently disabled for your IP address, ' + 'probably due to sustained spam attacks. Sorry for the ' + 'inconvenience.')) token = await oauth.sso.authorize_access_token(request) user = await oauth.sso.parse_id_token(request, token) sub = user.get("sub") # this is the SSO account ID in JWT terminology if not sub: - raise HTTPException(status_code=400, detail="JWT is missing its `sub` field.") + _ = get_translator_for_request(request) + raise HTTPException(status_code=400, detail=_("JWT is missing its `sub` field.")) aur_accounts = conn.execute(select([Users.c.ID]).where(Users.c.SSOAccountID == sub)) \ .fetchall() if not aur_accounts: return "Sorry, we don’t seem to know you Sir " + sub elif len(aur_accounts) == 1: - sid = open_session(conn, aur_accounts[0][Users.c.ID]) + sid = open_session(request, conn, aur_accounts[0][Users.c.ID]) response = RedirectResponse("/") # TODO redirect to the referrer response.set_cookie(key="AURSID", value=sid, httponly=True, -- 2.27.0

1 0