parse-community / Parse-Swift

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

Sign In with Apple + custom parameters #135

Closed lsmilek1 closed 3 years ago

lsmilek1 commented 3 years ago

Hello,

I already had implemented Apple Sign-In and had this working. Down the way I wanted to extend the User class with some custom fields and some I set as required in the Parse Dashboard. But the following code is now not working even the required fields are defined at struct init:

func authorizationController(controller: ASAuthorizationController, didCompleteWithAuthorization authorization: ASAuthorization) {
        if let appleIDCredential = authorization.credential as? ASAuthorizationAppleIDCredential {
            guard let token = appleIDCredential.identityToken else {
                fatalError("identity token is nil")
            }
            var newUser = PrsUser()
            newUser.username = appleIDCredential.email
            newUser.email = appleIDCredential.email
            var acl = ParseACL()
            acl.publicRead = false
            acl.publicWrite = false
            acl.setReadAccess(user: newUser, value: true)
            acl.setWriteAccess(user: newUser, value: true)
            newUser.ACL = acl
            print("newUser: \(newUser)")
            newUser.apple.login(user: appleIDCredential.user, identityToken: token) { result in
                switch result {
                case .success(let user):
                    print("login response: \(user)")
                    if self.userController?.currentProfile.gender == Gender.notDefined.rawValue {
                        self.showGenderSelection(animated: false, newPrsUser: user)
                    } else {
                        self.showSyncProgress(animated: false, syncUser: user)
                    }
                case .failure(let error):
                    let errorText = "Error \(error.code.rawValue) - \(error.message)"
                    UIAlerts.alertWith(message: errorText, viewController: self)
                    print("Error logging in: \(error.localizedDescription)")
                }
            }
        }
    }

the print right before apple.login() shows following:

newUser: _User ({"maxA":30,"minA":20,"maxD":20,"ACL":{}})

but the login results in error:

Error logging in: ParseError code=142 error=minA is required

When I set the fields as not required in the parse dashboard, the login proceed without error. It is not a big deal for me now, as I can save the fields in the second call. But it confused me quite a lot because I use the same logic also for sign-up with email and there it is throwing now error even when the same fields are initialised the same:

func signup() {
        guard let email = emailTextField.text, let password = passwordTextField.text else { return }
        var newUser = PrsUser()
        newUser.email = email
        newUser.password = password
        newUser.username = email
        var acl = ParseACL()
        acl.publicRead = false
        acl.publicWrite = false
        acl.setReadAccess(user: newUser, value: true)
        acl.setWriteAccess(user: newUser, value: true)
        newUser.ACL = acl
        print("newUser: \(newUser)")
        newUser.signup { result in
            switch result {
            case .success(let user):
                ...
            case .failure(let error):
                ...
            }
        }
    }

Did I understood the logic behind apple login wrong or might this indeed be a bug?

Thank you!

cbaker6 commented 3 years ago

When I set the fields as not required in the parse dashboard, the login proceed without error.

Does is still show 20 for minA in the Parse Dashboard when you set the fields to "not required"? If so, this may be something with the Dashboard or the server. If you see 20 in the Dashboard that means the Swift client sent it correctly.

cbaker6 commented 3 years ago

Also, your sever error code is giving you a hint that this is server side. Maybe you have cloud code interfering:

https://github.com/parse-community/Parse-Swift/blob/a7a6f40d022005a555a63982b26fa6c2ed1587ac/Sources/ParseSwift/Types/ParseError.swift#L199-L202

lsmilek1 commented 3 years ago

Hi Corey,

if I comment out the code after login, the values are not being set as the printed login response on success also indicates:

if let appleIDCredential = authorization.credential as? ASAuthorizationAppleIDCredential {
            guard let token = appleIDCredential.identityToken else {
                fatalError("identity token is nil")
            }
            var newUser = PrsUser()
            newUser.username = appleIDCredential.email
            newUser.email = appleIDCredential.email
            var acl = ParseACL()
            acl.publicRead = false
            acl.publicWrite = false
            acl.setReadAccess(user: newUser, value: true)
            acl.setWriteAccess(user: newUser, value: true)
            newUser.ACL = acl
            print("newUser: \(newUser)")
            newUser.apple.login(user: appleIDCredential.user, identityToken: token) { result in
                switch result {
                case .success(let user):
                    print("login response: \(user)")
//                    if self.userController?.currentProfile.gender == Gender.notDefined.rawValue {
//                        self.showGenderSelection(animated: false, newPrsUser: user)
//                    } else {
//                        self.showSyncProgress(animated: false, syncUser: user)
//                    }
                case .failure(let error):
                    let errorText = "Error \(error.code.rawValue) - \(error.message)"
                    UIAlerts.alertWith(message: errorText, viewController: self)
                    print("Error logging in: \(error.localizedDescription)")
                }
            }
        }
    }

