From: Stephan Bosch <stephan.bosch@open-xchange.com>
Date: Mon, 20 Nov 2023 15:53:05 +0000 (+0100)
Subject: auth: mech-oauth2 - Rework error handling
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=91417ad42cf9612f48355c76f31b0c89cb947209;p=thirdparty%2Fdovecot%2Fcore.git

auth: mech-oauth2 - Rework error handling

Treat passdb errors as "invalid_token" and treat request decoding problems as
"invalid_request" status, which makes more sense. Also simplify the overall
structure.
---

diff --git a/src/auth/mech-oauth2.c b/src/auth/mech-oauth2.c
index ec48f72ea8..e2dc2dd3d0 100644
--- a/src/auth/mech-oauth2.c
+++ b/src/auth/mech-oauth2.c
@@ -48,8 +48,7 @@ static bool oauth2_unescape_username(const char *in, const char **username_r)
 }
 
 static void
-oauth2_fail(struct oauth2_auth_request *oauth2_req, int code,
-	    const char *status)
+oauth2_fail(struct oauth2_auth_request *oauth2_req, const char *status)
 {
 	struct auth_request *request = &oauth2_req->auth;
 	const char *oidc_url = (oauth2_req->db == NULL ? "" :
@@ -59,11 +58,17 @@ oauth2_fail(struct oauth2_auth_request *oauth2_req, int code,
 
 	json_ostream_ndescend_object(joutput, NULL);
 	if (strcmp(request->mech->mech_name, "XOAUTH2") == 0) {
-		status = dec2str(code);
+		if (strcmp(status, "invalid_token") == 0)
+			json_ostream_nwrite_string(joutput, "status", "401");
+		else if (strcmp(status, "insufficient_scope") == 0)
+			json_ostream_nwrite_string(joutput, "status", "403");
+		else
+			json_ostream_nwrite_string(joutput, "status", "400");
 		json_ostream_nwrite_string(joutput, "schemes", "bearer");
+	} else {
+		i_assert(strcmp(request->mech->mech_name, "OAUTHBEARER") == 0);
+		json_ostream_nwrite_string(joutput, "status", status);
 	}
-
-	json_ostream_nwrite_string(joutput, "status", status);
 	json_ostream_nwrite_string(joutput, "scope", "mail");
 	json_ostream_nwrite_string(joutput, "openid-configuration", oidc_url);
 	json_ostream_nascend_object(joutput);
@@ -73,6 +78,16 @@ oauth2_fail(struct oauth2_auth_request *oauth2_req, int code,
 	auth_request_fail_with_reply(request, str_data(reply), str_len(reply));
 }
 
+static void oauth2_fail_invalid_request(struct oauth2_auth_request *oauth2_req)
+{
+	oauth2_fail(oauth2_req, "invalid_request");
+}
+
+static void oauth2_fail_invalid_token(struct oauth2_auth_request *oauth2_req)
+{
+	oauth2_fail(oauth2_req, "invalid_token");
+}
+
 static void
 oauth2_verify_callback(enum passdb_result result,
 		       const unsigned char *credentials ATTR_UNUSED,
@@ -88,10 +103,10 @@ oauth2_verify_callback(enum passdb_result result,
 	case PASSDB_RESULT_USER_DISABLED:
 	case PASSDB_RESULT_PASS_EXPIRED:
 		/* user is explicitly disabled, don't allow it to log in */
-		oauth2_fail(oauth2_req, 403, "insufficient_scope");
-		return;
+		oauth2_fail(oauth2_req, "insufficient_scope");
+		break;
 	case PASSDB_RESULT_PASSWORD_MISMATCH:
-		oauth2_fail(oauth2_req, 401, "invalid_token");
+		oauth2_fail(oauth2_req, "invalid_token");
 		break;
 	case PASSDB_RESULT_NEXT:
 	case PASSDB_RESULT_SCHEME_NOT_AVAILABLE:
@@ -230,7 +245,7 @@ mech_oauthbearer_auth_continue(struct auth_request *request,
 		return;
 	}
 	if (data_size == 0) {
-		oauth2_fail(oauth2_req, 401, "invalid_token");
+		oauth2_fail_invalid_request(oauth2_req);
 		return;
 	}
 
@@ -245,7 +260,7 @@ mech_oauthbearer_auth_continue(struct auth_request *request,
 	/* ensure initial field is OK */
 	if (*fields == NULL || *(fields[0]) == '\0') {
 		e_info(request->mech_event, "Invalid continued data");
-		oauth2_fail(oauth2_req, 401, "invalid_token");
+		oauth2_fail_invalid_request(oauth2_req);
 		return;
 	}
 
@@ -255,13 +270,13 @@ mech_oauthbearer_auth_continue(struct auth_request *request,
 		case 'f':
 			e_info(request->mech_event,
 			       "Client requested non-standard mechanism");
-			oauth2_fail(oauth2_req, 400, "request_not_supported");
+			oauth2_fail_invalid_request(oauth2_req);
 			return;
 		case 'p':
 			/* channel binding is not supported */
 			e_info(request->mech_event,
 			       "Client requested and used channel-binding");
-			oauth2_fail(oauth2_req, 400, "request_not_supported");
+			oauth2_fail_invalid_request(oauth2_req);
 			return;
 		case 'n':
 		case 'y':
@@ -272,7 +287,7 @@ mech_oauthbearer_auth_continue(struct auth_request *request,
 			    !oauth2_unescape_username((*ptr)+2, &username)) {
 				e_info(request->mech_event,
 				       "Invalid username escaping");
-				oauth2_fail(oauth2_req, 400, "invalid_request");
+				oauth2_fail_invalid_request(oauth2_req);
 				return;
 			} else {
 				user_given = TRUE;
@@ -281,7 +296,7 @@ mech_oauthbearer_auth_continue(struct auth_request *request,
 		default:
 			e_info(request->mech_event,
 			       "Invalid gs2-header in request");
-			oauth2_fail(oauth2_req, 400, "invalid_request");
+			oauth2_fail_invalid_request(oauth2_req);
 			return;
 		}
 	}
@@ -294,7 +309,7 @@ mech_oauthbearer_auth_continue(struct auth_request *request,
 			} else {
 				e_info(request->mech_event,
 				       "Invalid continued data");
-				oauth2_fail(oauth2_req, 401, "invalid_token");
+				oauth2_fail_invalid_token(oauth2_req);
 				return;
 			}
 		}
@@ -303,17 +318,17 @@ mech_oauthbearer_auth_continue(struct auth_request *request,
 	if (user_given &&
 	    !auth_request_set_username(request, username, &error)) {
 		e_info(request->mech_event, "%s", error);
-		oauth2_fail(oauth2_req, 400, "invalid_request");
+		oauth2_fail_invalid_request(oauth2_req);
 		return;
 	}
 	if (user_given && token != NULL)
 		mech_oauth2_verify_token(oauth2_req, token);
 	else if (token == NULL) {
 		e_info(request->mech_event, "Missing token");
-		oauth2_fail(oauth2_req, 401, "invalid_token");
+		oauth2_fail_invalid_request(oauth2_req);
 	} else {
 		e_info(request->mech_event, "Missing username");
-		oauth2_fail(oauth2_req, 401, "invalid_request");
+		oauth2_fail_invalid_request(oauth2_req);
 	}
 }
 
@@ -334,7 +349,7 @@ mech_xoauth2_auth_continue(struct auth_request *request,
 		return;
 	}
 	if (data_size == 0) {
-		oauth2_fail(oauth2_req, 401, "invalid_token");
+		oauth2_fail_invalid_request(oauth2_req);
 		return;
 	}
 
@@ -361,7 +376,7 @@ mech_xoauth2_auth_continue(struct auth_request *request,
 			} else {
 				e_info(request->mech_event,
 				       "Invalid continued data");
-				oauth2_fail(oauth2_req, 401, "invalid_token");
+				oauth2_fail_invalid_token(oauth2_req);
 				return;
 			}
 		}
@@ -371,17 +386,17 @@ mech_xoauth2_auth_continue(struct auth_request *request,
 	if (user_given &&
 	    !auth_request_set_username(request, username, &error)) {
 		e_info(request->mech_event, "%s", error);
-		oauth2_fail(oauth2_req, 400, "invalid_request");
+		oauth2_fail_invalid_request(oauth2_req);
 		return;
 	}
 	if (user_given && token != NULL)
 		mech_oauth2_verify_token(oauth2_req, token);
 	else if (token == NULL) {
 		e_info(request->mech_event, "Missing token");
-		oauth2_fail(oauth2_req, 401, "invalid_token");
+		oauth2_fail_invalid_request(oauth2_req);
 	} else {
 		e_info(request->mech_event, "Missing username");
-		oauth2_fail(oauth2_req, 401, "invalid_token");
+		oauth2_fail_invalid_request(oauth2_req);
 	}
 }