Closed christian-recordedfuture closed 2 years ago
@christian-recordedfuture Static Tests results for commit 57b0a1729e856a0f36f0ddce9d8a47eb7c314e09 - https://drive.google.com/file/d/1N-hAl7b2raF6Ty0itd4ZHSH3Ke_j-zP9/view?usp=drivesdk
@christian-recordedfuture Static Tests results for commit f2107c61726653e696f1ca1e8cebe81bd3aebde0 - https://drive.google.com/file/d/16NS8X_VVzyxahFT-OxMP4OzJroVziRLp/view?usp=drivesdk
@christian-recordedfuture Static Tests results for commit 5629d9928078b03776ce27aede52c9a997fc2e0b - https://drive.google.com/file/d/1NLrrcz8WliVqi1nIrqDgrcOJxF1ZzCWk/view?usp=drivesdk
It seems like disabling semgrep errors/warnings do not work here. It all goes though when running locally. The reason for using verify=False is because we don't always have valid certificates on development machines (and this goes for a few customers as well), so we would not be able to get the sessionid otherwise.
It also seems like the static tests are failing with empty lists of errors. Also there seems to be no place to specify height and width for the logos, unless you want it inside of the svg files?
@dgopani-crest Static Tests results for commit 986debb7fc6e43b4b110c5c27606a1bc66717457 - https://drive.google.com/file/d/1RSTBTDgULrY0x9fuZPN2zHr3rJhG8pXd/view?usp=drivesdk
Hey @christian-recordedfuture, We are working on this app and we need your input on a few points.
Hey @christian-recordedfuture, can you please help us with the above points?
Good morning!
Reply to question no1: I have gone through your changes, and I am mainly fine with them but do we really need to rename the logos? In the original code, they were named after the colour they have, not the mode that they were going to be used for which I think makes it clearer (we also have a blue version).
We also have a question related to the changes of the license text - does it have to be changed? We prefer to keep the following as is, ie:
Reply to question no2: will be included in the next commit.
Reply to question no3: No breaking changes for the playbooks.
I will wait to push my changes to git, until I know your replies to my questions above.
Cheers, Ingrid
Hello @recordedfuture-puille @christian-recordedfuture,
For logo, as per SOAR app standard logo file name should be "logo": "<app>.svg"
for light theme and "logo_dark": "<app>_dark.svg"
for dark theme. On dark theme it will be in monotone colour as attached below
For License, we have moved our all apps to open source GitHub, which falls under Apache-2.0 License. But as you mentioned you want to keep the previous Recorded Future license as well, we can include both the Apache and Recorded Future license in the files. For example https://github.com/splunk-soar-connectors/lansweeper/blob/next/__init__.py. Let me know if this sounds good to you or not?
Thanks!
just a quick check: isn't the purpose of declaring the logo files, that they don't need default names?
I will check with our legal department what they want to do with the license.
We are keeping the logo name the same to make uniform across all the apps. That is just standard practice.
For Licensing adding @mattsayar-splunk, If he can give his inputs
Yeah let us know what your Legal team says. I don't think it's a conflict to have both.
@recordedfuture-puille Static Tests results for commit 901d787dc8b4b7ba306a8eb69f6d0afa11015974 - https://drive.google.com/file/d/1HHYzmMZrUYMMFiD4R1QHCoRleGEfMupo/view?usp=drivesdk
I have uploaded modifications instigated by your review: mainly license, correction of the logo files (so that the white logo is used for dark mode and vice versa) and release notes. There are some other tweaks too.
Let me know if you need anything else from me!
@dgopani-crest Static Tests results for commit 76293fd7e863431b61ac71a5941f19e0369d2c96 - https://drive.google.com/file/d/1umiH-_C_dDECaNn8boHePh85dONmLc5y/view?usp=drivesdk
Thanks @recordedfuture-puille, We will release this app asap. Thanks for contributing to SOAR!!
@dgopani-crest Static Tests results for commit 76293fd7e863431b61ac71a5941f19e0369d2c96 - https://drive.google.com/file/d/1VUEi-i_O82_i4sQE56tbBOXi7YwAlzRV/view?usp=drivesdk
@dgopani-crest Static Tests results for commit e0155152de82e2db269bb1b1f9eb7b743aa34512 - https://drive.google.com/file/d/1CYnMt-tA8H5KwePfoMKi6T-sxtomXXE3/view?usp=drivesdk
@dgopani-crest Static Tests results for commit 823463f2f9248f20e014ee44e51705fdf75cefe5 - https://drive.google.com/file/d/1g9GvDG2zXXeJLoeocmerOHjyMFDlJfUe/view?usp=drivesdk
Please ensure your pull request (PR) adheres to the following guidelines:
Pull Request Checklist
Please check if your PR fulfills the following requirements:
<App Name>: <PR Type> - <PR Description>
next
branch of the forked repo. Create separate feature branch for raising the PR.Pull Request Type
Please check the type of change your PR introduces:
Release Notes (REQUIRED)
Recorded Future Links have been added to the intelligence lookups. These are entities that have been verified to be linked together by research or technical analysis, unlike the related entities which have been associated with each other solely because they have been found in the same document(s).
The linked entities have been marked up by type and can be used without further modification for downstream actions in a playbook.
There are a few other improvements, such as better feedback for testing of connectivity, fetching of alerts, etc.
What is the current behavior? (OPTIONAL)
What is the new behavior? (OPTIONAL)
Other information (OPTIONAL)
Pay close attention to (OPTIONAL)
Screenshots (if relevant)
Thanks for contributing!