fix: if user's keys are unavailable, attempt to relog to recover them#207
Conversation
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR updates E2EE key resolution and login state handling so missing own private keys trigger a forced full reconnect, direct-message decrypt retries use direction-aware key selection, and refreshed login keys clear the reconnect marker. ChangesE2EE missing key detection and forced reconnect
Estimated code review effort: 4 (Complex) | ~55 minutes Sequence Diagram(s)sequenceDiagram
participant Handler as decryptMessageBody
participant Client as LineClient
participant E2EE as e2ee.Manager
participant Login as StartWithOverride
participant Meta as UserLoginMetadata
Handler->>E2EE: DecryptMessageV2(message)
E2EE-->>Handler: ErrMissingOwnPrivateKey
Handler->>Handler: decode sender/receiver key IDs
Handler->>E2EE: ensurePeerKeyByID / ensurePeerKey
Handler->>Client: markMissingE2EEKey(err)
Client->>Meta: set ForceFullE2EELogin
Client->>Client: emit StateBadCredentials
Login->>Meta: read ForceFullE2EELogin
Login->>Login: clear Certificate if forced
Login->>Meta: applyExportedLoginE2EEKeys(...)
Meta->>Meta: reset ForceFullE2EELogin = false
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="Running error: context loading failed: failed to load packages: failed to load packages: failed to load with go/packages: context deadline exceeded" Comment |
![indent-zero[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/connector/client.go`:
- Around line 320-334: The early return in the `UserLoginMetadata` handling
block of `client.go` is preventing the bad-credentials bridge state from being
emitted when `needsSave` is false. Update the logic around `needsSave`,
`lc.missingE2EEKeyMu`, and `lc.UserLogin.Save(ctx)` so the flag only controls
whether the save is attempted, while the later `StateBadCredentials`
notification still runs for the current client lifetime. Keep the existing
metadata update to `ForceFullE2EELogin` and `Certificate`, but avoid returning
before the reconnect prompt path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 59960cfd-fd52-4997-b8da-0720503d8221
📒 Files selected for processing (4)
pkg/connector/client.gopkg/connector/connector.gopkg/connector/handle_message.gopkg/connector/login_keys_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/connector/handle_message.go
- pkg/connector/connector.go
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: build-docker
- GitHub Check: Lint with 1.25
- GitHub Check: Lint with 1.25
- GitHub Check: build-docker
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Usego fmtfor code formatting across all Go files
Usegoimportswith-local "github.com/highesttt/matrix-line-messenger"flag to group project-local imports correctly
Usezerologfor logging throughout the codebase
Do not useMsgfin logging; useMsgwith structured fields instead
UseStringerinterface where applicable in Go code
Files:
pkg/connector/login_keys_test.gopkg/connector/client.go
**/!(ltsm)/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/!(ltsm)/**/*.go: Runstaticcheckon all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Rungo veton all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Files:
pkg/connector/login_keys_test.gopkg/connector/client.go
🔇 Additional comments (4)
pkg/connector/login_keys_test.go (1)
36-65: LGTM!pkg/connector/client.go (3)
49-53: LGTM!
346-351: LGTM!
616-626: LGTM!
| if meta, ok := lc.UserLogin.Metadata.(*UserLoginMetadata); ok { | ||
| needsSave := !meta.ForceFullE2EELogin || meta.Certificate != "" | ||
| meta.ForceFullE2EELogin = true | ||
| meta.Certificate = "" | ||
| if !needsSave { | ||
| lc.missingE2EEKeyMu.Unlock() | ||
| return | ||
| } | ||
| lc.missingE2EEKeyMu.Unlock() | ||
| if errSave := lc.UserLogin.Save(ctx); errSave != nil && lc.UserLogin.Bridge != nil { | ||
| lc.UserLogin.Bridge.Log.Warn().Err(errSave).Msg("Failed to save LINE E2EE reconnect requirement") | ||
| } | ||
| } else { | ||
| lc.missingE2EEKeyMu.Unlock() | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Early return on !needsSave skips the bad-credentials bridge state.
When metadata already carries ForceFullE2EELogin=true and an empty Certificate (e.g., after a restart), needsSave is false and the function returns at Line 324–326. But missingE2EEKeyMarked was just set to true, so the StateBadCredentials emission at Line 338 never runs for this client lifetime and the user is never prompted to reconnect. The needsSave check should gate only the Save call, not the notification.
🐛 Proposed fix: skip only the Save, keep the notification
if meta, ok := lc.UserLogin.Metadata.(*UserLoginMetadata); ok {
needsSave := !meta.ForceFullE2EELogin || meta.Certificate != ""
meta.ForceFullE2EELogin = true
meta.Certificate = ""
- if !needsSave {
- lc.missingE2EEKeyMu.Unlock()
- return
- }
lc.missingE2EEKeyMu.Unlock()
- if errSave := lc.UserLogin.Save(ctx); errSave != nil && lc.UserLogin.Bridge != nil {
- lc.UserLogin.Bridge.Log.Warn().Err(errSave).Msg("Failed to save LINE E2EE reconnect requirement")
+ if needsSave {
+ if errSave := lc.UserLogin.Save(ctx); errSave != nil && lc.UserLogin.Bridge != nil {
+ lc.UserLogin.Bridge.Log.Warn().Err(errSave).Msg("Failed to save LINE E2EE reconnect requirement")
+ }
}
} else {
lc.missingE2EEKeyMu.Unlock()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if meta, ok := lc.UserLogin.Metadata.(*UserLoginMetadata); ok { | |
| needsSave := !meta.ForceFullE2EELogin || meta.Certificate != "" | |
| meta.ForceFullE2EELogin = true | |
| meta.Certificate = "" | |
| if !needsSave { | |
| lc.missingE2EEKeyMu.Unlock() | |
| return | |
| } | |
| lc.missingE2EEKeyMu.Unlock() | |
| if errSave := lc.UserLogin.Save(ctx); errSave != nil && lc.UserLogin.Bridge != nil { | |
| lc.UserLogin.Bridge.Log.Warn().Err(errSave).Msg("Failed to save LINE E2EE reconnect requirement") | |
| } | |
| } else { | |
| lc.missingE2EEKeyMu.Unlock() | |
| } | |
| if meta, ok := lc.UserLogin.Metadata.(*UserLoginMetadata); ok { | |
| needsSave := !meta.ForceFullE2EELogin || meta.Certificate != "" | |
| meta.ForceFullE2EELogin = true | |
| meta.Certificate = "" | |
| lc.missingE2EEKeyMu.Unlock() | |
| if needsSave { | |
| if errSave := lc.UserLogin.Save(ctx); errSave != nil && lc.UserLogin.Bridge != nil { | |
| lc.UserLogin.Bridge.Log.Warn().Err(errSave).Msg("Failed to save LINE E2EE reconnect requirement") | |
| } | |
| } | |
| } else { | |
| lc.missingE2EEKeyMu.Unlock() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/connector/client.go` around lines 320 - 334, The early return in the
`UserLoginMetadata` handling block of `client.go` is preventing the
bad-credentials bridge state from being emitted when `needsSave` is false.
Update the logic around `needsSave`, `lc.missingE2EEKeyMu`, and
`lc.UserLogin.Save(ctx)` so the flag only controls whether the save is
attempted, while the later `StateBadCredentials` notification still runs for the
current client lifetime. Keep the existing metadata update to
`ForceFullE2EELogin` and `Certificate`, but avoid returning before the reconnect
prompt path.
1 participant
No description provided.