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

Support for abuseIPDB #70

Merged
merged 4 commits into from
Mar 9, 2024
Merged

Support for abuseIPDB #70

merged 4 commits into from
Mar 9, 2024

Conversation

zbalkan
Copy link
Contributor

@zbalkan zbalkan commented Feb 20, 2024

Currently failing some tests unrelated to abuseIPDB -it seems. When everything is fixed, I will update the documentation too.

@pirxthepilot
Copy link
Owner

pirxthepilot commented Feb 21, 2024

@zbalkan thanks for the PR. Pretty cool! Looking forward to your changes. I'm a bit swamped this week but will review as soon as I can.

Can you also share a preview screenshot of what the abuseIPDB section will look like? Thanks!

@pirxthepilot
Copy link
Owner

Btw, to run the tests locally you will need the hatch package. Then you can run them all:

hatch run test-all

You can also run each test separately:

Lint:

hatch run lint:all

Typechecking:

hatch run typecheck

Actual tests:

hatch run test
hatch run test-cov

@zbalkan
Copy link
Contributor Author

zbalkan commented Feb 21, 2024

I will solve the test problems first. Then I must write new ones.

FAILED tests/test_cli.py::TestDefaults::test_defaults_3 - SystemExit: 2
FAILED tests/test_cli.py::TestDefaults::test_defaults_4 - SystemExit: 2
FAILED tests/test_cli.py::TestGenEntityHandler::test_handler_domain_1 - assert False
FAILED tests/test_cli.py::TestGenEntityHandler::test_handler_domain_2 - assert False
FAILED tests/test_cli.py::TestGenEntityHandler::test_handler_domain_3 - assert False
FAILED tests/test_cli.py::TestGenEntityHandler::test_handler_domain_4 - assert False
FAILED tests/test_cli.py::TestGenEntityHandler::test_handler_ip_1 - assert False
FAILED tests/test_cli.py::TestMain::test_main_default - AssertionError: expected call not found.
============================================================= 8 failed, 105 passed in 4.53s ============================================================== 

When it comes to the UI, there will only be a single line just like Greynoise has. So the expected result is green or red depending on the score.
abuseipdb

@zbalkan zbalkan changed the title Support for abuseIPDB [draft] Support for abuseIPDB Feb 22, 2024
Copy link
Owner

@pirxthepilot pirxthepilot left a comment

Choose a reason for hiding this comment

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

@zbalkan just finished my initial review. Apologies for the multiple syntax/formatting changes. I didn't think anyone would actually contribute code here so I never got around to autoformatting and style guides yet 😅. But thanks for taking the time and putting up with my messy code!

tests/test_cli.py Show resolved Hide resolved
tests/test_cli.py Outdated Show resolved Hide resolved
tests/test_cli.py Outdated Show resolved Hide resolved
wtfis/clients/abuseipdb.py Outdated Show resolved Hide resolved
wtfis/main.py Outdated Show resolved Hide resolved
wtfis/models/abuseipdb.py Outdated Show resolved Hide resolved
wtfis/models/abuseipdb.py Outdated Show resolved Hide resolved
wtfis/models/abuseipdb.py Outdated Show resolved Hide resolved
wtfis/types.py Outdated Show resolved Hide resolved
wtfis/ui/base.py Outdated Show resolved Hide resolved
@zbalkan
Copy link
Contributor Author

zbalkan commented Feb 27, 2024

I did the changes requested. Yet, I need to fix the tests. I may need some guidance there.

@zbalkan
Copy link
Contributor Author

zbalkan commented Feb 27, 2024

Since there are no "Discussions" section, I'd like to ask here. I see that there are two types of data collected, the metadata on IP or domain -aka whois- and reputation.

Service WHOIS Reputation Notes
Virustotal + +
Passivetotal + - Has API for reputation
IP2Whois + -
IPWhois + -
Shodan + -
Greynoise - +
URLhaus - +

