Parse-Swift icon indicating copy to clipboard operation
Parse-Swift copied to clipboard

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

Open pmmlo opened this issue 1 year ago • 6 comments

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.

pmmlo avatar Sep 15 '22 17:09 pmmlo

Thanks for opening this issue!

  • 🎉 We are excited about your ideas for improvement!

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

pmmlo avatar Sep 15 '22 17:09 pmmlo

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

cbaker6 avatar Sep 15 '22 21:09 cbaker6

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?

pmmlo avatar Sep 15 '22 21:09 pmmlo

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

cbaker6 avatar Sep 16 '22 04:09 cbaker6

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.

pmmlo avatar Sep 16 '22 07:09 pmmlo