Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

publish breaks if user's website has a path #652

Closed
ckruse opened this issue Apr 20, 2016 · 31 comments
Closed

publish breaks if user's website has a path #652

ckruse opened this issue Apr 20, 2016 · 31 comments
Labels

Comments

@ckruse
Copy link

ckruse commented Apr 20, 2016

Brid.gy says that "Publish is not enabled" when sending a webmention, even when using the form on the website. But it is enabled:

image

Am I doing something wrong?

@ckruse
Copy link
Author

ckruse commented Apr 20, 2016

I also tried re-authenticating against twitter - same result.

@snarfed
Copy link
Owner

snarfed commented Apr 20, 2016

uh oh. sorry for the trouble! looks like this is because of #644. @mblaney, @kylewm, any ideas on how to handle cases like this where https://wwwtech.de/ is a single-user web site, but @ckruse happens to have https://wwwtech.de/about in his twitter profile instead of the home page?

@ckruse, in the short term, you can work around this by changing the link in your twitter profile to https://wwwtech.de/ and signing into bridgy again.

@mblaney
Copy link
Contributor

mblaney commented Apr 20, 2016

eek! sorry @ckruse. maybe multi-user sites will need to have multiple accounts registered with bridgy to take the path into account?

@snarfed
Copy link
Owner

snarfed commented Apr 20, 2016

@mblaney i'd be ok with that.

it means we'd still be open to @kylewm's attack in #644 (comment), ie if you're the first bridgy user on a domain, you can publish other users' posts on that domain (to your own silo account)...but you can do that without bridgy anyway, so it may not be worth thinking too hard about. thoughts?

@mblaney
Copy link
Contributor

mblaney commented Apr 20, 2016

yeah at least the solution to that attack will now be in the multi-user domain's hands... though it might not be entirely obvious. It seems the only other solution is to sacrifice the flexibility of paths on single user domains? Would be glad to find another way, but it's not a big deal if this turns out to be the only way to fix it.

@kylewm
Copy link
Contributor

kylewm commented Apr 20, 2016

ie if you're the first bridgy user on a domain, you can publish other users' posts on that domain (to your own silo account)

The fear is the other way around -- other users on the domain could send their posts to your silo account. I post unicyclic.com/kyle/i-m-voting-for-trump and send a webmention to bridgy publish, and it publishes to @mblaney's Twitter account (assuming he has signed up for Bridgy publish and I have not)...

I know you are against adding additional UI components (I am too) unless absolutely necessary, so please take this more as a strawman. We could add be a checkbox somewhere in the auth process -- "this is a multi-user domain, only allow publishing posts under http://unicyclic.com/mal/" ... Then you as an individual user can protect yourself.

Another way to do this might be to crawl and verify mutual rel=me's ... e.g. unicyclic.com/mal would not rel=me to unicyclic.com, but wwwtech.de/about would rel=me to wwwtech.de and vice-versa.

@snarfed
Copy link
Owner

snarfed commented Apr 20, 2016

thanks for the explanation @kylewm! much appreciated. both of those approaches sound OK to me. maybe not ideal, but there may not be an ideal answer.

here's a third option: if a single user domain wants to publish, require a home page link in their silo profile. ie keep the current behavior and just document that req't. thoughts?

i may raise this in IRC today. i don't feel like i have good intuition for UX/best practices around single vs multi user domains yet.

@snarfed
Copy link
Owner

snarfed commented Apr 20, 2016

asked on IRC, but haven't heard any clear votes or new ideas yet. :/

@mblaney
Copy link
Contributor

mblaney commented Apr 21, 2016

Yes my solution was to sign up for two bridgy accounts to prevent someone posting to your silo account. That's a bit counter-intuitive, but maybe it's a bonus that if you have multiple accounts on your domain (and you're the only one using it...) you can use any of them to publish to your silo account?

Saying that, I much prefer @kylewm's rel=me idea. As was mentioned on IRC, @ckruse already links wwwtech.de to his /about page (the reciprocal link isn't necessary by the way, we just want to know if we should store the domain only, or the domain and path).

