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

Move IP getting to backend once we have address selection component? #8

Open
ejolly opened this issue Mar 2, 2022 · 1 comment
Open
Labels
enhancement New feature or request needs discussion

Comments

@ejolly
Copy link
Contributor

ejolly commented Mar 2, 2022

So one less that ideal issue right now is that we're using the Abstract API to get a client's IP address and initial approximate location (before we try to refine it with the browser API). Since we're doing this all on the client-side we're exposing our API key, so a bad actor could run up the free limit of 20k req/month; no billing or other info is tied to the API key.

Alternatively, the speedtest itself has an option to hit a custom end-point to get a user's IP once we start a test. While we have full control over what this end-point does, we don't get easy access to its response (it's used directly by the speedtest library and we'd have to dive it to see how it's handled). I have that switched off on the main branch right now since it's redundant with us getting the client IP via the Abstract API and we have more flexibility on using it on the front-end.

However, I can see a case for moving this logic to the backend where we'd hide our API key too. It might make most sense to move this over whenever we finish building out our address selection component, as that's where we'll make use of the response from that end-point.

Leaving this here now to revisit this at that time

@ejolly ejolly added enhancement New feature or request needs discussion labels Mar 2, 2022
@ejolly
Copy link
Contributor Author

ejolly commented Aug 23, 2022

It looks like Sveltekit now exposes the client IP in event.clientAddress (see sveltejs/kit#4289). I've started a branch to remove our reliance on the Abstract API, but it's not merged in because we currently rely on that API to get both client IP, plus some additional approximate location info.

We would need to get this information in a different way (e.g. Mapquest) from their IP before we can fully remove the Abstract API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs discussion
Projects
None yet
Development

No branches or pull requests

1 participant