in the print before login I can see the fields being set being set for newUser:

newUser: _User ({"maxA":30,"minA":20,"maxD":20,"ACL":{}})

but here is login response (and that is how it then looks in the dashboard):

login response: _User ({"username":"okJ...Uo8","objectId":"BShb76JT9q","authData":{"apple":{"token":"eyJ...c6Q","id":"001...47"}},"createdAt":{"__type":"Date","iso":"2021-04-30T06:37:12.704Z"}})

In the cloud code I have currently only afterDelete trigger on User class to delete all related sessions.

Similarly when I comment out the after sign-up code in the username/password option, these fields are being set and I can see them in Dashboard:

func signup() {
        guard let email = emailTextField.text, let password = passwordTextField.text else { return }
        var newUser = PrsUser()
        newUser.email = email
        newUser.password = password
        newUser.username = email
        var acl = ParseACL()
        acl.publicRead = false
        acl.publicWrite = false
        acl.setReadAccess(user: newUser, value: true)
        acl.setWriteAccess(user: newUser, value: true)
        newUser.ACL = acl
        print("newUser: \(newUser)")
        newUser.signup { result in
            switch result {
            case .success(let user):
                print("after sign-up: \(user)")
//                if let parent = self.navigationController?.previousViewController as? OnboardingMainViewController {
//                    if self.userController?.currentProfile.gender == Gender.notDefined.rawValue {
//                        parent.showGenderSelection(animated: false, newPrsUser: user)
//                    } else {
//                        parent.showSyncProgress(animated: false, syncUser: user)
//                    }
//                }
//                self.navigationController?.popViewController(animated: true)
            case .failure(let error):
                if error.code == ParseError.Code.usernameTaken {
                    self.showError(type: .emailTaken)
                    self.setSegmentedControl(to: 1)
                } else {
                    let errorText = "Error \(error.code.rawValue) - \(error.message)"
                    self.showError(type: .server, text: errorText)
                    print("Error sign-up in: \(error.localizedDescription)")
                }
            }
        }
    }

For me is a bit confusing that there is no distinction between login and sign-up in the apple case. When there is a login first time, the ACL are also being set differently, it ignores acl.publicRead = false but set acl.setReadAccess(user: newUser, value: true) and acl.setWriteAccess(user: newUser, value: true) (you can see that comparing above and bellow login response: _User)

On subsequent logins with the same appleID I get in the response data that were in the User document previously saved. Here I tried to manually type maxA = 40 in Dashboard before second login under that appleID:

login response: _User ({"authData":{"apple":{"token":"eyJ...FSg","id":"00...247"}},"ACL":{"BShb76JT9q":{"write":true,"read":true},"*":{"read":true}},"username":"okJ...Uo8","objectId":"BShb76JT9q","updatedAt":{"type":"Date","iso":"2021-04-30T06:45:17.357Z"},"createdAt":{"type":"Date","iso":"2021-04-30T06:37:12.704Z"},"maxA":40})

this is indeed good, because subsequent apple signIn should download existing data instead of rewriting them with fresh new/empty. But I am not sure if the first login attempt behaves as it should. Is there any logic that I miss to distinct between first sign-up and following log-ins?

here is first and second sign-in response:

login response: _User ({ "username":"TTL…8k", "objectId":"eqspyeRmUw", "authData":{"apple":{"id":"001301…1247", "token":"eyJraW…gAozoQ"}}, "createdAt":{"__type":"Date","iso":"2021-04-30T07:03:21.423Z"}})

login response: _User ({ "ACL":{"*":{"read":true},"eqspyeRmUw":{"write":true,"read":true}}, "username":"TTL8SlAzD19eC7v890kPBcS8k", "objectId":"eqspyeRmUw", "updatedAt":{"type":"Date","iso":"2021-04-30T07:03:21.423Z"}, "authData":{"apple":{"id":"001301…1247", "token":"eyJra…Uq1Jg"}}, "createdAt":{"type":"Date","iso":"2021-04-30T07:03:21.423Z"}})

I could theoretically rely on that in the first response (new sign-up) there is no updatedAt field. The problem is that the updatedAt is exactly same as created on the second login also and I expect that the response might change in future where there would be also updatedAt returned upon first sign-up. So this check does not feel bullet-proof. I have to do a second call due to ACL anyway so the updatedAt will change. But the second call is also a workaround on something that seems to be a bug.

