Content-hashed external URI parsing support #2387

gnunicorn · 2024-11-26T14:44:57Z

This PR brings support for providing non matrix-to https links with content hashing support for additional security. These work analogous to the previous acter:-style-links but with that entire path put into the #-fragment (query stays the same, but the order might change) and replacing the path with a final hash of the entire URI in this style without that hash as the final parameter). This allows any server to provide that feature under any domain + path as long as the last path-item is the hash itself.

Example: acter:roomid/room:acter.global/e/someEvent?via=acter.global&via=example.org becomes https://app.acter.global/p/$HASH?via=acter.global&via=example.org#roomid/room:acter.global/e/someEvent (where $HASH = SHA1.of(https://app.acter.global/p/?via=acter.global&via=example.org#roomid/room:acter.global/e/someEvent)

This ensures that we can a) still parse most information from the URI itself without querying the server and b) ensure that the data wasn't messed with in transit or by any third party compared to the data that the server would be showing under that hash.

To the reviewer:

I renamed the files and functions for better lookup
I have refactored the code to be reusable for both tests of the acter: and new https-links, that's why the test file looks different
I decided to update the super-invite-uris to use the same pattern (rather than a host-url-one) to make it easier to reuse the code (which required updating our current qr-code generator)
links with a hash are checked for containing a user-id - if not the result is thrown in a particular error (allowing the UI to still use it, but make clear that this isn't the usual way it is meant to be)

gnunicorn · 2024-11-26T14:45:21Z

app/lib/features/deep_linking/parse_acter_uri.dart

@@ -32,14 +40,41 @@ UriParseResult _parseActerUri(Uri uri) {
  };
 }

+UriParseResult _parseHttpsUri(Uri uri) {


this is the core of the new scheme

github-actions · 2024-11-26T14:45:25Z

Hey there 👋,
and thanks for the contribution. But it seems like you forgot to

📰 Add a markdown file in .changes/ explaining what changed

gnunicorn · 2024-11-26T14:45:38Z

app/lib/features/deep_linking/parse_acter_uri.dart

@@ -32,14 +40,41 @@ UriParseResult _parseActerUri(Uri uri) {
  };
 }

