Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Uptake public key hash changes #1440

Open
wants to merge 7 commits into
base: trunk
Choose a base branch
from

Conversation

sitaram-kalluri
Copy link
Member

- What I did

  • Uptake "PublicKeyHash" changes into at_client package.
    • In "SharedKeyDecryption.dart", when decrypting the shared key, compare the hash value of the encryption public key with the hash value in atKey.metadata.pubKeyHash. If the value mismatch, throw exception.
    • Add publicKeyHash value in sync_service_impl.dart to sync the value to the remote secondary and fetch the value from the remote secondary.
    • Add the publicKeyHash value in at_notification.metadata.

- How to verify it

  • Updated the existing unit test to verify publicKeyHash is populated when encrypting the value.
  • Updated the existing unit test to verify the exception is thrown if publicKeyHash mismatch.
  • Updated the existing unit test to verify decryption is successful if publicKeyHash value matches.

- Description for the changelog

  • feat: Uptake public key hash changes

Note: In the pubspec.yaml, the at_persistence_secondary_server is set to "2121-uptake-public-key-hash-at-persistence_secondary-server" branch to consume the public key hash changes. Once the PR in at_server is merged, will replace with the hosted version.

// AtConstants.sharedWithPublicKeyHash. Therefore, check for null.
if (json['metadata'][AtConstants.sharedWithPublicKeyHash] != null) {
// AtConstants.sharedWithPublicKeyHash. Therefore, check for null String.
if (json['metadata'][AtConstants.sharedWithPublicKeyHash] != "null") {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems bad

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure Gary, will revisit the code and modify the IF condition.

Copy link
Member Author

@sitaram-kalluri sitaram-kalluri Dec 13, 2024

Choose a reason for hiding this comment

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

In the MonitorVerbHandler.dart file of at_secondary_server, the pubKeyHash object is converted into a JSON-encoded string and sent to the client. When the pubKeyHash value is null, the jsonEncode method returns a string with the value "null", which causes metadata.pubKeyHash to be set to "null" instead of null.

Code snippet from MonitorVerbHandler.dart:

"pubKeyHash": jsonEncode(atNotification.atMetadata?.pubKeyHash?.toJson())

To ensure that metadata.publicKeyHash contains a null object on the client side, modify the code in MonitorVerbHandler.dart as follows:

notification.metadata?.putIfAbsent(
          "pubKeyHash",
          () => (atNotification.atMetadata?.pubKeyHash != null)
              ? jsonEncode(atNotification.atMetadata?.pubKeyHash?.toJson())
              : null);

@gkc : Let me know if this approach looks good or suggest an alternate approach please.

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.

2 participants