Next Version

[bencorlett]

This PR serves as the basis for an entire rewrite of this package. The previous version has been around for a very long time and is quite long in the tooth.

Key improvements over the older version are:

1. The overall architecture in the older version leads to a lot of duplication in tests and our tests are in fact often not really testing anything meaningful.
2. After re-reading the spec, there are a couple of areas where we currently don't correctly address creating an OAuth signature. Obviously this works for most folks, however there's some edge cases which we don't account for. Upon searching the web for other OAuth 1 packages, it appears nobody (that I've seen) accounts for these. An example is where the same parameter is found in both the request body and the query string. Both instances should be used for generating the OAuth signature. The new version accounts for this.
3. The creation of additional providers which customise functionality was painful in the previous iteration. By utilising PSR-7, PSR-17 and PSR-18, providers have a lot more flexibility to setup unique requests if they need to instead of overriding a lot of functionality.
4. Again, with the addition of PSR, we have decoupled from requiring specific Guzzle versions. We suggest Guzzle as a perfect partner for this package, however our only dependency is an implementation of the relevant PSRs as well as a Guzzle support package for handling RFC 3986 encoding/decoding. The final iteration of this package may remove that dependency.
5. The rewrite express a Generic Provider, which any OAuth1-compliant server should work with. We will likely remove all additional providers by extracting them into external packages (up for debate) and supply only the Generic Provider out of the box.
6. The rewrite has extensive unit test coverage with many scenarios in key areas, such as signature generation and OAuth flow. We have taken examples right out the OAuth spec and plugged them into our tests to ensure that we catch the edge cases exposed by the spec. We also have 100% code coverage, obviously 😉.

To do:

• [ ] Decide if we extract providers out and ship with only a Generic Provider, or do we ship with the same providers as the old version? Thoughts: maybe we ship with the most common ones. Some less common ones like Magento 1 introduce the requirement of XML-based PHP extensions being dependencies…
• [ ] Write up detailed documentation with usage examples.
• [ ] Add a handy Guzzle middleware to sign authenticated OAuth 1 requests, or recommend the usage of the official OAuth subscriber.
• [ ] Decide what properties to officially support on the User object. At the time of writing, we support id, email, username and all other data is provider-specific and is accessible via the user's metadata array.
• [ ] Explore adding integrations/bridges (likely in separate repositories) for common frameworks such as Laravel/Symfony.

[mfn]

💥

😄

[bencorlett]

💥

😄

@mfn not so much anymore!

[mfn]

• Add a handy Guzzle middleware to sign authenticated OAuth 1 requests, or recommend the usage of the official OAuth subscriber.

I'm not familiar with this implementation (so many changes 🙀) and I'm not sure if this it the proper forum to bring this up.

guzzle/oauth-subscriber works by storing the token credentials on the class in the constructor. This means once instantiated, this "state" persists to all requests performed with it.

I've a use-case where I want to pass these credentials on a per request basis (by allowing credentials being passed via Guzzles $options). This frees the Guzzle client from being tied to a specific set of credentials and allows to be more conservative with resources and logic:

• a single Guzzle client instance can be used for all instances, as it's not tied to a set of credentials
• every call explicitly can pass a different set of credentials

This is useful when interacting with multiple identities but not requiring to have multiple HTTP clients for this.

---

I searched for an opportunity to mention this somewhere in the hopes there are peers recognizing this as a valid use-case. Right now, to solve this, I've had to copy OAuth1 into a private source tree and adapt it accordingly.

The discussion in https://github.com/thephpleague/oauth1-client/issues/88 triggered me to write this here.

[bencorlett]

I see.

Why don’t you want to create a separate client instance for each account you’re acting on behalf of?

Having said that, I can see your use-case being solved with this new version pretty much out of the box.

There’s a new $client->executeAuthenticatedRequest() method now. We take a PSR-7 request (guzzle satisfies this requirement) and also a PSR-18 HTTP client (guzzle also satisfies this).

I’ll put some more thought into it and come back with an example soon.

Sent from my iPhone

On 3 Sep 2020, at 2:29 pm, Markus Podar notifications@github.com wrote:

 Add a handy Guzzle middleware to sign authenticated OAuth 1 requests, or recommend the usage of the official OAuth subscriber. I'm not familiar with this implementation (so many changes 🙀) and I'm not sure if this it the proper forum to bring this up.

guzzle/oauth-subscriber works by storing the token credentials on the class in the constructor. This means once instantiated, this "state" persists to all requests performed with it.

I've a use-case where I want to pass these credentials on a per request basis (by allowing credentials being passed via Guzzles $options). This frees the Guzzle client from being tied to a specific set of credentials and allows to be more conservative with resources and logic:

