Handling SSL/TLS errors during WellKnown lookup #5965
Conversation
…s when triggered via the sign in with matrix id flow
ouchadam
force-pushed
the
feature/adm/matrix-id-certificate
branch
from
cf00e00 to
e97cdb0
Compare
ouchadam
requested review from
a team,
onurays and
ariskotsomitopoulos
and removed request for
a team
| ) | ||
| ) | ||
| } | ||
|
|
||
| private fun HomeServerConnectionConfig?.orWellKnownDefaults() = this ?: HomeServerConnectionConfig.Builder() |
There was a problem hiding this comment.
I wasn't too sure about this, I had originally planned to make the service entry point non null but it feels strange to pass the matrix id and a homeserver config (with the url already calculated from the matrix id)
Ideally the getWellKnownData could take the certificate parts of the homeserver config rather than the entire model
There was a problem hiding this comment.
I also tried with non-null parameter in my local but it produces ugly code as so said.
There was a problem hiding this comment.
Maybe keep your original plan and add a in the builder something like HomeServerConnectionConfig.Builder.from(matrixId) or withMatrixId(matrixId) to avoid using dummy.org? But it's maybe more confusing :/.
This is not a blocker for me.
There was a problem hiding this comment.
I'm happy to add a build method and then remove the matrixId from the getWellKnownData 👍
There was a problem hiding this comment.
I had a go at using the builder for this however the direct usage of Android's Uri breaks the OnboardingViewModel tests
https://github.com/vector-im/element-android/compare/feature/adm/matrix-id-via-builder
there's a bit more refactoring needed to take into account using fake uris
will merge as is for the time being
| ) | ||
| ) | ||
| } | ||
|
|
||
| private fun HomeServerConnectionConfig?.orWellKnownDefaults() = this ?: HomeServerConnectionConfig.Builder() |
There was a problem hiding this comment.
Maybe keep your original plan and add a in the builder something like HomeServerConnectionConfig.Builder.from(matrixId) or withMatrixId(matrixId) to avoid using dummy.org? But it's maybe more confusing :/.
This is not a blocker for me.
Matrix SDKIntegration Tests Results:
|
Type of change
Content
Applies the SSL/TLS certificate handling logic to the
sign in with matrix idflowHomeserverConnectionConfigwhich in turn meant the SSL/TLS socket factory was never appliedMotivation and context
To make the
sign in with matrix idandothersign in flows consistentScreenshots / GIFs
Tests
sign in with matrix idothera certificate error is shownTested devices