Thank you

cbaker6 commented 3 years ago

The SDK currently doesn't support modifying the user before being logged in using apple login or any other authentication type. If you want to do what you mentioned, you need to 1) login the user first 2) then make any necessary changes to the modified or current user 3) save the updated user.

In your case:

PrsUser.apple.login(user: appleIDCredential.user, identityToken: token) { result in
                switch result {
                case .success(var user):
                    print("login response: \(user)")
                    user.username = appleIDCredential.email
                    user.email = appleIDCredential.email
                   var acl = ParseACL()
                   acl.publicRead = false
                   acl.publicWrite = false
                   acl.setReadAccess(user: user, value: true) //These look like typical user ACL, you probably don't need to set them to can use the default ACL or CLP in the dashboard
                   acl.setWriteAccess(user: user, value: true)
                   user.ACL = acl
                   user.maxA = 30 //Note that if you have these as default values of PrsUser, you don't need to set here. I couldn't tell from your code
                   user.minA = 20
                   user.maxD = 20
                   user.save { result in ... }
                case .failure(let error):
                    let errorText = "Error \(error.code.rawValue) - \(error.message)"
                    UIAlerts.alertWith(message: errorText, viewController: self)
                    print("Error logging in: \(error.localizedDescription)")
                }
            }
cbaker6 commented 3 years ago

For me is a bit confusing that there is no distinction between login and sign-up in the apple case.

It’s not possible for a developer to determine the difference between login and signup. For authentication login purposes, I don’t see why someone would need to have a login and signup method. One should suffice. Apple discuses some of this in their documentation:

If the user is signed in at the system-level with their Apple ID, the sheet appears describing the Sign in with Apple feature, followed by another sheet allowing the user to edit the information in their account. The user can edit their first and last name, choose another email address as their contact information, and hide their email address from the app. If the user chooses to hide their email address from the app, Apple generates a proxy email address to forward email to the user’s private email address. Lastly, the user enters the password for the Apple ID, then clicks Continue to create the account.

You may also want to look at Apples documentation further as I don’t believe they keep giving you the full name and email (if the user chooses to provide it) if the user has already signed into your app before. This means in your current setup, you couldn’t be overwriting the username and email with nil.

lsmilek1 commented 3 years ago

Thank you for pointing out important mistakes in my code, I will adjust the code accordingly. That explains why I see such behaviour and understand that all custom user fields has to be "not required" in the database then, otherwise there would be the previously mentioned error popping up.

From the UI/UX point I still need to distinguish if it is new sign-up, or login for the client app to perform either sync "up" or "down" (from/to server) and adjusting UI flow to that. I have read the apple documentation and understood that the user info is passed only on first sign-in (although the email is always there in JWT, might just be fake one), perhaps that could be additional check to confirm that it is a first sign-up. I was hoping I would get a clear information about that from parse server login response.

Nevertheless I believe I just understood the logic wrong so feel free to close this issue. Setting the data during the login is nice to have, but then also it is necessary to know if it is login or signup before setting anything.

Thank you for clarification!

lsmilek1 commented 3 years ago

This means in your current setup, you couldn’t be overwriting the username and email with nil.

From your comment above and what I tested it seems to me that there is not "update these fields" method and when I save, it overrides all fields with nil, if I don't define them. That means that I have to make sure than client side has up-to-date data or fetch the object first before update? iOS docs say:

The client automatically figures out which data has changed so only “dirty” fields will be sent to Parse. You don’t need to worry about squashing data that you didn’t intend to update.

Does the ParseSwift behaves the same?

cbaker6 commented 3 years ago

From your comment above and what I tested it seems to me that there is not "update these fields" method and when I save, it overrides all fields with nil, if I don't define them. That means that I have to make sure than client side has up-to-date data or fetch the object first before update?

You can “fetch” after login and then update the fields on the client if they are not nil from Apple.

Does the ParseSwift behaves the same?

No, the Swift client has no notion of “dirty” as that’s used with “classes” and willChange/didChange property observers. All fields in a struct can be optional, so if you only want to send a ParseObject with dirty fields, create a new instance containing only the dirty fields.

lsmilek1 commented 3 years ago

if you only want to send a ParseObject with dirty fields, create a new instance containing only the dirty fields.

I see. I actually tried that first, but on the ParseUser object and there it seems to not work, throwing errors like "username missing", "password required"... Even I try this trick in the ParseUser, it miss the password (currentUser is not nil, it is logged in):

