[feature] Discover webfinger through host-meta #1588

daenney · 2023-03-04T16:06:59Z

Description

If this is a code change, please include a summary of what you've coded, and link to the issue(s) it closes/implements.

If this is a documentation change, please briefly describe what you've changed and why.

I'm opening this as a draft because:

The implementation makes me sad, please suggest improvements
~~I want to figure out how to properly test the fallback and ideally verify what gets set in the cache~~
Arguably, I'm inclined to not want this code at all because the host-meta redirect goes against current best practices. But it's hard for end users to figure out why search isn't working and if/when they do manage to get another instance admin to potentially update their configuration. It's the fediverse so in this case I think it's worthwhile to try to be MAX_COMPATIBLE

This implements a fallback for discovering the webfinger endpoint in case the /.well-known/webfinger endpoint wasn't properly redirected. Some instances do this because the recommendation used to be to use host-meta for the webfinger redirect in the before times.

Closes #1558.

Checklist

Please put an x inside each checkbox to indicate that you've read and followed it: [ ] -> [x]

If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want).

I/we have read the GoToSocial contribution guidelines.
I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
I/we have performed a self-review of added code.
I/we have written code that is legible and maintainable by others.
I/we have commented the added code, particularly in hard-to-understand areas.
I/we have made any necessary changes to documentation.
I/we have added tests that cover new code.
I/we have run tests and they pass locally with the changes.
I/we have run go fmt ./... and golangci-lint run.

daenney · 2023-03-04T16:54:36Z

I've added a test by redirecting to a different webfinger endpoint. Not sure if/how we want to test for the cached values?

internal/config/defaults.go

internal/transport/finger.go

tsmethurst

Looking good so far :) I'm wondering, do we need/want to cache cases where the webfinger URL didn't require any host-meta looking up, and just the initial constructed webfinger uri worked OK? I was thinking we could only cache cases where the host-meta lookup revealed a different uri from what we expected, if you see what I mean.

internal/transport/finger.go

daenney · 2023-03-05T12:03:16Z

Looking good so far :) I'm wondering, do we need/want to cache cases where the webfinger URL didn't require any host-meta looking up, and just the initial constructed webfinger uri worked OK? I was thinking we could only cache cases where the host-meta lookup revealed a different uri from what we expected, if you see what I mean.

I've updated that now. ~~Now only webfingerFromHostMeta can add new entry in the cache, and only if we got a cached entry + success do we renew the TTL. That should avoid us caching the default URL if that one worked.~~ We only set the returned URL by webfingerFromHostMeta in cache after the host == url check in Finger.

internal/transport/finger.go

daenney · 2023-03-05T20:40:31Z

internal/transport/finger_test.go

+	suite.True(wc.Has("misconfigured-instance.com"), "expect webfinger cache to have entry for misconfigured-instance.com")
+}
+
+func (suite *FingerTestSuite) TestFingerWithHostMetaCacheStrategy() {


This one's a bit long but it tests that things happen as currently intended:

If we get a webfinger redirect, cache it

Future successful requests extend the TTL

Future failed requests don't touch the TTL so the entry will eventually expire

daenney · 2023-03-05T20:41:02Z

internal/transport/transport_test.go

+	"github.com/superseriousbusiness/gotosocial/testrig"
+)
+
+type TransportTestSuite struct {


I cribbed this from other test suites. I don't know if I need all this. But it works 😅

daenney · 2023-03-05T20:44:30Z

internal/transport/transport_test.go

+	testrig.StandardDBSetup(suite.db, nil)
+	testrig.StandardStorageSetup(suite.storage, "../../testrig/media")
+
+	ts, err := suite.federator.TransportController().NewTransportForUsername(context.TODO(), "")


Idk if this should be here/how to better do this. But I need a transport.Transport to call Finger

This implements a fallback for discovering the webfinger endpoint in case the /.well-known/webfinger endpoint wasn't properly redirected. Some instances do this because the recommendation used to be to use host-meta for the webfinger redirect in the before times. Closes superseriousbusiness#1558.

This adds a test suite for transport and moves the finger cache tests into there instead of abusing the search test suite.

We don't really need a separate function for the oddly located webfinger response as we check the full URL string anyway

internal/transport/controller.go

internal/transport/finger.go

NyaaaWhatsUpDoc · 2023-03-08T11:17:16Z

Looks good! A few comments but then should be ready to go :)

When we receive an HTTP 410 on webfinger it means the resource we asked for (the account) is gone, but the endpoint itself responded. In such cases we want to treat the request as successful from a cache (renewal) point of view, while still returning an error from Finger. Follow-up for superseriousbusiness#1588

When we receive an HTTP 410 on webfinger it means the resource we asked for (the account) is gone, but the endpoint itself responded. In such cases we want to treat the request as successful from a cache (renewal) point of view, while still returning an error from Finger. Follow-up for #1588

daenney force-pushed the host-meta-client branch 3 times, most recently from 5c3a62a to ccf5573 Compare March 4, 2023 16:53

daenney force-pushed the host-meta-client branch 2 times, most recently from 7353e02 to b7214ca Compare March 4, 2023 17:01

