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

Removes Check Forcing Client ID to be a String #1233

Closed Sephster closed 3 years ago

Sephster commented 3 years ago

This PR removes the check forcing the client ID to be a string as this is breaking Laravel Passport which uses integers. It is likely other implementations are affected by this so I am reverting this change.

This should fix issue #1232

marc-mabe commented 3 years ago

I didn't know why laravel uses an int but the spec defines a string.

RFC 6794 Section 2.2 https://datatracker.ietf.org/doc/html/rfc6749#section-2.2

The authorization server issues the registered client a client identifier -- a unique string representing the registration information provided by the client. The client identifier is not a secret; it is exposed to the resource owner and MUST NOT be used alone for client authentication. The client identifier is unique to the authorization server.

The client identifier string size is left undefined by this specification. The client should avoid making assumptions about the identifier size. The authorization server SHOULD document the size of any identifier it issues.

Where I see this could be handled as a breaking change it should be deprecated and restricted in the next major.

Additionally it should catch other types (like array or float) to return a proper response and not end up in an Internal Server Error.

So I would allow string and int only for now but trigger a deprecation message on int.

Thoughts ?

eugene-borovov commented 3 years ago

The problem is that the value is checked, but not converted to a string. Inside the library, all values received from the request must be strings or NULL.

Index: src/Grant/AbstractGrant.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php
--- a/src/Grant/AbstractGrant.php   (revision 97dbc97b3b1bc4e613b70cb5e0e07d4b2d9372cc)
+++ b/src/Grant/AbstractGrant.php   (date 1622871883500)
@@ -354,8 +354,14 @@
     protected function getRequestParameter($parameter, ServerRequestInterface $request, $default = null)
     {
         $requestParameters = (array) $request->getParsedBody();
+        $value = $requestParameters[$parameter] ?? $default;
+        if (\is_string($value) || \is_int($value)) {
+            $value = \trim($value);
+        } else {
+            $value = null;
+        }

-        return $requestParameters[$parameter] ?? $default;
+        return $value === '' ? null : $value;
     }

     /**