thephpleague / oauth2-server

A spec compliant, secure by default PHP OAuth 2.0 Server
https://oauth2.thephpleague.com
MIT License
6.51k stars 1.12k forks source link

FATAL: Trying to include an un existing class #1244

Closed itay-moav closed 2 years ago

itay-moav commented 3 years ago

In file src/AuthorizationValidators/BearerTokenValidator.php You have the following code

        $this->jwtConfiguration->setValidationConstraints(
            \class_exists(StrictValidAt::class)
                ? new StrictValidAt(new SystemClock(new DateTimeZone(\date_default_timezone_get())))
                : new ValidAt(new SystemClock(new DateTimeZone(\date_default_timezone_get()))),
            new SignedWith(
                new Sha256(),
                InMemory::plainText($this->publicKey->getKeyContents(), $this->publicKey->getPassPhrase() ?? '')
            )
        );

Notice the \class_exists condition which is done on StrictValidAt::class This can not work. You need the class StrictValidAt to exist for this to work.... which would make this condition moot. You must use a simple string syntax there. Autoloader will try to include and fail at that line

[Wed Aug 18 12:24:56.175459 2021] [php7:warn] [pid 5674:tid 140327830636288] [client 127.0.0.1:41668] PHP Warning: include_once(Lcobucci/JWT/Validation/Constraint/StrictValidAt.php): failed to open stream: No such file or directory in

eugene-borovov commented 3 years ago

Could you describe your setup? I cannot reproduce this issue. Do you use custom class autoloading?

itay-moav commented 3 years ago

I just include the composer autoloader. But....Check the code itself. It is pretty clear this is a bug. U are using a class constand to check if the class exists. For PHP to know what the class constant is (MyClass::class), It needs to include said class ... catch 22

On Thu, Aug 19, 2021 at 1:59 AM Eugene Borovov @.***> wrote:

Could you describe your setup? I cannot reproduce this issue. Do you use custom class autoloading?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/thephpleague/oauth2-server/issues/1244#issuecomment-901633249, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQDUEUXR3IJXRJZA7DQX3T5SMV5ANCNFSM5CMNXGSQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

eugene-borovov commented 3 years ago

Class constant works with unknown class https://3v4l.org/kEpZH \class_exists has second parameter for control autoloading. I tested this code with PHP 7.2 and error reporting E_ALL and I didn`t get the warning.

itay-moav commented 3 years ago

hmmm... maybe I have an autoloader hidden somewhere. If that is the case, this code will sometimes break, sometimes not. Try with an autoloader that replaces both \ and _ to / and includes that file, on top of the vendor autoloader. include this : strreplace(['\',''],'/',$class_name) . '.php'

On Thu, Aug 19, 2021 at 3:24 AM Eugene Borovov @.***> wrote:

Class constant works with unknown class https://3v4l.org/kEpZH \class_exists has second parameter for control autoloading. I tested this code with PHP 7.2 and error reporting E_ALL and I didn`t get the warning.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/thephpleague/oauth2-server/issues/1244#issuecomment-901674598, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQDUHAGHPMP5I644ZNIB3T5SWRBANCNFSM5CMNXGSQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

eugene-borovov commented 3 years ago

I suggest to use PSR-0 autoloading for legacy code.

itay-moav commented 3 years ago

A good idea, but maybe give a facility for other users who might encounter this issue in the future.

On Fri, Aug 20, 2021 at 2:10 AM Eugene Borovov @.***> wrote:

I suggest to use PSR-0 autoloading https://getcomposer.org/doc/04-schema.md#psr-0 for legacy code.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/thephpleague/oauth2-server/issues/1244#issuecomment-902456658, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQDUB4ENXXGKEQ3A73SUTT5XWT7ANCNFSM5CMNXGSQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

Sephster commented 2 years ago

Thanks for reporting this @itay-moav and thanks to @eugene-borovov for his investigations. I think given Eugene's findings, it is unlikely we would work around this issue as it seems to be more a user-implementation issue rather than anything wrong with this library's code or use of PHP syntax.

If I am wrong with this I'd be happy to re-open this issue and take another look.