init(user: User, syncData: [UserFields]) {
        let currentUser = PrsUser.current
        self.objectId = currentUser?.objectId
        self.username = currentUser?.username
        self.password = currentUser?.password
        for field in syncData {
            set(field: field, from: user)
        }
}

I guess password is not stored on the client and in case of ParseUser object I have to send always the whole document? I am trying to understand it because I am trying to support multi-device client app.

Also when I init ParseObject and specify only certain fields, the rest is getting nilled in the database. Example:

init(profile: Profile, syncData: [ProfileFields]) {
        self.objectId = profile.objectId
        for field in syncData {
            set(field: field, from: profile)
        }        
    }

mutating func set(field: ProfileFields, from profile: Profile) {
        let discoverable = profile.name.count > 2 && profile.status == ProfileStatus.published.rawValue

        switch field {
        case .age: self.ag = profile.age
        //... <-other fields

        case .all:
            for field in ProfileFields.allCases {
                if field != .all {
                    set(field: field, from: profile)
                }
            }
        }
    }

}

enum ProfileFields: CaseIterable {
    case all

    case age
    //... <-other fields
}

Or did I understood wrong what you mean by the new instance?

cbaker6 commented 3 years ago

The password is definitely not stored on the client.

There may be some differences with ParseUser. You can also use operations when possible to only modify specific data and not sent your whole object. https://github.com/parse-community/Parse-Swift/blob/a7a6f40d022005a555a63982b26fa6c2ed1587ac/ParseSwift.playground/Pages/13%20-%20Operations.xcplaygroundpage/Contents.swift#L47-L58

cbaker6 commented 3 years ago

I guess password is not stored on the client and in case of ParseUser object I have to send always the whole document? I am trying to understand it because I am trying to support multi-device client app.

I'm not sure what you mean here. Multiple devices should be able to log into the same account (the server allows this). Also, since you have the sessionToken, you can always login with become http://parseplatform.org/Parse-Swift/api/Protocols/ParseUser.html#/s:10ParseSwift0A4UserPAAE6become12sessionToken7options13callbackQueue10completionySS_ShyAA3APIV6OptionOGSo17OS_dispatch_queueCys6ResultOyxAA0A5ErrorVGctF

Also when I init ParseObject and specify only certain fields, the rest is getting nilled in the database.

Depending on your usecase, it might be easier to make a mutable copy of the whole ParseObject and then mutate whatever fields needed, and then save or your can use the "operations" method I mentioned earlier.

lsmilek1 commented 3 years ago

By multi-device I mean that I want to use only dirty keys to send updates "up" and in the save response I would get the most actual state of the object, that I could store in the device/client. This way I would not override any field that was updated shortly before the save.

I am really struggling to understand how to update the object. I am able to save a new object with custom objectId no matter what other fields I set with following code:

    ```
    let prsProfile = PrsProfile(profile: profile, syncData: syncData)
    print(prsProfile)
    prsProfile.save { result in
        switch result {
        case .success(let profile):
            print("profile saved: \(String(describing: profile))")
        case .failure(let error):
            print("Error saving: \(error)")
        }
    }
    ```

but when I use the same logic for update, I get following error:

Error saving: ParseError(code: ParseSwift.ParseError.Code.duplicateValue, message: "A duplicate value for a field with unique values was provided")

Even when I try something simple as:

    ```
    var prsProfile = PrsProfile(objectId: profile.objectId)
    prsProfile.ag = 55
    prsProfile.save { result in .... }
    ```

in both server custom settings and parseSwift initialiser in AppDelegate I have allowCustomObjectId = true and I tried to take out all cloud functions also.

I could not find any example in ParseSwift.playgrounds. Could this be a bug?

cbaker6 commented 3 years ago

By multi-device I mean that I want to use only dirty keys to send updates "up" and in the save response I would get the most actual state of the object, that I could store in the device/client. This way I would not override any field that was updated shortly before the save.

This doesn't sound like a good way to sync data IMO. What happens if the previous value of a field you are attempting to update doesn't match the server? That's a conflict as your data isn't in sync to begin with. It seems like multiple things can go wrong with your method. Having the "dirty" key method will still cause issues for you. Syncing data correctly is a whole different problem. You should look at the recommendations mentioned in https://community.parseplatform.org/t/syncing-a-local-store/1455

I could not find any example in ParseSwift.playgrounds. Could this be a bug?

The playgrounds currently don't use a custom objectId so no examples there.

