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

Improve shared status verification #2525

Merged
merged 5 commits into from
Apr 27, 2017
Merged

Improve shared status verification #2525

merged 5 commits into from
Apr 27, 2017

Conversation

Gargron
Copy link
Member

@Gargron Gargron commented Apr 27, 2017

Instead of parsing shared status contents verbatim, make roundtrip to purported original URL. Confirm that the "original" URL is from the same domain as the author it claims to be from.

to purported original URL. Confirm that the "original" URL is from the
same domain as the author it claims to be from.
return FollowRemoteAccountService.new.call("#{username}@#{domain}")
account = FollowRemoteAccountService.new.call("#{username}@#{domain}")

return nil if confirmed_domain?(domain, account)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't quite read right to me, return nil if the domain is confirmed? If it's correct, a clarifying comment could be helpful 👍

Copy link
Contributor

@ashfurrow ashfurrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me 👍

Copy link
Contributor

@mjankowski mjankowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have a spec here showing that an invalid share gets replaced by the original, or rejected if there is no original.

@Gargron Gargron merged commit 2af4f3c into master Apr 27, 2017
@Gargron Gargron deleted the fix-status-verification branch April 27, 2017 15:06
seefood pushed a commit to Toootim/mastodon that referenced this pull request Apr 28, 2017
* Instead of parsing shared status contents verbatim, make roundtrip
to purported original URL. Confirm that the "original" URL is from the
same domain as the author it claims to be from.

* Fix obvious typo, add comment

* Use URI look-up first

* Add test, update Goldfinger dependency to make less useless HTTP requests per Webfinger lookup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants