Open doenietzomoeilijk opened 2 years ago
matthewp
commented
1 year ago I ran into another client that newline separates redirect_uris. GoToSocial treats this as a single URI only, which is the problem. Here's another server that had the same problem and fixe it: https://github.com/pixelfed/pixelfed/issues/2106
matthewp
commented
1 year ago I investigated this with another client that was doing the same thing. This is what I found:
GTS saves the RedirectURIs as the Domain on the client here: https://github.com/superseriousbusiness/gotosocial/blob/a1dda2267260472aa777ef42d9d2f5c36df648f3/internal/processing/app.go#L73
When creating the auth code, the oauth library checks to see if the redirect uri is part of the client's domain. Since our client's domain is not a valid URI, it throws.
So I think the proper solution would be to create multiple clients, one for each redirect URI. But that would probably also mean making multiple Applications since those contain the client id / secret as well. And that doesn't feel right.
So I'm not sure what to do about this now.
daenney
commented
1 year ago We shouldn't create multiple clients/applications, one for each redirect URL, that's probably going to cause more confusion. We need to handle multiple redirect URLs for the same app correctly.
The way to fix that would probably be to have an application_redirect_uris table, instead of redirect_uri column on applications. That way we can split the incoming redirect_uris on newline and add an entry with a reference back to the application in the application_redirect_uris. Then on the redirect we can fetch the URLs from that table for the application, and check if any of them match. We'd need to add a migration to copy the current value in redirect_uri into the new application_redirect_uris too and then drop the redirect_uri column.
The alternative would be not to normalise this and instead store it as a JSON list straight into the redirect_uri column. Though for clarity's sake we should then at least rename the column to redirect_uris in a migration. It's a little hideous but would work. In pg we could mark that as a JSONB column, but SQLite has no such concept though it can query using json_array. Normalisation is somewhat unnecessary here since each application would have its own set of redirect_uris, so a value in the application_redirect_uris would never be reused/shared.
kvibber
commented
2 months ago Another data point: Buffer appears to be doing the same thing when authenticating with OAUTH now. (I assume they weren't before, because I was able to connect my test server sometime last year.)
timestamp="27/08/2024 20:04:22.841" func=oauth.New.func1 level=ERROR msg="internal oauth error: parse \"https://account.buffer.com/channels/connect\\nhttps://account.buffer.com/onboarding/publishing\\nhttps://account.buffer.com/oauth/mastodon/callback\": net/url: invalid control character in URL"
Describe the bug with a clear and concise description of what the bug is.
After going through oauth authentication for brid.gy (I get redirected to login, I login and authenticate, get redirected back to brid.gy) I receive a 400 error.
The URL on brid.gy seems to be their callback endpoint. It contains a bit of state so I won't reproduce it here.
The logs on my end show this:
The newlines might be the invalid control characters mentioned...
When looking at the brid.gy code, the endpoint controller seems to be here (I think - I don't do Python/flask development, but it looks promising).
I have no idea why that new-line separated string ends up in my logs, it doesn't seem like that brid.gy controller is supposed to output that stuff.
What's your GoToSocial Version?
0.5.1-SNAPSHOT git-15be356
GoToSocial Arch
amd64 docker
Browser version
Firefox 104.0.2
What happened?
Tried to authenticate the Brid.gy Mastodon bits here.
What you expected to happen?
End up authenticated at brid.gy.
How to reproduce it?
Follow this link, enter a GTS instance URL, authenticate / authorize at the GTS end.
Anything else we need to know?
Discussed this with Tobi on Matrix, they weren't sure if this is a spec-compatible thing that brid.gy is doing (in which case it's a GTS bug) or Brid.gy is doing Something Weird™. They asked me to at least file a report for it, so here we are.