feat: Observe self client certificate revocation (WPB-6145) #2384

ohassine · 2024-01-18T10:06:55Z

PR Submission Checklist for internal contributors

The PR Title
- conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
- contains a reference JIRA issue number like SQPIT-764
- answers the question: If merged, this PR will: ... ³
The PR Description
- is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Description

I this PR, I am creating the logic that will be used in Android to show a dialog when self client's certificate is revoked:

A flag will be observed by Android through a use case called ObserveShouldNotifyForRevokedCertificateUseCase to show the dialog.

Needs releases with:

GitHub link to other pull request

Testing

Test Coverage (Optional)

I have added automated test to this contribution

How to Test

Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.

Notes (Optional)

Specify here any other facts that you think are important for this issue.

Attachments (Optional)

Attachments like images, videos, etc. (drag and drop in the text box)

PR Post Submission Checklist for internal contributors (Optional)

Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References

https://sparkbox.com/foundry/semantic_commit_messages
https://github.com/wireapp/.github#usage
E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

# Conflicts: # cryptography/src/appleMain/kotlin/com/wire/kalium/cryptography/MLSClientImpl.kt # cryptography/src/commonJvmAndroid/kotlin/com.wire.kalium.cryptography/MLSClientImpl.kt # cryptography/src/commonMain/kotlin/com/wire/kalium/cryptography/MLSClient.kt # cryptography/src/jsMain/kotlin/com/wire/kalium/cryptography/MLSClientImpl.kt # logic/src/commonMain/kotlin/com/wire/kalium/logic/data/e2ei/E2EIRepository.kt # network/src/commonMain/kotlin/com/wire/kalium/network/api/base/unbound/acme/ACMEApi.kt

…k_revocation_list

…ert_revoked_dialog

…ert_revoked_dialog # Conflicts: # logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/UserConfigRepository.kt # network/src/commonMain/kotlin/com/wire/kalium/network/api/base/unbound/acme/ACMEApi.kt # persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/unread/UserConfigDAO.kt

…alog' into show_your_device_cert_revoked_dialog

github-actions · 2024-01-18T11:06:15Z

Test Results

2 662 tests 2 652 ✔️ 3m 16s ⏱️
      3 suites     10 💤
      3 files       0 ❌

Results for commit ca03aa7.

♻️ This comment has been updated with latest results.

datadog-wireapp · 2024-01-18T11:20:36Z

Datadog Report

All test runs 51cf2bb 🔗

✅ 2 Total Test Services: 0 Failed, 2 Passed

Test Services

Service Name	Failed	Known Flaky	New Flaky	Passed	Skipped	Wall Time	Test Service View
`kalium-ios`	0	0	0	2592	80	5m 52s	Link
`kalium-jvm`	0	0	0	2761	105	8m 47.05s	Link

…ialog

mchenani

🔥

...n/com/wire/kalium/logic/feature/user/e2ei/ObserveShouldNotifyForRevokedCertificateUseCase.kt

borichellow · 2024-01-24T08:55:41Z

Question: how it should work?
With this implementation user will see the dialog every time he/she launches the app (if the certificate was revoked).
And (I guess) there is some option to dismiss that dialog (by MarkNotifyForRevokedCertificateAsNotifiedUseCase).

But I thought if my certificate was revoked it means that I'm a bad guy and should be logged out, isn't it?

codecov-commenter · 2024-01-24T09:05:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (release/candidate@d86d915). Click here to learn what that means.

Additional details and impacted files

@@                 Coverage Diff                  @@
##             release/candidate    #2384   +/-   ##
====================================================
  Coverage                     ?   58.65%           
  Complexity                   ?       21           
====================================================
  Files                        ?     1130           
  Lines                        ?    44262           
  Branches                     ?     4128           
====================================================
  Hits                         ?    25961           
  Misses                       ?    16460           
  Partials                     ?     1841

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d86d915...f22c54f. Read the comment docs.

ohassine · 2024-01-24T09:39:23Z

Question: how it should work? With this implementation user will see the dialog every time he/she launches the app (if the certificate was revoked). And (I guess) there is some option to dismiss that dialog (by MarkNotifyForRevokedCertificateAsNotifiedUseCase).

But I thought if my certificate was revoked it means that I'm a bad guy and should be logged out, isn't it?

Yes we show a dialog recommending to the user to logout.
Check this PR it contains demo video wireapp/wire-android#2594

And once the user dismisses the dialog we invoke markNotifyForRevokedCertificateAsNotified so dialog won't show next time

borichellow · 2024-01-24T10:14:04Z

Question: how it should work? With this implementation user will see the dialog every time he/she launches the app (if the certificate was revoked). And (I guess) there is some option to dismiss that dialog (by MarkNotifyForRevokedCertificateAsNotifiedUseCase).
But I thought if my certificate was revoked it means that I'm a bad guy and should be logged out, isn't it?

Yes we show a dialog recommending to the user to logout. Check this PR it contains demo video wireapp/wire-android#2594

And once the user dismisses the dialog we invoke markNotifyForRevokedCertificateAsNotified so dialog won't show next time

sounds strange to me (the fact the user could skip the logging out), but okay :)

…ialog # Conflicts: # logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/UserConfigRepository.kt # logic/src/commonMain/kotlin/com/wire/kalium/logic/data/e2ei/E2EIRepository.kt # logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/user/UserScope.kt # logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListUseCaseTest.kt # persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/unread/UserConfigDAO.kt

* feat(MLS): check revocation list * feat(MLS): cover CheckRevocationListUseCase with unit test * chore: detekt * chore: apply new changes from CC * feat: store urls with expiration time * feat: pass url as param to the use case * chore: detekt * chore: unit test * feat: Show dialog when current user's certificate is revoked * feat: update unit test for CheckRevocationListUseCase * chore: detekt * chore: cleanup * chore: resolve conflicts

…2408) * feat(MLS): check revocation list * feat(MLS): cover CheckRevocationListUseCase with unit test * chore: detekt * chore: apply new changes from CC * feat: store urls with expiration time * feat: pass url as param to the use case * chore: detekt * chore: unit test * feat: Show dialog when current user's certificate is revoked * feat: update unit test for CheckRevocationListUseCase * chore: detekt * chore: cleanup * chore: resolve conflicts

ohassine added 15 commits January 11, 2024 15:54

feat(MLS): check revocation list

af30446

feat(MLS): cover CheckRevocationListUseCase with unit test

5ec9d73

chore: detekt

d779f1f

chore: apply new changes from CC

ad94e4d

Merge branch 'develop' into check_revocation_list

ac1448e

feat: store urls with expiration time

e991e45

feat: pass url as param to the use case

19db4e5

Merge remote-tracking branch 'origin/check_revocation_list' into chec…

c0c02f8

…k_revocation_list

chore: detekt

aeba6d4

chore: unit test

0fbddf6

feat: Show dialog when current user's certificate is revoked

9993659

feat: update unit test for CheckRevocationListUseCase

c2868bb

Merge remote-tracking branch 'origin/develop' into show_your_device_c…

911d2fd

…ert_revoked_dialog

github-actions bot assigned ohassine Jan 18, 2024

github-actions bot added 🚨 Potential breaking changes 👕 size: XL type: feature ✨ labels Jan 18, 2024

Merge branch 'develop' into show_your_device_cert_revoked_dialog

cb59169

ohassine mentioned this pull request Jan 18, 2024

feat: Show a dialog when current client's certificate is revoked (WPB-6145) wireapp/wire-android#2594

Merged

8 tasks

ohassine added 2 commits January 18, 2024 11:55

chore: detekt

68b5456

Merge remote-tracking branch 'origin/show_your_device_cert_revoked_di…

bb94a90

…alog' into show_your_device_cert_revoked_dialog

ohassine added the WIP work in progress label Jan 19, 2024

chore: cleanup

c502523

ohassine removed the WIP work in progress label Jan 24, 2024

ohassine requested review from a team and removed request for a team January 24, 2024 08:38

ohassine requested review from typfel, borichellow, vitorhugods, Garzas and saleniuk January 24, 2024 08:38

ohassine changed the base branch from develop to release/candidate January 24, 2024 08:42

Merge branch 'release/candidate' into show_your_device_cert_revoked_d…

f22c54f

…ialog

mchenani approved these changes Jan 24, 2024

View reviewed changes

yamilmedina approved these changes Jan 24, 2024

View reviewed changes

borichellow reviewed Jan 24, 2024

View reviewed changes

...n/com/wire/kalium/logic/feature/user/e2ei/ObserveShouldNotifyForRevokedCertificateUseCase.kt Show resolved Hide resolved

ohassine added 2 commits January 25, 2024 10:23

chore: resolve conflicts

e42a5d7

ohassine enabled auto-merge (squash) January 25, 2024 09:51

github-actions bot added 👕 size: L and removed 👕 size: XL labels Jan 25, 2024

chore: cleanup

ca03aa7

ohassine merged commit 35d3229 into release/candidate Jan 25, 2024
17 checks passed

ohassine deleted the show_your_device_cert_revoked_dialog branch January 25, 2024 11:50

github-actions bot mentioned this pull request Jan 25, 2024

feat: Observe self client certificate revocation (WPB-6145) #2406

Closed

8 tasks

ohassine mentioned this pull request Jan 25, 2024

feat: Observe self client certificate revocation (WPB-6145) - cherrypick #2408

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Observe self client certificate revocation (WPB-6145) #2384

feat: Observe self client certificate revocation (WPB-6145) #2384

ohassine commented Jan 18, 2024 •

edited

Loading

github-actions bot commented Jan 18, 2024 •

edited

Loading

datadog-wireapp bot commented Jan 18, 2024 •

edited

Loading

mchenani left a comment

borichellow commented Jan 24, 2024

codecov-commenter commented Jan 24, 2024

ohassine commented Jan 24, 2024

borichellow commented Jan 24, 2024

feat: Observe self client certificate revocation (WPB-6145) #2384

feat: Observe self client certificate revocation (WPB-6145) #2384

Conversation

ohassine commented Jan 18, 2024 • edited Loading

PR Submission Checklist for internal contributors

What's new in this PR?

Description

Testing

Test Coverage (Optional)

How to Test

Notes (Optional)

Attachments (Optional)

PR Post Submission Checklist for internal contributors (Optional)

PR Post Merge Checklist for internal contributors

References

github-actions bot commented Jan 18, 2024 • edited Loading

Test Results

datadog-wireapp bot commented Jan 18, 2024 • edited Loading

Datadog Report

mchenani left a comment

Choose a reason for hiding this comment

borichellow commented Jan 24, 2024

codecov-commenter commented Jan 24, 2024

Codecov Report

ohassine commented Jan 24, 2024

borichellow commented Jan 24, 2024

ohassine commented Jan 18, 2024 •

edited

Loading

github-actions bot commented Jan 18, 2024 •

edited

Loading

datadog-wireapp bot commented Jan 18, 2024 •

edited

Loading