Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for custom signature methods to oauth1.Client #239

Merged
merged 2 commits into from
Apr 12, 2014
Merged

Add support for custom signature methods to oauth1.Client #239

merged 2 commits into from
Apr 12, 2014

Conversation

al-the-x
Copy link
Contributor

In attempting to connect to Pearson's Learning Center LMS product using their custom OAuth 1.0A Provider implementation via requests + requests-oauthlib + oauthlib, I need the ability to define a custom signature method for requests made via oauth1.Client. Pearson requires CMAC-AES encryption to sign requests and does so in a manner wholly different from the standard HMAC-SHA1 method defined in the RFC. However, this is their prerogative as the Provider, per the RFC:

OAuth provides three methods for the client to prove its rightful ownership of the credentials: "HMAC-SHA1", "RSA-SHA1", and "PLAINTEXT". These methods are generally referred to as signature methods, even though "PLAINTEXT" does not involve a signature. In addition, "RSA-SHA1" utilizes an RSA key instead of the shared-secrets associated with the client credentials.

OAuth does not mandate a particular signature method, as each implementation can have its own unique requirements. Servers are free to implement and document their own custom methods. Recommending any particular method is beyond the scope of this specification. Implementers should review the Security Considerations section (Section 4) before deciding on which method to support.

Currently, oauth1.Client only permits signature methods HMAC-SHA1, RSA-SHA1 and PLAINTEXT via hard-coded conditional statements; see:

Proposed

In order to support custom signature methods, oauth.Client should provide an API for registering new signature methods by name. For example, to provide PLAINTEXT method:

from oauthlib.oauth1 import Client

## Register the PLAINTEXT signature method as a callback...
Client.register_signature_method("PLAINTEXT", 
    lambda client, base_string: 
        return base_string)

Then oauth1.Client.sign should look up the signature method in the registered methods and invoke the registered callable, throwing ValueError if the value of oauth_signature_method has not been registered. Callables passed to oauth1.Client.register_signature_method should accept the client instance (self) and the base_string to be signed, as whatever information the signature method needs should be available as a property on the client instance. The current signature method functions – sign_hmac_sha1, sign_rsa_sha1, and sign_plaintext – can easily be refactored to match this signature.

Any existing tests for oauth1.Client or the sign_* functions should be updated accordingly, and new tests for oauth1.Client.register_signature_method should be added, along with supporting tests (re #28).

@al-the-x
Copy link
Contributor Author

Sorry for the verbosity on this issue, folks, but I'm using this with a client and want to cover all my bases. Please treat as an RFC for proposed solution. Suggestions for improvement are welcomed, as I'm actively working on the code.

@ib-lundgren
Copy link
Collaborator

I definitely think this is worth doing and the proposed solution sounds good :)

I might not have much time to help out coding until summer but definitely able to answer any questions you might have.

@al-the-x
Copy link
Contributor Author

@ib-lundgren No worries. I can handle it, I think, although I'll happily accept advice as I go! Thanks!

@al-the-x
Copy link
Contributor Author

Looks like test coverage for the verify_*() functions is missing. Since verify_hmac_sha1() utilizes sign_hmac_sha1(), I started down the path of writing some tests, but I couldn't get the base_string calculated by verify_hmac_sha1() to match the expected control_base_string in the tests. I'm probably doing something wrong.

In order to support adding custom signature methods, the current signature methods -- HMAC-SHA1, RSA-SHA1, and PLAINTEXT -- need to be implemented with a common interface. In a previous attempt, I tried changing those functions directly, but there are too many dependencies on their current signatures. By shimming them instead with these thin wrappers, I can provide the common interface I need without breaking everything else in the library.
The "PIZZA" signature method signs all requests with the string "PIZZA" as a trivial example of a custom signing method.
@al-the-x
Copy link
Contributor Author

Change of strategy: since so much else in the library relies on the current sign_*() methods, I've wrapped them with the proposed signature instead. PR shortly.

@ib-lundgren
Copy link
Collaborator

Looking forward to it :)

On Tue, Mar 25, 2014 at 10:04 PM, David Rogers notifications@github.comwrote:

Change of strategy: since so much else in the library relies on the
current sign_*() methods, I've wrapped them with the proposed signature
instead. PR shortly.

Reply to this email directly or view it on GitHubhttps://github.com//issues/239#issuecomment-38627024
.

@al-the-x
Copy link
Contributor Author

Attached the PR to this issue, as you can still do that on Github. Instead of refactoring the sign_*() functions directly, I've shimmed them with e.g. sign_hmac_sha1_with_client() that has a different signature. These sign_*_with_client() functions are registered via oauth1.Client.register_signature_method(), a @classmethod, that adds the callable into a dict. This allows a programmer to override an existing signature method, if necessary, and use a custom signature method with all instances of oauth1.Client. Please advise. Thanks.

@ib-lundgren
Copy link
Collaborator

Looks great @al-the-x! Very neat :) Opened an issue to add similar functionality to the server side as well.

Sorry it took me so long to get back to you. I will go ahead and merge this in now. Let me know if you would like to be added to AUTHORS (via a PR or comment).

ib-lundgren added a commit that referenced this pull request Apr 12, 2014
Add support for custom signature methods to `oauth1.Client`
@ib-lundgren ib-lundgren merged commit 6450527 into oauthlib:master Apr 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants