From 3840f43fa0026b06b15913379761c1bf6b6a1c68 Mon Sep 17 00:00:00 2001 From: Fabian Henneke Date: Thu, 3 Sep 2020 10:48:14 +0200 Subject: Refactor Git operations and auth (#1066) Co-authored-by: Harsh Shandilya --- .../java/com/zeapo/pwdstore/git/BaseGitActivity.kt | 104 ++++++++++++++++----- 1 file changed, 79 insertions(+), 25 deletions(-) (limited to 'app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt') diff --git a/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt b/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt index ed72c88d..e16b0c8b 100644 --- a/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt +++ b/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt @@ -6,9 +6,14 @@ package com.zeapo.pwdstore.git import android.view.MenuItem import androidx.appcompat.app.AppCompatActivity +import androidx.core.content.edit import com.github.ajalt.timberkt.Timber.tag +import com.github.ajalt.timberkt.d import com.github.ajalt.timberkt.e +import com.github.michaelbull.result.Err import com.google.android.material.dialog.MaterialAlertDialogBuilder +import com.github.michaelbull.result.Result +import com.zeapo.pwdstore.R import com.zeapo.pwdstore.git.config.GitSettings import com.zeapo.pwdstore.git.operation.BreakOutOfDetached import com.zeapo.pwdstore.git.operation.CloneOperation @@ -17,10 +22,15 @@ import com.zeapo.pwdstore.git.operation.PullOperation import com.zeapo.pwdstore.git.operation.PushOperation import com.zeapo.pwdstore.git.operation.ResetToRemoteOperation import com.zeapo.pwdstore.git.operation.SyncOperation -import com.zeapo.pwdstore.utils.PasswordRepository +import com.zeapo.pwdstore.utils.PreferenceKeys +import com.zeapo.pwdstore.utils.getEncryptedPrefs +import com.zeapo.pwdstore.utils.sharedPrefs +import net.schmizz.sshj.common.DisconnectReason +import net.schmizz.sshj.common.SSHException +import net.schmizz.sshj.userauth.UserAuthException /** - * Abstract AppCompatActivity that holds some information that is commonly shared across git-related + * Abstract [AppCompatActivity] that holds some information that is commonly shared across git-related * tasks and makes sense to be held here. */ abstract class BaseGitActivity : AppCompatActivity() { @@ -39,33 +49,77 @@ abstract class BaseGitActivity : AppCompatActivity() { * Attempt to launch the requested Git operation. * @param operation The type of git operation to launch */ - suspend fun launchGitOperation(operation: Int) { + suspend fun launchGitOperation(operation: Int): Result { if (GitSettings.url == null) { - setResult(RESULT_CANCELED) - finish() - return + return Err(IllegalStateException("Git url is not set!")) } - try { - val localDir = requireNotNull(PasswordRepository.getRepositoryDirectory()) - val op = when (operation) { - REQUEST_CLONE, GitOperation.GET_SSH_KEY_FROM_CLONE -> CloneOperation(localDir, GitSettings.url!!, this) - REQUEST_PULL -> PullOperation(localDir, this) - REQUEST_PUSH -> PushOperation(localDir, this) - REQUEST_SYNC -> SyncOperation(localDir, this) - BREAK_OUT_OF_DETACHED -> BreakOutOfDetached(localDir, this) - REQUEST_RESET -> ResetToRemoteOperation(localDir, this) - else -> { - tag(TAG).e { "Operation not recognized : $operation" } - setResult(RESULT_CANCELED) - finish() - return - } + val op = when (operation) { + REQUEST_CLONE, GitOperation.GET_SSH_KEY_FROM_CLONE -> CloneOperation(this, GitSettings.url!!) + REQUEST_PULL -> PullOperation(this) + REQUEST_PUSH -> PushOperation(this) + REQUEST_SYNC -> SyncOperation(this) + BREAK_OUT_OF_DETACHED -> BreakOutOfDetached(this) + REQUEST_RESET -> ResetToRemoteOperation(this) + else -> { + tag(TAG).e { "Operation not recognized : $operation" } + return Err(IllegalArgumentException("$operation is not a valid Git operation")) } - op.executeAfterAuthentication(GitSettings.authMode) - } catch (e: Exception) { - e.printStackTrace() - MaterialAlertDialogBuilder(this).setMessage(e.message).show() } + return op.executeAfterAuthentication(GitSettings.authMode) + } + + fun finishOnSuccessHandler(@Suppress("UNUSED_PARAMETER") nothing: Unit) { + finish() + } + + fun finishAfterPromptOnErrorHandler(err: Throwable) { + val error = rootCauseException(err) + if (!isExplicitlyUserInitiatedError(error)) { + getEncryptedPrefs("git_operation").edit { + remove(PreferenceKeys.HTTPS_PASSWORD) + } + sharedPrefs.edit { remove(PreferenceKeys.SSH_OPENKEYSTORE_KEYID) } + d(error) + MaterialAlertDialogBuilder(this) + .setTitle(resources.getString(R.string.jgit_error_dialog_title)) + .setMessage(ErrorMessages[error]) + .setPositiveButton(resources.getString(R.string.dialog_ok)) { _, _ -> + finish() + }.show() + } + } + + /** + * Check if a given [Throwable] is the result of an error caused by the user cancelling the + * operation. + */ + private fun isExplicitlyUserInitiatedError(throwable: Throwable): Boolean { + var cause: Throwable? = throwable + while (cause != null) { + if (cause is SSHException && + cause.disconnectReason == DisconnectReason.AUTH_CANCELLED_BY_USER) + return true + cause = cause.cause + } + return false + } + + /** + * Get the real root cause of a [Throwable] by traversing until known wrapping exceptions are no + * longer found. + */ + private fun rootCauseException(throwable: Throwable): Throwable { + var rootCause = throwable + // JGit's TransportException hides the more helpful SSHJ exceptions. + // Also, SSHJ's UserAuthException about exhausting available authentication methods hides + // more useful exceptions. + while ((rootCause is org.eclipse.jgit.errors.TransportException || + rootCause is org.eclipse.jgit.api.errors.TransportException || + (rootCause is UserAuthException && + rootCause.message == "Exhausted available authentication methods"))) { + rootCause = rootCause.cause ?: break + } + return rootCause } companion object { -- cgit v1.2.3