Skip to content

Av/315 single key encryption for a folder#361

Merged
anushkavidanage merged 9 commits intodevfrom
av/315_single_key_encryption_for_a_folder
Oct 22, 2025
Merged

Av/315 single key encryption for a folder#361
anushkavidanage merged 9 commits intodevfrom
av/315_single_key_encryption_for_a_folder

Conversation

@anushkavidanage
Copy link
Collaborator

@anushkavidanage anushkavidanage commented Oct 14, 2025

Pull Request Details

What issue does this PR address

Testing

To test this PR, please run the DemoPod app (example app) in solidpod. On the home page, you will see two new buttons under the section ACL Inheritance, called Create Resource with ACL Inheritance and Read Resource with ACL Inheritance. Use those two UIs to test the writing and reading of resources with inheritance.

Checklist

Complete the check-list below to ensure your branch is ready for PR.

Flutter Style Guide: https://survivor.togaware.com/gnulinux/flutter-style.html

  • Screenshots included in linked issue
  • Changes adhere to the team style and coding guideline
  • No confidential information
  • No duplicated content
  • No lint check errors related to your changes (make prep or flutter analyze lib)
  • Pre-exisiting lint errors noted: [HERE]
  • Tested on at least one device
    • Android Phone
    • Android Emulator
    • Chrome on Android
    • Chrome
    • iOS
    • Linux
    • MacOS
    • Windows
  • Added 2 reviewers (or 1 for private repositories then they add another)

Finalising

Once PR discussion is complete and 2 reviewers have approved:

  • Merge dev into the branch
  • Resolve any conflicts
  • Add one line summary into CHANGELOG.md
  • Bump appropriate version number in pubspec.yaml
  • Push to git repository and review
  • Merge PR into dev

Copy link
Collaborator

@cdawei cdawei left a comment

Choose a reason for hiding this comment

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

The changes look good, thanks @anushkavidanage.

I can read/write a file encrypted with a ancestor directory's encryption key in the example app.
image

There are a few minor points as described below, for your consideration.

final normalizedDirPath =
await normalizeFilePath(inheritedFrom, basePath);
final parentDirUrl = await getDirUrl(normalizedDirPath);
content = await getEncTTLStr(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a minor point: it seems there's opportunity to just call getEncTTLStr() once instead of also repeating it below at line 242?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tanks Dawei, all the comments are fixed except for this one. Here, we have two separate variables assigned in the two instances of function call, normalizedDirPath and parentDirUrl. And the normalizedDirPath is checked within the function to see whether the resource is inherited from this or not. I don't see a clear way of generalising this? Any ideas?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi Anushka, this is not essential but what in my head is something showing here av/315_single_key_encryption_for_a_folder...315_dc_suggestions

Feel free to incorporate (discard) these minor changes if they (don't) make sense to you :-)

@srodriguez142857
Copy link
Collaborator

I tested the Example Demo app for the ACL Inheritance feature.
All good; the app is working as expected.

Screenshot 2025-10-16 193035

I have the following observations:

  1. Does the functionality support traversing multiple directory levels recursively to find the associated encryption key?
    For instance, if we have the following path dir1/dir2/dir3/file.ttl, will the functionality look first in dir3, and if it cannot find it, will it continue looking in the upper-level directories (dir2 and dir1) until it either finds it or not?

  2. The current functionality seems to "ignore" paths with nested directories. If we have a path parentDir/parentDir2/another-file.ttl, the system associates its encryption key in the ind-keys.ttl file to a path pointing to /data/parentDir2/, "excluding" its nested structure (see below):

image
  1. Is there any ACL file associated with the directory (parentDir)? I couldn't find it.

Let's discuss this tomorrow.

@srodriguez142857
Copy link
Collaborator

Hi @anushkavidanage.
I did another round of testing.
The app is working as expected. I will proceed to approve this PR.


I only have the following observation: I got an error for the following case,

image
[ERROR:flutter/runtime/dart_vm_initializer.cc(40)] Unhandled Exception: Exception: Unable to locate the individual key for resource:
https://solid.dev.empwr.au/sergio101/exampleApp/data/dir1/dir2/
#0      KeyManager.getIndividualKey (package:solidpod/src/solid/utils/key_helper.dart:473:7)
#1      writePod (package:solidpod/src/solid/write_pod.dart:227:26)
<asynchronous suspension>
#2      CreateAclInheritedFileState._submitForm (package:demopod/features/create_acl_inherited_file.dart:73:24)
<asynchronous suspension>

The path dir1/dir2 exists. Not sure whether this should be permitted (a new functionality?).

Copy link
Collaborator

@srodriguez142857 srodriguez142857 left a comment

Choose a reason for hiding this comment

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

The functionality is working as expected.

@anushkavidanage
Copy link
Collaborator Author

Hi @srodriguez142857 and @cdawei, I had a look through the use cases we discussed today morning and it looks like we need a bit of changes to enable that. Also, there are few things that we might need to discuss further as well. I created a new issue around this. Please have a look: #403

@anushkavidanage anushkavidanage merged commit 894a30e into dev Oct 22, 2025
18 of 20 checks passed
@anushkavidanage anushkavidanage deleted the av/315_single_key_encryption_for_a_folder branch October 22, 2025 06:13
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.

3 participants

Comments