realm-tasks icon indicating copy to clipboard operation
realm-tasks copied to clipboard

Authentication via CloudKit

Open TimOliver opened this issue 9 years ago • 21 comments

This PR adds login via CloudKit as an alternative to the username / password login method of the RealmTasks app. It is still a WIP as it has not been properly tested with a local copy of the authentication server yet.

It works as follows:

  1. The app has been configured with CloudKit entitlements, and an App ID has been registered for it on Realm's iTunes Developer Account (I double-checked to make sure it's possible to later delete this ID after we're done!).
  2. A button labeled 'Connect with iCloud' has been placed on the Login View Controller.
  3. Tapping that button will send a request to iCloud, and will return the user's record ID. This is a unique string guaranteed to be the same for a user's iCloud account on each of their devices. Mine looked like: _950637393581ea5577efb01fce48fcef.
  4. Upon successfully receiving the token, it then uses the same logic as the username / password logic in that it is loaded into an HTTP request and sent to the Realm authentication server (But this will need to change when the Realm Sync Cocoa Binding API is fully decided)

A couple of things I'm still researching:

  1. iOS and Mac apps are treated as discrete apps, so I'm still trying to work out how you can guarantee the CloudKit token would be the same on both RealmTasks macOS and iOS.
  2. Once the authentication server receives the key, what steps should be taken to verify that it is a valid key on the server's end. I'm still looking through CloudKit's REST API to see what endpoints are available to external servers for this.
  3. We probably need to be very careful how we advertise this authentication method publicly. We need to make sure it is obvious that the extent of how much we're using iCloud is to only get a unique identity representing the user, and that everything else is managed by Realm (Obviously we need to be careful about Apple branding and the iCloud logo too)

TimOliver avatar Jul 27 '16 09:07 TimOliver

Great start on this @TimOliver! Those first two things you're still researching are super important, so keep us posted!

jpsim avatar Jul 27 '16 19:07 jpsim

A button labeled 'Connect to iCloud' has been placed on the Login View Controller.

Another likely scenario is that the login happens automatically if the iCloud is available. No need for a button in that case.

astigsen avatar Jul 27 '16 21:07 astigsen

Another likely scenario is that the login happens automatically if the iCloud is available. No need for a button in that case.

In this case, since we're actively working on auth strategies and incorporating many of them in this demo app, it makes sense to be explicit about which auth approach you want while this is in development. Obviously we'll want to support that promptless use case in general too.

jpsim avatar Jul 27 '16 21:07 jpsim

Thanks a lot @jpsim! @mrackwitz has offered to help look into the server-side aspect of authentication as well. We'll keep at it!

Another likely scenario is that the login happens automatically if the iCloud is available. No need for a button in that case.

