Skip to content

fix(otp): parse KeeOTP TOTP query fields#10

Open
ftorga wants to merge 1 commit into
crazytan:mainfrom
ftorga:fix/keeotp-totp-compatibility
Open

fix(otp): parse KeeOTP TOTP query fields#10
ftorga wants to merge 1 commit into
crazytan:mainfrom
ftorga:fix/keeotp-totp-compatibility

Conversation

@ftorga

@ftorga ftorga commented Jul 10, 2026

Copy link
Copy Markdown

Closes #9

Summary

  • Parse KeeOTP TOTP query payloads from supported entry fields.
  • Decode declared secret encodings safely and feed existing TOTP consumers.
  • Keep HOTP out of this slice.

Changes

File Change
KeeForge/Models/EncryptedValue.swift Tightened secret handling for OTP payload decoding
KeeForge/Models/Entry.swift Added KeeOTP-aware TOTP wiring
KeeForge/Models/KDBXParser.swift Parsed KeeOTP TOTP query fields
KeeForge/Models/TOTPGenerator.swift Adapted TOTP generation inputs
KeeForgeTests/KDBXParserTests.swift Added parser coverage
KeeForgeTests/TOTPGeneratorTests.swift Added generator coverage

Test Plan

  • Unit tests added for parser and generator behavior
  • Existing TOTP flows preserved
  • HOTP intentionally deferred

Checklist

@crazytan crazytan left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused compatibility work. I found two blocking correctness issues that should be addressed before merge. I also ran the relevant unit-test slice locally; it completed with one failure in KDBXParserTests.testKeeOTPSupportsEachDeclaredEncodingWithoutFallback() (the over-padded Base64 case). TOTPGeneratorTests and EncryptedValueTests passed. There are currently no GitHub workflow runs/statuses attached to this commit.

Please also add the required CHANGELOG.md entry under ## Unreleased for this bug fix.

guard let normalized = normalizeStrictUnpaddedBase32(key) else { return nil }
return TOTPGenerator.base32Decode(normalized).flatMap { $0.isEmpty ? nil : $0 }
case "base64":
return Data(base64Encoded: key).flatMap { $0.isEmpty ? nil : $0 }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Make Base64 decoding strict. Data(base64Encoded:) accepts the over-padded JBSWY3DP====, so this branch creates a TOTPConfig even though the declared Base64 spelling is malformed. This is why the PR's own testKeeOTPSupportsEachDeclaredEncodingWithoutFallback fails. Please validate canonical Base64 characters/padding (or round-trip decoded bytes to the canonical representation) before accepting the secret.

struct TOTPConfig: Sendable {
let secret: EncryptedValue
/// Pre-decoded secret bytes for formats whose declared encoding is not Base32.
let decodedSecret: EncryptedValue?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve the decoded secret/source encoding through edits. EntryDraftPayload and DatabaseDraft.makeTOTPConfig do not carry decodedSecret, so an unrelated entry edit reconstructs Base64/Hex/UTF-8 secrets as ordinary Base32 and breaks TOTP generation. For uppercase OTP/Otp, serialization also writes conflicting TimeOtp-* fields alongside the original custom field, and those fields take precedence on reload. Please preserve the KeeOTP source/encoding or decoded bytes through the edit and serialization pipeline, with an edit/save regression test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: recognize KeeOTP TOTP query fields

2 participants