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

Sign in with apple id #369

Closed
kevincobain2000 opened this issue Jun 9, 2019 · 27 comments
Closed

Sign in with apple id #369

kevincobain2000 opened this issue Jun 9, 2019 · 27 comments

Comments

@kevincobain2000
Copy link

Since the announcement of sign in with apple id. It'd be good to have it as a socialite provider for sign in with apple.

https://developer.apple.com/sign-in-with-apple/

@kevincobain2000 kevincobain2000 changed the title Sign in with apple id [Feature Request] Sign in with apple id Jun 9, 2019
@olivernybroe
Copy link

We are not accepting new adapters.
Adapters for other platforms are listed at the community driven Socialite Providers website.

So you should prob. create the issue in that package instead https://github.com/SocialiteProviders/Providers or just create your own package with it.

@driesvints
Copy link
Member

@olivernybroe I realize that this is in the readme but I suspect that this will be such a widely adopted adapter so we could maybe consider adding it in.

Might need https://developer.apple.com/documentation/signinwithapplejs ?

@taylorotwell
Copy link
Member

Feel free to PR.

@m1guelpf
Copy link

This PR to SocialiteProviders/Providers might be a good starting point.

@driesvints driesvints changed the title [Feature Request] Sign in with apple id Sign in with apple id Jun 13, 2019
@theodh
Copy link

theodh commented Jun 24, 2019

The email address will be available soon, see issue:
https://forums.developer.apple.com/thread/118209

We need to wait after the update from Apple, but we could implement this @jbrooksuk #371 (comment)
with a dummy param email_id and change the variable name after the update, so we could be live quickly.

Shall I make a PR from your repo @pxgamer https://github.com/pxgamer/socialite/tree/feature/apple-provider
so we could go live after the update form apple? Or do you want to implement this?

Perhaps a suggestion for the socialite documentation from apple with there anonymous-email-feature:

In order to contact users that use Apple’s private email relay service, you need to register domains and email addresses that your organization will use for communication. Domains and domains associated with email addresses must comply with Sender Policy Framework standards and be verified by Apple before they are successfully registered.

Update From #371 message @pxgamer, will start a PR from his repo and snippet @jbrooksuk

@theodh
Copy link

theodh commented Jun 25, 2019

The snippet has been implemented see theodh@bc70f9a
At this moment the scope is only name. The state at this moment: a unique verifier user will be created. If we add email in the scope it will throw an exception until Apple has added this param. If email is a requirement for socialite we should wait after the update, otherwise I could do a PR. Other suggestions?

@owenvoke
Copy link

I think it's probably worth waiting until the update by Apple. It might be worth removing commit 248c0ce though as this is IDE-specific and probably should be in your global ignore (correct me if I'm wrong though)?

Also, can the abstract getUserByToken() method be skipped out? 🤔
I know you've mentioned why in the user() method.

@theodh
Copy link

theodh commented Jun 25, 2019

I think it's probably worth waiting until the update by Apple. It might be worth removing commit 248c0ce though as this is IDE-specific and probably should be in your global ignore (correct me if I'm wrong though)?

Removed the .idea and reversed to scope name+email

Also, can the abstract getUserByToken() method be skipped out? 🤔
I know you've mentioned why in the user() method.

Good point! The override on the user method in the AppleProvider could cause maintenance issues. Because the AppleProvider is an exception on this method we could introduce a smaller override and we keep the abstract. I was thinking about a strategy pattern, but that is an overkill. We could also introduce an extra method. Then we don't have to change the other providers and I only need to override this new function in the AppleProvider:

AbstractProvider.php:

  
    public function user()
    {
        if ($this->hasInvalidState()) {
            throw new InvalidStateException;
        }

        $response = $this->getAccessTokenResponse($this->getCode());

        $token = Arr::get($response, 'access_token');
        $user = $this->mapUserToObject($this->getUserByAccessTokenResponse($response));

        return $user->setToken($token)
                    ->setRefreshToken(Arr::get($response, 'refresh_token'))
                    ->setExpiresIn(Arr::get($response, 'expires_in'));
    }

    protected function getUserByAccessTokenResponse($response)
    {
        return ($this->getUserByToken(
            $token = Arr::get($response, 'access_token')
        ));
    }

We should think about this, other ideas :)

@theodh
Copy link

theodh commented Jun 25, 2019

