search

bshaffer / oauth2-server-php

A library for implementing an OAuth2 Server in php
http://bshaffer.github.io/oauth2-server-php-docs
MIT License
3.26k stars 950 forks source link

php 7.3 Only one method may be used to authenticate at a time #954

Open stone86 opened 5 years ago

stone86 commented 5 years ago

this problem is relation with function "apache_request_headers" whu included in php 7.3 FPM

https://github.com/bshaffer/oauth2-server-php/blob/master/src/OAuth2/Request.php#L191

bshaffer commented 5 years ago

This error is defined here, and happens because the Access Token is being defined in more than one of the Auth Header (e.g. Authorization: xyz, the GET parameter (e.g. `?access_token=xyz"), or the POST parameter.

MikkelPaulson commented 5 years ago

The root cause of this problem is that the executing environment is bleeding through into the Request object. The $_SERVER superglobal is abstracted to a $server parameter passed to the constructor, which is good practice for separation of concerns, but the Request class then calls apache_request_headers(), which returns the real headers rather than what the parent class tells the Request that the headers are.

IMO lines 191-199 should be deleted. The Request has no business accessing the environment directly.

bshaffer commented 5 years ago

I am still unclear where this would cause an issue, as the Request class should only execute those lines if it otherwise cannot find an Authorization header. Perhaps a good implementation would be to add another conditional to prevent a conflict.

In general, I agree the Request class shouldn't access the environment. And for a long time this was the case, but we had so many bugs being filed because of issues specifically with Apache, I decided to add the fix. So I am reluctant to remove it, but I would love to better understand what is happening here and I can fix it.