-
Notifications
You must be signed in to change notification settings - Fork 321
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
Add IntSights support #276
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks this looks great!
The code looks good, can I just ask that you add a unit test for this with a mocked response.
Its quite simple to do and you can see examples for the other providers in test_tiproviders.py.
Please let us know if you have any issues with this or want some help.
Hi @petebryan, I've added some mock data based on what I saw in test_tiproviders.py. Let me know if I missed anything! |
Can you add a section to https://github.com/microsoft/msticpy/blob/main/msticpy/resources/mpconfig_defaults.yaml TIProviders:
....
XForce:
Args:
ApiID: *cred_key
AuthKey: *cred_key
Primary: bool(default=False)
Provider: "XForce" |
@ianhelle: Section added! |
So looking at the failing test here there are a couple for minor changes required. The first is you need to add the IntSight provider to msticpy\tests\testdata\msticpyconfig.yaml so that the TILookup class in the tests correctly loads it. Second of all in the mocked data you provide for the test you pass in some dates but in the wrong format. If you make these changes the test_tiproviders test file should complete properly. |
Sorry @FlorianBracq there was another test error that I forgot to include in my last message. On line 367 of https://github.com/FlorianBracq/msticpy/blob/IntSights/tests/sectools/test_tiproviders.py where you are mocking the response you have the query variable defined as: However with this provider there is no query element of the params so you should just take the params as is with: |
Add IntSights support
Simple PR related to #275