-
Notifications
You must be signed in to change notification settings - Fork 191
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
Conversation
- integration tested with react dapp
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 { |
There was a problem hiding this comment.
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 {
...
}
let pushMessage = try service.decryptMessage(topic: topic, ciphertext: ciphertext) | ||
bestAttemptContent.title = pushMessage.title | ||
bestAttemptContent.body = pushMessage.body | ||
contentHandler(bestAttemptContent) |
There was a problem hiding this comment.
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
PNEncryptedTest.apns
Outdated
@@ -0,0 +1,14 @@ | |||
{ |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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)
?
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok xd
Description
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.
Due Dilligence