joselfonseca / lighthouse-graphql-passport-auth

Add GraphQL mutations to get tokens from passport for https://lighthouse-php.com/
https://lighthouse-php-auth.com/
MIT License
228 stars 56 forks source link

OAuth Public key is not readable from .env file #149

Closed canatufkansu closed 3 years ago

canatufkansu commented 3 years ago

Hi,

We are using Passport package by adding encryption keys to the .env file as shown in official documentation.

Official Documentation -> https://laravel.com/docs/7.x/passport#deploying-passport

So we don't have oauth-public.key and oauth-private.key files under storage folder. All of the mutations are working but refreshToken mutation is giving error when we call it. The error is like the following

The path "/application/myApp/storage/oauth-public.key" does not contain a valid key file {"exception":"[object] (Lcobucci\\JWT\\Signer\\Key\\FileCouldNotBeRead(code: 0): The path \"/application/myApp/storage/oauth-public.key\" does not contain a valid key file at /application/myApp/vendor/lcobucci/jwt/src/Signer/Key/FileCouldNotBeRead.php:14)"

When we check the according resolver function from the schema file

refreshToken(input: RefreshTokenInput @spread): RefreshTokenPayload! @field(resolver: "Joselfonseca\\LighthouseGraphQLPassport\\GraphQL\\Mutations\\RefreshToken@resolve") @throttle(maxAttempts:60 decayMinutes:1)

we can see that this mutation is resolved by resolve method in RefreshToken class. In the parseToken method we saw that public and private keys are imported from the file without checking Passport's configuration.

public function parseToken($accessToken)
    {
        $public_key_path = Passport::keyPath('oauth-public.key');
        $private_key_path = Passport::keyPath('oauth-public.key');

        $config = Configuration::forAsymmetricSigner(new Sha256(), LocalFileReference::file($private_key_path), InMemory::file($public_key_path));

        $token = $config->parser()->parse((string) $accessToken);

        $claims = $token->claims();

        return $claims->get('sub');
    }

We think that there is missing control here, and also we saw that private_key_path is imported from oauth-public.key file which looks suspicious.

joselfonseca commented 3 years ago

Thanks for the the issue @canatufkansu

This pops up a question i've had for a couple of months. Do we really need to return the user from the refresh token mutation? The only reason we are using that parse token method is to read the JWT and get the user ID to query it and return it in the refresh token response, however I have a feeling we could just get rid of it as it may not even be necessary.

In your use case, do you need to get the user from that mutation?

canatufkansu commented 3 years ago

Hi,

My mutation is like the following, I don't want to return the user information from the query but still get the error. As you said I'm using this mutation just for refreshing the token.

mutation {
  refreshToken(input: { refresh_token: "def502007bd3fb4746869a9aaeb17e07c56ff1328130ea4997f81387919a548a2ca5b71ed69468710b641070d7190992f481ddf7384b11ede288831cad22c8c7455417ca8b641f6c4103347f816a4b976500343a15a238f086a4234a3f91900e9891ed791e54aa4ed380b3433434e239824bb78a070b9da54f57d0a01db20d4014e45afb0a241064181794350123eacb95b410db4a775cc620b7b9dc2d97fea92884aba1907345323b95635fbea4b54c924ce86b7f84d8196a91de6e550dcb76cf7d462ccdef09213be5c794457f9b76c21b141822326fa8e8d0328ffa9b9242d02a7dfcdc6e0527d49670ee79115565c37b3133e6d6f0e5400004f565ffb0ab29c2c2033e0b1c16622b2ba714e0d49f0b633a00816d88ceafce168d2ae4c491dcabdf377ae95a6a548f143aceecbd3d0fc0e5b4267ba9c07944f52fd47dc386041ab0ad368eff9b56912522c6d13495ecbba7dc206a64ff2b449860eb56017b2cb32a7ac001" }) {
    token_type
    access_token
    refresh_token
    expires_in
  }
}
joselfonseca commented 3 years ago

Ok, I think it will be a good idea to remove that code so it won't fail and since we don't really need to get the user from this mutation it will also remove some other issues that can happen. Will work on that.

joselfonseca commented 3 years ago

Actually, I don't have to do that, I found another way. Fix coming

joselfonseca commented 3 years ago

this should be fixed in https://github.com/joselfonseca/lighthouse-graphql-passport-auth/releases/tag/6.0.2