Remove installer interaction delegate API / refactor finishing update…

… cycle (sparkle-project#1925)

Documentation/Installation.md

@@ -13,9 +13,9 @@
 ### Launching the Installer
 SPUCoreBasedUpdateDriver invokes SPUInstallerDriver's extraction method which invokes SUInstallerLauncher's launch installer method. The launcher can be inside a XPC service or running inside the updater/sparkle framework itself. The launcher XPC bundle includes a copy of the installer and agent application in its bundle so it can compute the path to them itself (instead of relying on the updater to pass the paths), which is for security trust reasons.
 
-First, the launcher makes sure that if the updater delegate doesn't allow interaction, that installation can be done without interaction. If it can't, then the launcher fails. If the updater driver that invoked the launcher doesn't allow for interaction (in particular, the automatic/silent update driver does not allow interaction), then the launcher fails with a hint to try again with interaction enabled (in particular, with a UI based update driver). If this happens, the updater keeps a reference to the downloaded update and appcast item that is used for resuming with a UI based update driver later. The launcher in this case is telling that authorization can only be done if interaction is allowed from the driver, so the user can enter their password to start up the installer as root to install over a location that requires such privileges.
+First, if the updater driver that invoked the launcher doesn't allow for interaction (in particular, the automatic/silent update driver does not allow interaction), then the launcher fails with a hint to try again with interaction enabled (in particular, with a UI based update driver). If this happens, the updater keeps a reference to the downloaded update and appcast item that is used for resuming with a UI based update driver later. The launcher in this case is telling that authorization can only be done if interaction is allowed from the driver, so the user can enter their password to start up the installer as root to install over a location that requires such privileges.
 
-The launcher then looks for the installer and agent application first in its bundle's auxiliary directory path, and then its resources path. If the launcher is running inside a XPC service, the tools will be in the auxiliary path location, which allows the developer using the service to also code sign the tools. Once those paths are found, the launcher copies the progress agent application to a temporary cache location. The installer is not copied because it is a plain single executable that does not rely on external relative depedencies, whereas the agent application relies on bundle resources. This is important to consider because the old application may be moved around and removed when the installation process occurs. Leaving the installer inside its bundle also could leave it in a place the user doesn't necessarily have write privileges to.
+The launcher then looks for the installer and agent application first in its bundle's auxiliary directory path. Once those paths are found, the launcher copies the progress agent application to a temporary cache location. The installer is not copied because it is a plain single executable that does not rely on external relative dependencies, whereas the agent application relies on bundle resources. This is important to consider because the old application may be moved around and removed when the installation process occurs. Leaving the installer inside its bundle also could leave it in a place the user doesn't necessarily have write privileges to.
 
 The installer is first submitted. If it requires root privileges, it will ask for an admin user name and password. How it determines if root privileges are necessary is based on:
 
@@ -27,7 +27,7 @@ The installer makes sure, after extraction, that the expected installation type
 
 If the user cancels submitting the installer on authorization (if it's required), the launcher aborts the installation process silently. If everything does alright, the progress agent application is submitted next. If either of the submissions fail, the installation fails and aborts, otherwise the launcher succeeds and its job is done. Note before the submissions are done, the jobs are removed from launchd, and the jobs are submitted in a way that act as "one-time" jobs. If they fail or succeed, the jobs aren't restarted, and the jobs aren't installed in a system location for launchd to attempt launching again.
 
-The only argument passed to the installer is the host bundle identifier of the bundle to update. This bundle identifier is used so that the installer and updater know what Mach service to connect to for IPC. Everything else to the installer is sent via a XPC connection. The host bundle path is passed to the progress agent, which is just used for obtaining the bundle identifier, and for UI purposes (eg: displaying app icon and name when the app needs to show progress). The type of domain the installer is running in (user vs system) is also passed to the progress agent application so it can know which domain to use to connect to the installer.
+The most important argument passed to the installer is the host bundle identifier of the bundle to update. This bundle identifier is used so that the installer and updater know what Mach service to connect to for IPC. Mostly everything else to the installer is sent via a XPC connection. The host bundle path is passed to the progress agent, which is just used for obtaining the bundle identifier, and for UI purposes (eg: displaying app icon and name when the app needs to show progress). The type of domain the installer is running in (user vs system) is also passed to the progress agent application so it can know which domain to use to connect to the installer.
 
 ### Timeouts
 

InstallerLauncher/SUInstallerLauncher+Private.h

@@ -0,0 +1,29 @@
+//
+//  SUInstallerLauncher+Private.h
+//  SUInstallerLauncher+Private
+//
+//  Created by Mayur Pawashe on 8/21/21.
+//  Copyright © 2021 Sparkle Project. All rights reserved.
+//
+
+#ifndef SUInstallerLauncher_Private_h
+#define SUInstallerLauncher_Private_h
+
+#import <Sparkle/SUExport.h>
+
+// Chances are clients will need this too
+#import <Sparkle/SPUInstallationType.h>
+
+@class NSString;
+
+/**
+ Private API for determining if the system needs authorization access to update a bundle path
+ 
+ This API is not supported when used directly from a Sandboxed applications and will always return @c YES in that case.
+ 
+ @param bundlePath The bundle path to test if authorization is needed when performing an update that replaces this bundle.
+ @return @c YES if Sparkle thinks authorization is needed to update the @p bundlePath, otherwise @c NO.
+ */
+SU_EXPORT BOOL SPUSystemNeedsAuthorizationAccessForBundlePath(NSString *bundlePath);
+
+#endif /* SUInstallerLauncher_Private_h */

InstallerLauncher/SUInstallerLauncher.m

@@ -7,6 +7,7 @@
 //
 
 #import "SUInstallerLauncher.h"
+#import "SUInstallerLauncher+Private.h"
 #import "SUFileManager.h"
 #import "SULog.h"
 #import "SPUMessageTypes.h"
@@ -315,53 +316,62 @@ - (NSString *)pathForBundledTool:(NSString *)toolName extension:(NSString *)exte
     return resolvedAuxiliaryToolURL.path;
 }
 
-static BOOL SPUNeedsSystemAuthorizationAccess(NSString *path, NSString *installationType)
+BOOL SPUSystemNeedsAuthorizationAccessForBundlePath(NSString *bundlePath)
 {
+    NSFileManager *fileManager = [NSFileManager defaultManager];
+    BOOL hasWritability = [fileManager isWritableFileAtPath:bundlePath] && [fileManager isWritableFileAtPath:[bundlePath stringByDeletingLastPathComponent]];
+
     BOOL needsAuthorization;
-    if ([installationType isEqualToString:SPUInstallationTypeGuidedPackage]) {
+    if (!hasWritability) {
         needsAuthorization = YES;
-    } else if ([installationType isEqualToString:SPUInstallationTypeInteractivePackage]) {
-        needsAuthorization = NO;
     } else {
-        NSFileManager *fileManager = [NSFileManager defaultManager];
-        BOOL hasWritability = [fileManager isWritableFileAtPath:path] && [fileManager isWritableFileAtPath:[path stringByDeletingLastPathComponent]];
-        if (!hasWritability) {
+        // Just because we have writability access does not mean we can set the correct owner/group
+        // Test if we can set the owner/group on a temporarily created file
+        // If we can, then we can probably perform an update without authorization
+
+        NSString *tempFilename = @"permission_test" ;
+
+        SUFileManager *suFileManager = [[SUFileManager alloc] init];
+        NSURL *tempDirectoryURL = [suFileManager makeTemporaryDirectoryWithPreferredName:tempFilename appropriateForDirectoryURL:[NSURL fileURLWithPath:NSTemporaryDirectory()] error:NULL];
+
+        if (tempDirectoryURL == nil) {
+            // I don't imagine this ever happening but in case it does, requesting authorization may be the better option
             needsAuthorization = YES;
         } else {
-            // Just because we have writability access does not mean we can set the correct owner/group
-            // Test if we can set the owner/group on a temporarily created file
-            // If we can, then we can probably perform an update without authorization
-
-            NSString *tempFilename = @"permission_test" ;
-
-            SUFileManager *suFileManager = [[SUFileManager alloc] init];
-            NSURL *tempDirectoryURL = [suFileManager makeTemporaryDirectoryWithPreferredName:tempFilename appropriateForDirectoryURL:[NSURL fileURLWithPath:NSTemporaryDirectory()] error:NULL];
-
-            if (tempDirectoryURL == nil) {
-                // I don't imagine this ever happening but in case it does, requesting authorization may be the better option
+            NSURL *tempFileURL = [tempDirectoryURL URLByAppendingPathComponent:tempFilename];
+            if (![[NSData data] writeToURL:tempFileURL atomically:NO]) {
+                // Obvious indicator we may need authorization
                 needsAuthorization = YES;
             } else {
-                NSURL *tempFileURL = [tempDirectoryURL URLByAppendingPathComponent:tempFilename];
-                if (![[NSData data] writeToURL:tempFileURL atomically:NO]) {
-                    // Obvious indicator we may need authorization
-                    needsAuthorization = YES;
-                } else {
-                    needsAuthorization = ![suFileManager changeOwnerAndGroupOfItemAtRootURL:tempFileURL toMatchURL:[NSURL fileURLWithPath:path] error:NULL];
-                }
-
-                [suFileManager removeItemAtURL:tempDirectoryURL error:NULL];
+                needsAuthorization = ![suFileManager changeOwnerAndGroupOfItemAtRootURL:tempFileURL toMatchURL:[NSURL fileURLWithPath:bundlePath] error:NULL];
             }
+
+            [suFileManager removeItemAtURL:tempDirectoryURL error:NULL];
         }
     }
+
+    return needsAuthorization;
+}
+
+static BOOL SPUSystemNeedsAuthorizationAccess(NSString *path, NSString *installationType)
+{
+    BOOL needsAuthorization;
+    if ([installationType isEqualToString:SPUInstallationTypeGuidedPackage]) {
+        needsAuthorization = YES;
+    } else if ([installationType isEqualToString:SPUInstallationTypeInteractivePackage]) {
+        needsAuthorization = NO;
+    } else {
+        needsAuthorization = SPUSystemNeedsAuthorizationAccessForBundlePath(path);
+    }
     return needsAuthorization;
 }
 
 
 // Note: do not pass untrusted information such as paths to the installer and progress agent tools, when we can find them ourselves here
-- (void)launchInstallerWithHostBundlePath:(NSString *)hostBundlePath updaterIdentifier:(NSString *)updaterIdentifier authorizationPrompt:(NSString *)authorizationPrompt installationType:(NSString *)installationType allowingDriverInteraction:(BOOL)allowingDriverInteraction allowingUpdaterInteraction:(BOOL)allowingUpdaterInteraction completion:(void (^)(SUInstallerLauncherStatus, BOOL))completionHandler
+- (void)launchInstallerWithHostBundlePath:(NSString *)hostBundlePath updaterIdentifier:(NSString *)updaterIdentifier authorizationPrompt:(NSString *)authorizationPrompt installationType:(NSString *)installationType allowingDriverInteraction:(BOOL)allowingDriverInteraction completion:(void (^)(SUInstallerLauncherStatus, BOOL))completionHandler
 {
     dispatch_async(dispatch_get_main_queue(), ^{
-        BOOL needsSystemAuthorization = SPUNeedsSystemAuthorizationAccess(hostBundlePath, installationType);
+        BOOL needsSystemAuthorization = SPUSystemNeedsAuthorizationAccess(hostBundlePath, installationType);
 
         NSBundle *hostBundle = [NSBundle bundleWithPath:hostBundlePath];
         if (hostBundle == nil) {
@@ -372,18 +382,6 @@ - (void)launchInstallerWithHostBundlePath:(NSString *)hostBundlePath updaterIden
             return;
         }
 
-        if (needsSystemAuthorization && !allowingUpdaterInteraction) {
-            SULog(SULogLevelError, @"Updater is not allowing interaction to the launcher.");
-            completionHandler(SUInstallerLauncherFailure, needsSystemAuthorization);
-            return;
-        }
-
-        if (!allowingUpdaterInteraction && [installationType isEqualToString:SPUInstallationTypeInteractivePackage]) {
-            SULog(SULogLevelError, @"Updater is not allowing interaction to the launcher for performing an interactive type package installation.");
-            completionHandler(SUInstallerLauncherFailure, needsSystemAuthorization);
-            return;
-        }
-
         // if we need to use the system domain and we aren't allowed interaction, then try sometime later when interaction is allowed
         if (needsSystemAuthorization && !allowingDriverInteraction) {
             completionHandler(SUInstallerLauncherAuthorizeLater, needsSystemAuthorization);
@@ -462,10 +460,4 @@ - (void)launchInstallerWithHostBundlePath:(NSString *)hostBundlePath updaterIden
     });
 }
 
-- (void)checkIfApplicationInstallationRequiresAuthorizationWithHostBundlePath:(NSString *)hostBundlePath reply:(void(^)(BOOL))reply
-{
-    // No need to execute on main queue
-    reply(SPUNeedsSystemAuthorizationAccess(hostBundlePath, SPUInstallationTypeApplication));
-}
-
 @end

InstallerLauncher/SUInstallerLauncherProtocol.h

@@ -11,8 +11,6 @@
 
 @protocol SUInstallerLauncherProtocol
 
-- (void)launchInstallerWithHostBundlePath:(NSString *)hostBundlePath updaterIdentifier:(NSString *)updaterBundleIdentifier authorizationPrompt:(NSString *)authorizationPrompt installationType:(NSString *)installationType allowingDriverInteraction:(BOOL)allowingDriverInteraction allowingUpdaterInteraction:(BOOL)allowingUpdaterInteraction completion:(void (^)(SUInstallerLauncherStatus, BOOL))completionHandler;
-
-- (void)checkIfApplicationInstallationRequiresAuthorizationWithHostBundlePath:(NSString *)hostBundlePath reply:(void(^)(BOOL requiresAuthorization))reply;
+- (void)launchInstallerWithHostBundlePath:(NSString *)hostBundlePath updaterIdentifier:(NSString *)updaterBundleIdentifier authorizationPrompt:(NSString *)authorizationPrompt installationType:(NSString *)installationType allowingDriverInteraction:(BOOL)allowingDriverInteraction completion:(void (^)(SUInstallerLauncherStatus, BOOL))completionHandler;
 
 @end