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: at_client put method - introduce shouldEncrypt flag #1398

Merged
merged 18 commits into from
Sep 27, 2024

Conversation

murali-shris
Copy link
Member

@murali-shris murali-shris commented Sep 16, 2024

Fixes #1401
Depends on atsign-foundation/at_server#2099 for intermittent functional test failure in sync_multiple_client_test

- What I did

  • Ability for clients to set shouldEncrypt=false when they don't want to encrypt data for self and shared key

- How I did it

  • added a flag shouldEncrypt with default value=true in PutRequestOptions. This value will be ignored for public keys(metadata.isEncrypted is always false). For self and sharedkey this value will be honoured and data will be encrypted(shouldEncrypt=true) or not encrypted(shouldEncrypt=false)
  • Refactored put_request_transformer for readability
  • Changes in get_response_transformer to handle two scenarios 1) isEncrypted for new data(post these changes where isEncrypted=true for self/shared keys) 2) isEncrypted=false for old self/shared key.
  • modified sync_multiple_client_test to replicate the intermittent issue consistently
  • added unit and functional test

- How to verify it

@murali-shris murali-shris changed the title test: verify at_commons branch feat: at_client put method - introduce shouldEncrypt flag Sep 20, 2024
@murali-shris
Copy link
Member Author

fix for functional test issue in server
https://github.com/atsign-foundation/at_server/pull/2099/files

@murali-shris murali-shris marked this pull request as ready for review September 25, 2024 05:43
@murali-shris murali-shris requested a review from gkc September 26, 2024 04:41
!(decodedResponse['key'].startsWith('cached:public:'))) {
var decryptionService = AtKeyDecryptionManager(_atClient)
.get(tuple.one, _atClient.getCurrentAtSign()!);
// For new encrypted data after AtClient v3.2.1, isEncrypted will be true by default
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "by default" mean? Is it more accurate to say that "isEncrypted will be set to true when data is encrypted"?

Copy link
Member Author

Choose a reason for hiding this comment

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

isEncrypted will be set to true for self/shared keys, if caller doesn't set anything in PutRequestionOptions. I will change the wording.

Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

@murali-shris murali-shris requested a review from gkc September 26, 2024 11:09
@gkc gkc merged commit fb088ea into trunk Sep 27, 2024
10 checks passed
@gkc gkc deleted the update_verb_isencrypted_changes branch September 27, 2024 08:10
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.

Introduce shouldEncrypt in PutRequestOptions for clients to not encrypt self key/shared key data
2 participants