AspNet.Security.OAuth.Providers icon indicating copy to clipboard operation
AspNet.Security.OAuth.Providers copied to clipboard

Mixcloud Does Not Appear to Support the `state` Parameter

Open Mike-E-angelo opened this issue 4 years ago โ€ข 22 comments

Describe the bug In the Mixcloud provider implementation, efforts are being made to "bundle" the state parameter and then "unbundle" it upon authentication. The reason for this is that it appears that Mixcloud does not currently support the state parameter and does not pass it back when the code exchange occurs.

Steps To reproduce Use the provider without the state bundling as exists. The provider complains that the state cannot be read. This is because it is not provided/returned back from Mixcloud.

Expected behaviour state is returned by Mixcloud.

Actual behaviour state is not returned, so bundling must take place. Honestly, I can't believe I got that to work. ๐Ÿ˜… Ideally, we can get this resolved on Mixcloud's side and remove the bundling/unbundling as currently implemented. If this occurs, I can create a PR to get things working as expected.

System information: N/A

Additional context I have opened a ticket with Mixcloud support and they have assigned the issue to an internal developer. Creating this ticket to properly capture the issue and also allow for Mixcloud to collaborate/input here if needed.

Mike-E-angelo avatar Jun 25 '21 09:06 Mike-E-angelo

Hey @Mike-E-angelo , Mixcloudโ€™s customer support has forwarded me your emails and Iโ€™ll be looking into this as soon as I can

Iโ€™ll let you know here once I have more info/if weโ€™ve done the changes you asked :)

PS: I work for Mixcloud

Edit: This isn't forgotten, I had some time-off/got pulled elsewhere : )

miguelSWE avatar Jun 28 '21 07:06 miguelSWE

Hi @limaAniceto - any update on this from Mixcloud?

martincostello avatar Nov 12 '21 09:11 martincostello

FWIW I have reached out to Mixcloud's customer support to see if they are aware of any updates here.

Mike-E-angelo avatar Nov 15 '21 16:11 Mike-E-angelo

Alright, I have heard back from MixCloud customer support. This issue has been identified as a bug on their end and is slated to be worked on fixing it Soonโ„ข.

https://youtu.be/Bro-WwwroEA

No timetable/estimate, however.

Mike-E-angelo avatar Nov 16 '21 15:11 Mike-E-angelo

Cool - thanks for the update. I'll leave this issue open until we hear back from them.

martincostello avatar Nov 16 '21 15:11 martincostello

Hey @limaAniceto. Do you have (good) news for us? ๐Ÿ˜ƒ

kevinchalet avatar Apr 25 '22 17:04 kevinchalet

FWIW I am in active correspondence with MixCloud support (again :P ) to see what is happening here. I have to say I am a bit embarrassed by the developments here. A representative of a company updates a thread to let everyone know that this issue "isn't forgotten", and then ghosts the thread to do exactly that. It reflects poorly not only on the organization but also on me for being the one to loop them in and recommend them. That's how I feel, at least.

I get that this is open source and everyone is working for free, but it's been nearly a year now with multiple back-and-forths and one's patience can only reasonably last for so long. At least, that's the case for me. :P

The OTHER issue is that upon further usage of MixCloud's API I found a rather dubious design flaw that prohibited me from actively using it with my own project, which was the whole reason I engaged in the PR in the first place. Namely, their unique identifier is the user name. They are both one and the same. You change one, they both change. So, if someone changes their user name, they also change their "globally" unique identifier and how you identify them in your system. There appears to be real unique identifier (GUID) but that is not exposed via their API, just the user name. As there is no "event" to correlate the two values, you essentially get a new account without any easy/obvious way of referencing the old one.

I raised this with MixCloud and it's been the same treatment as we're seeing with this issue. All said and stated I do not have the warm and fuzzies about this provider, am not actively using it (nor would I recommend doing so), and I am actually OK with removing it if that is a consideration/possibility.

Mike-E-angelo avatar May 21 '22 16:05 Mike-E-angelo

Naturally, after my Grumpy McGrumpyface rant, I received a reply from MixCloud that this should be fixed now that we should try again.

๐Ÿ˜๐Ÿ˜…๐Ÿ˜ญ

I will add this to my things to do. I suddenly got swamped with a ton of tasks so it should be later this week, if not the weekend. I will update here either way as soon as I can. ๐Ÿ‘

Mike-E-angelo avatar May 24 '22 16:05 Mike-E-angelo

A quick update here: I attempted to use the latest API and it still appears to not work. I have notified MixCloud but I also wanted to quickly verify my understanding here to ensure that it is accurate:

  1. ASP.NET (MixCloud) Oauth Provider takes the dictionary state of the authentication and encyrpts it as an encrypted string value
  2. This encrypted string value gets sent to the Oauth2 authentication endpoint as the state parameter
  3. Oauth2 authentication endpoints performs authentication
  4. Upon successful authentication, the exact same state passed in step 2 gets sent back to the configured "callback/return" URI (e.g. https://mydomain.com/signin-mixcloud?other=parameters&state=<encrypted_state>)

It is at step 4 where we are broken here. That is, no state is being passed back, and/or what is, is invalid. In my case the value that I get in return is login which does not appear to be encrypted. :)

If I have something misunderstood that would be valuable to know. ๐Ÿ‘

Additionally, I have asked to have the engineer/resource being used to implement/solve this to contribute to this thread with any further discussion/questions.

I will continue to update here with any further news.

Mike-E-angelo avatar Jun 08 '22 11:06 Mike-E-angelo

I got an update from MixCloud support stating that this bug (which was claimed to have been fixed) has been assigned to someone to address (again). The time to fix is indeterminate.

Perhaps my earlier angst was not misplaced after all. ๐Ÿ˜ž

Mike-E-angelo avatar Jun 20 '22 08:06 Mike-E-angelo

The time to fix is indeterminate.

Haha, it's indeed getting really ridiculous, given how easy this should be to fix. For reference, in OpenIddict, it's implemented in literally 3 lines of code ๐Ÿคฃ

// If the user agent is expected to be redirected to the client application, attach the request
// state to the authorization response to help the client mitigate CSRF/session fixation attacks.
//
// Note: don't override the state if one was already attached to the response instance.
if (!string.IsNullOrEmpty(context.RedirectUri) && string.IsNullOrEmpty(context.Response.State))
{
    context.Response.State = context.Request?.State;
}

kevinchalet avatar Jun 20 '22 16:06 kevinchalet

Hi @Mike-E-angelo ,

Do you have an example (runnable test case) that I can work with? I'm not a Cโ™ฏ dev, but some runnable code which spells out your expectation would help me to get this resolved quickly. Thanks!

ben-xo avatar Jul 11 '22 14:07 ben-xo

@ben-xo hey. I'm not sure a runnable test case would be useful as state is a core concept of the OAuth 2.0 specification and not something specific to a language or a dev environment.

It's actually really trivial to implement as the only thing you have to do is extract the state parameter from the authorization request and append it to the redirect_uri you construct when generating the authorization response, as clearly explained in the OAuth 2.0 specification:

image

https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2

kevinchalet avatar Jul 11 '22 17:07 kevinchalet

It's worth mentioning that the state parameter is required and must be returned even for errored responses, as indicated in the next section: https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1

kevinchalet avatar Jul 11 '22 17:07 kevinchalet

@Mike-E-angelo , @kevinchalet ,

Regardless of whether or not it's in the spec, with no client to test with, it's hard to validate that any fix fixes your bug. If you have a 4 or 5 line program that demonstrates what you are expecting, I can then go away and get it fixed without any back-and-forth.

ben-xo avatar Jul 11 '22 18:07 ben-xo

Woo, to be honest, this is getting quite ridiculous, don't you guys have test clients you use internally to test your own server implementation? (well, I guess not, otherwise we wouldn't have this thread opened for more than a year ๐Ÿคฃ)

Anyway, it's easy to test using a program like Postman (authorization tab -> OAuth 2.0: fill in the form and click on "Get New Access Token") or manually by constructing the request URL yourself and using your preferred browser:

https://www.mixcloud.com/oauth/authorize?client_id=the_client_id_of_a_registered_client&redirect_uri=the_redirect_uri_of_a_registered_client&state=xyz

If you're redirected to the_redirect_uri_of_a_registered_client?code=abc&state=xyz (if the authorization is approved) or the_redirect_uri_of_a_registered_client?error=access_denied&state=xyz (it you refused the authorization demand), it works. Otherwise, it doesn't.

kevinchalet avatar Jul 11 '22 18:07 kevinchalet

Regardless of whether or not it's in the spec, with no client to test with, it's hard to validate that any fix fixes your bug

Thank you very much for checking in here @ben-xo and for your time investigating this issue. At the risk of piling on here, however, I do share the concern and frustration with others here about how long it has taken to address this issue as well as the continued misunderstanding on what exactly the issue here is.

  1. The issue in no way involves C# outside of the fact that a C# client is involved to integrate with your provided API. You can think of this client as a wrapper around the CURL/web calls that you yourself provide in your own documentation here.
  2. As this is indeed defined -- and as noted, required -- by the OAuth2 specification, you are free to use any client you originally used to test your Oauth2 implementation and API. This is overkill, however, as a simple call via Postman (as has already been suggested) is all you need to test/verify on your end.
  3. There appears to be confusion about this thread's intent. The intent of this thread was to track a broken API on Mixcloud's side. And, unfortunately, it still is tracking over a year after the fact. ๐Ÿ˜ž It was opened here as it appears that we were the first to notice this issue and wanted to have a way to bring it to your attention, as well as offer a place for any collaboration/communication. That is, we invited you here to discuss and collaborate to get it fixed on your end. Getting it fixed on your end means that it works on all the clients/integrations that you already own and have published. This would mean the Ruby client mentioned at the bottom of this page, and basic web requests as provided by your documentation. Once all those are working, then we can verify it here as well (again since what we do here is a basic CURL wrapper it should "just work" along with all the other clients).

To summarize, there is nothing special about C# or this repository with regards to this issue, except that it expects a fully-compliant Oauth2 as developed by your team. As such, this issue as identified is an incomplete implementation of the Oauth2 specification on Mixcloud's side, and the GitHub issue here is meant to track it in a C# client implementation which in turn expects a fully operational/compliant Oauth2 implementation.

At the risk of a bad pun, I hope this further clarifies the "state" of this issue here. ๐Ÿ˜

Mike-E-angelo avatar Jul 11 '22 23:07 Mike-E-angelo

I'm well aware that this has nothing to do with C#. But I can't prioritise a bug I'm not experiencing. A series of curl commands that demonstrate the problem would do just fine.

As you've linked to the curl commands in our doc, can I take it they're sufficient to show the problem?

I understand your frustration, but this is very low priority for me, and so the easier you make this on me the more likely I am to have a go at fixing it.

If you leave it to me to interpret the RFC and play with any of the sample clients, rather than showing me how you would use the client to achieve what you want, the most likely outcome is that the bug will not get fixed the way you expect (or only fixed in some circumstances and not others), and you'll be back here complaining next month. Help me to help you, and we can get this resolved.

ben-xo avatar Jul 12 '22 06:07 ben-xo

Anyway, it's easy to test using a program like Postman (authorization tab -> OAuth 2.0: fill in the form and click on "Get New Access Token") or manually by constructing the request URL yourself and using your preferred browser:

https://www.mixcloud.com/oauth/authorize?client_id=the_client_id_of_a_registered_client&redirect_uri=the_redirect_uri_of_a_registered_client&state=xyz

If you're redirected to the_redirect_uri_of_a_registered_client?code=abc&state=xyz (if the authorization is approved) or the_redirect_uri_of_a_registered_client?error=access_denied&state=xyz (it you refused the authorization demand), it works. Otherwise, it doesn't.

Thanks, this is extremely helpful.

ben-xo avatar Jul 12 '22 08:07 ben-xo

Thank you for your reply @ben-xo. For reference, here is code that is utilized in another project that fulfills this requirement. It is C# but, hopefully, you can see the straightforward nature of the issue here. Whatever state is passed to your service, is passed back to the resulting response when the call is made on your end to the callback URL.

So in the case of your documentation, it should be updated to show the following:

https://www.mixcloud.com/oauth/authorize?client_id=YOUR_CLIENT_ID&redirect_uri=YOUR_REDIRECT_URI&state=YOUR_STATE

Please note that I was incorrect in stating/implying that state is always required. It is required if it is initially provided to your service in the initial call. So, it is an optional parameter, but if the client (us) passes it to you, then it is required as passing back to the calling origin as originally passed in.

Additionally, I did try to set up some CURL calls for you but the authorize "endpoint" is actually an HTML/browser-based request and not an actual API endpoint. However, I see that with your latest post that it appears you are situated correctly now?

Please do let me know if you have any further questions here and I will do my best to assist you.

Mike-E-angelo avatar Jul 12 '22 08:07 Mike-E-angelo

Hi @ben-xo I got a marketing email from Mixcloud today and it made me wonder if Mixcloud had managed to implement the 4 lines of code here, so I wanted to check-in on your progress and/or if you had any additional questions about this issue.

Mike-E-angelo avatar Sep 12 '22 07:09 Mike-E-angelo

Got another marketing email @ben-xo and passing along the reminder to you. Any luck here with those four lines of code? Please let us know if you have any questions around this issue reported in June 2021.

https://howlongagogo.com/date/2021/june/25

Mike-E-angelo avatar Oct 08 '22 08:10 Mike-E-angelo

I have sent a message to Mixcloud support and asked if @ben-xo is still employed there.

Mike-E-angelo avatar Oct 14 '22 12:10 Mike-E-angelo

I'm personally speechless.

Proper state support is absolutely essential to the security of an OAuth 2.0 client as it prevents session fixation/forged requests attacks (and, when implemented correctly, helps reduce the ability to steal authorization codes via referrer leakages). For the aspnet-contrib provider, we used a workaround to have a state-like mechanism but I really doubt most Mixcloud clients in the wild went that far. The number of applications vulnerable to the classes of attacks I mentioned should be astonishing...

kevinchalet avatar Oct 14 '22 13:10 kevinchalet

Retweet for awareness: https://twitter.com/Mike_E_angelo/status/1580925898789789698

Mike-E-angelo avatar Oct 14 '22 14:10 Mike-E-angelo

I added a Mixcloud integration to the new OpenIddict client (https://github.com/openiddict/openiddict-core/pull/1594) and I can confirm the state issue hasn't been fixed and that a workaround is still needed.

(oh, and they return JSON responses as text/javascript responses ๐Ÿคฃ)

kevinchalet avatar Nov 27 '22 04:11 kevinchalet

Closing for now. If Mixcloud fixes/improves their implementation in the future, we'll revisit that.

kevinchalet avatar Nov 29 '22 17:11 kevinchalet

Whew finally putting this thing out of its misery. I will never have anything to do with Mixcloud again and regret ever opening this issue ๐Ÿ˜ญ

Mike-E-angelo avatar Nov 29 '22 18:11 Mike-E-angelo