Update league/oauth2-server to 7.1.x

Thanks for the patch!

Tests are failing for the new version of the dependency, retesting just in case.

@alan_blake what is new in the new version of the library?

Hmm it seems that tests are consistently failing.

We still want this. Bubbling it up.

A method signature also needed updating to make this work. Otherwise, pretty straightforward.

Thanks for the patch!! You are awesome.

@bradjones1 it seems that tests are not passing. Maybe there something broken with our integration upon updating to the new version?

We still want this in the release, pls

Indeed...

Drupal\Tests\simple_oauth_extras\Functional\AuthCodeFunctionalTest::testAuthCodeGrant
Failed asserting that 400 matches expected 200.

et. al.

Not sure if the test needs updating or if this breaks something more globally.

So this took quite a bit of digging and local debugging, but I believe the test failures were the proximate result of a missing redirect_uri parameter in the auth code flow. Why this hadn't been an issue before is beyond what I explored, but I did find this note in the upstream queue relating to whether the parameter is optional (as set out in the spec) in this implementation. It's a bit old but from a read of the code in 7.1.x and this note, it appears the maintainer made the conscious choice to require it, despite the flexibility in the spec.

I also ran into an issue where some requests to the test site in functional tests were made without the cookie indicating the simpletest db to use; it would then result in errors where the site was not able to properly load the previously-created client consumer record. I replaced the request method in RequestHelperTrait with a post method that adds the appropriate cookie data.

I am admittedly not very good at writing tests so hopefully this moves us in the right direction. On the bright side I don't think there's anything particular in the newer version beyond a function signature that needed changing. So this is mostly a test fix.

Missed a few calls but that's progress.

Addressing testbot failures.

+++ b/simple_oauth_extras/tests/src/Functional/AuthCodeFunctionalTest.php
@@ -272,10 +272,9 @@ class AuthCodeFunctionalTest extends TokenBearerFunctionalTestBase {
-    return $this->request('POST', $this->url, [
-      'form_params' => $valid_payload,
-    ]);
+    return $this->post($this->url, $valid_payload);

Why these changes?

I removed what I suspect that are unnecessary changes.

This should've also been reverted.

@e0ipso - I appreciate the review... the changes you reverted as unnecessary were required to be able to run these tests locally, specifically the post method which adds in the test UA cookie for simpletest. I wasn't able to get these to run locally without them.

I understand if you'd like to limit the scope of the patch but I did have to invest a lot of time into tracking down this particular issue on local testing, and I'm concerned other developers might have similar trouble working on this in the future? Up to you of course as maintainer. Turns out the key was the redirect_url item.

Good point. I'm not convinced about the encryption keys, but that can be dealt with later.

Merging #16, thanks for your contribution!!!

Thank you. I moved the encryption keys because in my case, I was testing with a pair of docker containers and the "from" file location on one machine may not be the same as the other, so I tried to simplify this code by storing the data in the class. Agreed it's probably not mandatory but it helped to simplify things when I was debugging simpletest.

Cheers!