librespeed / speedtest

Self-hosted Speed Test for HTML5 and more. Easy setup, examples, configurable, mobile friendly. Supports PHP, Node, Multiple servers, and more
https://librespeed.org
GNU Lesser General Public License v3.0
11.91k stars 2.16k forks source link

Feature request: Using IPinfo IP to Country ASN database #641

Closed abdullahdevrel closed 1 month ago

abdullahdevrel commented 1 month ago

Description

I am requesting to add support for IPinfo's IP to Country database to the project. The database has the following features:

Database schema

Field Name Example Data Type Description
start_ip 1.0.16.0 TEXT Starting IP address of an IP address range
end_ip 1.0.31.255 TEXT Ending IP address of an IP address range
country JP TEXT ISO 3166 country code of the location
country_name Japan TEXT Name of the country
continent AS TEXT Continent code of the country
continent_name Asia TEXT Name of the continent
asn AS2519 TEXT Autonomous System Number
as_name ARTERIA Networks Corporation TEXT Name of the AS (Autonomous System) organization
as_domain arteria-net.com TEXT Official domain or website of the AS organization

Documentation: https://ipinfo.io/developers/ip-to-country-asn-database

Samples are available here: https://github.com/ipinfo/sample-database/tree/main/IP%20to%20Country%20ASN

The database can be downloaded simply by accessing the storage URI with an access token.

curl -L https://ipinfo.io/data/free/country_asn.mmdb?token=<YOUR_TOKEN> -o country_asn.mmdb

Why it should be implemented

Implemeting this feature would enable adding country and ASN (ISP) information on the client IP address.

Optional: implementation suggestions

I work for IPinfo, so I can provide the database-specific support as needed.

Optional: screenshots

image

adolfintel commented 1 month ago

Hi, I saw your comment on the other issue.

First of all, thanks for supporting the project a few years back. I'm not sure why the account was limited and why nobody replied to my emails but this would solve the issue so I'm not going to complain.

I would be interested in replacing the current ipinfo implementation with this. Since it's CC-BY-SA it wouldn't be an issue if I redistribute it with the repo for convenience, right?

@sstidl thoughts?

abdullahdevrel commented 1 month ago

Hey @adolfintel, Nice to meet you.

First of all, thanks for supporting the project a few years back. I'm not sure why the account was limited and why nobody replied to my emails but this would solve the issue so I'm not going to complain.

This is very unusual for us, but I sincerely apologize for the oversight.

We try our best to actively stay on top of everything related to our users, developers and open source projects. I initially made the request to use our free IP database first, but then I decided to look into the code a bit. I was quite surprised to see that you are already using project data! I was not aware of this. I will try my best to reestablish the relationship we had and improve upon it.

Please do not hesitate to post in our community next time: https://community.ipinfo.io. Our entire team is active in the community, and we will be able to jump and respond to any issues the project might have.

I would be interested in replacing the current ipinfo implementation with this. Since it's CC-BY-SA it wouldn't be an issue if I redistribute it with the repo for convenience, right?

Of course! The free IP database is designed to be re-distributed with attribution. We tried our best to make it as OSS-friendly as possible. Feel free to look into projects to explore how they implemented our free database:

Even though we permit redistribution, you may not want to do that in this instance. The reason is that the database is extremely granular, even for a country and ASN database, and thus is a bit on the bigger side.

image

We have a storage URI-based download mechanism, so downloading the database is easy.

https://ipinfo.io/data/free/country_asn.mmdb?token=$TOKEN -o country_asn.mmdb

I would recommend that individual users create their own free IPinfo account and download the database using their own free token instead of using your project's token, as that would minimize the project's responsibility to supply the IPinfo access token.

But ultimately, the distribution of the IP database and IPinfo access token is your decision. The database is licensed under CC-BY-SA, so everything is more or less fair game. You make the engineering decision however you want, and we will provide you with accurate data from our end.

You need to use the MMDB version of the database and an MMDB reader to get started. Let me know, what you have in mind.

sstidl commented 1 month ago

I followed the discussion so far. Thank you for the open and solution orientated procedure. I see an unsolved problem when everyone should use his own key... When we provide docker images we could package the db into the image. This would lower the effort to use it. This would also mean that we regularly update the image to update the database. So far this would work with a dedicated key used by the build process. It would be different if someone would like to use his own key and update mechanism.