Wouldn't it be nice to categorize them based on those? Though PT reputation endpoint may put it in both categories just like VT. Currently, the term "enrichment" is used yet it does not add much semantically.

@pirxthepilot
Copy link
Owner

@zbalkan sorry for the late reply, weekdays are usually busy. I will try to review tomorrow and help with the tests. Thank you for the changes!

@pirxthepilot
Copy link
Owner

Since there are no "Discussions" section, I'd like to ask here. I see that there are two types of data collected, the metadata on IP or domain -aka whois- and reputation.
Service WHOIS Reputation Notes
Virustotal + +
Passivetotal + - Has API for reputation
IP2Whois + -
IPWhois + -
Shodan + -
Greynoise - +
URLhaus - +

Wouldn't it be nice to categorize them based on those? Though PT reputation endpoint may put it in both categories just like VT. Currently, the term "enrichment" is used yet it does not add much semantically.

Good call out. Technically, these are all enrichments anyway. I like the categorization, but I wonder if there's a better term to describe the non-reputation data other than "whois" (though, I'm not opposed to the term).

@zbalkan
Copy link
Contributor Author

zbalkan commented Mar 3, 2024

Since there are no "Discussions" section, I'd like to ask here. I see that there are two types of data collected, the metadata on IP or domain -aka whois- and reputation.
Service WHOIS Reputation Notes
Virustotal + +
Passivetotal + - Has API for reputation
IP2Whois + -
IPWhois + -
Shodan + -
Greynoise - +
URLhaus - +
Wouldn't it be nice to categorize them based on those? Though PT reputation endpoint may put it in both categories just like VT. Currently, the term "enrichment" is used yet it does not add much semantically.

Good call out. Technically, these are all enrichments anyway. I like the categorization, but I wonder if there's a better term to describe the non-reputation data other than "whois" (though, I'm not opposed to the term).

Well, I agree with "whois" not being a good term. I just wanted to put things in a frame. You can call it any term you want. I thought of "basics" and "metadata" but didn't like it. So I kept it that way.

I agree that they are all enrichments, the ownership is a matter of registration and static while reputation is a collective measure from multiple reports and security products and dynamic. The nature of the data is different. I wanted to point out the nuance.

wtfis/clients/abuseipdb.py Outdated Show resolved Hide resolved
wtfis/models/abuseipdb.py Outdated Show resolved Hide resolved
wtfis/models/abuseipdb.py Outdated Show resolved Hide resolved
wtfis/clients/abuseipdb.py Outdated Show resolved Hide resolved
wtfis/models/abuseipdb.py Outdated Show resolved Hide resolved
wtfis/models/abuseipdb.py Outdated Show resolved Hide resolved
@pirxthepilot
Copy link
Owner

@zbalkan I will go ahead and merge this. Appreciate all the work here, thank you! This will be included in the next release - just give me another week or so to write additional tests and include some tweaks not related to this.

@pirxthepilot pirxthepilot merged commit 4ec8031 into pirxthepilot:main Mar 9, 2024
13 checks passed
@zbalkan
Copy link
Contributor Author

zbalkan commented Mar 11, 2024

Thanks for the comment. I'm glad I did some contribution to the tool I almost used daily. I found out that the documentation must be improved a little bit more. I have added the enrichment section, but the main screenshot and the animated GIF are the same. I hesitated to touch that. Also, the .env.wtfis.example must be updated, too. Should I create another PR for these?

@pirxthepilot
Copy link
Owner

@zbalkan sorry again for the late response. Yes, would appreciate the update to the README! I haven't gotten around to writing the additional tests yet, unfortunately. If you do have extra time, do you want to give it a shot? I can give you some instructions on what to add. Thanks!

@zbalkan
Copy link
Contributor Author

zbalkan commented Mar 22, 2024

Just tell me what I can do. I'll try to find some time.

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.

None yet

2 participants