Adding support for DrupalOAuth2 provider

A small fix on the previous patch: Used field keys_id for client_id (instead of keys_key).
This patch must be applied after the first one.

The patch on HybridAuth library has been applied already (https://github.com/hybridauth/hybridauth/pull/148). I am also a maintainer of that library now.
Can we apply these two patches as well? I can also help with maintaining this module (as much as I can). In particular, I would like to see different providers having their own modules (instead of being hardcoded into the main module).

Thanks Andy.

I believe that it is better not to include the DrupalOAuth2 provider inside this module.
I am going to create a new Drupal module for it, called hybridauth_drupaloauth2. Until yesterday I could not promote modules to drupal.org, but now I can. I don't think it should be included inside https://drupal.org/project/oauth2_loginprovider, because this one is a server module, hybridauth_drupaloauth2 is a client module, so it is better to keep them separate.

I also think that some other providers should be separated into their own modules, because this would make the core module cleaner and easier to maintain. However you are the maintainer, so you decide about it.

AndyB,

I intend to create and maintain only one module, namely hybridauth_DrupalOAuth2. You can see it on the patch or from github (it is not yet on drupal.org): https://github.com/dashohoxha/hybridauth_DrupalOAuth2

I have seen in the code of hybridauth that specific configurations are made for Google, for Facebook, etc. This just bloats the code. Instead the hooks can be used to provide specific configurations for specific providers. These hooks are:

• hook_form_hybridauth_admin_provider_settings_alter(&$form, &$form_state, $form_id)
• hook_hybridauth_provider_config_alter(&$config, $provider_id)

You can see here how I have used them: https://github.com/dashohoxha/hybridauth_DrupalOAuth2/blob/7.x-1.x/hybri...
So, the idea is to get out of the main code things that are specific to a certain provider.

But there is a problem here. The patch that I submitted includes also icons for the new provider and css code for it (which you have not committed). I totally forgot about them since I have done this patch a long time ago. You have to find a way in your module to make icons and css code provider specific as well. For example, if I store them in the provider module (hybridauth_DrupalOAuth2), the core module (hybridauth) should be able to pick them from there.

AndyB, I understand you.
If you want to keep the configuration code of main providers (FB, Google, etc.) in the core module, that is fine for me. You are the maintainer, so you decide what is better and easier for you.

But I think that you cannot afford to include into the core module the code for special configurations that are needed for all the providers that are out there. An example is the provider DrupalOAuth2, which needs to use special configuration fields for hybridauth. I think you agree with this.

Fortunately, Drupal has a powerful hook system, which allows modules to customize and extend other modules. This is what I have used in the module hybridauth_DrupalOAuth2 to get the job done without needing to ask from you special configurations in the core module (hybridauth) in order to support the provider DrupalOAuth2. Thanks for committing the modifications that make this possible (about config alter etc.)

However the job is not completed because I cannot define a CSS and an icon for this provider, without asking you to add them into the core module. I beleive that this problem can be solved using the hooks and other tools that Drupal provides. Can I try to find a solution for this and then submit a patch about it?

Regards,
Dashamir

AndyB,

I tried but couldn't find anything that works well.

I think that ctools plugin system makes it easy for themers to create a new set of icons, and if needed, they can include in this set the icon for DrupalOAuth2. Meanwhile I have added some icons to the project: https://github.com/dashohoxha/hybridauth_DrupalOAuth2/tree/7.x-1.x/icons

If you can find a better solution, please let me know.

Thanks,
Dashamir

AndyB,

Thanks for catching those 2 lines of code. Maybe I have not been sure how the keys_id and keys_key were being used by the library, so I have decided to set both of them. However keys_key is not actually needed. I just tested the latest code of hybridauth (with these 2 lines removed) and it worked well.

So, the good news is that the plugin that you have added for DrupalOAuth2 is working fine (I just tested it).

The other good news is that a separate module also works fine, if you add a css file to it (for the icons) and load the css file on hook_init(), like this:
- https://github.com/dashohoxha/hybridauth_DrupalOAuth2/blob/7.x-1.x/hybri...
- https://github.com/dashohoxha/hybridauth_DrupalOAuth2/blob/7.x-1.x/hybri...
I just tested this solution as well (removed the plugin and installed the module, and it worked, icons were displayed correctly).

However I am not sure how well both of these two solutions work with the plugin icon_pack. I didn't test it, but I think that both of them don't work well. Anyway this should not be a big problem, because a customized theme can always override the default css code of the modules (and the icons as well).

Another drawback of both solutions is that drupal_add_css() is deprecated and is removed in Drupal8 (as far as I know).

In general, I am not familiar with ctool plugins, so I don't see what advantages they have compared to a Drupal module. I think that a Drupal module is designed to be like a plugin unit: you need to add some functionality to an application, you enable a module, and that's it. Why to add extra complexity? (I don't know yet all the rules for building Drupal modules, now I should learn the rules for building ctool plugins as well!). Also, in a Drupal module I can check for the existence of the library provider on which this module depends (the hybridauth provider for DrupalOAuth2), and give instructions for how to get and install it, if needed. (Maybe this can be done on a ctools plugin as well, I don't know.)

So, it is good to have two solutions. I would prefer the one with a module (either as a submodule of hybridauth, or as an independent module), because it seems to me more flexible, more standard solution, and provides better modularity. However the ctools plugin is OK as well. So, keep whichever you prefer.

Regards,
Dashamir