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

[Push] notification decryption #647

Merged
merged 27 commits into from Jan 12, 2023
Merged

[Push] notification decryption #647

merged 27 commits into from Jan 12, 2023

Conversation

llbartekll
Copy link
Contributor

@llbartekll llbartekll commented Jan 3, 2023

Description

  • add resubscription service for push sdk
  • add NotificationService target for example wallet that decrypt PNs delivered by the echo server
  • add PushDecryptionService that shares the keychain group with push sdk

Resolves # (issue)

How Has This Been Tested?

Delivery of PNs has been tested with Push Notification Tester. Here is an example payload, encrypted message has been generated by JS dapp.

{"aps":{
     "mutable-content" : 1
},
"topic":"54855cd02500161515c75d97540c39caf193ce056d88d7cc36a41da895493f29",
"ciphertext":"AIvB700nz1RdAHOxzIVrBhtn/z8Zu9eXN91YJHjHh1pLGmvPasEMazkD3vadMewd9j6T8mtJNKYWizh81vIH21E5DSHKefSsYwa638jBsyp75jIMvT9IdU4kk2CZNJ+jwnSpwq3bo4Tw9XRl2ABXKLSsWsGfr6kDViZUSu5wt8A5rIbtUQ4xNeshbOyOCxnk6P3IafiFFsQUS/jN2Ya1GKkZ7vlEjqxL+kSdLffxwu3vb8WqhJrGbSlbD9XRaGA18XMEmPG5gRsGUU+wGg=="
}

Due Dilligence

  • Breaking change
  • Requires a documentation update

@llbartekll llbartekll marked this pull request as draft January 3, 2023 11:43
@arein arein added the accepted label Jan 3, 2023
@llbartekll llbartekll self-assigned this Jan 5, 2023
@llbartekll llbartekll marked this pull request as ready for review January 5, 2023 13:30
override func serviceExtensionTimeWillExpire() {
// Called just before the extension will be terminated by the system.
// Use this as an opportunity to deliver your "best attempt" at modified content, otherwise the original push payload will be used.
if let contentHandler = contentHandler, let bestAttemptContent = bestAttemptContent {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you use the same name when unwrapping, you can make it shorter like

if let contentHandler, let bestAttemptContent {
   ...
}

…V2 into PNs-ui-testing

# Conflicts:
#	Example/ExampleApp.xcodeproj/project.pbxproj
let pushMessage = try service.decryptMessage(topic: topic, ciphertext: ciphertext)
bestAttemptContent.title = pushMessage.title
bestAttemptContent.body = pushMessage.body
contentHandler(bestAttemptContent)
Copy link
Contributor

Choose a reason for hiding this comment

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

should you return here? otherwise you will call contentHandler twice

@@ -0,0 +1,14 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

why you need PNEncryptedTest and PNTest?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use them for testing in the simulator but I can remove PNEncryptedTest now

}

public func register(deviceToken: Data) async throws {
try await registerService.register(deviceToken: deviceToken)
}

public func decryptMessage(topic: String, ciphertext: String) throws -> String {
try decryptionService.decryptMessage(topic: topic, ciphertext: ciphertext)
#if DEBUG
Copy link
Contributor

Choose a reason for hiding this comment

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

why we cannot use always register(deviceToken: Data) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is for simulator id as you get it as a string


import Foundation

public final class GroupKeychainStorage: KeychainStorageProtocol {
Copy link
Contributor

Choose a reason for hiding this comment

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

a lot of code duplicates with KeychainStorage ig, is it possible to reuse one instance? And configure it as shared/not shared

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know and I was thinking about it but didn't want to modify existing code.
will take a look again, any suggestions?

public func decryptMessage(topic: String, ciphertext: String) throws -> PushMessage {
let rpcRequest: RPCRequest = try serializer.deserialize(topic: topic, encodedEnvelope: ciphertext)
guard let params = rpcRequest.params else { throw Errors.malformedPushMessage }
let pushMessage = try params.get(PushMessage.self)
Copy link
Contributor

Choose a reason for hiding this comment

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

return try params.get(PushMessage.self)? xD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok xd

@llbartekll llbartekll merged commit c2cba73 into develop Jan 12, 2023
@llbartekll llbartekll deleted the PNs-ui-testing branch January 12, 2023 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants