Skip to content

Commit

Permalink
Infrastructure changes to allow for seamless Single-Sign-On implement…
Browse files Browse the repository at this point in the history
…ations. (#28)

# Infrastructure changes to allow for seamless Single-Sign-On
implementations.

## ♻️ Current situation & Problem
While `IdentityProvider` are currently supported by `SpeziAccount`,
there are some issues when trying to accommodate more extended use cases
or certain limitations when combining different functionality.

Below is a list of certain pain points including general improvements
this PR makes:
* Currently, account values requirements are globally applied. E.g., a
configured password key is required for **all** account services. This
doesn't fit very well for SSO providers as they intentionally don't
collect passwords. Instead one would need a way of declaring AccountKeys
required in the context of a given account services. This is resolved by
changing the effect of the `RequiredAccountKeys` configuration an
account service would provide and consequentially removing the need for
the user to declare the `password` account value globally required.
* We added a new `FollowUpInfoSheet` that automatically pops up after
account setup to assist the user to provide any additional account
details that are configured to be required. That is especially useful
when using Identity Provider like Sign in with Apple that only provide a
fixed set of user information.
A new `verifyRequiredAccountDetails(_:)` modifier was added to implement
the same logic on the global application level. This can be used that
existing users always are in line with the latest configuration of your
app.
* The `accountRequired(_:setupSheet:)` modifier was added to enforce a
user account at all times.
* Added a new globally unique and stable `AccountId`, mandatory for each
user account.
* Fixed an issue where the SignupForm cancel confirmation would always
pop up when using AccountKeys with default values.
* Other fixes and improvements.

## ⚙️ Release Notes 
Added several infrastructure enhancements that allow for an improved
user experience and compatibility when using identity providers.

### Breaking Changes
* We introduce a new `accountId` (see `AccountIdKey`) account value that
is mandatory for all user accounts. This is now also used as a primary
identifier for `AccountStorageStandard`s.

## 📚 Documentation
Documentation was updated respectively.


## ✅ Testing
Test cases were added for new functionality or fixed functionality.

## 📝 Code of Conduct & Contributing Guidelines 

By submitting creating this pull request, you agree to follow our [Code
of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md):
- [x] I agree to follow the [Code of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md).

---------

Co-authored-by: Paul Schmiedmayer <[email protected]>
  • Loading branch information
Supereg and PSchmiedmayer authored Oct 23, 2023
1 parent 1eca293 commit 2f88e04
Show file tree
Hide file tree
Showing 60 changed files with 1,260 additions and 340 deletions.
38 changes: 30 additions & 8 deletions Sources/SpeziAccount/Account.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import SwiftUI
/// ### Managing Account state
/// This section provides an overview on how to manage and manipulate the current user account as an ``AccountService``.
///
/// - ``supplyUserDetails(_:)``
/// - ``supplyUserDetails(_:isNewUser:)``
/// - ``removeUserDetails()``
///
/// ### Initializers for your Preview Provider
Expand Down Expand Up @@ -89,7 +89,7 @@ public class Account: ObservableObject, Sendable {
///
/// - Note: This array also contains ``IdentityProvider``s that need to be treated differently due to differing
/// ``AccountSetupViewStyle`` implementations (see ``IdentityProviderViewStyle``).
let registeredAccountServices: [any AccountService]
public let registeredAccountServices: [any AccountService]

/// Initialize a new `Account` object by providing all properties individually.
/// - Parameters:
Expand All @@ -112,8 +112,8 @@ public class Account: ObservableObject, Sendable {
logger.warning(
"""
Your AccountConfiguration doesn't have the \\.userId (aka. UserIdKey) configured. \
A primary and unique user identifier is expected with most SpeziAccount components and \
will result in those components breaking.
A primary, user-visible identifier is recommended with most SpeziAccount components for \
an optimal user experience. Ignore this warning if you know what you are doing.
"""
)
}
Expand Down Expand Up @@ -183,10 +183,30 @@ public class Account: ObservableObject, Sendable {
/// This method is called by the ``AccountService`` every time the state of the user account changes.
/// Either if the went from no logged in user to having a logged in user, or if the details of the user account changed.
///
/// - Parameter details: The ``AccountDetails`` of the currently logged in user account.
public func supplyUserDetails(_ details: AccountDetails) async throws {
/// - Parameters:
/// - details: The ``AccountDetails`` of the currently logged in user account.
/// - isNewUser: An optional flag that indicates if the provided account details are for a new user registration.
/// If this flag is set to `true`, the ``AccountSetup`` view will render a additional information sheet not only for
/// ``AccountKeyRequirement/required``, but also for ``AccountKeyRequirement/collected`` account values.
/// This is primarily helpful for identity providers. You might not want to set this flag
/// if you using the builtin ``SignupForm``!
public func supplyUserDetails(_ details: AccountDetails, isNewUser: Bool = false) async throws {
precondition(
details.contains(AccountIdKey.self),
"""
The provided `AccountDetails` do not have the \\.accountId (aka. AccountIdKey) set. \
A primary, unique and stable user identifier is expected with most SpeziAccount components and \
will result in those components breaking.
"""
)

var details = details

if isNewUser {
details.patchIsNewUser(true)
}


// Account details will always get built by the respective Account Service. Therefore, we need to patch it
// if they are wrapped into a StandardBacked one such that the `AccountDetails` carry the correct reference.
for service in registeredAccountServices {
Expand All @@ -206,7 +226,9 @@ public class Account: ObservableObject, Sendable {

if let standardBacked = details.accountService as? any StandardBacked,
let storageStandard = standardBacked.standard as? any AccountStorageStandard {
let recordId = AdditionalRecordId(serviceId: standardBacked.backedId, userId: details.userId)
let recordId = AdditionalRecordId(serviceId: standardBacked.backedId, accountId: details.accountId)

try await standardBacked.preUserDetailsSupply(recordId: recordId)

let unsupportedKeys = details.accountService.configuration
.unsupportedAccountKeys(basedOn: configuration)
Expand All @@ -232,7 +254,7 @@ public class Account: ObservableObject, Sendable {
if let details,
let standardBacked = details.accountService as? any StandardBacked,
let storageStandard = standardBacked.standard as? any AccountStorageStandard {
let recordId = AdditionalRecordId(serviceId: standardBacked.backedId, userId: details.userId)
let recordId = AdditionalRecordId(serviceId: standardBacked.backedId, accountId: details.accountId)

await storageStandard.clear(recordId)
}
Expand Down
43 changes: 13 additions & 30 deletions Sources/SpeziAccount/AccountConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,6 @@ public final class AccountConfiguration: Component, ObservableObjectProvider {
public func configure() {
// assemble the final array of account services
let accountServices = (providedAccountServices + self.accountServices).map { service in
// verify that the configuration matches what is expected by the account service
verifyAccountServiceRequirements(of: service)

// Verify account service can store all configured account keys.
// If applicable, wraps the service into an StandardBackedAccountService
let service = verifyConfigurationRequirements(against: service)
Expand All @@ -99,36 +96,22 @@ public final class AccountConfiguration: Component, ObservableObjectProvider {
self.account?.injectWeakAccount(into: standard)
}

private func verifyAccountServiceRequirements(of service: any AccountService) {
let requiredValues = service.configuration.requiredAccountKeys

// A collection of AccountKey.Type which aren't configured by the user or not configured to be required
// but the Account Service requires them.
let mismatchedKeys: [any AccountKeyWithDescription] = requiredValues.filter { keyWithDescription in
let key = keyWithDescription.key
let configuration = configuredAccountKeys[key]
return configuration == nil
|| (key.isRequired && configuration?.requirement != .required)
}

guard !mismatchedKeys.isEmpty else {
return
}

// Note: AccountKeyWithDescription has a nice `debugDescription` that pretty prints the KeyPath property name
preconditionFailure(
"""
You configured the AccountService \(service) which requires the following account values to be configured: \
\(mismatchedKeys.description).
Please modify your `AccountServiceConfiguration` to have these account values configured.
"""
)
}

private func verifyConfigurationRequirements(against service: any AccountService) -> any AccountService {
logger.debug("Checking \(service.description) against the configured account keys.")

// if account service states exact supported keys, AccountIdKey must be one of them
if case let .exactly(keys) = service.configuration.supportedAccountKeys {
precondition(
keys.contains(AccountIdKey.self),
"""
The account service \(type(of: service)) doesn't have the \\.accountId (aka. AccountIdKey) configured \
as an supported key. \
A primary, unique and stable user identifier is expected with most SpeziAccount components and \
will result in those components breaking.
"""
)
}

// collect all values that cannot be handled by the account service
let unmappedAccountKeys: [any AccountKeyConfiguration] = service.configuration
.unsupportedAccountKeys(basedOn: configuredAccountKeys)
Expand Down
61 changes: 53 additions & 8 deletions Sources/SpeziAccount/AccountHeader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import SpeziViews
import SwiftUI


/// A summary view for ``SpeziAccountOverview`` that can be used as a Button to link to ``SpeziAccountOverview``.
/// A account summary view that can be used to link to the ``AccountOverview``.
///
/// Below is a short code example on how to use the `AccountHeader` view.
///
Expand All @@ -37,7 +37,8 @@ public struct AccountHeader: View {
public enum Defaults {
/// Default caption.
@_documentation(visibility: internal)
public static let caption = LocalizedStringResource("ACCOUNT_HEADER_CAPTION", bundle: .atURL(from: .module)) // swiftlint:disable:this attributes
public static let caption = LocalizedStringResource("ACCOUNT_HEADER_CAPTION", bundle: .atURL(from: .module))
// swiftlint:disable:previous attributes
}

@EnvironmentObject private var account: Account
Expand All @@ -47,12 +48,23 @@ public struct AccountHeader: View {
let accountDetails = account.details

HStack {
UserProfileView(name: accountDetails?.name ?? PersonNameComponents(givenName: "Placeholder", familyName: "Placeholder"))
.frame(height: 60)
.redacted(reason: account.details == nil ? .placeholder : [])
.accessibilityHidden(true)
if let accountDetails,
let name = accountDetails.name {
UserProfileView(name: name)
.frame(height: 60)
.accessibilityHidden(true)
} else {
Image(systemName: "person.crop.circle.fill")
.resizable()
.frame(width: 60, height: 60)
.foregroundColor(Color(.systemGray3))
.accessibilityHidden(true)
}

VStack(alignment: .leading) {
Text(accountDetails?.name?.formatted() ?? "Placeholder")
let nameTitle = accountDetails?.name?.formatted(.name(style: .long)) ?? accountDetails?.userId ?? "Placeholder"

Text(nameTitle)
.font(.title2)
.fontWeight(.semibold)
.redacted(reason: account.details == nil ? .placeholder : [])
Expand Down Expand Up @@ -89,7 +101,7 @@ public struct AccountHeader: View {
let details = AccountDetails.Builder()
.set(\.userId, value: "[email protected]")
.set(\.name, value: PersonNameComponents(givenName: "Andreas", familyName: "Bauer"))

return NavigationStack {
Form {
Section {
Expand All @@ -103,4 +115,37 @@ public struct AccountHeader: View {
}
.environmentObject(Account(building: details, active: MockUserIdPasswordAccountService()))
}

#Preview {
let details = AccountDetails.Builder()
.set(\.userId, value: "[email protected]")

return NavigationStack {
Form {
Section {
NavigationLink {
AccountOverview()
} label: {
AccountHeader()
}
}
}
}
.environmentObject(Account(building: details, active: MockUserIdPasswordAccountService()))
}

#Preview {
NavigationStack {
Form {
Section {
NavigationLink {
AccountOverview()
} label: {
AccountHeader()
}
}
}
}
.environmentObject(Account(MockUserIdPasswordAccountService()))
}
#endif
4 changes: 2 additions & 2 deletions Sources/SpeziAccount/AccountOverview.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import SpeziViews
import SwiftUI


/// The essential ``SpeziAccount`` view to view and modify the active account details.
/// The essential `SpeziAccount` view to view and modify the active account details.
///
/// This provides an overview of the current account details. Further, it allows the user to modify their
/// account values.
Expand Down Expand Up @@ -52,7 +52,7 @@ import SwiftUI
/// }
/// ```
///
/// - Note: The ``init(isEditing:)`` initializer allows to pass an optional `Bool` Binding to retrieve the
/// - Note: The ``init(isEditing:additionalSections:)`` initializer allows to pass an optional `Bool` Binding to retrieve the
/// current edit mode of the view. This can be helpful to, e.g., render a custom `Close` Button if the
/// view is not editing when presenting the AccountOverview in a sheet.
public struct AccountOverview<AdditionalSections: View>: View {
Expand Down
7 changes: 4 additions & 3 deletions Sources/SpeziAccount/AccountService/AccountService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ import SwiftUI
/// You may improve the user experience or rely on user interface defaults if you adopt protocols like
/// ``EmbeddableAccountService`` or ``UserIdPasswordAccountService``.
///
/// - Note: `SpeziAccount` provides the generalized ``UserIdKey`` unique user identifier that can be customized
/// using the ``UserIdConfiguration``.
/// - Important: Every user account is expected to have a primary and unique user identifier.
/// SpeziAccount requires a stable and internal ``AccountIdKey`` unique user identifier and offers
/// a user visible ``UserIdKey`` which can be customized using the ``UserIdConfiguration``.
///
/// You can learn more about creating an account service at: <doc:Creating-your-own-Account-Service>.
///
Expand Down Expand Up @@ -52,7 +53,7 @@ public protocol AccountService: AnyObject, Hashable, CustomStringConvertible, Se

/// Create a new user account for the provided ``SignupDetails``.
///
/// - Note: You must call ``Account/supplyUserDetails(_:)`` eventually once the user context was established after this call.
/// - Note: You must call ``Account/supplyUserDetails(_:isNewUser:)`` eventually once the user context was established after this call.
/// - Parameter signupDetails: The signup details
/// - Throws: Throw an `Error` type conforming to `LocalizedError` if the signup operation was unsuccessful,
/// inorder to present a localized description to the user.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,16 @@
import Spezi


/// The collection of ``AccountKey``s that are required to use the associated ``AccountService``.
/// The collection of ``AccountKey``s that are required when using the associated ``AccountService``.
///
/// A ``AccountService`` may set this configuration to communicate that a certain set of ``AccountKey``s are
/// required to be configured in the ``AccountValueConfiguration`` provided in the ``AccountConfiguration`` in
/// order to user the account service.
///
/// Upon startup, `SpeziAccount` automatically verifies that the user-configured account values match the expectation
/// set by the ``AccountService`` through this configuration option.
/// required to use the given account service. For example, a password-based account service defines the password
/// to be required using this configuration as the account value is only required in the context of using this specific
/// account service.
///
/// Access the configuration via the ``AccountServiceConfiguration/requiredAccountKeys``.
///
/// Below is an example on how to provide this option.
/// Below is an example configuration for a userid-password-based account service.
///
/// ```swift
/// let configuration = AccountServiceConfiguration(/* ... */) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
///
/// Access the configuration via the ``AccountServiceConfiguration/supportedAccountKeys``.
///
/// Belo is an example on how to provide a fixed set of supported account keys.
/// Below is an example on how to provide a fixed set of supported account keys.
///
/// ```swift
/// let supportedKeys = AccountKeyCollection {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ protocol StandardBacked: AccountService {
init(service: Service, standard: AccountStandard)

func isBacking(service accountService: any AccountService) -> Bool

func preUserDetailsSupply(recordId: AdditionalRecordId) async throws
}


Expand All @@ -39,6 +41,8 @@ extension StandardBacked {
}
return self.accountService.objId == service.objId
}

func preUserDetailsSupply(recordId: AdditionalRecordId) async throws {}
}


Expand Down
Loading

0 comments on commit 2f88e04

Please sign in to comment.