-
Notifications
You must be signed in to change notification settings - Fork 497
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
hide account deactivation in OIDC #7652
Changes from all commits
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 |
---|---|---|
|
@@ -112,11 +112,14 @@ class HomeserverAddress: NSObject { | |
let brand: String? | ||
/// The icon field is an optional field that points to an icon representing the identity provider. If present then it must be an HTTPS URL to an image resource. | ||
let iconURL: String? | ||
/// Dertermines if this provider uses OIDC | ||
let isOIDC: Bool | ||
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. I think for sanity this should also be |
||
|
||
init(id: String, name: String, brand: String?, iconURL: String?) { | ||
init(id: String = "", name: String, brand: String?, iconURL: String?, isOIDC: Bool = false) { | ||
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. I'm not really a fan of the default value for id now I look at it. It feels optional now, whereas it really isn't. Would be better imo to explicitly set this when we create the mock provider with a comment that its fine as there is only one. |
||
self.id = id | ||
self.name = name | ||
self.brand = brand | ||
self.iconURL = iconURL | ||
self.isOIDC = isOIDC | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -290,9 +290,16 @@ class AuthenticationService: NSObject { | |
// Get the login flow | ||
let loginFlowResponse = try await client.getLoginSession() | ||
|
||
let identityProviders = loginFlowResponse.flows?.compactMap { $0 as? MXLoginSSOFlow }.first?.identityProviders ?? [] | ||
let loginFlow = loginFlowResponse.flows?.compactMap { $0 as? MXLoginSSOFlow }.first | ||
var identityProviders = loginFlow?.ssoIdentityProviders ?? [] | ||
if identityProviders.isEmpty { | ||
// Provide a backup for homeservers that support SSO but don't offer any identity providers | ||
// https://spec.matrix.org/latest/client-server-api/#client-login-via-sso | ||
identityProviders = [SSOIdentityProvider(name: "SSO", brand: nil, iconURL: nil, isOIDC: loginFlow?.delegatedOIDCCompatibility ?? false)] | ||
} | ||
|
||
return LoginFlowResult(supportedLoginTypes: loginFlowResponse.flows?.compactMap { $0 } ?? [], | ||
ssoIdentityProviders: identityProviders.sorted { $0.name < $1.name }.map(\.ssoIdentityProvider), | ||
ssoIdentityProviders: identityProviders.sorted { $0.name < $1.name }, | ||
homeserverAddress: client.homeserver) | ||
} | ||
|
||
|
@@ -309,7 +316,14 @@ class AuthenticationService: NSObject { | |
} | ||
|
||
extension MXLoginSSOIdentityProvider { | ||
var ssoIdentityProvider: SSOIdentityProvider { | ||
SSOIdentityProvider(id: identifier, name: name, brand: brand, iconURL: icon) | ||
@objc func makeSSOIdentityProvider(isOIDC: Bool) -> SSOIdentityProvider { | ||
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. Same on this property name |
||
SSOIdentityProvider(id: identifier, name: name, brand: brand, iconURL: icon, isOIDC: isOIDC) | ||
} | ||
} | ||
|
||
extension MXLoginSSOFlow { | ||
@objc var ssoIdentityProviders: [SSOIdentityProvider] { | ||
identityProviders.map { $0.makeSSOIdentityProvider(isOIDC: delegatedOIDCCompatibility)} | ||
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. I would question if this part is correct. AIUI, if there are multiple providers, then the presence of this flag means "don't show them anyway" but as we're not doing that, I would be surprised if these are then using OIDC compatibility (as they're basically Sign in with Google/Apple etc which aren't). |
||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Deactivate account option is disabled when logging in with OIDC. |
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.
Too much time writing Swift, I don't think you need the new
in there 😆