+UriParseResult _parseHttpsUri(Uri uri) {
+  if (uri.host == 'matrix.to') {
+    return _parseMatrixHttpsUri(uri);


matrix.to is a special case with a special format for legacy links

gnunicorn · 2024-11-26T14:46:35Z

app/lib/features/deep_linking/parse_acter_uri.dart

+  final hash = uri.pathSegments.lastOrNull;
+  if (hash == null || hash.isEmpty) {
+    throw IncorrectHashError();
+  }
+
+  final pathWithoutHash = uri.path.substring(0, uri.path.length - hash.length -1);
+  final strippedUri = uri.replace(path: pathWithoutHash);
+  final hashableUri = strippedUri.toString();
+  final calculatedHash = sha1.convert(utf8.encode(hashableUri)).toString();
+  if (calculatedHash != hash) {
+    throw IncorrectHashError();
+  }


look for the hash

remove the hash

compare the hash

Example: acter:roomid/room:acter.global/e/someEvent?via=acter.global&via=example.org becomes https://app.acter.global/p/$HASH?via=acter.global&via=example.org#roomid/room:acter.global/e/someEvent (where $HASH = SHA1.of(https://app.acter.global/p/?via=acter.global&via=example.org#roomid/room:acter.global/e/someEvent)

Not quite understand link structure and logic. Sorry but looks complex to me to understand.

gnunicorn · 2024-11-26T14:47:09Z

app/lib/features/deep_linking/parse_acter_uri.dart

+  final extractableUri = strippedUri.replace(path: strippedUri.fragment, fragment: null);
+  final result = _parseActerUri(extractableUri);


extract the fragment as the path to just use the existing parsing code

gnunicorn · 2024-11-26T14:47:24Z

app/lib/features/deep_linking/parse_acter_uri.dart

+  final extractableUri = strippedUri.replace(path: strippedUri.fragment, fragment: null);
+  final result = _parseActerUri(extractableUri);
+  if (result.preview.userId == null || result.preview.userId?.isEmpty == true) {
+    throw MissingUserError(result: result);


fail with the result if no userId was found.

gnunicorn · 2024-11-26T14:48:15Z

app/test/features/deep_linking/parse_acter_uri_test.dart

+
+// re-usable test cases
+
+void acterObjectLinksTests(UriMaker makeUri) {


these are the same as before, just that we have new UriMaker so we can resuse the same tests suites for multiple "schemes"

codecov · 2024-11-26T14:53:32Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 27.89%. Comparing base (b1eb5c5) to head (8bcb398).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2387      +/-   ##
==========================================
+ Coverage   27.86%   27.89%   +0.02%     
==========================================
  Files         617      617              
  Lines       42261    42279      +18     
==========================================
+ Hits        11778    11793      +15     
- Misses      30483    30486       +3

Flag	Coverage Δ
integration-test	`36.88% <ø> (-0.02%)`	⬇️
unittest	`20.11% <ø> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kumarpalsinh25

Deep-link structure looks complex to me, not sure if I am not getting it well or it is actually complex itself.

kumarpalsinh25 · 2024-11-27T07:29:40Z

app/test/features/deep_linking/parse_acter_uri_test.dart

+
+// re-usable test cases
+
+void acterObjectLinksTests(UriMaker makeUri) {


kumarpalsinh25 · 2024-11-27T07:38:18Z

app/test/features/deep_linking/parse_acter_uri_test.dart

+  group('Testing fallback https://app.acter.global/:-links', () =>
+   newSpecLinksTests(makeUriMakerForPublicPrefix('https://app.acter.global/p/')),
+  );
+
+  group('Testing https://app.acter.global/ object-links', () =>
+    acterObjectLinksTests(makeUriMakerForPublicPrefix('https://app.acter.global/p/')),
+  );
+
+  group('Testing https://app.acter.global/ invite-links',  () =>
+    acterInviteLinkTests(makeUriMakerForPublicPrefix('https://app.acter.global/p/')),
+  );
+
+  group('Testing https://app.acter.global/ preview data', () =>
+    acterObjectPreviewTests(makeUriMakerForPublicPrefix('https://app.acter.global/p/')),
+  );
+
+  group('Testing broken https://app.acter.global/ ', () {
+    test('faulty hash fails', () async {
+      expect(() => parseActerUri(
+          Uri.parse('http://acter.global/p/faultyHash?via=acter.global&via=example.org#roomid/room:acter.global/e/someEvent'),
+        ),
+        throwsA(TypeMatcher<IncorrectHashError>()),
+      );


Getting bit difficulty to grasp entire logic.

kumarpalsinh25 · 2024-11-27T07:44:27Z

app/lib/features/deep_linking/parse_acter_uri.dart

+  final hash = uri.pathSegments.lastOrNull;
+  if (hash == null || hash.isEmpty) {
+    throw IncorrectHashError();
+  }
+
+  final pathWithoutHash = uri.path.substring(0, uri.path.length - hash.length -1);
+  final strippedUri = uri.replace(path: pathWithoutHash);
+  final hashableUri = strippedUri.toString();
+  final calculatedHash = sha1.convert(utf8.encode(hashableUri)).toString();
+  if (calculatedHash != hash) {
+    throw IncorrectHashError();
+  }


Example: acter:roomid/room:acter.global/e/someEvent?via=acter.global&via=example.org becomes https://app.acter.global/p/$HASH?via=acter.global&via=example.org#roomid/room:acter.global/e/someEvent (where $HASH = SHA1.of(https://app.acter.global/p/?via=acter.global&via=example.org#roomid/room:acter.global/e/someEvent)

Not quite understand link structure and logic. Sorry but looks complex to me to understand.

gnunicorn added 4 commits November 26, 2024 13:30

Refactor links for easier reusability

b169713

New https://-based URIs with proper hashing

9651342

Rename utils to parse_acter_uri

688dc0b

Fix user missing tests

8bcb398

gnunicorn added the flutter label Nov 26, 2024

gnunicorn requested a review from kumarpalsinh25 November 26, 2024 14:44

gnunicorn commented Nov 26, 2024

View reviewed changes

kumarpalsinh25 reviewed Nov 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Content-hashed external URI parsing support #2387

Content-hashed external URI parsing support #2387

gnunicorn commented Nov 26, 2024

gnunicorn Nov 26, 2024

github-actions bot commented Nov 26, 2024

gnunicorn Nov 26, 2024

gnunicorn Nov 26, 2024

kumarpalsinh25 Nov 27, 2024

gnunicorn Nov 26, 2024

gnunicorn Nov 26, 2024

gnunicorn Nov 26, 2024

kumarpalsinh25 Nov 27, 2024

codecov bot commented Nov 26, 2024 •

edited

Loading

kumarpalsinh25 left a comment

kumarpalsinh25 Nov 27, 2024

kumarpalsinh25 Nov 27, 2024

kumarpalsinh25 Nov 27, 2024

		final extractableUri = strippedUri.replace(path: strippedUri.fragment, fragment: null);
		final result = _parseActerUri(extractableUri);


		// re-usable test cases

		void acterObjectLinksTests(UriMaker makeUri) {

Content-hashed external URI parsing support #2387

Are you sure you want to change the base?

Content-hashed external URI parsing support #2387

Conversation

gnunicorn commented Nov 26, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 26, 2024 • edited Loading

Codecov Report

kumarpalsinh25 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 26, 2024 •

edited

Loading