Provide human-readable message why a login attempt failed

Signed-off-by: DL6ER <[email protected]>
pi-hole · Apr 25, 2024 · dce5e74 · dce5e74
1 parent 4734e01
commit dce5e74
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 8 deletions.
diff --git a/src/api/api.c b/src/api/api.c
@@ -113,6 +113,7 @@ int api_handler(struct mg_connection *conn, void *ignored)
 		http_method(conn),
 		NULL,
 		NULL,
+		NULL,
 		API_AUTH_UNAUTHORIZED,
 		double_time(),
 		{ false, NULL, NULL, NULL, 0u },

diff --git a/src/api/auth.c b/src/api/auth.c
@@ -186,7 +186,8 @@ int check_client_auth(struct ftl_conn *api, const bool is_api)
 
 	if(!sid_avail)
 	{
-		log_debug(DEBUG_API, "API Authentication: FAIL (no SID provided)");
+		api->message = "no SID provided";
+		log_debug(DEBUG_API, "API Authentication: FAIL (%s)", api->message);
 		return API_AUTH_UNAUTHORIZED;
 	}
 
@@ -212,21 +213,28 @@ int check_client_auth(struct ftl_conn *api, const bool is_api)
 		}
 		else
 		{
-			log_debug(DEBUG_API, "API Authentication: FAIL (Cookie authentication without CSRF token)");
+			api->message = "Cookie authentication without CSRF token";
+			log_debug(DEBUG_API, "API Authentication: FAIL (%s)", api->message);
 			return API_AUTH_UNAUTHORIZED;
 		}
 	}
 
+	bool expired = false;
 	for(unsigned int i = 0; i < max_sessions; i++)
 	{
 		if(auth_data[i].used &&
-		   auth_data[i].valid_until >= now &&
 		   strcmp(auth_data[i].sid, sid) == 0)
 		{
+			// Check if session is known but expired
+			if(auth_data[i].valid_until < now)
+				expired = true;
+
+			// Check CSRF if authentiating via cookie
 			if(need_csrf && strcmp(auth_data[i].csrf, csrf) != 0)
 			{
-				log_debug(DEBUG_API, "API Authentication: FAIL (CSRF token mismatch, received \"%s\", expected \"%s\")",
-				          csrf, auth_data[i].csrf);
+				api->message = "CSRF token mismatch";
+				log_debug(DEBUG_API, "API Authentication: FAIL (%s, received \"%s\", expected \"%s\")",
+				          api->message, csrf, auth_data[i].csrf);
 				return API_AUTH_UNAUTHORIZED;
 			}
 			user_id = i;
@@ -266,7 +274,8 @@ int check_client_auth(struct ftl_conn *api, const bool is_api)
 	}
 	else
 	{
-		log_debug(DEBUG_API, "API Authentication: FAIL (SID invalid/expired)");
+		api->message = expired ? "session expired" : "session unknown";
+		log_debug(DEBUG_API, "API Authentication: FAIL (%s)", api->message);
 		return API_AUTH_UNAUTHORIZED;
 	}
 
@@ -314,6 +323,7 @@ static int get_session_object(struct ftl_conn *api, cJSON *json, const int user_
 		JSON_ADD_BOOL_TO_OBJECT(session, "totp", strlen(config.webserver.api.totp_secret.v.s) > 0);
 		JSON_ADD_NULL_TO_OBJECT(session, "sid");
 		JSON_ADD_NUMBER_TO_OBJECT(session, "validity", -1);
+		JSON_REF_STR_IN_OBJECT(session, "message", api->message);
 		JSON_ADD_ITEM_TO_OBJECT(json, "session", session);
 		return 0;
 	}
@@ -326,6 +336,7 @@ static int get_session_object(struct ftl_conn *api, cJSON *json, const int user_
 		JSON_REF_STR_IN_OBJECT(session, "sid", auth_data[user_id].sid);
 		JSON_REF_STR_IN_OBJECT(session, "csrf", auth_data[user_id].csrf);
 		JSON_ADD_NUMBER_TO_OBJECT(session, "validity", auth_data[user_id].valid_until - now);
+		JSON_REF_STR_IN_OBJECT(session, "message", api->message);
 		JSON_ADD_ITEM_TO_OBJECT(json, "session", session);
 		return 0;
 	}
@@ -335,6 +346,7 @@ static int get_session_object(struct ftl_conn *api, cJSON *json, const int user_
 	JSON_ADD_BOOL_TO_OBJECT(session, "totp", strlen(config.webserver.api.totp_secret.v.s) > 0);
 	JSON_ADD_NULL_TO_OBJECT(session, "sid");
 	JSON_ADD_NUMBER_TO_OBJECT(session, "validity", -1);
+	JSON_REF_STR_IN_OBJECT(session, "message", api->message);
 	JSON_ADD_ITEM_TO_OBJECT(json, "session", session);
 	return 0;
 }

diff --git a/src/api/docs/content/specs/auth.yaml b/src/api/docs/content/specs/auth.yaml
@@ -280,6 +280,7 @@ components:
             - sid
             - csrf
             - validity
+            - message
             - totp
           properties:
             valid:
@@ -299,6 +300,10 @@ components:
             validity:
               type: integer
               description: Remaining lifetime of this session unless refreshed (seconds)
+            message:
+              type: string
+              description: Human-readable message optionally describing the reason for an authentication failure
+              nullable: true
 
     password:
       type: object
@@ -439,6 +444,7 @@ components:
           sid: null
           csrf: null
           validity: 300
+          message: null
     login_okay:
       summary: Login successful
       value:
@@ -448,6 +454,7 @@ components:
           sid: "vFA+EP4MQ5JJvJg+3Q2Jnw="
           csrf: "Ux87YTIiMOf/GKCefVIOMw="
           validity: 300
+          message: null
     no_login_required:
       summary: No login required for this client
       value:
@@ -457,6 +464,7 @@ components:
           sid: null
           csrf: null
           validity: -1
+          message: null
     login_required:
       summary: Login required, 2FA disabled
       value:
@@ -466,6 +474,7 @@ components:
           sid: null
           csrf: null
           validity: -1
+          message: null
     login_required_2fa:
       summary: Login required, 2FA enabled
       value:
@@ -475,6 +484,7 @@ components:
           sid: null
           csrf: null
           validity: -1
+          message: null
     login_failed:
       summary: Login failed
       value:
@@ -484,6 +494,7 @@ components:
           sid: null
           csrf: null
           validity: -1
+          message: no SID provided
     errors:
       no_payload:
         summary: Bad request (no valid JSON payload)

diff --git a/src/webserver/http-common.h b/src/webserver/http-common.h
@@ -37,6 +37,7 @@ struct ftl_conn {
 	const enum http_method method;
 	char *action_path;
 	const char *item;
+	const char *message;
 	int user_id;
 	double now;
 	struct {

diff --git a/src/webserver/lua_web.c b/src/webserver/lua_web.c
@@ -162,8 +162,10 @@ int request_handler(struct mg_connection *conn, void *cbdata)
 			free(target);
 
 			// User is not authenticated, redirect to login page
-			log_web("Authentication required, redirecting to %slogin?target=%s", config.webserver.paths.webhome.v.s, encoded_target);
-			mg_printf(conn, "HTTP/1.1 302 Found\r\nLocation: %slogin?target=%s\r\n\r\n", config.webserver.paths.webhome.v.s, encoded_target);
+			log_web("Authentication required, redirecting to %slogin?target=%s",
+			        config.webserver.paths.webhome.v.s, encoded_target);
+			mg_printf(conn, "HTTP/1.1 302 Found\r\nLocation: %slogin?target=%s\r\n\r\n",
+			          config.webserver.paths.webhome.v.s, encoded_target);
 			free(encoded_target);
 			return 302;
 		}