Description: Drupal module for OAuth2 clients. It is based (depends) on the library https://github.com/adoy/PHP-OAuth2, and is supposed to work well with https://drupal.org/project/oauth2_server.
Sandbox: https://drupal.org/sandbox/dashohoxha/2133827
Git: git clone --branch 7.x-1.0 http://git.drupal.org/sandbox/dashohoxha/2133827.git oauth2_client

Comments

dashohoxha’s picture

dashohoxha commented
Title: [D7] oauth2_client » [D7] OAuth2 Client
PA robot’s picture

PA robot commented
Status: Needs review » Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxdashohoxha2133827git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

dashohoxha’s picture

dashohoxha commented
Priority: Normal » Major
Status: Needs work » Needs review

I fixed the "errors" detected by the automatic reviewer. Some of them are quite stupid and I think that the automatic reviewer needs a review itself. Anyway, now it is clean and can be reviewed.

neerajskydiver’s picture

neerajskydiver commented

you should not use "master" branch in git. You should create a version specific branch.

update your git clone link, it should be like this
git clone --branch master http://git.drupal.org/sandbox/dashohoxha/2133827.git oauth2_client

in .module file instead of 

      $message = t('The PHP-OAuth2 client library is required for the oauth2_client module to function. Download the library from <a href="https://github.com/adoy/PHP-OAuth2" target="_blank">GitHub</a> and place it in <em>!path</em>.', array('!path' => $path));
      drupal_set_message($message, 'error');

you can use like this
drupal_set_message(t('The PHP-OAuth2 client library is required for the oauth2_client module to function. Download the library from <a href="https://github.com/adoy/PHP-OAuth2" target="_blank">GitHub</a> and place it in <em>!path</em>.', array('!path' => $path)), 'error');

And also

Line 168: Remove PHP debugging function calls. [production_php]
print '<xmp>'; print_r($e); print '</xmp>';

severity: normalLine 177: Remove PHP debugging function calls. [production_php]
print '<xmp>'; print_r($response); print '</xmp>';

use proper comments in oauth2_client.inc file
https://drupal.org/node/1354

dashohoxha’s picture

dashohoxha commented
Issue summary: View changes
dashohoxha’s picture

dashohoxha commented

@neerajskydiver, thanks for reviewing it.
I have created the branch 7.x-1.0 and updated the git clone link.
I have also corrected the drupal_set_message(), just to satisfy your request (I don't think it's important).

Regarding debug functions, they are part of a testing function and should not be removed. However I separated the testing functionality into another module (oauth2_client_test), and I hope that this makes the code of the main module more clean and clear.

About the comments in oauth2_client.inc, it seems to me that it is properly and extensively commented.

By the way, I am not getting notifications for the new comments on this post. Does anybody know why? Is this a bug introduced by the latest upgrade of drupal.org, or maybe I have to fix anything on my settings? Thanks.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
Priority: Major » Normal
Status: Needs review » Postponed (maintainer needs more info)
Patch existing modules instead?

It seems that Oauth and Oauth2 provide very similar functionality. Could you describe how your module differs?

We prefer collaboration over competition, therefore we want to prevent having duplicating modules on drupal.org. If the differences between these modules are not too fundamental for patching the existing one, we would love to see you joining forces and concentrate all power on enhancing one module. (If the existing module is abandoned, please think about taking it over).

If that fails for whatever reason please get back to us and set this back to "needs review".

I did not do a code review.

----
Top Shelf Modules - Crafted, Curated, Contributed.

dashohoxha’s picture

dashohoxha commented
Status: Postponed (maintainer needs more info) » Needs review

First of all, sorry for the late reply. I don't get notifications for the updates to this issue and I still don't know why.

Ask yourself, why there is a module Oauth2 while already is a module Oauth? Is this not against the principle of collaboration over competition? The truth is that Oauth is about version 1, which is totally different from version 2 (Oauth2). Some people claim that version 2 is not as secure as version 1, and maybe they are right, but the fact is that Oauth2 has become the standard everywhere, with Google, Facebook, Twitter, GitHub etc. using it. It was rather surprising for me that Drupal still does not have a proper implementation of Oauth2.

Now, besides the module Oauth2, there is also a module oauth2_server. Ask yourself, is this not against the principle of collaboration over competition? Why oauth2_server was allowed to become a full module, while there was already the module Oauth2? I don't know about the last question, but I will give my reasons.

The module Oauth2 tries to implement both a server and a client, and also other things as well (for example proxy). This is not the right approach (in my opinion) because a Drupal application is usually either an oauth2 server, or an oauth2 client. It is also a bit bloated and difficult to understand what is going on.

I had to implement an oauth2 server and I had difficulties doing it with the module oauth2. It even says in its README:

WARNING! This version is not suitable for production use yet!

On the other hand, the module oauth2_server worked better, was more easy to understand, and also had better documentation. So I went with it. After implementing the server, I needed an oauth2 client. I could have done something just to solve my problem, but I decided to do something more general, that can be used by everybody, although it took me more time to implement (and also is taking some headaches to submit it as a full project and to convince everybody that it is OK).

Another reason is that Oauth2 is based on the library oauth2-php, which supports oauth-2.09. On the other hand, the module oauth2_server is based on the library oauth2-server-php, which supports oauth-2.15, and the module oauth2_client is based on the library PHP-OAuth2. So, they are based on different PHP libraries and cannot just extend each-other, they have to be different solutions.

In my opinion, the combination oauth2_server + oauth2_client makes a better solution than the module oauth2. The module oauth2 simply didn't work for me, so how could I extend something that does not work and I could not understand.

bojanz’s picture

bojanz commented

I will review this today.

There is no duplication, there is currently no oauth2 client module for Drupal.
The oauth2 namespace is controlled by people who don't want to share access or accept patches, my own oauth2_server module was created as a new project because of that (after many attempts to connect, even personal communication via backchannels and d.o admins).

Thank you for your efforts dashohoxha.

dashohoxha’s picture

dashohoxha commented

Thanks bojanz. Let me know, if you find something that could be improved. If you find it OK, then please change the status to 'Reviewed and Tested By the Community'.

In general, the module is very simple, it just manages an access_token for each oauth2 client. It is so simple because it is designed to be integrated with other modules and not to repeat the work done by them. For example it can be integrated with the modules http_client and wsclient. I have also written patches for these modules, please help to check and review them as well: #2138617: OAuth2 authentication for http_client #2138627: Create plugin for OAuth2 authentication

PA robot’s picture

PA robot commented
Status: Needs review » Closed (duplicate)
Multiple Applications
It appears that there have been multiple project applications opened under your username:

Project 1: https://drupal.org/node/2164201

Project 2: https://drupal.org/node/2138549

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Related issues: +#2164201: [D7] OAuth2 Login Provider