Skip to content

Commit e553cba

Browse files
beveradbclaude
andcommitted
Fix KeeShare code issues and add KeePassXC format support
This commit addresses reviewer feedback from PR PhilippC#3106: **Error Handling** - Fix Import method to rethrow exceptions instead of silently swallowing them - Allows proper error propagation and user notification via outer exception handlers **Path Resolution** - Refactor ResolvePath to use IFileStorage methods consistently for both local and remote files - Removes System.IO.Path usage in favor of abstraction layer - Ensures all file operations go through proper storage backends **LoadDB Handler Logic** - Add clarifying comment to WrapHandlerForKeeShare explaining that OnLoadCompleteKeeShareCheck runs in both null and non-null handler cases - Addresses maintainer question about intentionality of the null check **KeePassXC Interoperability** - Add HasKeePassXCFormat() to detect groups created in KeePassXC - Add TryImportKeePassXCConfig() to import KeePassXC's KeeShare configuration - Support multiple KeePassXC CustomData formats: * KeeShareReference.Path/Type/Password * KPXC_KeeShare_Path/Type/Password * Legacy KeeShare XML format - Integrate detection into HasKeeShareGroups() for automatic import - Maintains backward compatibility with KP2A format - Allows device-specific path overrides using existing KP2A mechanism **Notes** - Status logging already implemented (line 482: "Exporting KeeShare database group") - Build workflow already includes dotnet workload restore (lines 312-314, 322) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent f0b5e2e commit e553cba

2 files changed

Lines changed: 138 additions & 24 deletions

File tree

