From 8bd2a53ee739b36d2022eeedb752ae59d8982a79 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Tue, 10 Jan 2023 00:03:25 +0100
Subject: [PATCH] security: rework previous fixes to use the new
 use_remote_user_var_in_nginx_conf in ssowat conf introduced in yunohost
 11.1.2

---
 access.lua  | 21 ++++++++++++++++-
 helpers.lua | 67 +++++++++++++++++++++++++----------------------------
 2 files changed, 52 insertions(+), 36 deletions(-)

diff --git a/access.lua b/access.lua
index 74cc709..ea1021c 100644
--- a/access.lua
+++ b/access.lua
@@ -315,9 +315,28 @@ for permission_name, permission_infos in pairs(conf["permissions"]) do
     end
 end
 
+
+---
+--- 5. CHECK CLIENT-PROVIDED AUTH HEADER (should almost never happen?)
+---
+
+if permission ~= nil then
+    perm_user_remote_user_var_in_nginx_conf = permission["use_remote_user_var_in_nginx_conf"]
+    if perm_user_remote_user_var_in_nginx_conf == nil or perm_user_remote_user_var_in_nginx_conf == true then
+        is_logged_in_with_basic_auth = hlp.validate_or_clear_basic_auth_header_provided_by_client()
+
+        -- NB: is_logged_in_with_basic_auth can be false, true or nil
+        if is_logged_in_with_basic_auth == false then
+            return ngx.exit(ngx.HTTP_UNAUTHORIZED)
+        elseif is_logged_in_with_basic_auth == true then
+            is_logged_in = true
+        end
+    end
+end
+
 --
 --
--- 5. APPLY PERMISSION
+-- 6. APPLY PERMISSION
 --
 --
 
diff --git a/helpers.lua b/helpers.lua
index 2f8f1f0..c3b18df 100644
--- a/helpers.lua
+++ b/helpers.lua
@@ -256,36 +256,40 @@ function refresh_logged_in()
         return false
     end
 
-    -- If client set the Authorization/Proxy-Authorization header before reaching the SSO,
-    -- we want to match user and password against the user database.
-    --
-    -- It allows to bypass the cookie-based procedure with a per-request
-    -- authentication. This is useful to authenticate on the SSO during
-    -- curl requests for example.
-
-    local auth_header = ngx.req.get_headers()["Authorization"] or ngx.req.get_headers()["Proxy-Authorization"]
-
-    -- Ignore this for PROPFIND routes used by Nextcloud (et al.?) which also rely on basic auth with totally yunohost-unrelated credentials ...
-    if auth_header and ngx.var.request_method ~= "PROPFIND" then
-        logger.debug(auth_header)
-        _, _, b64_cred = string.find(auth_header, "^Basic%s+(.+)$")
-        if b64_cred == nil then
-            return is_logged_in
-        end
-        _, _, user, password = string.find(ngx.decode_base64(b64_cred), "^(.+):(.+)$")
-        user = authenticate(user, password)
-        if user then
-            logger.debug("User got authenticated through basic auth")
-            authUser = user
-            is_logged_in = true
-        else
-            return ngx.exit(ngx.HTTP_UNAUTHORIZED)
-        end
-    end
-
     return is_logged_in
 end
 
+function validate_or_clear_basic_auth_header_provided_by_client()
+
+    -- Ignore if no Auth header
+    local auth_header = ngx.req.get_headers()["Authorization"]
+    if auth_header == nil then
+        return nil
+    end
+
+    -- Ignore if not a Basic auth header
+    _, _, b64_cred = string.find(auth_header, "^Basic%s+(.+)$")
+    if b64_cred == nil then
+        return nil
+    end
+
+    -- Try to authenticate the user,
+    -- or remove the Auth header if not valid
+    _, _, user, password = string.find(ngx.decode_base64(b64_cred), "^(.+):(.+)$")
+    user = authenticate(user, password)
+    if user then
+        logger.debug("User got authenticated through basic auth")
+        authUser = user
+        return true
+    else
+        ngx.req.clear_header("Authorization")
+        return false -- ngx.exit(ngx.HTTP_UNAUTHORIZED)
+    end
+
+end
+
+
+
 -- Check whether a user is allowed to access a URL using the `permissions` directive
 -- of the configuration file
 function has_access(permission, user)
@@ -419,14 +423,7 @@ end
 --   - app requests that no authentication headers be sent
 -- Prevents user from pretending to be someone else on public apps
 function clear_headers()
-    -- Clear auth header only if it's a 'Basic' auth stuff, not 'Bearer' stuff
-    -- Also ignore PROPFIND routes used by Nextcloud (et al.?)
-    if ngx.var.request_method ~= "PROPFIND" and ngx.req.get_headers()["Authorization"] then
-        _, _, b64_cred = string.find(ngx.req.get_headers()["Authorization"], "^Basic%s+(.+)$")
-        if b64_cred ~= nil then
-            ngx.req.clear_header("Authorization")
-        end
-    end
+    -- NB: Basic Auth header is cleared in validate_or_clear_basic_auth_header_provided_by_client
     for k, v in pairs(conf["additional_headers"]) do
         ngx.req.clear_header(k)
     end