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

don't decrypt attachments before tapping + show modal on error #1233

Closed
tomholub opened this issue Dec 17, 2021 · 3 comments · Fixed by #1323
Closed

don't decrypt attachments before tapping + show modal on error #1233

tomholub opened this issue Dec 17, 2021 · 3 comments · Fixed by #1323
Assignees
Labels
actionable PR Submitted PR is submitted for this issue

Comments

@tomholub
Copy link
Collaborator

also mostly fixed #1143. The error was that while the text could be decrypted, the attachments were throwing decrypt errors. I've made it recognize decrypt errors and download the original file, but it needs another issue to actually render the decrypt error before downloading, make sure that's what the user wanted.

image

Originally posted by @tomholub in #1219 (comment)

@tomholub
Copy link
Collaborator Author

Additionally, there is no need to decrypt all attachments when rendering. Decrypt when tapping attachment.

MessageAttachment should also have an isEncrypted: bool, and we should decide whether to decrypt based on that, when user taps the file.

                // todo - this is decrypting attachments too early
                //   there is no need to decrypt attachments when rendering message,
                //   since user may never tap them.
                //   Instead decrypt them when actually tapped.
                let decrypted = try await core.decryptFile(
                    encrypted: meta.data,
                    keys: keys,
                    msgPwd: nil
                )
                if let decryptSuccess = decrypted.decryptSuccess {
                    attachment = MessageAttachment(
                        name: decryptSuccess.name,
                        data: decryptSuccess.data
                    )
                } else if let decryptErr = decrypted.decryptErr {
                    // todo - once above todo is done and we are decrypting
                    //   at the time of tapping an attachment, render error modal
                    // if user confirms the modal, they can download the original msg
                    // then can remove the warning
                    logger.logWarning("attachment could not be decrypted due to error: \(decryptErr.error.type) - \(decryptErr.error.message)")
                    attachment = MessageAttachment(
                        name: meta.name,
                        data: meta.data
                    )
                } else {
                    throw AppErr.unexpected("decryptFile: expected one of decryptErr, decryptSuccess to be present")
                }

@tomholub tomholub added this to the 0.7.0: Forward milestone Dec 17, 2021
@tomholub
Copy link
Collaborator Author

tomholub commented Dec 17, 2021

Since we're adding isEncrypted to each attachment, we could also surface that in UI / list of attachments, with encrypted (green) or not encrypted (red) badge for each attachment?

@tomholub
Copy link
Collaborator Author

tomholub commented Dec 17, 2021

This will also need an appium test for the above message. Tapping the message should produce a modal:

Error decrypting attachment: \(decryptErr.error.type) - \(decryptErr.error.message)

This will likely download a corrupted file. Download anyway?

[yes] [no]

Taping yes downloads the original (encrypted) file. Tapping no will hide the modal.

@sosnovsky sosnovsky added the in progress Work on the issue is in progress label Jan 17, 2022
sosnovsky added a commit that referenced this issue Jan 18, 2022
sosnovsky added a commit that referenced this issue Jan 18, 2022
@sosnovsky sosnovsky added PR Submitted PR is submitted for this issue and removed in progress Work on the issue is in progress labels Jan 20, 2022
tomholub pushed a commit that referenced this issue Jan 21, 2022
* #1233 decrypt attachments on tap

* #1233 update attachment decryption

* #1233 handle attachment decrypt errors

* #1233 improve text attachments rendering

* fix tests

* update MessageAttachment

* fix ui test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actionable PR Submitted PR is submitted for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants