{"action":"created","issue":{"url":"https://api.github.com/repos/aspnet/Security/issues/1695","repository_url":"https://api.github.com/repos/aspnet/Security","labels_url":"https://api.github.com/repos/aspnet/Security/issues/1695/labels{/name}","comments_url":"https://api.github.com/repos/aspnet/Security/issues/1695/comments","events_url":"https://api.github.com/repos/aspnet/Security/issues/1695/events","html_url":"https://github.com/aspnet/Security/issues/1695","id":304903966,"number":1695,"title":"Twitter Handler OAuth Signing code is incorrect","user":{"login":"speige","id":2002030,"avatar_url":"https://avatars1.githubusercontent.com/u/2002030?v=4","gravatar_id":"","url":"https://api.github.com/users/speige","html_url":"https://github.com/speige","followers_url":"https://api.github.com/users/speige/followers","following_url":"https://api.github.com/users/speige/following{/other_user}","gists_url":"https://api.github.com/users/speige/gists{/gist_id}","starred_url":"https://api.github.com/users/speige/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/speige/subscriptions","organizations_url":"https://api.github.com/users/speige/orgs","repos_url":"https://api.github.com/users/speige/repos","events_url":"https://api.github.com/users/speige/events{/privacy}","received_events_url":"https://api.github.com/users/speige/received_events","type":"User","site_admin":false},"labels":[{"id":93052239,"url":"https://api.github.com/repos/aspnet/Security/labels/1%20-%20Ready","name":"1 - Ready","color":"CCCCCC","default":false},{"id":85920071,"url":"https://api.github.com/repos/aspnet/Security/labels/bug","name":"bug","color":"fc2929","default":true}],"state":"open","locked":false,"assignee":{"login":"Tratcher","id":1821173,"avatar_url":"https://avatars3.githubusercontent.com/u/1821173?v=4","gravatar_id":"","url":"https://api.github.com/users/Tratcher","html_url":"https://github.com/Tratcher","followers_url":"https://api.github.com/users/Tratcher/followers","following_url":"https://api.github.com/users/Tratcher/following{/other_user}","gists_url":"https://api.github.com/users/Tratcher/gists{/gist_id}","starred_url":"https://api.github.com/users/Tratcher/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Tratcher/subscriptions","organizations_url":"https://api.github.com/users/Tratcher/orgs","repos_url":"https://api.github.com/users/Tratcher/repos","events_url":"https://api.github.com/users/Tratcher/events{/privacy}","received_events_url":"https://api.github.com/users/Tratcher/received_events","type":"User","site_admin":false},"assignees":[{"login":"Tratcher","id":1821173,"avatar_url":"https://avatars3.githubusercontent.com/u/1821173?v=4","gravatar_id":"","url":"https://api.github.com/users/Tratcher","html_url":"https://github.com/Tratcher","followers_url":"https://api.github.com/users/Tratcher/followers","following_url":"https://api.github.com/users/Tratcher/following{/other_user}","gists_url":"https://api.github.com/users/Tratcher/gists{/gist_id}","starred_url":"https://api.github.com/users/Tratcher/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Tratcher/subscriptions","organizations_url":"https://api.github.com/users/Tratcher/orgs","repos_url":"https://api.github.com/users/Tratcher/repos","events_url":"https://api.github.com/users/Tratcher/events{/privacy}","received_events_url":"https://api.github.com/users/Tratcher/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/aspnet/Security/milestones/29","html_url":"https://github.com/aspnet/Security/milestone/29","labels_url":"https://api.github.com/repos/aspnet/Security/milestones/29/labels","id":2918445,"number":29,"title":"2.2.0","description":null,"creator":{"login":"Eilon","id":202643,"avatar_url":"https://avatars3.githubusercontent.com/u/202643?v=4","gravatar_id":"","url":"https://api.github.com/users/Eilon","html_url":"https://github.com/Eilon","followers_url":"https://api.github.com/users/Eilon/followers","following_url":"https://api.github.com/users/Eilon/following{/other_user}","gists_url":"https://api.github.com/users/Eilon/gists{/gist_id}","starred_url":"https://api.github.com/users/Eilon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Eilon/subscriptions","organizations_url":"https://api.github.com/users/Eilon/orgs","repos_url":"https://api.github.com/users/Eilon/repos","events_url":"https://api.github.com/users/Eilon/events{/privacy}","received_events_url":"https://api.github.com/users/Eilon/received_events","type":"User","site_admin":false},"open_issues":16,"closed_issues":0,"state":"open","created_at":"2017-11-16T22:27:22Z","updated_at":"2018-04-13T17:32:38Z","due_on":null,"closed_at":null},"comments":6,"created_at":"2018-03-13T19:27:59Z","updated_at":"2018-04-13T17:32:50Z","closed_at":null,"author_association":"NONE","body":"I'm not aware of a specific bug caused by this. My guess is it work 99% of the time with the current api calls, but if it were used for any other twitter api calls, they'd all fail.\r\n\r\nIf you read the documentation here:\r\nhttps://developer.twitter.com/en/docs/basics/authentication/guides/creating-a-signature\r\nyou'll see that the code here doesn't match:\r\nhttps://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterHandler.cs\r\n\r\nOne example is: All uses of UrlEncoder.Encode should be replaced with Uri.EscapeDataString\r\nThere are several others. I found this because I copied the code in order to call other twitter apis & they failed until I made corrections.\r\n\r\nIn TwitterHandler.cs all the methods have a slightly different copy/pasted OAuth code. I've extracted it to a generic method. I've tested & it is working on several different twitter apis, I haven't tested it on all of them, so it's possible there's still an issue somewhere (for example, I expect all data to be in the query params, but some apis may have a body instead).\r\n\r\nThe existing requests in TwitterHandler would need to be refactored to use this generic method. I haven't coded that since I'm using the built-in version for authentication & my version for extra api calls.\r\n\r\n```\r\nusing System;\r\nusing System.Collections.Generic;\r\nusing System.Collections.Immutable;\r\nusing System.Globalization;\r\nusing System.Linq;\r\nusing System.Net.Http;\r\nusing System.Security.Claims;\r\nusing System.Security.Cryptography;\r\nusing System.Text;\r\nusing System.Text.Encodings.Web;\r\nusing System.Threading.Tasks;\r\nusing Microsoft.AspNetCore.Authentication.Twitter;\r\nusing Newtonsoft.Json.Linq;\r\n\r\nnamespace Microsoft.AspNetCore.Authentication.Twitter\r\n{\r\n\tpublic class TwitterHandler\r\n\t{\r\n\t\tprotected string ComputeSignature(string consumerSecret, string tokenSecret, string signatureData)\r\n\t\t{\r\n\t\t\tusing (HMACSHA1 algorithm = new HMACSHA1())\r\n\t\t\t{\r\n\t\t\t\talgorithm.Key = Encoding.ASCII.GetBytes(string.Format(CultureInfo.InvariantCulture, \"{0}&{1}\", Uri.EscapeDataString(consumerSecret), string.IsNullOrEmpty(tokenSecret) ? string.Empty : Uri.EscapeDataString(tokenSecret)));\r\n\t\t\t\treturn Convert.ToBase64String(algorithm.ComputeHash(Encoding.ASCII.GetBytes(signatureData)));\r\n\t\t\t}\r\n\t\t}\r\n\r\n\t\tprotected async Task<JObject> ExecuteRequestAsync(AccessToken accessToken, ClaimsIdentity identity, string url, HttpMethod httpMethod, Dictionary<string, string> queryParameters = null)\r\n\t\t{\r\n\t\t\tStringBuilder canonicalizedRequestBuilder = new StringBuilder();\r\n\t\t\tcanonicalizedRequestBuilder.Append(httpMethod.Method);\r\n\t\t\tcanonicalizedRequestBuilder.Append(\"&\");\r\n\t\t\tcanonicalizedRequestBuilder.Append(Uri.EscapeDataString(url));\r\n\r\n\t\t\tSortedDictionary<string, string> authorizationParts = new SortedDictionary<string, string>()\r\n\t\t\t{\r\n\t\t\t\t{ \"oauth_consumer_key\", Options.ConsumerKey },\r\n\t\t\t\t{ \"oauth_nonce\", Guid.NewGuid().ToString(\"N\") },\r\n\t\t\t\t{ \"oauth_signature_method\", \"HMAC-SHA1\" },\r\n\t\t\t\t{ \"oauth_timestamp\", GenerateTimeStamp() },\r\n\t\t\t\t{ \"oauth_token\", accessToken.Token },\r\n\t\t\t\t{ \"oauth_version\", \"1.0\" }\r\n\t\t\t};\r\n\r\n\t\t\tImmutableSortedDictionary<string, string> signatureParts = (queryParameters ?? new Dictionary<string, string>()).Union(authorizationParts).ToImmutableSortedDictionary(x => x.Key, x => x.Value);\r\n\t\t\tstring parameterString = signatureParts.Select(x => string.Format(\"{0}={1}\", Uri.EscapeDataString(x.Key), Uri.EscapeDataString(x.Value))).Aggregate((x, y) => string.Format(\"{0}&{1}\", x, y));\r\n\t\t\tcanonicalizedRequestBuilder.Append(\"&\");\r\n\t\t\tcanonicalizedRequestBuilder.Append(Uri.EscapeDataString(parameterString));\r\n\r\n\t\t\tstring signature = ComputeSignature(Options.ConsumerSecret, accessToken.TokenSecret, canonicalizedRequestBuilder.ToString());\r\n\t\t\tauthorizationParts.Add(\"oauth_signature\", signature);\r\n\r\n\t\t\tstring queryString = (queryParameters != null && queryParameters.Any() ? \"?\" + queryParameters.Select(x => x.Key + \"=\" + x.Value).Aggregate((x, y) => x + \"&\" + y) : \"\");\r\n\t\t\tHttpRequestMessage request = new HttpRequestMessage(httpMethod, url + queryString);\r\n\t\t\trequest.Headers.Add(\"Authorization\", \"OAuth \" + authorizationParts.Select(x => string.Format(\"{0}=\\\"{1}\\\"\", x.Key, Uri.EscapeDataString(x.Value))).Aggregate((x, y) => string.Format(\"{0},{1}\", x, y)));\r\n\r\n\t\t\tHttpResponseMessage response = await _client.SendAsync(request);\r\n\t\t\tif (!response.IsSuccessStatusCode)\r\n\t\t\t{\r\n\t\t\t\tresponse.EnsureSuccessStatusCode(); // throw\r\n\t\t\t}\r\n\r\n\t\t\tstring responseText = await response.Content.ReadAsStringAsync();\r\n\t\t\treturn JObject.Parse(responseText);\r\n\t\t}\r\n\t}\r\n}\r\n```"},"comment":{"url":"https://api.github.com/repos/aspnet/Security/issues/comments/381207389","html_url":"https://github.com/aspnet/Security/issues/1695#issuecomment-381207389","issue_url":"https://api.github.com/repos/aspnet/Security/issues/1695","id":381207389,"user":{"login":"Eilon","id":202643,"avatar_url":"https://avatars3.githubusercontent.com/u/202643?v=4","gravatar_id":"","url":"https://api.github.com/users/Eilon","html_url":"https://github.com/Eilon","followers_url":"https://api.github.com/users/Eilon/followers","following_url":"https://api.github.com/users/Eilon/following{/other_user}","gists_url":"https://api.github.com/users/Eilon/gists{/gist_id}","starred_url":"https://api.github.com/users/Eilon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Eilon/subscriptions","organizations_url":"https://api.github.com/users/Eilon/orgs","repos_url":"https://api.github.com/users/Eilon/repos","events_url":"https://api.github.com/users/Eilon/events{/privacy}","received_events_url":"https://api.github.com/users/Eilon/received_events","type":"User","site_admin":false},"created_at":"2018-04-13T17:32:50Z","updated_at":"2018-04-13T17:32:50Z","author_association":"OWNER","body":"Moving to 2.2.0 because we just don't have time to get this into 2.1.0."}}
{"actor":{"display_login":"Eilon"}}