src/Kp2aBusinessLogic/database/edit/LoadDB.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ public LoadDb(IKp2aApp app, IOConnectionInfo ioc, Task<MemoryStream> databaseDat
6868

6969
private static OnOperationFinishedHandler WrapHandlerForKeeShare(IKp2aApp app, OnOperationFinishedHandler originalHandler)
7070
{
71+
// Note: OnLoadCompleteKeeShareCheck runs in BOTH cases below (null or not null)
72+
// The null check is just to handle the two scenarios differently:
73+
// - If null: create a simple handler that only runs KeeShare check
74+
// - If not null: wrap the existing handler to run KeeShare check before it
7175
if (originalHandler == null)
7276
{
7377
return new ActionOnOperationFinished(app, (success, message, importantMessage, exception, context) =>

src/keepass2android-app/KeeShare.cs

Lines changed: 134 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -187,13 +187,126 @@ public static void DisableKeeShare(PwGroup group)
187187
group.CustomData.Remove(FilePathKey);
188188
group.CustomData.Remove(PasswordKey);
189189
group.CustomData.Remove("KeeShare.TrustedCertificate");
190-
190+
191191
string deviceKey = GetDeviceFilePathKey();
192192
group.CustomData.Remove(deviceKey);
193-
193+
194194
group.Touch(true, false);
195195
}
196196

197+
/// <summary>
198+
/// Checks if a group has KeePassXC-style KeeShare configuration.
199+
/// KeePassXC stores share info in CustomData with keys like "KeeShareReference.Path", etc.
200+
/// </summary>
201+
public static bool HasKeePassXCFormat(PwGroup group)
202+
{
203+
if (group == null) return false;
204+
205+
// Check for KeePassXC's CustomData keys
206+
return group.CustomData.Exists("KeeShareReference.Path") ||
207+
group.CustomData.Exists("KPXC_KeeShare_Path") ||
208+
(group.CustomData.Exists("KeeShare") &&
209+
!group.CustomData.Exists(ActiveKey)); // Has old KeeShare key but not our format
210+
}
211+
212+
/// <summary>
213+
/// Attempts to import KeePassXC KeeShare configuration into KP2A format.
214+
/// This allows groups created in KeePassXC to work in KP2A.
215+
/// Does NOT overwrite existing KP2A configuration.
216+
/// </summary>
217+
public static bool TryImportKeePassXCConfig(PwGroup group)
218+
{
219+
if (group == null || !HasKeePassXCFormat(group)) return false;
220+
221+
// Don't overwrite existing KP2A configuration
222+
if (group.CustomData.Exists(ActiveKey)) return false;
223+
224+
try
225+
{
226+
string path = null;
227+
string type = "Synchronize"; // KeePassXC default
228+
string password = null;
229+
230+
// Try to extract path from various KeePassXC formats
231+
if (group.CustomData.Exists("KeeShareReference.Path"))
232+
{
233+
path = group.CustomData.Get("KeeShareReference.Path");
234+
}
235+
else if (group.CustomData.Exists("KPXC_KeeShare_Path"))
236+
{
237+
path = group.CustomData.Get("KPXC_KeeShare_Path");
238+
}
239+
else if (group.CustomData.Exists("KeeShare"))
240+
{
241+
// Try to parse XML or structured format
242+
string data = group.CustomData.Get("KeeShare");
243+
// Simple heuristic: look for path= pattern
244+
int pathIdx = data.IndexOf("path=", StringComparison.OrdinalIgnoreCase);
245+
if (pathIdx >= 0)
246+
{
247+
pathIdx += 5; // skip "path="
248+
if (pathIdx < data.Length)
249+
{
250+
char quote = data[pathIdx];
251+
if (quote == '"' || quote == '\'')
252+
{
253+
pathIdx++;
254+
int endIdx = data.IndexOf(quote, pathIdx);
255+
if (endIdx > pathIdx)
256+
{
257+
path = data.Substring(pathIdx, endIdx - pathIdx);
258+
}
259+
}
260+
}
261+
}
262+
}
263+
264+
// Try to extract type
265+
if (group.CustomData.Exists("KeeShareReference.Type"))
266+
{
267+
string xtype = group.CustomData.Get("KeeShareReference.Type");
268+
// Map KeePassXC types to KP2A types
269+
if (xtype.Equals("Export", StringComparison.OrdinalIgnoreCase))
270+
type = "Export";
271+
else if (xtype.Equals("Import", StringComparison.OrdinalIgnoreCase))
272+
type = "Import";
273+
else if (xtype.Equals("Sync", StringComparison.OrdinalIgnoreCase) ||
274+
xtype.Equals("Synchronize", StringComparison.OrdinalIgnoreCase))
275+
type = "Synchronize";
276+
}
277+
else if (group.CustomData.Exists("KPXC_KeeShare_Type"))
278+
{
279+
string xtype = group.CustomData.Get("KPXC_KeeShare_Type");
280+
if (xtype.Equals("0")) type = "Export";
281+
else if (xtype.Equals("1")) type = "Import";
282+
else if (xtype.Equals("2")) type = "Synchronize";
283+
}
284+
285+
// Try to extract password
286+
if (group.CustomData.Exists("KeeShareReference.Password"))
287+
{
288+
password = group.CustomData.Get("KeeShareReference.Password");
289+
}
290+
else if (group.CustomData.Exists("KPXC_KeeShare_Password"))
291+
{
292+
password = group.CustomData.Get("KPXC_KeeShare_Password");
293+
}
294+
295+
if (!string.IsNullOrEmpty(path))
296+
{
297+
EnableKeeShare(group, type, path, password);
298+
Kp2aLog.Log("KeeShare: Successfully imported KeePassXC configuration for group " + group.Name);
299+
return true;
300+
}
301+
}
302+
catch (Exception ex)
303+
{
304+
Kp2aLog.Log("KeeShare: Failed to import KeePassXC configuration: " + ex.Message);
305+
}
306+
307+
return false;
308+
}
309+
197310
/// <summary>
198311
/// Resolves a file path, handling relative paths for local files.
199312
/// Relative paths are resolved relative to the current database's directory.
@@ -228,30 +341,17 @@ internal static IOConnectionInfo ResolvePath(IKp2aApp app, string path)
228341
return IOConnectionInfo.FromPath(path);
229342
}
230343

231-
if (currentIoc.IsLocalFile())
344+
// Use IFileStorage methods for all paths (local and remote) for consistency
345+
try
232346
{
233-
string dir = Path.GetDirectoryName(currentIoc.Path);
234-
if (string.IsNullOrEmpty(dir))
235-
{
236-
Kp2aLog.Log("KeeShare: Cannot resolve relative path - database directory is empty. Using path as-is: " + path);
237-
return IOConnectionInfo.FromPath(path);
238-
}
239-
string fullPath = Path.Combine(dir, path);
240-
return IOConnectionInfo.FromPath(fullPath);
347+
var fileStorage = app.GetFileStorage(currentIoc);
348+
IOConnectionInfo parentPath = fileStorage.GetParentPath(currentIoc);
349+
return fileStorage.GetFilePath(parentPath, path);
241350
}
242-
else
351+
catch (Exception ex)
243352
{
244-
try
245-
{
246-
var fileStorage = app.GetFileStorage(currentIoc);
247-
IOConnectionInfo parentPath = fileStorage.GetParentPath(currentIoc);
248-
return fileStorage.GetFilePath(parentPath, path);
249-
}
250-
catch (Exception ex)
251-
{
252-
Kp2aLog.Log("KeeShare: Failed to resolve relative path using IFileStorage methods: " + ex.Message + ". Using path as-is: " + path);
253-
return IOConnectionInfo.FromPath(path);
254-
}
353+
Kp2aLog.Log("KeeShare: Failed to resolve relative path using IFileStorage methods: " + ex.Message + ". Using path as-is: " + path);
354+
return IOConnectionInfo.FromPath(path);
255355
}
256356
}
257357

@@ -363,7 +463,16 @@ internal static bool HasKeeShareGroups(PwGroup group)
363463
{
364464
if (group.CustomData.Get(ActiveKey) == "true")
365465
return true;
366-
466+
467+
// Check for KeePassXC format and try to import it
468+
if (HasKeePassXCFormat(group))
469+
{
470+
TryImportKeePassXCConfig(group);
471+
// After import attempt, check if we now have active KeeShare
472+
if (group.CustomData.Get(ActiveKey) == "true")
473+
return true;
474+
}
475+
367476
foreach (var sub in group.Groups)
368477
{
369478
if (HasKeeShareGroups(sub)) return true;
@@ -881,6 +990,7 @@ private void Import(PwGroup targetGroup, string path, string password, string ty
881990
catch (Exception ex)
882991
{
883992
Kp2aLog.Log("KeeShare import failed for group " + targetGroup.Name + ": " + ex.Message);
993+
throw;
884994
}
885995
}
886996

0 commit comments

Comments
 (0)