node-apple-signin icon indicating copy to clipboard operation
node-apple-signin copied to clipboard

Add support for multiple apple public keys

Open tomislavherman opened this issue 5 years ago • 4 comments

Currently we use only first public key provided by apple to verify ID token. It seems that apple issues ID tokens with different keys (3 in this moment). This fix uses header.kid field from ID token in order to identify which public key will be used to verify ID token.

tomislavherman avatar Feb 13 '20 17:02 tomislavherman

Can we move the discussion in https://github.com/Techofficer/node-apple-signin/pull/9 ?

IMO, it would be good to move to jwks-rsa and use jwks properly for the sake of clarity and simplicity

alexabidri avatar Feb 15 '20 05:02 alexabidri

@alexabidri I applied your suggestion to this PR. I hope this is ok. Instead of jwks-rsa I used jwks-rsa-promisified which exposes promisified method versions of jwks client so I don't have to deal with callbacks inside async verifyIdToken(...).

tomislavherman avatar Feb 18 '20 16:02 tomislavherman

@tomislavherman Good job!

Personnaly I would prefer to use jwks-rsa because jwks-rsa-promisified is not maintained a lot and we dont take advantage of recurring new releases of jwks-rsa.

As apple-signin is a package aim to be used by some startup/corporations, I would avoid using a non maintained module which doesnt provide big improvements except to promisify a already existing module.

Some hours ago, jwks-rsa got a new release (1.7.0) This release includes a change to the default caching mechanism. Caching is on now by default, with the decrease of the default time of 10hours to 10minutes. This change introduces better support for signing key rotation.

Here is a simple wrapper to promisified jwks-rsa

// eslint-disable-next-line
const jwksClient = require('jwks-rsa');

function getApplePublicKey(kid) {
  const client = jwksClient({
    jwksUri: 'https://appleid.apple.com/auth/keys',
    cache: true,
  });

  return new Promise((resolve, reject) => {
    // eslint-disable-next-line
    client.getSigningKey(kid, (err, key) => {
      if (err) {
        reject(err);
      }
      resolve(key);
    });
  });
}

const key = await getApplePublicKey(kid);

alexabidri avatar Feb 19 '20 01:02 alexabidri

Thanks, @alexabidri. I agree with you regarding the jwks-rsa package.

I would just instantiate jwks client in the scope of the module instead of in the scope of getApplePublicKey function. Otherwise, each call to getApplePublicKey would create a new client, and we would lose the caching capability provided by client. Do you agree?

tomislavherman avatar Feb 19 '20 07:02 tomislavherman