ihabunek / toot

toot - Mastodon CLI & TUI
GNU General Public License v3.0
1.15k stars 110 forks source link

toot login gets the instance info wrong when logging into Akkoma server #347

Closed danschwarz closed 1 year ago

danschwarz commented 1 year ago

toot login

Enter instance URL [https://mastodon.social]: https://blob.cat Looking up instance info... Found instance Blob.cat running Mastodon version 2.7.2 (compatible; Akkoma 3.3.1-0-gbba467b39-blob)

... go through the login process. ... ... check config.json ...

Note the instance is https://blob.cat

image

This is with the latest master. I have confirmed that doing the same process against a stock Mastodon server results in the correct instance data being stored in config.json (no https:// prefix)

Not sure whether this is the fault of Akkoma, or toot, yet.

laleanor commented 1 year ago

It comes from the "uri" field from /api/v1/instance (which by the way is deprecated). According to the mastodon API documentation, that field is not supposed to contain an URI, but the domain name. Presumably the Akkoma developers assumed the field called "uri" is supposed to be a URI. What should toot do? Either have a hack to handle both cases, or do the sensible thing and get the handle from /api/v1/accounts/:id, like you're supposed to.

laleanor commented 1 year ago

Relevant patch: https://lists.sr.ht/~ihabunek/toot-discuss/patches/40675

danschwarz commented 1 year ago

Thank you, looking forward to trying this patch. @ihabunek didn't want to upgrade to the non deprecated version 2 service yet, so this workaround is perfect.

danschwarz commented 1 year ago

Tested your patch against Akkoma 3.3.1, stock Mastodon 4.1.0, and Mastodon glitch-soc 4.1.0. Works great with all 3 👍

rjp commented 1 year ago

FWIW, GotoSocial also returns the URL in the uri key but doesn't have any extensions in the returned JSON for identifying it as being from a GTS instance. Might be saner to parse out the domain name from the uri key rather than special-casing these two instance types?

(epicyon and takahe both seem to set uri to be the domain from a quick check of their sources)

curl -s https://social.browser.org/api/v1/instance | jq -r '.uri'
https://social.browser.org
danschwarz commented 1 year ago

Might be saner to parse out the domain name from the uri key rather than special-casing these two instance types?

Maybe. We could check if the key looks like a full URL vs a domain, and parse/don't parse accordingly. This would remove the need to have special case checks for instance containing the string "Pleroma" etc.

I'm still curious what @laleanor meant by the statement

do the sensible thing and get the handle from /api/v1/accounts/:id, like you're supposed to.

Given that we are failing on initial connect to the server, I'm not sure how we could do that? What am I missing here?

rjp commented 1 year ago

I'd be inclined to try the v2 endpoint first, then the v1 if that is missing, and use the appropriate info from whichever.

Although that does bring up the whole "if you support /v2/instance, where's the other /v2/ support?" can of worms and, obviously, toot instance X will output different data depending on the instance's support for /v2/instance.

Could also just always use urlparse and use path when netloc is empty (ie when the URI is a domain, not a URL) which is basically @laleanor's patch but not special-cased for Pleroma.

I did file this as a bug with Akkoma but the response was basically "the client is wrong, /v1/instance is deprecated" (even though it doesn't support /v2/instance) and then "it should interpret it as a URI, not a domain name" contradicting the spec.

Given that we are failing on initial connect to the server

It's not failing as much as "storing the wrong key in the config". I've just auth'd successfully against Akkoma using the "broken" toot and you can still do any commands that work off the active user (which shows as username@https://domain.) What you can't do is refer to that user by -u username@domain because toot only knows it as username@https://domain.

What am I missing here?

I think the argument is that once you've authorized, you then have the user information you need to store the app information keyed by domain in the config file rather than storing it immediately before the user auths.

Means that if the user fails to auth / cancels before auth, you wouldn't get an app block for that domain (as you do now) but that's not necessarily bad? Could add a distinct toot create-app step if people are relying on this behaviour.

Happy to have a go at this change in flow.

danschwarz commented 1 year ago

It's not failing as much as "storing the wrong key in the config

You're right.

It's been a month since I've looked at any of that code.

I'm thinking to go with the quick hack that checks the uri format and parses if necessary before storing the name.

Then once that's in place, consider replacing with an alternative that changes the flow, does the right thing as @laleanor suggested.

If you want to try a cleaner fix that changes the code flow, I'd encourage that, but probably best to get @ihabunek 's opinion on this.