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

Oauth1 server support, RSA-SHA1 signature method and important bug fixes #4

Open
wants to merge 4 commits into
base: fork/nbspou
Choose a base branch
from

Conversation

hoylen
Copy link

@hoylen hoylen commented Jul 9, 2019

This pull request makes changes to allow OAuth1 servers to be implemented using the library (i.e. not just OAuth clients), and implements the RSA-SHA1 signature method.

It also fixes several bugs. The most significant is that it replaced the usage of Uri.encodeComponent with a correct implementation of the percent encoding algorithm from section 3.6 of RFC 5849. When the parameters contained certain characters, it wasn't producing the correct signature base string and therefore didn't produce the correct signatures.

Also, parameters now support multiple values correctly. Previously, it would discard/ignore multiple values with the same parameter name, and produce the wrong signature. Therefore, the requests will be rejected by correctly implemented OAuth1 servers.

Changes

I've tried to keep the changes backward compatible. But this was not possible in some cases.

But most of the changes have been made to the "AuthorizationRequest" class, and that was an internal class - so most users of the library should not be greatly affected. The class used to be called AuthorizatonHeader, but that was not technically correct. The signed parameters do not just come from the header, and OAuth protocol parameters do not have to be transmitted in a HTTP Authorization header. The new name makes more sense for both clients and servers.

Client credentials for RSA-SHA1 was also tricky. The old classes worked for a shared secret, which is the same for both the client and server; but doesn't work when the client must have one type of credentials (RSA private key) and the server must have a different type of credential (RSA public key).

Issues

RSA-SHA1 requires an implementation of RSA. The pointycastle package was used for this.

The problem is, the package is large which makes command line Dart programs take about 2 seconds longer to start up. This is an unnecessary impact on programs that don't use RSA-SHA1.

The problem might not be so bad for Web applications, since Dart's tree-shaking JavaScript compiler might be able to exclude the RSA code, if it doesn't get used (this needs to be checked).

I think a better solution is to refactor the RSA-SHA1 implementation into its own package. But to do this elegantly, breaking changes will be needed for the ClientCredentials and SignatureMethod classes.

@kaetemi
Copy link
Member

kaetemi commented Jul 20, 2020

Interesting! I'll try this out sometime, thanks!

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.

2 participants