timekeeper

commit f24da15b101d9ff59a2e9e788ecb7cabc1c5a89f
parent 9d4d0b357b18093737927460423b06ff785242b7
Author: Jacob R. Edwards <jacob@jacobedwards.org>
Date:   Wed,  6 Mar 2024 20:44:48 -0800

Progagate errors in user login functions

To start out with, if any errors occurred I would call exit(2).
This is obviously not ideal, so now errors are propagated through
return value.

Diffstat:
M
page.c
 | 
10
++++++----
M
pages/login.c
 | 
26
+++++++++-----------------
M
user.c
 | 
92
+++++++++++++++++++++++++++++++++++++++++++++++--------------------------------
M
user.h
 | 
17
+++++++++++------

4 files changed, 81 insertions(+), 64 deletions(-)
diff --git a/page.c b/page.c
@@ -14,7 +14,7 @@
 enum kcgi_err
 loadpagerequest(struct kfcgi *fcgi, struct pagedata *pd)
 {
-	enum kcgi_err status;
+	int status;
 
 	status = khttp_fcgi_parse(fcgi, &pd->req);
 	if (status != KCGI_OK)
@@ -24,10 +24,12 @@ loadpagerequest(struct kfcgi *fcgi, struct pagedata *pd)
         if (!pd->req.cookiemap[KeyHash])
                 return KCGI_OK;
 
-        pd->user = getuser(pd, "hash", pd->req.cookiemap[KeyHash]->parsed.s);
-	/* propagate errors from getuser to here and handle them */
+        status = getuser(&pd->user, pd, "hash", pd->req.cookiemap[KeyHash]->parsed.s);
+	if (status == LoginValid || status == LoginNotFound)
+		return KCGI_OK;
 
-	return KCGI_OK;
+	khttp_free(&pd->req);
+	return KCGI_SYSTEM;
 }
 
 void
diff --git a/pages/login.c b/pages/login.c
@@ -35,25 +35,23 @@ pagelogin(struct pagedata *pd)
 	enum kcgi_err status;
 	struct user *user = NULL;
 	char *fuser, *fpass;
-	enum LoginStatus ls;
-	char *msg;
+	enum user_error userstatus;
 
 	if (pd->user)
 		return redirect(pd, pd->pages[PageIndex], "Logged in");
 
-	ls = LoginUntried;
+	userstatus = LoginLast;
 	if (pd->req.fieldmap[KeyUsername] && pd->req.fieldmap[KeyPassword]) {
 		fuser = pd->req.fieldmap[KeyUsername]->parsed.s;
 		fpass = pd->req.fieldmap[KeyPassword]->parsed.s;
 
 		if (pd->req.fieldmap[KeyCreate]) {
 			if (adduser(pd,  fuser, fpass))
-				ls = LoginError;
-		
+				userstatus = LoginSystem;
 		}
-		if (ls == LoginUntried) {
-			ls = loginuser(&user, pd, fuser, fpass);
-			if (ls == LoginValid) {
+		if (userstatus == LoginLast) {
+			userstatus = loginuser(&user, pd, fuser, fpass);
+			if (userstatus == LoginValid) {
 				status = khttp_head(&pd->req, kresps[KRESP_SET_COOKIE], "%s=%s; Path=/",
 				    pd->keys[KeyHash].name, user->hash);
 				freeuser(user);
@@ -70,15 +68,9 @@ pagelogin(struct pagedata *pd)
 	if ((status = htmlwithin(pd, KELEM_H1, "Login")) != KCGI_OK)
 		return status;
 
-	if (ls > 0) {
-		if (ls == LoginInvalid)
-			msg = "Error: Invalid credentials";
-		else if (ls == LoginError)
-			msg = "Error: System error";
-		else
-			msg = "Error: Undefined error";
-	
-		if ((status = htmlwithin(pd, KELEM_P, msg)) != KCGI_OK)
+	if (userstatus && userstatus < LoginLast) {
+		kutil_warn(&pd->req, NULL, "Bad login: %s", user_errors[userstatus]);
+		if ((status = htmlwithin(pd, KELEM_P, user_errors[userstatus])) != KCGI_OK)
 			return status;
 	}
 
diff --git a/user.c b/user.c
@@ -1,3 +1,5 @@
+#include <assert.h>
+
 #define const
 
 #include <sys/types.h>
@@ -18,6 +20,14 @@
 #include "user.h"
 #include "util.h"
 
+char *user_errors[] = {
+	[LoginValid] = "Login valid",
+	[LoginNotFound] = "User not found",
+	[LoginInvalid] = "Invalid user credentials",
+	[LoginSystem] = "System error",
+	[LoginCorrupt] = "User database is corrupt"
+};
+
 int
 adduser(struct pagedata *pd, char *name, char *key)
 {
@@ -38,6 +48,16 @@ adduser(struct pagedata *pd, char *name, char *key)
 	return 0;
 }
 
+int
+deleteuser(struct pagedata *pd, char *hash)
+{
+	struct sqlbox_parm p = {
+		.sparm = hash, .type = SQLBOX_PARM_STRING
+	};
+
+	return sqlbox_exec(pd->db, 0, StmtDeleteUser, 1, &p, 0) != SQLBOX_CODE_OK;
+}
+
 void
 freeuser(struct user *user)
 {
@@ -47,12 +67,12 @@ freeuser(struct user *user)
 	free(user->name);
 }
 
-struct user *
-getuser(struct pagedata *pd, char *field, char *value)
+enum user_error
+getuser(struct user **ruser, struct pagedata *pd, char *field, char *value)
 {
-	struct user *user;
 	size_t stmtid;
 	enum StmtID gets;
+	struct user *user;
 	struct sqlbox_parmset *res;
 	struct sqlbox_parm p[] = {
 		{ .sparm = value, .type = SQLBOX_PARM_STRING }
@@ -63,63 +83,60 @@ getuser(struct pagedata *pd, char *field, char *value)
 	else if (strcmp(field, "hash") == 0)
 		gets = StmtGetUserByHash;
 	else
-		err(1, "Expected valid field");
+		gets = -1;
 
-	if (!(stmtid = sqlbox_prepare_bind(pd->db, pd->dbid, gets, Len(p), p, 0)))
-		err(1, "sdqlb");
+	assert(gets >= 0);
 
+	if (!(stmtid = sqlbox_prepare_bind(pd->db, pd->dbid, gets, Len(p), p, 0)))
+		return LoginSystem;
 	if (!(res = sqlbox_step(pd->db, stmtid)))
-		err(1, "step");
+		return LoginSystem;
 
 	if (!res->psz && res->code == SQLBOX_CODE_OK) {
-		kutil_info(&pd->req, NULL, "getuser: %s: %s not found", value, field);
 		sqlbox_finalise(pd->db, stmtid);
-		return NULL;
+		return LoginNotFound;
 	}
 
 	user = calloc(1, sizeof(*user));
 	if (!user)
-		err(1, "IDC");
+		return LoginSystem;
 
-	if (res->psz != 2)
-		errx(1, "%zu: Invalid number of fields in user-data", res->psz);
-	user->hash = strdup(res->ps[0].sparm);
-	if (!user->hash)
-		err(1, "IDC");
-	user->name = strdup(res->ps[1].sparm);
-	if (!user->name)
-		err(1, "IDC");
+	if (res->psz != 2) {
+		freeuser(user);
+		return LoginCorrupt;
+	}
 
-	if (!sqlbox_finalise(pd->db, stmtid))
-		err(1, "finalize");
+	if (!(user->hash = strdup(res->ps[0].sparm)) ||
+	    !(user->name = strdup(res->ps[1].sparm))) {
+		freeuser(user);
+		return LoginSystem;
+	}
 
-	return user;
-}
+	if (!sqlbox_finalise(pd->db, stmtid)) {
+		freeuser(user);
+		return LoginSystem;
+	}
 
-int
-deleteuser(struct pagedata *pd, char *hash)
-{
-	struct sqlbox_parm p = {
-		.sparm = hash, .type = SQLBOX_PARM_STRING
-	};
+	assert(ruser);
+	*ruser = user;
 
-	return sqlbox_exec(pd->db, 0, StmtDeleteUser, 1, &p, 0) != SQLBOX_CODE_OK;
+	return LoginValid;
 }
 
-enum LoginStatus
-loginuser(struct user **userp, struct pagedata *pd, char *name, char *key)
+enum user_error 
+loginuser(struct user **ruser, struct pagedata *pd, char *name, char *key)
 {
 	struct user *user;
 	char *testhash;
+	enum user_error status;
 
-	user = getuser(pd, "name", name);
-	if (!user)
-		return LoginInvalid;
+	if ((status = getuser(&user, pd, "name", name)))
+		return status;
 
 	testhash = bcrypt(key, user->hash);
 	if (!testhash) {
 		freeuser(user);
-		return LoginError;
+		return LoginSystem;
 	}
 
 	if (strcmp(user->hash, testhash) != 0) {
@@ -127,9 +144,10 @@ loginuser(struct user **userp, struct pagedata *pd, char *name, char *key)
 		return LoginInvalid;
 	}
 
-	if (userp)
-		*userp = user;
+	if (ruser)
+		*ruser = user;
 	else
 		freeuser(user);
+
 	return LoginValid;
 }
diff --git a/user.h b/user.h
@@ -3,11 +3,13 @@
 #define PassMin 8
 #define PassMax 72
 
-enum LoginStatus {
-	LoginUntried = -1,
+enum user_error {
 	LoginValid,
+	LoginNotFound,
 	LoginInvalid,
-	LoginError
+	LoginSystem,
+	LoginCorrupt,
+	LoginLast
 };
 
 struct user {
@@ -17,9 +19,12 @@ struct user {
 
 struct pagedata;
 
+extern char *user_errors[];
+
 int	adduser(struct pagedata *pd, char *name, char *key);
-void	freeuser(struct user *user);
-struct user	*getuser(struct pagedata *pd, char *field, char *value);
 int	deleteuser(struct pagedata *pd, char *hash);
-enum LoginStatus	loginuser(struct user **userp, struct pagedata *pd,
+void	freeuser(struct user *user);
+enum user_error	getuser(struct user **user, struct pagedata *pd,
+    char *field, char *value);
+enum user_error	loginuser(struct user **ruser, struct pagedata *pd,
     char *name, char *key);