-
Notifications
You must be signed in to change notification settings - Fork 297
OIDC configuration #4623
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
OIDC configuration #4623
Changes from all commits
d28d80b
912d6f2
0bea29a
8d7a57b
ae56172
3cdc38d
9cc1cc5
c750287
4960065
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ package io.element.android.libraries.matrix.impl.auth | |
|
||
import io.element.android.libraries.matrix.api.auth.AuthenticationException | ||
import org.matrix.rustcomponents.sdk.ClientBuildException | ||
import org.matrix.rustcomponents.sdk.OidcException | ||
|
||
fun Throwable.mapAuthenticationException(): AuthenticationException { | ||
val message = this.message ?: "Unknown error" | ||
|
@@ -24,6 +25,13 @@ fun Throwable.mapAuthenticationException(): AuthenticationException { | |
is ClientBuildException.WellKnownLookupFailed -> AuthenticationException.Generic(message) | ||
is ClientBuildException.EventCache -> AuthenticationException.Generic(message) | ||
} | ||
is OidcException -> when (this) { | ||
is OidcException.Generic -> AuthenticationException.Oidc(message) | ||
is OidcException.CallbackUrlInvalid -> AuthenticationException.Oidc(message) | ||
is OidcException.Cancelled -> AuthenticationException.Oidc(message) | ||
is OidcException.MetadataInvalid -> AuthenticationException.Oidc(message) | ||
is OidcException.NotSupported -> AuthenticationException.Oidc(message) | ||
} | ||
Comment on lines
+28
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we're mapping them to the same type, are we sure the messages are clear enough to track the root cause of the issue? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, for what I have seen, it seems to be clear enough. We may want to improve the mapping if it's not the case. Previously the homeserver reachability was designed as the cause of the failure, so I guess this is already much better. |
||
else -> AuthenticationException.Generic(message) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,24 +7,22 @@ | |
|
||
package io.element.android.libraries.matrix.impl.auth | ||
|
||
import io.element.android.libraries.core.meta.BuildMeta | ||
import io.element.android.libraries.matrix.api.auth.OidcConfig | ||
import org.matrix.rustcomponents.sdk.OidcConfiguration | ||
import javax.inject.Inject | ||
|
||
class OidcConfigurationProvider @Inject constructor() { | ||
class OidcConfigurationProvider @Inject constructor( | ||
private val buildMeta: BuildMeta, | ||
) { | ||
fun get(): OidcConfiguration = OidcConfiguration( | ||
clientName = "Element", | ||
clientName = buildMeta.applicationName, | ||
redirectUri = OidcConfig.REDIRECT_URI, | ||
clientUri = "https://element.io", | ||
logoUri = "https://element.io/mobile-icon.png", | ||
tosUri = "https://element.io/acceptable-use-policy-terms", | ||
policyUri = "https://element.io/privacy", | ||
contacts = listOf( | ||
"[email protected]", | ||
), | ||
// Some homeservers/auth issuers don't support dynamic client registration, and have to be registered manually | ||
staticRegistrations = mapOf( | ||
"https://id.thirdroom.io/realms/thirdroom" to "elementx", | ||
), | ||
clientUri = OidcConfig.CLIENT_URI, | ||
logoUri = OidcConfig.LOGO_URI, | ||
tosUri = OidcConfig.TOS_URI, | ||
policyUri = OidcConfig.POLICY_URI, | ||
contacts = null, | ||
staticRegistrations = OidcConfig.STATIC_REGISTRATIONS, | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pixlwave this map is not "externally" configurable, do you know if we should do something about it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine for now. IIRC we added the static registrations specifically for compatibility with Third Room, but given basically everyone will be using MAS, dynamic registration will be supported.