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

Pass CryptKey instance to oauth2 with permission check disabled #10

Merged
merged 4 commits into from
Dec 20, 2018
Merged

Pass CryptKey instance to oauth2 with permission check disabled #10

merged 4 commits into from
Dec 20, 2018

Conversation

gschafra
Copy link
Contributor

Fix permission check (introduced with thephpleague/oauth2-server#776) for private keys
always failing in windows environments (also docker). Shamelessly stolen from laravel/passport#454.

…on check disabled.

Fix permission check (introduced with thephpleague/oauth2-server#776) for private keys
always failing in windows environments (also docker). Shamelessly stolen from laravel/passport#454.
@gschafra
Copy link
Contributor Author

O.k... it's my first PR ever... so be tolerant 😟

@gschafra gschafra changed the title Pass CryptKey instance to oauth2 with permission check disabled WIP: Pass CryptKey instance to oauth2 with permission check disabled Dec 19, 2018
@spideyfusion spideyfusion self-assigned this Dec 20, 2018
Copy link
Contributor

@spideyfusion spideyfusion left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. I've left some feedback for you to address. :-)

DependencyInjection/TrikoderOAuth2Extension.php Outdated Show resolved Hide resolved
DependencyInjection/TrikoderOAuth2Extension.php Outdated Show resolved Hide resolved
DependencyInjection/TrikoderOAuth2Extension.php Outdated Show resolved Hide resolved
@spideyfusion
Copy link
Contributor

@gschafra Don't forget to lint your code after you're done with the PR.

@gschafra
Copy link
Contributor Author

Unfortunately, when I'm trying to run php via docker on my Windows machine here (company conventions, sorry), running Docker for Windows using:
docker-compose run --rm --no-deps php -v
i've got

D:\vlinux\oauth2-bundle (disable-private-key-permission-check -> origin) λ docker-compose run --rm --no-deps php -v
WARNING: The HOST_USER_ID variable is not set. Defaulting to a blank string.
WARNING: The HOST_GROUP_ID variable is not set. Defaulting to a blank string.
sh: can't open 'docker-entrypoint': No such file or directory

Already tried to set autocrlf to input on cloning as suggested here: bigchaindb/bigchaindb#2215.
Any suggestions/ideas?

@codecov-io
Copy link

Codecov Report

Merging #10 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #10      +/-   ##
=========================================
+ Coverage     85.97%   86%   +0.02%     
  Complexity      197   197              
=========================================
  Files            37    37              
  Lines           599   600       +1     
=========================================
+ Hits            515   516       +1     
  Misses           84    84
Impacted Files Coverage Δ Complexity Δ
DependencyInjection/TrikoderOAuth2Extension.php 91.66% <100%> (+0.11%) 14 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d0eac3...dd342fd. Read the comment docs.

- Applied on tests and extension class
@gschafra gschafra changed the title WIP: Pass CryptKey instance to oauth2 with permission check disabled Pass CryptKey instance to oauth2 with permission check disabled Dec 20, 2018
@gschafra
Copy link
Contributor Author

gschafra commented Dec 20, 2018

So... applied everything as suggested, run lint and unit tests (also locally). There's still the requested change open (see above)... is there something left open to do for me (concerning this PR of course ;-) )?

@spideyfusion spideyfusion merged commit a24415a into trikoder:master Dec 20, 2018
@gschafra gschafra deleted the disable-private-key-permission-check branch December 20, 2018 20:38
@spideyfusion spideyfusion removed their assignment Jan 18, 2019
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.

3 participants