daenney mentioned this pull request Mar 4, 2023

[feature] Provide .well-known/host-meta endpoint #1561

Closed

9 tasks

daenney commented Mar 4, 2023

View reviewed changes

internal/config/defaults.go Show resolved Hide resolved

daenney commented Mar 4, 2023

View reviewed changes

internal/config/defaults.go Show resolved Hide resolved

daenney commented Mar 4, 2023

View reviewed changes

internal/config/defaults.go Outdated Show resolved Hide resolved

daenney commented Mar 4, 2023

View reviewed changes

internal/transport/finger.go Outdated Show resolved Hide resolved

daenney force-pushed the host-meta-client branch from b7214ca to 3bc79ec Compare March 4, 2023 17:51

daenney commented Mar 4, 2023

View reviewed changes

internal/transport/finger.go Outdated Show resolved Hide resolved

daenney force-pushed the host-meta-client branch 2 times, most recently from 899495b to f15bd91 Compare March 4, 2023 23:06

daenney marked this pull request as ready for review March 4, 2023 23:07

daenney force-pushed the host-meta-client branch from f15bd91 to cd9e456 Compare March 5, 2023 11:31

tsmethurst reviewed Mar 5, 2023

View reviewed changes

internal/transport/finger.go Outdated Show resolved Hide resolved

internal/transport/finger.go Outdated Show resolved Hide resolved

internal/transport/finger.go Outdated Show resolved Hide resolved

daenney force-pushed the host-meta-client branch 4 times, most recently from 454e14b to 458f440 Compare March 5, 2023 12:24

daenney commented Mar 5, 2023

View reviewed changes

internal/transport/finger.go Outdated Show resolved Hide resolved

daenney force-pushed the host-meta-client branch from 58029f4 to 5013c56 Compare March 5, 2023 17:31

daenney commented Mar 5, 2023

View reviewed changes

daenney force-pushed the host-meta-client branch from ec28755 to 4fed212 Compare March 5, 2023 20:43

daenney commented Mar 5, 2023

View reviewed changes

daenney force-pushed the host-meta-client branch from 4fed212 to ed380f4 Compare March 5, 2023 20:53

daenney mentioned this pull request Mar 6, 2023

[bug] Issues Federating with Mastodon Instances with different WEB_DOMAIN and LOCAL_DOMAIN #1445

Closed

daenney added 4 commits March 7, 2023 13:24

[bug] Ensure we only ever update cache on success

059bd71

[chore] Move finger tests to their own place

d99df50

This adds a test suite for transport and moves the finger cache tests into there instead of abusing the search test suite.

[chore] cleanup the test a bit more

082306d

We don't really need a separate function for the oddly located webfinger response as we check the full URL string anyway

daenney force-pushed the host-meta-client branch from ed380f4 to 082306d Compare March 7, 2023 12:25

NyaaaWhatsUpDoc reviewed Mar 8, 2023

View reviewed changes

internal/transport/controller.go Outdated Show resolved Hide resolved

NyaaaWhatsUpDoc reviewed Mar 8, 2023

View reviewed changes

internal/transport/finger.go Outdated Show resolved Hide resolved

NyaaaWhatsUpDoc reviewed Mar 8, 2023

View reviewed changes

internal/transport/finger.go Outdated Show resolved Hide resolved

daenney added 3 commits March 8, 2023 12:43

Address review comments

714bc96

[chore] update config example

569b2e9

[chore] access DB only through state in controller

edd504d

daenney force-pushed the host-meta-client branch from 7874c42 to edd504d Compare March 8, 2023 12:53

NyaaaWhatsUpDoc merged commit e397272 into superseriousbusiness:main Mar 8, 2023

daenney deleted the host-meta-client branch March 8, 2023 13:06

daenney mentioned this pull request Mar 8, 2023

[bug] Handle 410 on webfinger properly #1601

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature] Discover webfinger through host-meta #1588

[feature] Discover webfinger through host-meta #1588

daenney commented Mar 4, 2023 •

edited

Loading

daenney commented Mar 4, 2023

tsmethurst left a comment

daenney commented Mar 5, 2023 •

edited

Loading

daenney Mar 5, 2023

daenney Mar 5, 2023

daenney Mar 5, 2023

NyaaaWhatsUpDoc commented Mar 8, 2023

[feature] Discover webfinger through host-meta #1588

[feature] Discover webfinger through host-meta #1588

Conversation

daenney commented Mar 4, 2023 • edited Loading

Description

Checklist

daenney commented Mar 4, 2023

tsmethurst left a comment

Choose a reason for hiding this comment

daenney commented Mar 5, 2023 • edited Loading

daenney Mar 5, 2023

Choose a reason for hiding this comment

daenney Mar 5, 2023

Choose a reason for hiding this comment

daenney Mar 5, 2023

Choose a reason for hiding this comment

NyaaaWhatsUpDoc commented Mar 8, 2023

daenney commented Mar 4, 2023 •

edited

Loading

daenney commented Mar 5, 2023 •

edited

Loading