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 Consumer Key double URL encoded #2065

Closed
j-duckworth opened this issue Apr 14, 2023 · 6 comments
Closed

OAuth1 Consumer Key double URL encoded #2065

j-duckworth opened this issue Apr 14, 2023 · 6 comments
Labels

Comments

@j-duckworth
Copy link

Describe the bug
When using OAuth1Authenticator.ForProtectedResource, exclamation marks in the Consumer Key are URL encoded twice causing authentication to fail.

To Reproduce
Authenticate a request using OAuth1Authenticator.ForProtectedResource with a consumer key that contains an exclamation mark.

I called the authenticator factory method like this and made a simple GET request:
OAuth1Authenticator.ForProtectedResource(_consumerKey, _consumerSecret, string.Empty, string.Empty, OAuthSignatureMethod.HmacSha256);

Using Fiddler, you should see that the key in the header contains %2521 instead of %21.

Expected behavior
This is the unit test that I wrote to show the issue:

    [Theory]
    [InlineData(OAuthType.ProtectedResource)]
    public void Authenticate_ShouldUriEncodeConsumerKey_OnHttpAuthorizationHeaderHandling(OAuthType type) {
        // Arrange
        const string url = "https://no-query.string";

        var client = new RestClient(url);
        var request = new RestRequest();
        _authenticator.Type = type;
        _authenticator.ConsumerKey = "my@consumer!key";
        _authenticator.ConsumerSecret = null;

        // Act
        _authenticator.Authenticate(client, request);

        // Assert
        var authParameter = request.Parameters.Single(x => x.Name == KnownHeaders.Authorization);
        var value = (string)authParameter.Value;

        Assert.NotNull(value);
        Assert.NotEmpty(value);
        Assert.Contains("OAuth", value!);
        Assert.Contains($"oauth_consumer_key=\"my%40consumer%21key", value);
    }

Stack trace
Copy the full stack trace here if you get an exception.

Desktop (please complete the following information):

  • OS: Windows 11
  • .NET version 4.7.2
  • Version 110.2.0

Additional context
The RFC 3986 encoding in UrlEncodeRelaxed encodes the exclamation mark to %21, then the RFC 2396 escaping encodes the resulting percent sign.

It should work if we do the RFC 2396 escaping first, because the RFC 3986 won't encode percentage signs. I'm not sure whether this would have any unexpected side effects, as the order seems deliberate.

@stale
Copy link

stale bot commented May 22, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@GreatBarrier86
Copy link

Is there any info on how to possibly work-around this? We can obviously use a regular HttpClient if we need to and manually generate the necessary token...etc, but it would be cool if this was on the roadmap to be fixed.

We've run into the same issue with Oracle NetSuite's !transform endpoint.

@alexeyzimarev
Copy link
Member

I would expect someone who is using this feature to spend a bit of time fixing it.

@GreatBarrier86
Copy link

GreatBarrier86 commented Jul 8, 2023

I'd be willing to do a PR for it, but for @alexeyzimarev specifically, should reversing the order of the escaping be satisfactory? I don't know the library well enough to know if that might have side effects as @j-duckworth mentioned,

The reason i ask is because a comment in UrlEncodeRelaxed here specifically says to do RFC3986 first but no reason as to why.

@alexeyzimarev
Copy link
Member

I would start the PR by adding the test and then you can make it pass. Apparently, current OAuth1 tests pass but they seem to be incomplete? It's not my code, and I don't use OAuth1, so I am not even sure where the issue is.

@alexeyzimarev
Copy link
Member

Apparently, the order of escaping in UrlEncodeRelaxed was just wrong. It first replaces RFC 3986 characters, and then calls EscapeDataString, which then escapes the percent sign. I don't think it ever worked.

As you mentioned, changing the order fixed the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants