When dealing with an OAuth-Server which is not supporting the authorization with credentials inside the request body, we can't get the token, only a 401 'Unauthorized' response.
This is because the OAuth 2.0 Client library sets the 'PostAuthOptionProvidor' as default.
When changing this to 'HttpBasicAuthOptionProvider', we get a token from the OAuth-Server.
But i don't think it's a good idea to change default behaviour of the library, so i patched this module by adding the possibility to add the 'optionProvider' in the plugin definition.

For example :
/**
* OAuth2 Client to authenticate with OAuth services
*
* @Oauth2Client(
* id = "my_id",
* name = @Translation("MY ID"),
* grant_type = "client_credentials",
* client_id = "XXXX",
* client_secret = "YYYY",
* authorization_uri = "",
* token_uri = "https://my.token.xy/oauth/token",
* resource_owner_uri = "",
* option_provider = "League\OAuth2\Client\OptionProvider\HttpBasicAuthOptionProvider",
* )
*/

I'm pretty sure there will be a better (writing) solution for this, but these are my first steps into OO.
Any suggestions are appreciated.

CommentFileSizeAuthor
#15 change_optionprovider-8.x-3.0-beta1-3046879-7.patch2.79 KBpbosmans
#6 change_optionprovider-8.x-2.x-3046879-6.patch2.97 KBimclean
#5 change_optionprovider-8.x-2.x-3046879-5.patch1.18 KBsfuchsbe
#4 change_optionprovider-3046879-4.patch1.11 KBsfuchsbe
oauth2_client-optionProvider.patch1.19 KBpbosmans

Issue fork oauth2_client-3046879

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pbosmans created an issue. See original summary.

Ashley.Sommer’s picture

Ashley.Sommer commented

@pbosmans

Thank you for creating this issue thread. This looks like a good feature addition, and will likely be merged with the next round of feature addition merges. I might change it slightly (by removing "League\OAuth2\Client\OptionProvider\" and having only the final class name).

mrdalesmith’s picture

mrdalesmith commented

This patch does not cleanly apply because of the /Drupal/web/ paths within it. Can confirm it worked when cleaned up.

sfuchsbe’s picture

sfuchsbe commented
StatusFileSize
new change_optionprovider-3046879-4.patch1.11 KB

I'm currently facing the same issue and would like to follow up on this thread.
I agree that havint the FQCN configured in the annotation is not perfect, but I was ending up with the same, since class discovery without having the namespace configured seem impossible at the moment - at least for me ;)

I reworked the patch hopefully following the guidelines besed on the mentioned version 8.x-2.0-beta3 - additionally I'm going to provide the same patch for 8.x-2.x branch in a moment.

sfuchsbe’s picture

sfuchsbe commented
Status: Active » Needs review
StatusFileSize
new change_optionprovider-8.x-2.x-3046879-5.patch1.18 KB

This is a version for 8.x-2.x branch.
In my opinion we should go ahead with reviewing.

imclean’s picture

imclean commented
StatusFileSize
new change_optionprovider-8.x-2.x-3046879-6.patch2.97 KB

This looks good and probably essential as Basic auth is required by the OAuth2 spec.

I've made a couple of changes:

  1. Uses a more generic "collaborators" key to include any type of collaborator *
  2. Updates the interface and uses a getter to retrieve the collaborators

* PHP League currently states:

Collaborators include`grantFactory`, `requestFactory`, and `httpClient`. Individual providers may introduce more collaborators, as needed.

It checks for "optionProvider" even though it doesn't mention it in the comment.

It requires a patch from this issue to remove the error on the test page: #3101522: Array to string conversion in Drupal\oauth2_client\Form\ClientTestForm

imclean’s picture

imclean commented

Just a note that the library's ClientCredentialsGrantService->getAccessToken() doesn't support scope or any other options.

See the issue on github for a possible solution.

imclean’s picture

imclean commented

We're using #6 on a production site and it's working well. It would be good to have some feedback on the change of approach.

imclean’s picture

imclean commented
Version: 8.x-2.0-beta3 » 8.x-2.x-dev
Category: Support request » Feature request

This is almost a bug report as the OAuth2 spec requires Basic Auth.

imclean’s picture

imclean commented

Pull request for oauth2-client.

imclean’s picture

imclean commented
Status: Needs review » Needs work

#7

Just a note that the library's ClientCredentialsGrantService->getAccessToken() doesn't support scope or any other options.

See: https://github.com/thephpleague/oauth2-client/issues/774#issuecomment-71...

You could probably achieve what you need by extending HttpBasicAuthOptionProvider and overriding getAccessTokenOptions(), adding the scope to the headers, as needed.

We'll need to roll our own HttpBasicAuthOptionProvider to add the scope.

imclean’s picture

imclean commented
Status: Needs work » Needs review

I think the patch in #6 is fine. To use scope with HTTP Basic Auth you just need to extend the class.

Can anyone else test #6?

fathershawn’s picture

fathershawn
Primary language English
Location New York
commented
Assigned: Unassigned » fathershawn

Hi! @perelesnyk and I have stepped up as maintainers, officially for D9 compatible and forward. I've done a quick review of the patch and it's an interesting approach to passing options to the provider. We will review in more detail after #3072062: Refactor Authorization Token Flow is completed.

fathershawn’s picture

fathershawn
Primary language English
Location New York
commented
Assigned: fathershawn » perelesnyk
Status: Needs review » Needs work
pbosmans’s picture

pbosmans commented
StatusFileSize
new change_optionprovider-8.x-3.0-beta1-3046879-7.patch2.79 KB

Patch updated for version 8.x-3.0-beta1

pbosmans’s picture

pbosmans commented
Version: 8.x-2.x-dev » 8.x-3.0-beta1
Status: Needs work » Needs review
fathershawn’s picture

fathershawn
Primary language English
Location New York
commented
Assigned: perelesnyk » fathershawn
Status: Needs review » Needs work

This seems like a workable idea, but we should add the collaborators property to the annotation class and set it to a default empty array.

Second, the feature request changes code currently in MR in #3256272: Additional parameters to get Access token so let's get that change in first.

fathershawn’s picture

fathershawn
Primary language English
Location New York
commented

Well now we need an option provider in #3157501: Add Scopes to ClientCredentials Grant Type so adding this feature :)

FatherShawn opened merge request !10

  • FatherShawn committed 3cc3658 on 8.x-3.x 
    Issue #3046879 by pbosmans, FatherShawn: Provide collaborators to...
fathershawn’s picture

fathershawn
Primary language English
Location New York
commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.