adolfintel commented 1 month ago

I pushed a simple implementation of this to the ipinfo_offlinedb branch, it works like this:

I'm using maxmind's library to read the mmdb file, @abdullahdevrel not sure if that's the best option (and I don't like including the phar file directly in the repo).

@sstidl can you give it a try?

sstidl commented 1 month ago

I tried it quickly, looks good for a proof of concept.

I looked into the code and I think we need to refactor the getIP.php At the moment there is a lot of duplicated code and I think the logic isn't as complicated as it looks now.

adolfintel commented 1 month ago

Yeah, it needs some cleanup, there's some duplicated code and it's overly complicated for what it does. I'd just replace it with a couple of functions, one for the online db, and the other for the offline db, plus a function to get the IP.

sstidl commented 1 month ago

i just added the workflow to fetch the db while building the image. all you @adolfintel have to do, is setup a secret named IPINFO_APIKEY with the key that should be used to download it. i didnt test, if that works, so please try.

adolfintel commented 1 month ago

The db is already in the repo and I can keep it updated regularly, are you sure you want to get rid of it and force the user to register to download a free db?

sstidl commented 1 month ago

The db is already in the repo and I can keep it updated regularly, are you sure you want to get rid of it and force the user to register to download a free db?

the user is not forced to register... thats the magic of docker images. we build the image here on github with your key and every week we build an updated image with a new version of the db in it. noone would need to register at ipinfo this way.

sstidl commented 1 month ago

and also that saves you time as you dont ever need to download a new version of the db anymore (at least if people use the docker image)

adolfintel commented 1 month ago

Sorry, I misread your previous post, I thought you modified entrypoint.sh. If you don't already have the key, send me an email at info@fdossena.com and I'll send it to you.

sstidl commented 1 month ago

Sorry, I misread your previous post, I thought you modified entrypoint.sh. If you don't already have the key, send me an email at info@fdossena.com and I'll send it to you.

i dont have permissions in the repo to set secrets... so dont send it to me, just set the secret IPINFO_APIKEY here in github to your api-key.

adolfintel commented 1 month ago

Done, give it a try

sstidl commented 1 month ago

looks good: https://github.com/librespeed/speedtest/actions/runs/10132636147/job/28016839202

the md5 in the image is different from the one in the backend directory so I assume the newly downloaded version made it into the docker image. 👍

adolfintel commented 1 month ago

Great. I'll do some refactoring of getIP then I'll merge it to master in the next few days.

abdullahdevrel commented 1 month ago

Thank you for implementing the new database. sstidl and adolfintel.

I see an unsolved problem when everyone should use his own key... When we provide docker images we could package the db into the image. This would lower the effort to use it. This would also mean that we regularly update the image to update the database. [...] If the user has no ipinfo api key, it uses the offline db (a copy is in the repo, I'll update it every 1-2 months) If the user has an ipinfo api key, it uses the full api and the offline db as a fallback

Perfect. Thank you.

@adolfintel

I'm using maxmind's library to read the mmdb file, @abdullahdevrel not sure if that's the best option (and I don't like including the phar file directly in the repo).

That looks good. This library from MaxMind is only their MMDB reader library, which is used to read the MMDB file. It is not their API library.

A small documentation note is that the database is updated daily. If the user opts to use their own access token and database, they can update the database daily. But for the repository's packaged database, updating with long intervals is fine.

adolfintel commented 1 month ago

I pushed the refactor yesterday and tested it a bit, it seems to work fine. @sstidl give it a try and then we'll merge it.

sstidl commented 1 month ago

Also tested it a bit. Looks good

We should add some Info to the readme

adolfintel commented 1 month ago

Updated the documentation.

I also took the chance to make some minor improvements to the UI (dark theme, unified single server and multiple servers for index.html).

The docker version still has the old UI, I'll unify it tomorrow and then we can release 5.4.

The wiki needs some updating too (or it can just be removed since it's the same stuff that we have in doc.md)

sstidl commented 1 month ago

Good to see the project gets some love. 😃

I started a pull request #645 to better see what's changed

sstidl commented 1 month ago

Solved in #645