@kylewm
Copy link
Contributor

kylewm commented Apr 21, 2016

the reciprocal link isn't necessary

good point! so:

  • if user auths with a non-root path X
    • check the root path for a rel=me link to X
    • If it exists, then it is equivalent to authing at the root

@mblaney
Copy link
Contributor

mblaney commented Apr 21, 2016

sounds like a plan! any volunteers? :-)

@snarfed
Copy link
Owner

snarfed commented Apr 21, 2016

the one catch i wonder about is rel-me adoption. i suspect the majority of users with non-home-page links right now don't have homepage rel-me links to them. :/ I'd love to see some stats from remote_api_shell!

@mblaney
Copy link
Contributor

mblaney commented Apr 21, 2016

ok here's another idea then!

If user auths with a non-root path, check the root path for a specific rel value ie: rel=bridgy-multi-user.
If it exists, then the path must be kept, otherwise store just the domain for the user.

I could then put the following link on https://unicyclic.com/
<a href="https://brid.gy" rel="bridgy-multi-user">This site supports Brid.gy Publish</a>

@snarfed
Copy link
Owner

snarfed commented Apr 21, 2016

interesting idea, i like it! or even reuse existing mf2 or other markup for "this is a multi user site" if it exists. cc @tantek and @kevinmarks in case they know.

@mblaney
Copy link
Contributor

mblaney commented Apr 21, 2016

reusing an existing rel value is a good idea, especially since the bridgy- prefix in the rel value is redundant if https://brid.gy is given as the href. I just found rel=group at http://microformats.org/wiki/existing-rel-values, and the description there actually makes sense for this case.

@ckruse
Copy link
Author

ckruse commented Apr 25, 2016 via email

@snarfed snarfed changed the title Brid.gy says "Publish not enabled" when sending a webmention publish breaks if user's website has a path May 1, 2016
@snarfed snarfed removed the now label May 1, 2016
@snarfed
Copy link
Owner

snarfed commented Jul 24, 2017

fwiw this is related to #629, but not quite the same. #629 is for supporting multiple users on a single domain. this is just about profile URLs with paths.

@mblaney
Copy link
Contributor

mblaney commented Jul 25, 2017

#629 is about supporting multiple silo accounts on a single domain, and the way bridgy works now is that I can create multiple paths on my own domain, and link them to different silo accounts. So you could argue that #644 closes #629... not in the way suggested, but there is a way to do what's been requested. :-)

I would still like to add the rel=group fix mentioned above, I think that's the simplest way to fix this issue.

@snarfed
Copy link
Owner

snarfed commented Jan 21, 2018

@jernst hit this recently too, with a different setup. his root domain http://upon2020.com/ 303 (!) redirects to https://upon2020.com/blog/ , so when he signs up, we follow that redirect and store that URL, with the path. then, when he tries to publish e.g. https://upon2020.com/banter/2018/01/19/testing-2-2-3-please-ignore/ , it doesn't work because we check that it has /blog/ as a prefix, and it doesn't.

@snarfed snarfed added the now label Jan 21, 2018
@jernst
Copy link

jernst commented Jan 21, 2018

Ok, fine :-) no more 303 in the next @uboslinux update.

@snarfed
Copy link
Owner

snarfed commented Jan 21, 2018

lol! @jernst definitely don't feel like you need to change it on my account. bridgy should handle anything users throw at it.

@snarfed
Copy link
Owner

snarfed commented Jan 22, 2018

i ended up fixing this with a dumber hack than anything proposed above. if just one account matches the requested domain, we now go ahead and use it even if the path doesn't match.

this definitely doesn't support multiple users per domain (#629), and it's also arguably a security hole for multi-user domains...but i'm not actually sure we have any of those live on bridgy right now. #629 is hypothetical. so, we'll cross that bridge when we come to it.

