I had some issues with the client credentials OAuth2 plugin of the client currently in use, I think a much more maintained and supported option is https://github.com/thephpleague/oauth2-client. The server version of this package is being used in https://www.drupal.org/project/simple_oauth v2.

CommentFileSizeAuthor
#24 2879665-24.diff7.76 KBherved
#14 2879665-14-oauth2-auth-plugin.patch3.45 KBkrisahil
#12 2879665-12-oauth2-auth-plugin.patch3.84 KBkrisahil
#10 2879665-10-oauth2-auth-plugin.patch3.83 KBjon nunan
#4 2879665-2.patch3.12 KBpcambra
#2 2879665-1.patch3.16 KBpcambra

Issue fork migrate_plus-2879665

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

pcambra created an issue. See original summary.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
StatusFileSize
new 2879665-1.patch3.16 KB

The yml settings would be like this:

  source:
    plugin: url
    data_fetcher_plugin: http
    data_parser_plugin: json
    ids:
      nid:
        type: integer
    authentication:
      plugin: oauth2
      grant_type: client_credentials
      provider_options:
        clientId: {id}
        clientSecret: {XXXX}
        urlAuthorize: http://example.com/oauth2/authorize
        urlAccessToken: http://example.com/oauth2/token
        urlResourceOwnerDetails: http://example.com/oauth2/UserInfo
      grant_options:
        scope: {scope}

Two subarrays with options, one for the provider, and another for the grants, such as user/password.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Status: Active » Needs review
pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
StatusFileSize
new 2879665-2.patch3.12 KB

Sorry, a leftover :)

heddn’s picture

heddn
Primary language English
Location Illinois
commented

This would mean a bit of a BC break. What are the specific difficulties with one OAuth library vs another?

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented

I understand that, however, the library is marked as optional and the proposed one is wildly more supported and more future proof. If you check the github stars it's quite clear which one is more popular and the oauth server library from the same vendor is the one used in simple_oauth which it's quite probable to make it to core eventually as part of the API first initiative.

My setup is a Drupal 7 with oauth2_server and I could not make the client credentials grant type work with the current one, I don't think it transforms the forms bit very well, whereas with the one proposed, everything worked as expected.

heddn’s picture

heddn
Primary language English
Location Illinois
commented

I think I'm ok with the swap but I'd like to hear input from mikeryan.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Needs work

I'm leaning towards accepting this approach. However, I'd like to see some tests. And docs on how to implement this. Perhaps in the doxygen of OAuth2

heddn’s picture

heddn
Primary language English
Location Illinois
commented

I checked with @mikeryan and he's also OK with swapping out libraries. Only missing some simple tests and docs.

jon nunan’s picture

jon nunan commented
StatusFileSize
new 2879665-10-oauth2-auth-plugin.patch3.83 KB

Re-roll for 8.x-5.x

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs work » Closed (duplicate)

I don't think we can just replace this. We'll need to add it as a second plugin. Otherwise, we'd break existing sites.

Can we do that, and upload the results to the duplicate issue on gitlab: https://gitlab.com/drupalspoons/migrate_plus/-/issues/186

krisahil’s picture

krisahil commented
StatusFileSize
new 2879665-12-oauth2-auth-plugin.patch3.84 KB

Re-rolled against 6.0.x branch.

dbrowncode’s picture

dbrowncode commented
Status: Closed (duplicate) » Needs work
krisahil’s picture

krisahil commented
StatusFileSize
new 2879665-14-oauth2-auth-plugin.patch3.45 KB

Here's a patch that applies to releases 6.0.9 and 6.0.10.

heddn’s picture

heddn
Primary language English
Location Illinois
commented

Can we add this as a MR?

krisahil’s picture

krisahil commented
Version: 8.x-4.x-dev » 6.0.x-dev

krisahil opened merge request !153

jsacksick made their first commit to this issue’s fork.

jsacksick’s picture

jsacksick commented
Priority: Normal » Major

Bumping the priority because I'm currently unable to update my project due to the sainsburys/guzzle-oauth2-plugin library requiring firebase/php-jwt ~3.0 which is affected by a security advisory:

Problem 1
- Root composer.json requires sainsburys/guzzle-oauth2-plugin ^3.0 -> satisfiable by sainsburys/guzzle-oauth2-plugin[v3.0.0, ..., v3.0.7].
sainsburys/guzzle-oauth2-plugin[v3.0.0, ..., v3.0.1] require firebase/php-jwt ~3.0 -> found firebase/php-jwt[v3.0.0] but these were not loaded, because they are affected by security advisories ("PKSA-y2cr-5h3j-g3ys", "PKSA-2kqm-ps5x-s4f5"). Go to https://packagist.org/security-advisories/

jsacksick’s picture

jsacksick commented
Priority: Major » Normal

Disregard my previous message... I had another composer.json included from a custom module requiring the same library... So this wasn't coming from Migrate plus.

herved’s picture

herved commented

#19 Same here! That SA just popped up today I believe.
I need firebase/php-jwt > 7.0 but sainsburys/guzzle-oauth2-plugin v3.0.7 requires firebase/php-jwt (^3.0|^4.0|^5.0|^6.0).
sainsburys/guzzle-oauth2-plugin doesn't seem maintained anymore and at the moment the oauth2 plugin here relies on it. So this is major IMO.

herved’s picture

herved commented

#11 Do we really want to maintain the old plugin which relies on an insecure package? and if so, how to name the new plugin?

Edit: I'll create a different MR with a new plugin named oauth2_client (?), and mark the old plugin as deprecated.

herved opened merge request !156

herved’s picture

herved commented
StatusFileSize
new 2879665-24.diff7.76 KB

Static patch from MR156

herved changed the visibility of the branch 2879665-replace-oauth2-client to hidden.

herved’s picture

herved commented
Status: Needs work » Needs review
heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Needs work

Posted some comments on the MR.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs work » Reviewed & tested by the community

This is look pretty nice. About to merge.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Reviewed & tested by the community » Fixed

Change record published and code merged.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • heddn committed c6432058 on 6.0.x authored by herved 
    feat: #2879665 Replace OAuth2 client by the one from thephpleague

By:...

Status: Fixed » Closed (fixed)

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