Creating oAuth request from django request is duplicated

Vladimir Mihailenco avatarVladimir Mihailenco created an issue

Currently I know at least 2 functions, that create oauth request from django request: initialize_server_request(request) and get_oauth_request(request), but there are some differences in their work. Is this is done intentionally? Can initialize_server_request can be refactored to use get_oauth_request()?

As result CheckOAuth creates two different oAuth requests. It is wasteful and can be confusing.

I can contribute these changes if needed.

Comments (5)

  1. Vladimir Mihailenco
        # Don't include extra parameters when request.method is POST and 
        # request.MIME['CONTENT_TYPE'] is "application/x-www-form-urlencoded" 
        # (See http://oauth.net/core/1.0a/#consumer_req_param).
        # But there is an issue with Django's test Client and custom content types
        # so an ugly test is made here, if you find a better solution...
    

    AFAIK at least Google Chrome JavaScript implementation does not make any special case for none "application/x-www-form-urlencoded" parameters:

    ChromeExOAuth.prototype.sendSignedRequest = function(url, callback,
                                                         opt_params) {
      var method = opt_params && opt_params['method'] || 'GET';
      var body = opt_params && opt_params['body'] || null;
      var params = opt_params && opt_params['parameters'] || {};
      var headers = opt_params && opt_params['headers'] || {};
    
      var signedUrl = this.signURL(url, method, params);
    
      ChromeExOAuth.sendRequest(method, signedUrl, headers, body, function (xhr) {
        if (xhr.readyState == 4) {
          callback(xhr.responseText, xhr);
        }
      });
    };
    

    For sure I can pass it explicitly like 'body', but I think in 99% none will bother himself with such details. So I propose by default not to be too smart in this case and just pass request.REQUEST to python-oauth2.

  2. Vladimir Mihailenco

    Pull request: https://bitbucket.org/david/django-oauth-plus/pull-request/1/issue-3

    I have removed duplication and made code PEP-8 compliant. The biggest change is handling "application/x-www-form-urlencoded" requests. As I said:

    - Google Chrome does not check content_type at all. I did not check other implementations.

    - You are currently added quick fix for POST and not testserver environment, which is ugly + there are also PUT requests.

    - Parameters remain parameters whether they are urlencoded or JSON-encoded or whatever.

  3. David Larlet
    • changed status to open

    Hi Vladimir,

    Thanks for your bug report(s) and fixes. I'll take a look at your suggestions carefully. Are you sure that Google Chrome JavaScript is doing the right thing here? I wonder if it's standardized like that in the spec.

    Cheers, David

  4. Log in to comment
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.