@mblaney
Copy link
Contributor

mblaney commented Jan 23, 2018

hi @snarfed I like the idea of this, but it doesn't currently work for me. I'm the only bridgy-registered user on a multi-user domain, which means the non-registered accounts can currently post as me.

I think this means I need to either register another user with bridgy (to get the count above 1 in the new code), or implement the rel=group fix above. I'm leaning towards the latter as I think I've already put it off long enough! ;-)

@snarfed
Copy link
Owner

snarfed commented Jan 23, 2018

aha. thanks @mblaney! you're right, it doesn't work for you.

I'm now thinking I'll just detect and remember redirects from the home page. let me try that first.

@snarfed snarfed reopened this Jan 23, 2018
snarfed added a commit that referenced this issue Jan 23, 2018
@snarfed
Copy link
Owner

snarfed commented Jan 23, 2018

ok, done. @mblaney the post URL publish checks are back to how they were before. @jernst if you sign up for twitter again, it should now see that you own all of upon2020.com, not just /blog/, so publishing should still work.

@mblaney
Copy link
Contributor

mblaney commented Jan 24, 2018

thanks @snarfed, I'm going to find some time to fix this with rel=group. Sorry to point out more issues, but your recent text change in the error messages are a bit confusing. They are returned to the calling site, so if I display an error message containing: <a href="/">sign up again</a> that is a link on my site, whereas I want them to visit bridgy so it needs to be the full url...

I also don't think your last change can close this issue #652 as we still haven't fixed the original problem from @ckruse. Since I'm the instigator of the trouble here I'll try and get it done soon. :-)

@snarfed
Copy link
Owner

snarfed commented Jan 24, 2018

damn, thanks @mblaney. right on both counts.

i actually haven't really planned for bridgy's error messages to be rendered verbatim elsewhere, like on your site... but I'd also forgotten that they show up in webmention responses as well as interactive. I'll fully qualify the links.

re the full fix, i think i now prefer the original rel=me idea to rel=group. (heh, sorry.) rel=me is widely enough used and understood, esp relative to =group, and the default behavior of checking path prefix is secure. users would have to explicitly opt into single user mode.

i think the actual change is pretty contained. we'd keep cc6ca37, and immediately afterward, if both the initial and final (resolved) profile url have a path, we fetch the root and look for rel=me to any of the urls in the redirect chain. if we find a match, we add the root instead of the path, like cc6ca37 does.

sound ok? thanks for keeping me honest!

@snarfed snarfed reopened this Jan 24, 2018
@mblaney
Copy link
Contributor

mblaney commented Jan 24, 2018

yes that sounds great and I agree using rel=me is much better than introducing rel=group. I was just hesitant to expect people to add rel=me to the root of their domain, and I was happy to add something in my case as it's the only one that would require it.

I'm happy to help if you want to point me in the direction of where to pull their root domain and check for rel=me?

@snarfed
Copy link
Owner

snarfed commented Jan 24, 2018

hey, sure, thanks! it's basically right next to cc6ca37, both the change and the test.

the new test would be the same as test_create_new_domain_url_redirects_to_path, except the initial profile url would be something like http://site/path instead of http://site, and you'd expect_requests_get http://site to return HTML with the rel-me link.

the new code could be a new first conditional block in https://github.com/snarfed/bridgy/blob/master/models.py#L562, only if the profile url has a path. example HTTP fetch and mf2 parse:
https://github.com/snarfed/bridgy/blob/master/publish.py#L476-L478

@mblaney
Copy link
Contributor

mblaney commented Feb 8, 2018

thanks @snarfed I'm pretty sure your description wrote the code for me.... so glad we can finally close this one! :-)

@snarfed
Copy link
Owner

snarfed commented Mar 6, 2018

thanks again @mblaney! implemented in #791.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants