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.

Sign up for GitHub

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

Jump to bottom

fix(e2ei): force login to idp to update certificate (WPB-6877) #2743

Merged
merged 4 commits into from
Mar 4, 2024
Merged

fix(e2ei): force login to idp to update certificate (WPB-6877) #2743

merged 4 commits into from
Mar 4, 2024

Conversation

AndroidBob
Copy link
Collaborator

@AndroidBob AndroidBob commented Feb 27, 2024 •
edited by github-actions bot
Loading

BugWPB-6877 [Android] Fix Update E2EI Certificate

Cherry pick from the original PR:

⚠️ Conflicts during cherry-pick:
app/src/main/kotlin/com/wire/android/ui/e2eiEnrollment/GetE2EICertificateUI.kt
kalium

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
    • 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?

Since the update mechanism and design required some extra work, and we already have couple of suggestion about other approaches, we decided to enforce the user to login everytime agains the idp and not using the refresh token.

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
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. .

@AndroidBob AndroidBob added the cherry-pick PR is cherry-picking changes from another banch label Feb 27, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 27, 2024 •
edited
Loading

Test Results

827 tests  ±0   827 ✅ ±0   9m 20s ⏱️ - 2m 33s
115 suites ±0     0 💤 ±0 
115 files   ±0     0 ❌ ±0 

Results for commit 0ce3a65. ± Comparison against base commit 7a2c430.

♻️ This comment has been updated with latest results.

@AndroidBob
Copy link
Collaborator Author

Build 3351 failed.

@AndroidBob
Copy link
Collaborator Author

Build 3392 failed.

@alexandreferris
Merge branch 'develop' into release/fix/add-force-login-flag-to-oauth…
0ce3a65 
…-cherry-pick

# Conflicts:
#	app/src/main/kotlin/com/wire/android/ui/e2eiEnrollment/GetE2EICertificateUI.kt
#	kalium
borichellow
borichellow reviewed Mar 1, 2024
View reviewed changes
app/src/main/kotlin/com/wire/android/ui/e2eiEnrollment/GetE2EICertificateUI.kt
@@ -43,7 +43,7 @@ fun GetE2EICertificateUI(
LaunchedEffect(Unit) {
viewModel.requestOAuthFlow.onEach {
OAuthUseCase(context, it.target, it.oAuthClaims, it.oAuthState).launch(
context.getActivity()!!.activityResultRegistry
context.getActivity()!!.activityResultRegistry, forceLoginFlow = true
Copy link
Contributor

@borichellow borichellow Mar 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

this is the only place where OAuthUseCase.launch is called so the parameter forceLoginFlow is always true. What is the point of it then?

@codecov Codecov
Copy link

codecov bot commented Mar 1, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 43.14%. Comparing base (7a2c430) to head (0ce3a65).
Report is 1 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #2743      +/-   ##
===========================================
- Coverage    43.19%   43.14%   -0.05%     
===========================================
  Files          409      409              
  Lines        13583    13583              
  Branches      2397     2398       +1     
===========================================
- Hits          5867     5861       -6     
- Misses        7029     7036       +7     
+ Partials       687      686       -1
Files Coverage Δ
...tlin/com/wire/android/feature/e2ei/OAuthUseCase.kt 0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

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 7a2c430...0ce3a65. Read the comment docs.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 1, 2024

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator Author

Build 3424 succeeded.

The build produced the following APK's:

@alexandreferris alexandreferris added this pull request to the merge queue Mar 4, 2024
Merged via the queue into develop with commit 42d82dd Mar 4, 2024
14 checks passed
@alexandreferris alexandreferris deleted the release/fix/add-force-login-flag-to-oauth-cherry-pick branch March 4, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexandreferris alexandreferris alexandreferris approved these changes

@borichellow borichellow borichellow approved these changes

Assignees

@mchenani mchenani

Labels
cherry-pick PR is cherry-picking changes from another banch size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AndroidBob @alexandreferris @borichellow @mchenani