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

Be able to let the user trust several Fingerprints during login flow. #8193

Merged
merged 1 commit into from
Mar 8, 2023
Merged

Be able to let the user trust several Fingerprints during login flow. #8193

merged 1 commit into from
Mar 8, 2023

Conversation

bmarty
Copy link
Member

@bmarty bmarty commented Mar 1, 2023

It was the case before, see LoginViewModel.handleUserAcceptCertificate(...)

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Motivation and context

Screenshots / GIFs

Tests

  • Step 1
  • Step 2
  • Step ...

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@bmarty
Be able to let the user trust several Fingerprints during login flow.
448374f 
It was the case before, see `LoginViewModel.handleUserAcceptCertificate(...)`
bmarty
bmarty commented Mar 2, 2023
View reviewed changes
vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt
@@ -293,13 +295,14 @@ class OnboardingViewModel @AssistedInject constructor(
private fun handleUserAcceptCertificate(action: OnboardingAction.UserAcceptCertificate) {
// It happens when we get the login flow, or during direct authentication.
// So alter the homeserver config and retrieve again the login flow
trustedFingerprints.add(action.fingerprint)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the fix, we aggregate the trusted fingerprints, instead of replacing with the last one.

@bmarty bmarty requested review from a team and yostyle and removed request for a team March 7, 2023 09:08
@bmarty bmarty added the Z-NextRelease For issues and PRs which should be included in the NextRelease. label Mar 7, 2023
@bmarty bmarty marked this pull request as ready for review March 7, 2023 09:08
yostyle
yostyle approved these changes Mar 7, 2023
View reviewed changes
Copy link
Contributor

@yostyle yostyle left a comment

Choose a reason for hiding this comment

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

SGTM

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

90.0% 90.0% Coverage
0.0% 0.0% Duplication

@ganfra ganfra merged commit fc79615 into develop Mar 8, 2023
@ganfra ganfra deleted the feature/bma/certList branch March 8, 2023 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yostyle yostyle yostyle approved these changes

Assignees
No one assigned
Labels
X-Regression Z-NextRelease For issues and PRs which should be included in the NextRelease.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bmarty @yostyle @ganfra