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

feat: validate url as a Lemmy instance before making API requests #30

Merged
merged 6 commits into from Aug 10, 2023
Merged

feat: validate url as a Lemmy instance before making API requests #30

merged 6 commits into from Aug 10, 2023

Conversation

aidandenlinger
Copy link
Contributor

@aidandenlinger aidandenlinger commented Aug 9, 2023

Closes #14. This checks that a getSite call returns without error before allowing Api queries to the URL. This ensures credentials are only sent to Lemmy instances and prevents simple attempts at stealing credentials by typosquatting.

main this PR
main getsite
Fails after sending username / password to URL, even though it's not a lemmy instance Fails before sending username / password, since the URL isn't a lemmy instance

Creation of an Api now returns a Result, as the Api will fail to be created if the url does not return successfully from a getSite query.

I opted against checking the data returned from the getSite query - I figure any malicious actors who would spoof a getSite query to seem like a Lemmy instance would just spin up a full Lemmy instance so any validation like that wouldn't be useful, let me know if I'm thinking about that the wrong way. I have ideas for providing a dropdown of common Lemmy instances to really try to mitigate typosquatting, but that's a future issue/PR.

Let me know if you have alternate solutions in mind or anything else you'd like me to change!

@CMahaff
Copy link
Owner

CMahaff commented Aug 10, 2023

This looks great, thanks for picking it up!

You're right though, it's a half-measure that isn't going to stop a sophisticated hacker.

In a perfect world you're right that we'd have some kind of drop-down or auto-complete on the instance names to fill-in the most popular instances. Slint seems pretty limited though, so I'm not sure we can make it "look nice".

Even a simple green "check-mark" to the right of the instance field when it's recognized from a popular list (that we could store in LASIM) would probably be a good measure against fat-fingering an instance. If we wanted to get really fancy you could even save off instances that users successfully log in to and give them the green check-mark treatment.

But yes, an enhancement for another day :)

@CMahaff CMahaff merged commit b10fed8 into CMahaff:main Aug 10, 2023
3 checks passed
@aidandenlinger aidandenlinger deleted the getsite branch August 15, 2023 06:07
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.

Validate login target is a Lemmy instance before sending credentials
2 participants