This seems like a separate issue and I recommend opening another issue. I currently don't use the custom objectId in my own apps, but I tested the save/update capability when I initially added it. If you want to create a PR and add a playground page testing custom objectId, I can take a look to see what the issue is. The testcases are also here: https://github.com/parse-community/Parse-Swift/blob/a7a6f40d022005a555a63982b26fa6c2ed1587ac/Tests/ParseSwiftTests/ParseObjectCustomObjectIdTests.swift#L545-L574 you should be able to test those against a local server

cbaker6 commented 3 years ago

I've added the playground example. It seems to work fine in playgrounds for me: https://github.com/parse-community/Parse-Swift/pull/137

The only time I see:

Error saving: ParseError(code: ParseSwift.ParseError.Code.duplicateValue, message: "A duplicate value for a field with unique values was provided")

Is when you are trying to update a ParseObject that has already been saved (same objectId), but createdAt is nil. If a ParseObject is saved to the server, you must always have a date for createdAt.

cbaker6 commented 3 years ago

In a simple case of syncing, you will probably be better off using a Parse LiveQuery subscription combined with a fetch to stay in sync (at least almost stay in sync anyways) across devices. When ever your app is in the foreground it should fetch the latest user from the parse-server and then subscribe to a query of the respective user. I left a similar answer to this on stack https://stackoverflow.com/a/66555468/4639041

lsmilek1 commented 3 years ago

Ah, that was the issue, the missing createdAt. I see it working now. I will just need to init the object with createdAt, where I can pass obviously any Date() and it is not being overwritten. In the save response I become a new createdAt, but in the Dashboard I see the old still.

Thank you for the example also, it made it clear. Perhaps one detail I still could not see there. How would you clear any field so that it becomes undefined again? Setting nil in the struct is not bringing anything. Is the operation unset the only way? (it seems that I can't send multiple operations in one call) Perhaps there is some "API.Option.nilAsUndefined" feature?

Regarding the sync, thank you very much for your inputs. The simple scenario with LiveQuery and fetch is on my todo in combination with such "dirty" updates. Just to start the prototype quickly. I will have a look into that vector updates also as there might be also many users updating one travel itinerary...

Thank you for your patience!

cbaker6 commented 3 years ago

How would you clear any field so that it becomes undefined again? Setting nil in the struct is not bringing anything.

I’m confused, previously in https://github.com/parse-community/Parse-Swift/issues/135#issuecomment-831303466 you said:

Also when I init ParseObject and specify only certain fields, the rest is getting nilled in the database

Is the operation unset the only way? (it seems that I can't send multiple operations in one call)

Operations can be chained (like query), have you tried to chain multiple unset calls? For example you can do:

let operations = prsProfile.operation
            .unset("maxA")
            .unset("minA")
operations.save { result in ... }

Perhaps there is some "API.Option.nilAsUndefined" feature?

There is currently nothing like this, but I don’t quite see the need for it.

cbaker6 commented 3 years ago

Closing this issue as I think the apple sign-in with custom parameters is solved.

Feel free to open up additional issues if you continue to have the other problems.

lsmilek1 commented 3 years ago

Sorry for the confusion. As I found out, previously the fields were not nilled due to either missing createdAt or conflict with cloud code beforeSave trigger I think. New object were added with nil values, I just did not see that. Chaining the operations works, just the mutated struct that the chain is attached to does not get saved. Example:

        let prsProfile = PrsProfile(profile: profile, syncData: syncData) //setting fields "on", "co", "tn", "tr"
        var operations = prsProfile.operation.unset("ag") //unseting field from above struct
        operations = operations.unset("da")
        operations.save { result in
            switch result {
            case .success(let profile):
                print("profile saved: \(String(describing: profile))")
                success(true)

response shows mutated fields, but in the dashboard I see only the "unset" chain was saved:

profile saved: PrsProfile ({"on":true,"co":1,"tn":1,"objectId":"HCnenmfKzX","updatedAt":{"type":"Date","iso":"2021-05-04T13:18:46.234Z"},"createdAt":{"type":"Date","iso":"2021-05-04T13:18:46.035Z"},"tr":1})

Nevertheless, thank you for the explanation and patience. I will re-think how to store the data. Currently I wanted to unset certain fields because then they are deleted from ElasticSearch indexes and such documents are not returned from ElasticSearch query. But this is something I can achieve in the cloud code anyway.

cbaker6 commented 3 years ago

Operations is a separate save, if you mutated the object, you will need to save that also. You can try to batch the object and operation changes with saveAll

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

The label type:feature cannot be used in combination with type:improvement.