On Thu, 28 Jan 2016, Dave Mielke wrote:

I have written a patch to allow polkit-based authentication for
brlapi.

Cool! Thanks. Your patch has now been committed to the repository. I've made a
few formatting changes so please verify that I haven't broken anything.

There are a couple of error paths that don't log the problem. I'd appreciate it
if you could ensure that all error paths log the reason for the failure.

The subject variable gets assigned by a function call but it isn't checked for
success. Is that call always successful?

I'm attaching a patch that fixes a few things:
- logs if polkit_unix_process_new_for_owner fails.
- Adds comments to the arguments to polkit_authority_check_authorization_sync(). - Move the g_object_unref() so that it isn't called until we're finished with the result.

Maybe the logSystemError("polkit_authority_check_authorization_sync") should log the contents of error_local->message somehow.

Thanks,
-Mike
diff --git a/Programs/auth.c b/Programs/auth.c
index 1913640..6de3844 100644
--- a/Programs/auth.c
+++ b/Programs/auth.c
@@ -486,27 +486,30 @@ authPolkit_server (AuthDescriptor *auth, FileDescriptor 
fd, void *data) {
     logMessage(LOG_DEBUG, "attempting to authenticate pid %d via polkit", 
cred.pid);
 
     PolkitSubject *subject = polkit_unix_process_new_for_owner(cred.pid, -1, 
-1);
-    GError *error_local = NULL;
-
-    PolkitAuthorizationResult *result = 
polkit_authority_check_authorization_sync(
-      polkit->authority,
-      subject,
-      "org.brltty.write-display",
-      NULL,
-      POLKIT_CHECK_AUTHORIZATION_FLAGS_NONE,
-      NULL,
-      &error_local
-    );
-
-    if (result) {
-      g_object_unref(result);
-
-      int isAuthorized = polkit_authorization_result_get_is_authorized(result);
-      logMessage(LOG_DEBUG, "polkit_authority_check_authorization_sync 
returned %d", isAuthorized);
-      return isAuthorized;
+    if (subject) {
+      GError *error_local = NULL;
+
+      PolkitAuthorizationResult *result = 
polkit_authority_check_authorization_sync(
+        polkit->authority,                     /* authority */
+        subject,                               /* PolkitSubject for client */
+        "org.brltty.write-display",            /* name of polkit action */
+        NULL,                                  /* details */
+        POLKIT_CHECK_AUTHORIZATION_FLAGS_NONE, /* disallow interaction */
+        NULL,                                  /* GCancellable */
+        &error_local                           /* returned error */
+      );
+
+      if (result) {
+        int isAuthorized = 
polkit_authorization_result_get_is_authorized(result);
+        g_object_unref(result);
+        logMessage(LOG_DEBUG, "polkit_authority_check_authorization_sync 
returned %d", isAuthorized);
+        return isAuthorized;
+      } else {
+        logSystemError("polkit_authority_check_authorization_sync");
+        g_error_free(error_local);
+      }
     } else {
-      logSystemError("polkit_authority_check_authorization_sync");
-      g_error_free(error_local);
+      logSystemError("polkit_unix_process_new_for_owner");
     }
   } else {
     logSystemError("getsockopt[SO_PEERCRED]");
_______________________________________________
This message was sent via the BRLTTY mailing list.
To post a message, send an e-mail to: [email protected]
For general information, go to: http://mielke.cc/mailman/listinfo/brltty

Reply via email to