Decrypt stored password before comparing in the 'auth' backend command - #64
Open
monperrus wants to merge 1 commit into
Open
Decrypt stored password before comparing in the 'auth' backend command#64monperrus wants to merge 1 commit into
monperrus wants to merge 1 commit into
Conversation
The __accounts__ 'auth' command (used for JMAP Basic-auth login on
/session and /jmap) compared the plaintext password from the request
directly against iserver.password, which JMAP::CredentialStore stores
encrypted whenever JMAP_SECRET_KEY (or OpenBao) is configured. The
comparison never matched, so Basic auth against any IMAP-backed
account failed with 401 regardless of correct credentials, on any
deployment using the documented/recommended encrypted credential
storage.
The sibling 'verify_credentials' command already does this correctly
(require JMAP::CredentialStore; decrypt before comparing) — this
brings 'auth' in line with it.
Confirmed against a live deployment (Docker image with JMAP_SECRET_KEY
set): before this change, GET /session with valid Basic-auth
credentials returned 401 {"type":"unauthorized"}; after, it returns
200 with a full JMAP Session object, and Email/query + Email/get
against the account return real synced mail.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
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.
Bug
Basic-auth login (
GET /session,POST /jmap) against an IMAP-backedaccount fails with
401 {"type":"unauthorized"}on any deploymentthat sets
JMAP_SECRET_KEY(or configures OpenBao) — i.e. thedocumented, recommended production credential-storage setup.
_authenticate()inbin/jmap-proxy.plsends Basic-auth credentialsto the
__accounts__child'sauthcommand, which does:iserver.passwordis written viaJMAP::CredentialStore->encrypt()during account setup, so it's stored as
enc1:base64(...)(orvault:v1:...), never as plaintext. Comparing that directly againstthe plaintext password from the request can never succeed.
The sibling
verify_credentialscommand (a few lines below, samefile) already does this correctly:
authwas just missing the decrypt step.Fix
Bring
authin line withverify_credentials: decrypt the storedvalue before comparing.
Testing
Verified against a live deployment
(
ghcr.io/jmapio/jmap-proxy:latest,JMAP_SECRET_KEYset):curl -u 'user@example.com:correct-password' https://.../session→401 {"type":"unauthorized"}200with a full JMAP Session object, anda follow-up
Email/query+Email/getcall against/jmapreturned real synced mail from the account.
Related to #63 (a separate bug in the same account-setup path) but
independent — this fixes the login path specifically.