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

Send CryptKey instance to oauth2 with new permission check disabled #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jonathonsim
Copy link

When running in some development environments, eg Vagrant on Windows (or most other methods you might use to develop in windows, eg docker or a local PHP install) it's impossible to make the oauth-private.key file match the exact permissions that thephpleague/oauth2-server now checks for, and this means everything will fail with an exception

Passport fixed this in this PR laravel/passport#454 and this code implements a similar solution in benbjurstrom/passport-custom-jwt-claims

@@ -32,7 +33,7 @@ public function makeAuthorizationServer()
$this->app->make(AccessTokenRepository::class), // BenBjurstrom\JwtClaims\AccessTokenRepository
$this->app->make(ScopeRepository::class),
new CryptKey( 'file://'.Passport::keyPath('oauth-private.key'), null, false ),
new CryptKey('file://'.Passport::keyPath('oauth-public.key'), null, false )
app('encrypter')->getKey()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benbjurstrom FYI, my original commit here was wrong (this second argument is expected to be a secret encryption key as a string, rather than a CryptKey instance.

However looking at the upstream they use app('encrypter')->getKey() here rather than the public key and as far as I can see this key is supposed to be a private secret string unique to the app rather than a filename

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

Successfully merging this pull request may close these issues.

1 participant