Mercurial > dovecot > core-2.2

changeset 21733:01ffe59436af

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
auth: oauth2 - remove db_oauth2_request.result It's not a persistent state. When it's set, the callback needs to be called. This way it's more difficult to forget to set it.
author Timo Sirainen <timo.sirainen@dovecot.fi>
date Thu, 16 Mar 2017 00:38:39 +0200
parents 78b6f3032cc6
children 8b5f6e2ff4a6
files src/auth/db-oauth2.c src/auth/db-oauth2.h src/auth/passdb-oauth2.c
diffstat 3 files changed, 36 insertions(+), 29 deletions(-) [+]
line wrap: on
line diff
--- a/src/auth/db-oauth2.c	Thu Mar 16 00:33:24 2017 +0200
+++ b/src/auth/db-oauth2.c	Thu Mar 16 00:38:39 2017 +0200
@@ -346,6 +346,7 @@
 
 static bool
 db_oauth2_template_export(struct db_oauth2_request *req,
+			  enum passdb_result *result_r ATTR_UNUSED,
 			  const char **error_r ATTR_UNUSED)
 {
 	/* var=$ expands into var=${oauth2:var} */
@@ -396,22 +397,24 @@
 	}
 }
 
-static void db_oauth2_callback(struct db_oauth2_request *req, bool success,
+static void db_oauth2_callback(struct db_oauth2_request *req,
+			       enum passdb_result result, bool success,
 			       const char *error)
 {
 	db_oauth2_lookup_callback_t *callback = req->callback;
 	req->callback = NULL;
 
-	i_assert(req->result == PASSDB_RESULT_OK || (!success && error != NULL));
+	i_assert(result == PASSDB_RESULT_OK || (!success && error != NULL));
 
 	if (callback != NULL) {
 		DLLIST_REMOVE(&req->db->head, req);
-		callback(req->db, success, req, error, req->context);
+		callback(req->db, result, success, req, error, req->context);
 	}
 }
 
 static bool
-db_oauth2_validate_username(struct db_oauth2_request *req, const char **error_r)
+db_oauth2_validate_username(struct db_oauth2_request *req,
+			    enum passdb_result *result_r, const char **error_r)
 {
 	struct var_expand_table table[] = {
 		{ 'u', NULL, "user" },
@@ -423,7 +426,7 @@
 		auth_fields_find(req->fields, req->db->set.username_attribute);
 
 	if (username_value == NULL) {
-		req->result = PASSDB_RESULT_INTERNAL_FAILURE;
+		*result_r = PASSDB_RESULT_INTERNAL_FAILURE;
 		req->failed = TRUE;
 		*error_r = "No username returned";
 		return FALSE;
@@ -444,7 +447,7 @@
 	if (!str_equals(username_req, username_val)) {
 		*error_r = t_strdup_printf("Username '%s' did not match '%s'",
 					str_c(username_req), str_c(username_val));
-		req->result = PASSDB_RESULT_USER_UNKNOWN;
+		*result_r = PASSDB_RESULT_USER_UNKNOWN;
 		req->failed = TRUE;
 	}
 
@@ -452,7 +455,8 @@
 }
 
 static bool
-db_oauth2_user_is_enabled(struct db_oauth2_request *req, const char **error_r)
+db_oauth2_user_is_enabled(struct db_oauth2_request *req,
+			  enum passdb_result *result_r, const char **error_r)
 {
 	if (*req->db->set.active_attribute != '\0') {
 		const char *active_value = auth_fields_find(req->fields, req->db->set.active_attribute);
@@ -460,7 +464,7 @@
 		    (*req->db->set.active_value != '\0' &&
 		     strcmp(req->db->set.active_value, active_value) != 0)) {
 			*error_r = "User account is not active";
-			req->result = PASSDB_RESULT_USER_DISABLED;
+			*result_r = PASSDB_RESULT_USER_DISABLED;
 			req->failed = TRUE;
 		}
 	}
@@ -468,7 +472,8 @@
 }
 
 static bool
-db_oauth2_token_in_scope(struct db_oauth2_request *req, const char **error_r)
+db_oauth2_token_in_scope(struct db_oauth2_request *req,
+			 enum passdb_result *result_r, const char **error_r)
 {
 	if (*req->db->set.scope != '\0') {
 		bool found = FALSE;
@@ -480,7 +485,7 @@
 		if (!found) {
 			*error_r = t_strdup_printf("Token is not valid for scope '%s'",
 						   req->db->set.scope);
-			req->result = PASSDB_RESULT_USER_DISABLED;
+			*result_r = PASSDB_RESULT_USER_DISABLED;
 			req->failed = TRUE;
 		}
 	}
@@ -489,18 +494,19 @@
 
 static void db_oauth2_process_fields(struct db_oauth2_request *req)
 {
+	enum passdb_result result;
 	const char *error = NULL;
-	if (db_oauth2_validate_username(req, &error) &&
-	    db_oauth2_user_is_enabled(req, &error) &&
-	    db_oauth2_token_in_scope(req, &error) &&
-	    db_oauth2_template_export(req, &error) &&
+	if (db_oauth2_validate_username(req, &result, &error) &&
+	    db_oauth2_user_is_enabled(req, &result, &error) &&
+	    db_oauth2_token_in_scope(req, &result, &error) &&
+	    db_oauth2_template_export(req, &result, &error) &&
 	    !req->failed) {
-		req->result = PASSDB_RESULT_OK;
+		result = PASSDB_RESULT_OK;
 	} else {
-		i_assert(req->result != PASSDB_RESULT_OK && error != NULL);
+		i_assert(result != PASSDB_RESULT_OK && error != NULL);
 	}
 
-	db_oauth2_callback(req, !req->failed, error);
+	db_oauth2_callback(req, result, !req->failed, error);
 }
 
 static void
@@ -511,9 +517,8 @@
 
 	if (!result->success) {
 		/* fail here */
-		req->result = PASSDB_RESULT_INTERNAL_FAILURE;
 		req->failed = TRUE;
-		db_oauth2_callback(req, FALSE, result->error);
+		db_oauth2_callback(req, PASSDB_RESULT_INTERNAL_FAILURE, FALSE, result->error);
 		return;
 	}
 	db_oauth2_fields_merge(req, result->fields);
@@ -548,11 +553,11 @@
 
 	if (!result->success || !result->valid) {
 		/* no point going forward */
-		req->result = result->success ?
+		enum passdb_result passdb_result = result->success ?
 			PASSDB_RESULT_PASSWORD_MISMATCH :
-			PASSDB_RESULT_INTERNAL_FAILURE,
+			PASSDB_RESULT_INTERNAL_FAILURE;
 		req->failed = TRUE;
-		db_oauth2_callback(req, FALSE, result->error == NULL ? "Invalid token" : result->error);
+		db_oauth2_callback(req, passdb_result, FALSE, result->error == NULL ? "Invalid token" : result->error);
 		return;
 	}
 
--- a/src/auth/db-oauth2.h	Thu Mar 16 00:33:24 2017 +0200
+++ b/src/auth/db-oauth2.h	Thu Mar 16 00:38:39 2017 +0200
@@ -5,7 +5,9 @@
 struct oauth2_request;
 struct db_oauth2_request;
 
-typedef void db_oauth2_lookup_callback_t(struct db_oauth2 *db, bool success,
+typedef void db_oauth2_lookup_callback_t(struct db_oauth2 *db,
+					 enum passdb_result result,
+					 bool success,
 					 struct db_oauth2_request *request,
 					 const char *error,
 					 void *context);
@@ -28,7 +30,6 @@
 	void *context;
 	verify_plain_callback_t *verify_callback;
 
-	enum passdb_result result;
 	bool failed:1;
 };
 
@@ -41,7 +42,7 @@
 void db_oauth2_lookup(struct db_oauth2 *db, struct db_oauth2_request *req, const char *token, struct auth_request *request, db_oauth2_lookup_callback_t *callback, void *context);
 #define db_oauth2_lookup(db, req, token, request, callback, context) \
 	db_oauth2_lookup(db, req, token + \
-		CALLBACK_TYPECHECK(callback, void(*)(struct db_oauth2*, bool, struct db_oauth2_request *req, const char*, typeof(context))), \
+		CALLBACK_TYPECHECK(callback, void(*)(struct db_oauth2*, enum passdb_result, bool, struct db_oauth2_request *req, const char*, typeof(context))), \
 		request, (db_oauth2_lookup_callback_t*)callback, (void*)context)
 
 #endif
--- a/src/auth/passdb-oauth2.c	Thu Mar 16 00:33:24 2017 +0200
+++ b/src/auth/passdb-oauth2.c	Thu Mar 16 00:38:39 2017 +0200
@@ -10,18 +10,19 @@
 };
 
 static void
-oauth2_verify_plain_continue(struct db_oauth2 *db ATTR_UNUSED, bool success,
+oauth2_verify_plain_continue(struct db_oauth2 *db ATTR_UNUSED,
+			     enum passdb_result result, bool success,
 			     struct db_oauth2_request *req, const char *error,
 			     struct auth_request *request)
 {
-	i_assert(success || req->result != PASSDB_RESULT_OK);
-	if (!success && req->result == PASSDB_RESULT_INTERNAL_FAILURE)
+	i_assert(success || result != PASSDB_RESULT_OK);
+	if (!success && result == PASSDB_RESULT_INTERNAL_FAILURE)
 		auth_request_log_error(request, AUTH_SUBSYS_DB, "oauth2 failed: %s",
 				       error);
 	else if (!success)
 		auth_request_log_info(request, AUTH_SUBSYS_DB, "oauth2 failed: %s",
 				      error);
-	req->verify_callback(req->result, request);
+	req->verify_callback(result, request);
 	auth_request_unref(&request);
 }