parse-community / Parse-Swift

The Swift SDK for Parse Platform (iOS, macOS, watchOS, tvOS, Linux, Android, Windows)
https://parseplatform.org
MIT License
308 stars 69 forks source link

setAccessGroup does not allow the keychain name (the service name) to be set. #415

Open pmmlo opened 2 years ago

pmmlo commented 2 years ago

New Feature / Enhancement Checklist

Current Limitation

Currently, setAccessGroup() enables *.parseswift.sdk services to be manually accessed through keychain sharing, but if someone wants to login to multiple apps with a single Parse login, it is not possible.

Feature / Enhancement Description

If devs want to enable login to multiple apps with a single Parse login, we need to be able to set the service name, not automatically to "*.parseswift.sdk"

It could be something like: setAccessGroup

@discardableResult static public func setAccessGroup(_ accessGroup: String?, service: String?,
                                                         synchronizeAcrossDevices: Bool) throws -> Bool /// Setting kSecAttrService as service.

// Call the method
do {
  try ParseSwift.setAccessGroup("name.of.access.group", service: "com.pmmlo.foo", synchronizeAcrossDevices: false)
} catch {
  // Handle error
}

Importantly, it cannot be a simple duplication of the accessGroup string, because in some cases, the appPrefix must be added to the accessGroup, while it may not be desired in the service name.

Example Use Case

Alternatives / Workarounds

There is currently no way that I see to allow login to multiple apps with Parse-Swift by logging in to one of the family of apps.

3rd Party References

Not from an open-source or SDK-side. From a consumer product, there are a number of enterprise suite apps that implement this in slightly different ways, usually with a proprietary, server-side client verification. An example off the top of my head is iCloud, I believe logs you in to mail, calendar, and other apps.

parse-github-assistant[bot] commented 2 years ago

Thanks for opening this issue!

pmmlo commented 2 years ago

I'm afk for a bit, but will try to show what I mean with a PR later.

cbaker6 commented 2 years ago

I believe .parseSwift.sdk should be able to stay the same to distinguish it as a the Swift SDK Keychain. It seems like you would like something to specify the bundle id? https://github.com/parse-community/Parse-Swift/blob/085f5ad30d26c43251d82a3d0f412bfb9be3b473/Sources/ParseSwift/Storage/KeychainStore.swift#L38-L47

pmmlo commented 2 years ago

True, but I think it would still need another condition through the configurations, setAccessGroups() or somewhere else to avoid replacing.

But, replacing may be ok as long as it is done during initialization.

Also, if we want to enforce the suffix ".parseSwift.sdk" (not personally opposed), then we would want additional logic in the init().

/// e.g. Were you thinking something like this?  This may still need some additional logic in .set()
let keychain = KeychainStore(service: "com.example.shared.parseSwift.sdk")
try keychain.copy(KeychainStore.shared,
                               oldAccessGroup: ParseSwift.configuration.keychainAccessGroup,
                               newAccessGroup: ParseSwift.configuration.keychainAccessGroup)

Is this what you had in mind?

cbaker6 commented 2 years ago

The current init for the Keychain makes all instances have the correct ending if they are suppose to. Code that creates instances by passing in the service and doesn’t end in parseSwift.sdk are situations that need to connect to the Obj-C Keychain

pmmlo commented 2 years ago

Yeah, I saw the legacy shared and obj-c. I understandably couldn't instantiate KeychainStore() from the app, so either the struct/init need to be public or there needs to be a setter. People may not be very interested in this feature, so maybe not worth hashing this out.