fix: support Zimbra 2FA via password + OTP code#40
Open
HKNDerRollo wants to merge 7 commits into
Open
Conversation
|
|
Author
|
Sorry, I got it wrong. You can take it out |
HKNDerRollo
commented
May 15, 2026
HKNDerRollo
left a comment
HKNDerRollo
left a comment
Author
There was a problem hiding this comment.
“build/tools/composer.phar” and “0001-fix-zimbra-2fa-app-passwords.patch” removed
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
When Zimbra 2FA is enabled, password-based authentication fails and the app crashes on every request boot with: 'Could not boot integration_zimbra: Authenticated ciphertext could not be decoded.' This commit fixes both the crash and adds proper 2FA support via app-specific passwords (generated in Zimbra Preferences → Accounts). Changes: - ZimbraAPIService: wrap decryptIfNotEmpty() in try/catch so a broken or missing ciphertext no longer crashes the app at boot - ZimbraAPIService: add getEffectivePassword() which prefers the stored app_password over the regular password, enabling seamless 2FA bypass - ZimbraAPIService: use getEffectivePassword() in checkTokenExpiration() and isUserConnected() so token refresh works with app-specific passwords - ConfigController: persist app_password (encrypted) when provided; use it as the login credential when present; delete it on logout; block it in the non-sensitive config route - Settings/Personal: expose app_password_is_set (boolean) in initial state so the Vue component can show the correct placeholder text - PersonalSettings.vue: add optional 'App-specific password' input field with descriptive hint text; include it in the connect payload; reset it on logout - Migration Version1014Date20250707000000: scan all Zimbra users on upgrade and delete any credentials that can no longer be decrypted, so affected users see the login form instead of a 500 error Fixes: nextcloud#33 Signed-off-by: Dominik <drauer@hkn.de>
From: dominikrauer <dominikrauer@users.noreply.github.com> Date: Thu, 7 May 2026 07:59:34 +0000 Subject: [PATCH] fix: support app-specific passwords for Zimbra 2FA compatibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Zimbra 2FA is enabled, password-based authentication fails and the app crashes on every request boot with: 'Could not boot integration_zimbra: Authenticated ciphertext could not be decoded.' This commit fixes both the crash and adds proper 2FA support via app-specific passwords (generated in Zimbra Preferences → Accounts). Changes: - ZimbraAPIService: wrap decryptIfNotEmpty() in try/catch so a broken or missing ciphertext no longer crashes the app at boot - ZimbraAPIService: add getEffectivePassword() which prefers the stored app_password over the regular password, enabling seamless 2FA bypass - ZimbraAPIService: use getEffectivePassword() in checkTokenExpiration() and isUserConnected() so token refresh works with app-specific passwords - ConfigController: persist app_password (encrypted) when provided; use it as the login credential when present; delete it on logout; block it in the non-sensitive config route - Settings/Personal: expose app_password_is_set (boolean) in initial state so the Vue component can show the correct placeholder text - PersonalSettings.vue: add optional 'App-specific password' input field with descriptive hint text; include it in the connect payload; reset it on logout - Migration Version1014Date20250707000000: scan all Zimbra users on upgrade and delete any credentials that can no longer be decrypted, so affected users see the login form instead of a 500 error Fixes: nextcloud#33 --- lib/Controller/ConfigController.php | 13 +++- .../Version1014Date20250707000000.php | 62 +++++++++++++++++++ lib/Service/ZimbraAPIService.php | 29 ++++++++- lib/Settings/Personal.php | 8 ++- src/components/PersonalSettings.vue | 25 ++++++++ 5 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 lib/Migration/Version1014Date20250707000000.php diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index 90d589f..7311ada 100644 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -63,8 +63,16 @@ class ConfigController extends Controller { public function setSensitiveConfig(array $values): DataResponse { if (isset($values['url'], $values['login'], $values['password'])) { $this->config->setUserValue($this->userId, Application::APP_ID, 'url', $values['url']); + // Store app-specific password if provided (used to bypass Zimbra 2FA) + if (!empty($values['app_password'])) { + $this->config->setUserValue($this->userId, Application::APP_ID, 'app_password', $this->crypto->encrypt($values['app_password'])); + } else { + $this->config->deleteUserValue($this->userId, Application::APP_ID, 'app_password'); + } $secondFactor = ($values['two_factor_code'] ?? null) ?: null; - return $this->loginWithCredentials($values['login'], $values['password'], $secondFactor); + // Use app_password for login if provided, otherwise use regular password + $loginPassword = !empty($values['app_password']) ? $values['app_password'] : $values['password']; + return $this->loginWithCredentials($values['login'], $loginPassword, $secondFactor); } $result = []; @@ -79,6 +87,7 @@ class ConfigController extends Controller { $this->config->deleteUserValue($this->userId, Application::APP_ID, 'token'); $this->config->deleteUserValue($this->userId, Application::APP_ID, 'login'); $this->config->deleteUserValue($this->userId, Application::APP_ID, 'password'); + $this->config->deleteUserValue($this->userId, Application::APP_ID, 'app_password'); $this->config->deleteUserValue($this->userId, Application::APP_ID, '2fa_expires_at'); $this->config->deleteUserValue($this->userId, Application::APP_ID, 'zimbra_version'); $result['user_id'] = ''; @@ -102,7 +111,7 @@ class ConfigController extends Controller { */ public function setConfig(array $values): DataResponse { foreach ($values as $key => $value) { - if (in_array($key, ['url', 'login', 'password', 'token'])) { + if (in_array($key, ['url', 'login', 'password', 'app_password', 'token'])) { throw new OCSForbiddenException(); } diff --git a/lib/Migration/Version1014Date20250707000000.php b/lib/Migration/Version1014Date20250707000000.php new file mode 100644 index 0000000..c024918 --- /dev/null +++ b/lib/Migration/Version1014Date20250707000000.php @@ -0,0 +1,62 @@ +<?php +/** + * Nextcloud - Zimbra + * + * This file is licensed under the Affero General Public License version 3 or + * later. See the COPYING file. + */ + +namespace OCA\Zimbra\Migration; + +use OCA\Zimbra\AppInfo\Application; +use OCP\IConfig; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; +use OCP\Security\ICrypto; + +/** + * Reset broken encrypted Zimbra credentials that prevent the app from booting. + * This fixes the "Authenticated ciphertext could not be decoded" error (issue nextcloud#33). + */ +class Version1014Date20250707000000 extends SimpleMigrationStep { + + public function __construct( + private IConfig $config, + private ICrypto $crypto, + ) { + } + + public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options): void { + $appId = Application::APP_ID; + + // Find all users that have a Zimbra login configured + $usersWithLogin = $this->config->getUsersForUserValue($appId, 'login', ''); + $allUsers = $this->config->getUsersForUserValue($appId, 'url', ''); + $candidates = array_unique(array_merge($usersWithLogin, $allUsers)); + + $resetCount = 0; + foreach ($candidates as $userId) { + foreach (['token', 'password', 'app_password'] as $key) { + $encryptedValue = $this->config->getUserValue($userId, $appId, $key, ''); + if ($encryptedValue === '') { + continue; + } + try { + $this->crypto->decrypt($encryptedValue); + } catch (\Exception $e) { + $output->warning( + sprintf('Resetting broken Zimbra %s for user "%s" (cannot decrypt).', $key, $userId) + ); + $this->config->deleteUserValue($userId, $appId, $key); + // Also clear dependent values so the user sees the login form + $this->config->deleteUserValue($userId, $appId, 'token_expires_at'); + $resetCount++; + } + } + } + + if ($resetCount > 0) { + $output->info(sprintf('Zimbra migration: reset %d broken credential(s). Affected users must reconnect.', $resetCount)); + } + } +} diff --git a/lib/Service/ZimbraAPIService.php b/lib/Service/ZimbraAPIService.php index 9020b0f..c09fe27 100644 --- a/lib/Service/ZimbraAPIService.php +++ b/lib/Service/ZimbraAPIService.php @@ -71,7 +71,30 @@ class ZimbraAPIService { if ($value === '') { return $value; } - return $this->crypto->decrypt($value); + try { + return $this->crypto->decrypt($value); + } catch (\Exception $e) { + $this->logger->warning('Zimbra: could not decrypt credential, resetting.', [ + 'app' => Application::APP_ID, + 'exception' => $e, + ]); + return ''; + } + } + + /** + * Returns the app-specific password if one is stored, otherwise falls back + * to the regular password. App-specific passwords bypass Zimbra 2FA. + */ + private function getEffectivePassword(string $userId): string { + $encAppPw = $this->config->getUserValue($userId, Application::APP_ID, 'app_password', ''); + if ($encAppPw !== '') { + $appPw = $this->decryptIfNotEmpty($encAppPw); + if ($appPw !== '') { + return $appPw; + } + } + return $this->decryptIfNotEmpty($this->config->getUserValue($userId, Application::APP_ID, 'password', '')); } public function isUserConnected(string $userId): bool { @@ -81,7 +104,7 @@ class ZimbraAPIService { $userName = $this->config->getUserValue($userId, Application::APP_ID, 'user_name'); $token = $this->decryptIfNotEmpty($this->config->getUserValue($userId, Application::APP_ID, 'token')); $login = $this->config->getUserValue($userId, Application::APP_ID, 'login'); - $password = $this->decryptIfNotEmpty($this->config->getUserValue($userId, Application::APP_ID, 'password')); + $password = $this->getEffectivePassword($userId); return $url && $userName && $token && $login && $password; } @@ -507,7 +530,7 @@ class ZimbraAPIService { if ($nowTs > $tokenExpiresAt - 60) { // try login with credentials $login = $this->config->getUserValue($userId, Application::APP_ID, 'login'); - $password = $this->decryptIfNotEmpty($this->config->getUserValue($userId, Application::APP_ID, 'password')); + $password = $this->getEffectivePassword($userId); $loginResult = $this->login($userId, $login, $password); if (isset($loginResult['error'])) { $this->logger->debug('Zimbra token refresh error : ' . $loginResult['error'], ['app' => Application::APP_ID]); diff --git a/lib/Settings/Personal.php b/lib/Settings/Personal.php index 47587de..2f31dbc 100644 --- a/lib/Settings/Personal.php +++ b/lib/Settings/Personal.php @@ -39,7 +39,11 @@ class Personal implements ISettings { if ($value === '') { return $value; } - return $this->crypto->decrypt($value); + try { + return $this->crypto->decrypt($value); + } catch (\Exception $e) { + return ''; + } } /** @@ -54,6 +58,7 @@ class Personal implements ISettings { $zimbraUserDisplayName = $this->config->getUserValue($this->userId, Application::APP_ID, 'user_displayname'); $adminUrl = $this->config->getAppValue(Application::APP_ID, 'admin_instance_url'); $url = $this->config->getUserValue($this->userId, Application::APP_ID, 'url', $adminUrl) ?: $adminUrl; + $appPasswordIsSet = $this->config->getUserValue($this->userId, Application::APP_ID, 'app_password', '') !== ''; $userConfig = [ 'token' => $token ? 'dummyTokenContent' : '', @@ -63,6 +68,7 @@ class Personal implements ISettings { 'user_displayname' => $zimbraUserDisplayName, 'search_mails_enabled' => $searchMailsEnabled, 'navigation_enabled' => $navigationEnabled, + 'app_password_is_set' => $appPasswordIsSet, ]; $this->initialStateService->provideInitialState('user-config', $userConfig); return new TemplateResponse(Application::APP_ID, 'personalSettings'); diff --git a/src/components/PersonalSettings.vue b/src/components/PersonalSettings.vue index 30856f5..0bd6e5c 100644 --- a/src/components/PersonalSettings.vue +++ b/src/components/PersonalSettings.vue @@ -50,6 +50,21 @@ :placeholder="t('integration_zimbra', '123456')" @keyup.enter="onConnectClick"> </div> + <div v-show="showLoginPassword" class="field"> + <label for="zimbra-app-password"> + <LockIcon :size="20" class="icon" /> + {{ t('integration_zimbra', 'App-specific password (optional)') }} + </label> + <input id="zimbra-app-password" + v-model="appPassword" + type="password" + :placeholder="appPasswordPlaceholder" + @keyup.enter="onConnectClick"> + </div> + <p v-show="showLoginPassword" class="settings-hint"> + <InformationOutlineIcon :size="24" class="icon" /> + {{ t('integration_zimbra', 'If Zimbra 2FA is enabled, generate an app-specific password in Zimbra under Preferences → Accounts and enter it here instead of your regular password.') }} + </p> <NcButton v-if="!connected" id="zimbra-connect" :disabled="loading === true || !(login && password)" @@ -137,6 +152,7 @@ export default { loading: false, login: '', password: '', + appPassword: '', twoFactorRequired: false, twoFactorCode: '', } @@ -152,6 +168,12 @@ export default { showLoginPassword() { return !this.connected }, + appPasswordPlaceholder() { + if (this.state.app_password_is_set) { + return t('integration_zimbra', 'Leave empty to keep existing app-specific password') + } + return t('integration_zimbra', 'Enter app-specific password for 2FA accounts') + }, }, watch: { @@ -175,7 +197,9 @@ export default { this.state.token = '' this.login = '' this.password = '' + this.appPassword = '' this.twoFactorCode = '' + this.state.app_password_is_set = false }, onSearchChange(newValue) { this.saveOptions({ search_mails_enabled: newValue ? '1' : '0' }, false) @@ -255,6 +279,7 @@ export default { this.saveOptions({ login: this.login, password: this.password, + app_password: this.appPassword, url: this.state.url, two_factor_code: this.twoFactorCode, }, true) -- 2.43.0 Signed-off-by: Dominik <drauer@hkn.de>
Signed-off-by: Dominik <drauer@hkn.de>
Signed-off-by: HKNDerRollo <drauer@hkn.de> Signed-off-by: Dominik <drauer@hkn.de>
Signed-off-by: HKNDerRollo <drauer@hkn.de> Signed-off-by: Dominik <drauer@hkn.de>
Author
|
@juliushaertl oder @come-nc – could you take a look at this PR? It fixes Zimbra 2FA support (issue #33). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When Zimbra 2FA is enabled, users could not connect the Nextcloud integration. Two distinct issues caused this:
App crash at boot: A broken or expired encrypted credential caused decryptIfNotEmpty() to throw an uncaught exception, crashing every request with:
Could not boot integration_zimbra: Authenticated ciphertext could not be decoded.
2FA login impossible: The OTP input field was only revealed after a first login attempt returned twoFactorRequired: true in the AuthResponse. However, Zimbra deployments that enforce 2FA strictly return account.AUTH_FAILED immediately for password-only requests — the twoFactorRequired flag is never set, so the OTP field never appeared and the user was stuck.
Solution
Resilient credential decryption (ZimbraAPIService, Settings/Personal):
decryptIfNotEmpty() is now wrapped in a try/catch. A corrupt or undecryptable credential silently returns an empty string instead of crashing the app.
Always-visible OTP field (PersonalSettings.vue):
The second-factor input is now shown unconditionally alongside the password field, not only after a failed first attempt. Users with 2FA simply fill in their current OTP code before clicking Connect. The password + OTP code are submitted together in a single AuthRequest, which is what Zimbra requires.
Simplified connect flow (PersonalSettings.vue, ConfigController):
Removed the intermediate twoFactorRequired state and the associated two-step UX. The app no longer tries to infer 2FA from a failed response — it always sends whatever the user provided.
Database migration (Version1014Date20250707000000):
On upgrade, scans all Zimbra users and deletes any credentials that can no longer be decrypted. Affected users see the login form instead of a 500 error.
Notes
Users without 2FA are unaffected — leaving the OTP field empty works exactly as before.
Token refresh for 2FA accounts relies on a pre-auth key configured by the admin, or the user must reconnect manually when the Zimbra session token expires (existing behaviour).
Fixes #33