The apple clientsecret key has an expiration (max 6 months). All users should maintain or use a script like https://github.com/aaronpk/sign-in-with-apple-example/blob/master/client-secret.rb This is needed to maintain the laravel services.apple.clientsecret config value. We'll get security issues if we create this automatically (but it is possible I saw some php composer libraries to create this key). Security problem: someone is snitching the apple private key. In that case the developer should use an external script and create a file with the clientsecret with an expire date (example: in the storage/framework path). Or is this out of scope?

@theodh
Copy link

theodh commented Jul 4, 2019

Status update: from https://forums.developer.apple.com/thread/118209

tsombrero Jul 3, 2019 6:19 PM
(in response to HalvinCarris)
Saw this for the first time today, there are now four items in the posted form data on successful
login: state, code, id_token, and user. User has firstName, lastName, and email in a json blob.
Should we be concerned about replay attacks, since the user info is unsigned? In other words, if a nefarious actor replays a previous post but changes the "user" to something else, is that detectable?

Current issues:

  1. The authorize request from apple must be Http Post back (param response_mode=form_post) to get the form variable. The laravel app should handle this post instead of a get. theodh@1c38c90
  2. AppleId will only sent this once from the first authorize
user: {"name":{"firstName":"Jane","middleName":"","":"Doe"},"email":"j123easj2@privaterelay.appleid.com"}
  1. Blocker: I could not verify the user data (man-in-the-middle etc). Or we should trust the HTTP_ORIGIN host: appleid.apple.com then I could make a workaround.

It's all beta, shall we put this issue onhold until they change issue 3

@driesvints
Copy link
Member

@theodh yeah we're not gonna implement anything until the final release.

theodh added a commit to theodh/socialite that referenced this issue Jul 17, 2019
@theodh
Copy link

theodh commented Jul 17, 2019

@driesvints

Isn’t it better that we delay this feature until the final ios versions have been released? Not sure how much will change until then?

Good point, other view, we should test this in some environments. I'm already having this provider in the production environment. I'm also monitoring the log. My suggestion a beta-branch in your repo, or that other developers could use a VCS to mine.

Laravel app: composer.json

Change require:

"laravel/socialite": "dev-apple-issue-369",

Add

"repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/theodh/socialite"
        }
    ]

I need some testers 💂‍♂️ 🎺

Notes:
The apple provider has some different behaviour as a 'common' oauth provider, perhaps notes in the documentation:

  • You only get the email address in the first login of the user. You should save the email address (user->email) and the apple identifier (user->id). The second time you use this identifier to find the user in your laravel applications.
  • Clientsecret will expire, the user should maintain this.
  • The apple user could use the anonymous email feature. To sent emails please configure
    https://developer.apple.com/account/resources/services/configure

In order to contact users that use Apple’s private email relay service, you need to register domains and email addresses that your organization will use for communication. Domains and domains associated with email addresses must comply with Sender Policy Framework standards and be verified by Apple before they are successfully registered.

@owenvoke
Copy link

@theodh, which branch are you using? apple-issue-369 or dev-apple-issue-369?

@theodh
Copy link

theodh commented Jul 17, 2019

@pxgamer apple-issue-369

@theodh
Copy link

theodh commented Sep 16, 2019

Status update: apple revoked the feature that it is possible to handle a GET-request with the auth-token. In my production environment I created a custom subroutine without laravel/socialite. We need to handle (security etc) that a post request from host appleid.apple.com is allowed in the laravel framework.

A lot of developers asked to apple to get this feature back...

@alinmiron
Copy link

Hello people, any updates on this issue?

@karlshea
Copy link

karlshea commented Oct 4, 2019

What needs to happen or where can I help to move this forward?

@alinmiron
Copy link

alinmiron commented Oct 4, 2019

What needs to happen or where can I help to move this forward?

I managed to use this tutorial to do a quick and dirty test:
https://developer.okta.com/blog/2019/06/04/what-the-heck-is-sign-in-with-apple. I will try to use this library https://github.com/kelvinmo/simplejwt instead of the ruby script.

Since this documentation is almost useless: https://developer.apple.com/documentation/signinwithapplerestapi,

I dug into these also:
https://medium.com/techulus/how-to-setup-sign-in-with-apple-9e142ce498d4
https://forums.developer.apple.com/thread/119826
https://forums.developer.apple.com/thread/118209#385600
https://forums.developer.apple.com/message/383083#383083

From what I can tell beside the issues identified by the people above, main issue (beside getting name and email only at first call) is the creation of the client_secret from the private key obtained from Apple and the fact that this key is valid only 6 months. <--- The generated JWToken (client secret) is valid maximum 6 months, not the private key (.p8 file) !!!!
I'm not sure if Apple's REST API exposes an endpoint to test the validity of client_secret (or private key).

At this moment I try to put everything together and build a custom piece (without passport/socialite/etc) of code that will handle just the token verification (because it's an API) (the other part will be handled from a mobile app)

@driesvints
Copy link
Member

I also saw this passing by on Twitter, not sure if it's relevant: https://openid.net/2019/09/30/apple-successfully-implements-openid-connect-with-sign-in-with-apple/

@theodh
Copy link

theodh commented Oct 5, 2019

Indeed @alinmiron @driesvints, also made custom routines to handle the social, this is an odd duck...

It is not the socialite code implementation (see draft #396) but the side-effects in the laravel framework:

We could implement the draft PR, but we should make a documentation about the side-effects in the framework. We could make a note in the documentation?

The laravel framework developer should

  • Handle a post request of the authorization token.
  • Refresh the client_secret apple key each six months (write a automatic cronjob)
  • If the user is using his anonymous email-address, a standard email relay (mandrill, sendgrid) is not possible at this moment.
  • You only get the email address in the first login of the user. You should save the email address (user->email) and the apple identifier (sub) (user->id). The second time you use this identifier to find the user in your laravel applications.

If we're going further, still need testers, made a small update, had no time to test the code:
#369 (comment)

@theodh
Copy link

theodh commented Oct 6, 2019

Made an example of how the socialite of apple can be used. Also tested the PR and it works! :)

@alinmiron
Copy link

alinmiron commented Oct 7, 2019

Made an example of how the socialite of apple can be used. Also tested the PR and it works! :)

I've taken a quick look over the example provided, seems ok.
I didn't had time to test, I will try to make some time for testing this.
Regarding the obtaining of the client_secret, we could implement a method that reads the Apple private key and generate the client_secret without ruby (and maybe without cronjob).
here's what I did in my code (this is something custom, not socialite related):

    public function generateClientSecret()
    {
        $privateKeyFile = Storage::disk('local')->get('crapple.pkey');
        $team_id = ''; //value got from apple
        $key_id = ''; //value got from apple
        $signer = new \Lcobucci\JWT\Signer\Ecdsa\Sha256();
        $privateKey = new Key($privateKeyFile);
        $token = (new Builder())->issuedBy($team_id)// Configures the issuer (iss claim)
        ->permittedFor("https://appleid.apple.com")// Configures the audience (aud claim)
        ->issuedAt(time())// Configures the time that the token was issue (iat claim)
        ->expiresAt(time() + 86400 * 180)// Configures the expiration time of the token (exp claim)
        ->relatedTo(config('app.apple_client_id')) //Configures the subject
        ->withHeader('kid', $key_id)
            ->withHeader('type', 'JWT')
            ->withHeader('alg', 'ES256')
            ->getToken($signer, $privateKey); // Retrieves the generated token
        //$a = $token->getHeaders(); // Retrieves the token headers
        //$b = $token->getClaims(); // Retrieves the token claims

        return $token->__toString();
    }

First, I check to see if there is a value for a CLIENT_SECRET .env variable. If it's not, I call the above method and generate the client_secret then I'm writing it in the .env variable.
At each instantiation of the class I'm using, I test the token validity (remember, it's a JWT token that expires in 6 months!). This is the piece of code I'm using:

        $token = (new Parser())->parse((string)$token); // Parses from a string
        return $token->isExpired();

if the client_secret is expired, I'll regenerate using the same method above.

@alinmiron
Copy link

Hi @theodh. I was able to test your example. Everything looks good!

Since iOS 13 was released on 19th of September this should be merged, I think Apple will push the "Sign in with Apple" as a privacy-focused alternative to the existing 3rd party sign in methods (google, facebook ...) .

IMHO the only big issue will be the e-mail sending if the users login with anonymous e-mail address, but this is more like a configuration issue (registration of domain/email addresses with apple).

The client_secret generation can be handled easy. Also few mentions from here should be added to the Readme file and we should be good to go.

@theodh
Copy link

theodh commented Oct 8, 2019

Hi @alinmiron thank you for testing the example! Also put your reference of your php client secret solution in the example. Indeed now iOS 13 has been released, this issue has been resolved.

theodh added a commit to theodh/socialite that referenced this issue Oct 8, 2019
@driesvints
Copy link
Member

Looks like Taylor decided he doesn't wants to take on the maintenance burden after all which is understandable. Feel free to pr this to SocialiteProviders.

@mikebronner
Copy link

For anyone looking for this functionality, I have created a socialite package for this, which extended functionality specific for SIWA: https://github.com/GeneaLabs/laravel-sign-in-with-apple. Hope this is useful.

@ankurk91
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests