Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Support for an EncryptionKey was added in in New property on AuthorizationServer to receive an encryption key which is used for future encryption/decryption instead of keybased encryption/decryption.
As of Trigger an E_USER_DEPRECATED notice instead of an error a notice will be trggered every time a key is requested.
Proposed resolution
Use the AuthorizationServer::setEncryptionKey() method when an AuthorizationServer is instantiated. The only place I found that creates an AuthorizationServer is Oauth2GrantManager. Add the setEncryptionKey method to AuthorizationServer instances.
Remaining tasks
- Add Tests?
- Verify that using a random string is correct here and that Drupal\Component\Utility\Random is random enough. See https://oauth2.thephpleague.com/v5-security-improvements/
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 2901180-8.patch | 1.07 KB | balsama |
| #7 | 2901180-7.patch | 872 bytes | balsama |
| #5 | 2901180-Reverting-composer-json.patch | 370 bytes | joum |
| #3 | 2901180-V5-Security-Improvements-2.patch | 756 bytes | joum |
Comments
Comment #2
joumThe token attribution works correctly, but when the token is used to GET a resource, a 401 is returned with the message:
{
"message": "No authentication credentials provided."
}
Drupal's watchdog reports the following:
User deprecated function: You must set the encryption key going forward to improve the security of this library - see this page for more information https://oauth2.thephpleague.com/v5-security-improvements/
Comment #3
joumI created my own patch for this, still not tested.
Comment #4
joumWell, the patch didn't do anything. -_-'
The watchdog reports the following:
Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException: No authentication credentials provided. in Drupal\basic_auth\Authentication\Provider\BasicAuth->challengeException() (line 133 of /vagrant/www/core/modules/basic_auth/src/Authentication/Provider/BasicAuth.php).
Comment #5
joumHere's a patch to force the 5.1.3 version of the League package until this is properly fixed (not sure if it will work).
Comment #6
joumThe latest patch was absolute idiocy on my behalf. Composer installs the wrong dependency before applying the patch.
Running out of ideas here, any help would be very welcome.
Comment #7
balsamaThat seems like the right approach to me and actually fixes the issue for us. I cleaned up the patch a bit and used Random::string to generate the key. We have a test for this in our project, but this should probably have a test here too.
Comment #8
balsamaSorry, was thinking that Random::string was static. Fixed and added a comment. Also updated IS.
Comment #9
balsamaSetting priority as I'm pretty sure this isn't critical.
Also noticed that joum had already linked to some documentation that implies a random string is expected and suitable, which I had raised as a question in the IS (thanks! https://oauth2.thephpleague.com/v5-security-improvements/)
Comment #10
balsamaComment #12
e0ipsoThis will be addressed in https://www.drupal.org/node/2883862#comment-12226499. Sorry for mixing stuff but I worked on it on a plane and didn't have the numbers.