@astigsen Yep. In a production app, that's how I'd want it to work. The whole experience would ideally be completely seamless and automatic to the user. (Assuming they're actively logged into an iCloud account on the device. If they're not, we'll need to figure out how to account for this.)

Yeah, obviously a setting for it would need to be exposed for it somewhere. Clear itself simply has a single button that says 'iCloud' in its Settings menu:

fullsizerender

But as I was saying above, I'd be very wary of how we'd word a similar setting. We don't want to give users the impression their data is actually being stored in iCloud.

TimOliver avatar Jul 28 '16 02:07 TimOliver

I think the idea is that you just attempt to retrieve the iCloud ID and use that to identify the user without blocking them to use the app. We are building an "anonymous" login which lets you start opening sync'd realms immediately but you could then add the iCloud login to that Realm user so that they don't have to do anything to see data on two devices.

bigfish24 avatar Jul 28 '16 02:07 bigfish24

So nothing actually needs to be visible to the user it's a transparent login since they don't have to enter anything.

bigfish24 avatar Jul 28 '16 02:07 bigfish24

I get an "Unknown provider" error when running this against the realm-sync-services server at version realm-sync-0.26.2-2. I suppose I need realm/realm-sync-services#47 to test this out?

jpsim avatar Jul 29 '16 00:07 jpsim

Alright. I think this is pretty much done now. It works as follows:

  1. When Connect to iCloud is tapped, it first queries CloudKit to see if a Realm User ID was already saved to the user's private database.
  2. If the CloudKit query returns a 'record not found' error, then it assumes this is a new user registering for the first time.
  3. It requests the user record ID from CloudKit (A string unique to each iCloud user in a given CloudKit container), and then submits that to Realm Sync Services.
  4. Realm Sync Services' role is to use server-to-server communication to verify that the record ID string is indeed valid, and not an arbitrarily made up string.
  5. Upon confirmation, Realm Sync Services returns an access token in its response to the app.
  6. The app then takes this token, and saves it to the user's private database in their iCloud account.
  7. The next time the app is opened, the access token is queried from the private database.

A couple of points I realised:

  1. I originally tried writing the access token to a custom CloudKit zone in the hopes it would minimise visibility to the user while they're working in CloudKit, but it turns out that's not really necessary since the name of the model object already segregates it sufficiently.
  2. Pursuant to our discussion today on how we're no longer calling this product 'Realm Sync', I'm referring to the object model that stores the access token as a RealmServer object, and the primary key used to identify the object itself as io.realm.server.defaultuser. We may need to change this.
  3. I'm curious to see what would happen if the app is opened for the first time, on two separate devices simultaneously. Obviously one would win out, and hopefully it would simply be a matter of the second one identifying the first one already on record. Realm Sync Services is going to need some testing for these sorts of scenarios.
  4. Apparently users can go into the Settings app on an iOS device and completely blow away any CloudKit records an app might have. I'm wondering if there's anything we need to do to account for this, but that can obviously come later.
  5. At the moment, Realm Tasks itself is saving a local copy of the access token to disk. In a third party app, I'm wondering if that's something we should have as a feature of the binding (i.e., it keeps a copy of the token in the key chain which can be used in the interim while the app it still starting up), or make that a developer decision.

TimOliver avatar Aug 04 '16 13:08 TimOliver

If the CloudKit query returns a 'record not found' error, then it assumes this is a new user registering for the first time.

Is it safe to assume this? Is it possible that CloudKit hasn't finished syncing and you wouldn't see the record? What about if two devices attempt to "register" at the same time?

jpsim avatar Aug 04 '16 18:08 jpsim

I am confused why we have to save anything to iCloud. Didn't we confirm that you can receive a unique user ID from iCloud, which is all that we are after?

bigfish24 avatar Aug 04 '16 22:08 bigfish24

@jpsim Yeah I mentioned that possibility as point 3 there. At the moment, the Sync Services code will search for an existing Realm tied against the user record ID itself (Which I'm not crazy about since it probably means we're still saving a copy of the record on our servers), but at the very least, that will mean that two separate registration actions will still resolve to the same user account the server.

I did some more research, but it doesn't look like there's any way to 'lock' a write transaction across multiple devices. If a second write transaction occurs, it will overwrite the original value with the new value.

I'll think about this some more. I might have a solution.

@bigfish24 We continued the discussion here: https://github.com/realm/realm-sync-services/issues/44#issuecomment-236886341

To continue my emphasis, I want to minimize our reliance on using the CloudKit user record as the single source of truth for authenticating a user. It'd be like treating a single string as both the username and password, which can also never be revoked by us. It needs to be used in conjunction with some other piece of data that we can explicitly invalidate if we need to. And to ensure that data is available to all of the users' devices, it needs to be stored in CloudKit.

TimOliver avatar Aug 05 '16 07:08 TimOliver

I did some more research, but it doesn't look like there's any way to 'lock' a write transaction across multiple devices. If a second write transaction occurs, it will overwrite the original value with the new value.

There seems to be a way for that. If you store the retrieved user ID / access token on the user record and modify that via a CKModifyRecordsOperation with the savePolicy to CKRecordSaveIfServerRecordUnchanged:

A policy that saves the record only if the local copy of the record is based on the record still on the server. The server maintains a change tag for each record automatically. When you fetch a record, that change tag is included with the rest of the record’s data. If the change tag in your local record matches the change tag of the record on the server, the save operation proceeds normally. If the server record contains a newer change tag, the record is not saved and an error of type CKErrorServerRecordChanged is reported.

mrackwitz avatar Aug 05 '16 10:08 mrackwitz

Still don't get why we need to save anything? Since the CloudKit user ID identifies the user that should be passed to realm Auth to exchange for a Realm user ID. Am I missing something?

bigfish24 avatar Aug 05 '16 12:08 bigfish24

@mrackwitz Cool! Thanks for that Marius! I tested it out, and it returns an NSError citing a partial record write failure. I've added it in now!

@bigfish24 We're doing that. But then we're saving the Realm user ID to CloudKit so we can detect if the user logs out of their iCloud account (In which case the ID becomes inaccessible).

The alternative methods would be to save the Realm user ID locally on the device, but then we'd need to manually check that the user doesn't log out or change iCloud accounts after the fact (Which would probably mean saving a local copy of the user record ID as well so we can compare the two).

I'm definitely all for a simpler method. Please follow up the conversation here: https://github.com/realm/realm-sync-services/issues/44

TimOliver avatar Aug 05 '16 17:08 TimOliver

See my comment. Delegating control to iCloud is an application decision and as such could be something we want for RealmTasks (though I don't really see why), but it should not be the default for our support for iCloud login.

bigfish24 avatar Aug 05 '16 17:08 bigfish24

Okay I'm trying to get this one done and merged in now. It's been a while since this one was created so I've given it a bit of an overhaul.

  1. Merged in the latest copy of master.
  2. Separated Password and CloudKit auth options as separate Xcode schemes.
  3. Created a separate auth view controller for CloudKit to display and display error messages if needed.
  4. Removed the manual server logic we had and added in the new Realm Cocoa APIs we have now.

The main thing I want to get feedback on is is using multiple Xcode schemes a good way for custom defining the build settings so we can produce two different builds of the app, one with passwords and one with CloudKit integration?

TimOliver avatar Sep 08 '16 18:09 TimOliver

I can't provide feedback at the moment. @stel can you review this please?

jpsim avatar Sep 08 '16 23:09 jpsim

I have just a few suggestions:

  1. Looks like RealmSyncLoginManager.swift was added by mistake
  2. Text in label is truncated screen shot 2016-09-09 at 23 00 26
  3. There is a space added back to the app name: "Realm Tasks" (but actually I like it more :))
  4. Some extra spaces are added in LogInViewController.swift

And one more thing for Xcode scheme. I think it's better to make separate schemes for different targets because now you need to change target in scheme settings instead of just changing a scheme itself. But why not to include everything in one target and one view controller as it was at the beginning of this PR?

stel avatar Sep 09 '16 21:09 stel

Any updates? @TimOliver would it be possible to merge this? It would be very helpful to have this in the demo app...

ivnsch avatar Feb 06 '17 13:02 ivnsch

@i-schuetz Actually, I've started to try and pull away of all of the custom logic in Realm Tasks in favor of making it more reusable in a generic level library. I'll make a note to add iCloud integration there and then bring it in here.

TimOliver avatar Feb 06 '17 14:02 TimOliver

@TimOliver awesome, thanks!

ivnsch avatar Feb 06 '17 14:02 ivnsch