Skip to content
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

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

Open
pmmlo opened this issue Sep 15, 2022 · 6 comments
Open
Labels
type:feature New feature or improvement of existing feature

Comments

@pmmlo
Copy link
Contributor

pmmlo commented Sep 15, 2022

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
Copy link

Thanks for opening this issue!

  • 🎉 We are excited about your ideas for improvement!

@pmmlo
Copy link
Contributor Author

pmmlo commented Sep 15, 2022

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

@cbaker6 cbaker6 added the type:feature New feature or improvement of existing feature label Sep 15, 2022
@cbaker6
Copy link
Contributor

cbaker6 commented Sep 15, 2022

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?

init(service: String? = nil) {
var keychainService = ".parseSwift.sdk"
if let service = service {
keychainService = service
} else if let identifier = Bundle.main.bundleIdentifier {
keychainService = "\(identifier)\(keychainService)"
} else {
keychainService = "com\(keychainService)"
}
self.service = keychainService

@pmmlo
Copy link
Contributor Author

pmmlo commented Sep 15, 2022

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
Copy link
Contributor

cbaker6 commented Sep 16, 2022

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
Copy link
Contributor Author

pmmlo commented Sep 16, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests

2 participants