#903770: Integrate with Mollom moderation introduced some code to validate 2-legged OAuth requests to moderate local site content from mollom.com.

Properly validating inbound OAuth requests, even if only 2-legged, isn't particularly easy and therefore shouldn't happen in a few lines of custom code (which needs to be maintained).

Ideally, we should move the Mollom Moderation integration into a sub-module that depends on http://drupal.org/project/oauth

However, I've looked into OAuth module and it's definitely not in a good shape currently:

  1. The OAuth provider code has a (huge) dependency on CTools. That's unacceptable for Mollom module's use-case, and also a bit stupid, since CTools is only required for importing OAuth contexts/provider definitions defined in code.
  2. There is no built-in API functionality to support 2-legged OAuth. The module only targets 3-legged OAuth currently.
  3. The module has been rewritten into major version 3.x recently; to improve Services module integration and incorporate the functionality of some third-party OAuth modules, which caused quite some confusion in the past. However, the entire code base looks very rough and bogus at this point, and has no tests at all.
    Example: #1237022: oauth_common_form_authorization has wrong argument list
  4. It contains plenty of code that's not leveraging existing core functionality for unknown reasons (raw cURL instead of drupal_http_request()), and in general, uses un-Drupalish code in various places (sprintf(), etc).
    Example: #1308368: DrupalOauthClient::get() does not send POST parameters
  5. There's no documentation about its (3.x) API and how it's supposed to be used, so any implementation relying on the current code involves a high risk of being outdated/bogus.

Recommendation

  • Keep (, complete, and fix) the custom code in Mollom module, as long as we need 2-legged OAuth only.
  • As soon as we need 3-legged OAuth, invest (quite) some resources to fix/rewrite OAuth module, and move the Mollom Moderation integration into a sub-module.

Comments

sun’s picture

Issue tags: +API change
sun’s picture

Status: Postponed » Closed (won't fix)