a single Guzzle client instance can be used for all instances, as it's not tied to a set of credentials every call explicitly can pass a different set of credentials This is useful when interacting with multiple identities but not requiring to have multiple HTTP clients for this.

I searched for an opportunity to mention this somewhere in the hopes there are peers recognizing this as a valid use-case. Right now, to solve this, I've had to copy OAuth1 into a private source tree and adapt it accordingly.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[mfn]

Why don’t you want to create a separate client instance for each account you’re acting on behalf of?

Persistent background worker job connecting to the same remote OAuth1 system on behalf of identities likely different for each job.

To conserve memory and reduce GC, I try to make objects "stay" if possible, make them state-less so they can be shared like this.

Thanks!

[bencorlett]

Why don’t you want to create a separate client instance for each account you’re acting on behalf of?

Persistent background worker job connecting to the same remote OAuth1 system on behalf of identities likely different for each job.

To conserve memory and reduce GC, I try to make objects "stay" if possible, make them state-less so they can be shared like this.

Thanks!

Hi @mfn, here's a snippet of how I would solve your problem. I've included all of the boilerplate as well as I'd like to know your thoughts on it overall:

<?php

require_once __DIR__.'/vendor/autoload.php';

// Boilerplate to create a Twitter provider
$twitter = new League\OAuth1\Client\Provider\Twitter(
    new League\OAuth1\Client\Credentials\ClientCredentials(
        'app-id',
        'app-secret',
        'https://api.client.com/callback',
    )
);

// Create an OAuth client, giving a provider, PSR-17 HTTP request factory and PSR-18 client.
//
// You can use:
// 1.  `http-interop/http-factory-guzzle` for the PSR-17 HTTP request factory
// 2a. `guzzlehttp/guzzle` (v7 for PHP 7.2+) or
// 2b. `php-http/guzzle6-adapter` (for PHP 7.1)
//
// I'm currently testing the package in PHP 7.1, so I'm using 1 and 2b from above
$oauthClient = new League\OAuth1\Client\Client(
    $twitter,
    new Http\Factory\Guzzle\RequestFactory(),
    new Http\Adapter\Guzzle6\Client()
);

// Now you need a PSR-7 request, I use `guzzlehttp\psr7` for this
$request = new GuzzleHttp\Psr7\Request('GET', 'https://api.example.com/me');

// Here's your token credentials / access tokens
$tokenCredentials = new League\OAuth1\Client\Credentials\Credentials('token-id', 'token-secret');

// Now, you could either set this with the client for continual use:
//
// $oauthClient->setTokenCredentials($tokenCredentials);
// $response = $oauthClient->executeAuthenticatedRequest($request);
//
// However you won't want to do this as this sets the token credentials for the client overall.
// You might want to send unique token credentials every time:

try {
    $response = $oauthClient->executeAuthenticatedRequest($request, $tokenCredentials);
} catch (Psr\Http\Client\ClientExceptionInterface $e) {
    // 
} catch (Throwable $e) {
    //
}

[bencorlett]

@mfn out of curiosity - are you integrating with a known OAuth 1 provider? I'm looking through what still uses OAuth 1 and there isn't a lot!

[mfn]

@bencorlett The code in general looks fine to me and decouples the token credentials, seems exactly what I was talking about.

are you integrating with a known OAuth 1 provider?

Yes, Twitter ;)

Currently using guzzle/oauth-subscriber absorbed in the code base and adapted to serve the use-case.

Once a (dev) release is available, I can try integrating it and see how it works out. At first the code looks boilerplate-y though, but I might judge this wrong. I'm just used to directly use Guzzle 6 and not having to deal with the abstractions too much.

Thanks!

[bencorlett]

Yeah the cost of decoupling from dependency implementations is you must supply said implementations.

Having said that, we’ll likely ship with a Guzzle 6/7 factory for convenience.

Sent from my iPhone

On 4 Sep 2020, at 4:18 pm, Markus Podar notifications@github.com wrote:

 @bencorlett The code in general looks fine to me and decouples the token credentials, seems exactly what I was talking about.

are you integrating with a known OAuth 1 provider?

Yes, Twitter ;)

Currently using guzzle/oauth-subscriber absorbed in the code base and adapted to serve the use-case.

Once a (dev) release is available, I can try integrating it and see how it works out. At first the code looks boilerplate-y though, but I might judge this wrong. I'm just used to directly use Guzzle 6 and not having to deal with the abstractions too much.

Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[bencorlett]

Will open additional PRs for further work. Develop will point to the next version from now on.