Gem Security pack #33434
Gem Security pack #33434
Conversation
|
|
|
@liormgem - Thanks for the contribution. When filling out the Contribution Form, use |
Pack includes: 1 Automation 3 Classifiers 16 Incident Fields 1 Incident Type 1 Integration 1 Layout 3 Playbooks 1 Pre-process Rule
liormgem
force-pushed
the
gem_security_pack
branch
from
fc5f850
to
80080ee
Compare
@liormgem can you please address this comment, so i can go ahead and review the pr |
|
Hi @liormgem, I've reviewed your contribution. Thank you for your effort and dedication. I would appreciate it if you could address the following feedback: Classification and Mapping
PlaybooksGem Handle Alert for Root Usage
Gem Handle ec2
Gem Validate triggering event
Incident Fields
Layout
Please let me know if there are any more revisions needed or if there's anything else I can assist you with. Best regards, |
|
Hi @melamedbn, All your remarks were addressed and fixed, answers to your clarification questions are below:
There are small differences in the way the webhook sends the Gem alerts and the way the fetching Endpoint serves it.
It is.
The playbook is not relevant in this case.
It’s still relevant manually and I think that’s ok Thanks, |
merit-maita
requested review from
JudahSchwartz
and removed request for
merit-maita,
yucohen and
michal-dagan
JudahSchwartz
requested review from
JasBeilin
and removed request for
JudahSchwartz
Great. Please modify the content items and ping me back for review verification. Ben |
Hi @melamedbn, |
|
@liormgem - See https://app.circleci.com/pipelines/github/demisto/content/359788/workflows/d8957b00-e011-469b-abea-dcd1ab231f03/jobs/747032?invite=true#step-116-217_45. Can you please sync your fork with upstream? |
Packs/Gem/Integrations/Gem/Gem.yml
Outdated
| - display: Service Account ID | ||
| additionalinfo: The Service Account ID to use for connection | ||
| name: client_id | ||
| type: 0 | ||
| section: Connect | ||
| required: true | ||
| - display: Service Account Secret | ||
| additionalinfo: The Service Account Secret to use for connection | ||
| name: client_secret | ||
| type: 4 | ||
| section: Connect | ||
| required: true |
There was a problem hiding this comment.
Type 4 is deprecated, please use type 9 instead.
For example:
| - display: Service Account ID | |
| additionalinfo: The Service Account ID to use for connection | |
| name: client_id | |
| type: 0 | |
| section: Connect | |
| required: true | |
| - display: Service Account Secret | |
| additionalinfo: The Service Account Secret to use for connection | |
| name: client_secret | |
| type: 4 | |
| section: Connect | |
| required: true | |
| - display: Service Account ID | |
| name: credentials | |
| defaultvalue: "" | |
| type: 9 | |
| required: true | |
| section: Connect | |
| displaypassword: Service Account Secret |
Packs/Gem/Integrations/Gem/Gem.py
Outdated
| client_id=params['client_id'], | ||
| client_secret=params['client_secret'] |
There was a problem hiding this comment.
After changing to type 9 since type 4 is deprecated:
| client_id=params['client_id'], | |
| client_secret=params['client_secret'] | |
| client_id=demisto.getParam('credentials')['identifier'], | |
| client_secret=demisto.getParam('credentials')['password'] |
| def http_request(self, method: str, url_suffix='', full_url=None, headers=None, json_data=None, params=None, auth=True): | ||
| """ | ||
| Sends an HTTP request to the specified URL, adding the required headers and authentication token. | ||
|
|
||
| Args: | ||
| method (str): The HTTP method to use (e.g., GET, POST, PUT, DELETE). | ||
| url_suffix (str, optional): The URL suffix to append to the base URL. Defaults to ''. | ||
| full_url (str, optional): The full URL to send the request to. If provided, `url_suffix` will be ignored. | ||
| Defaults to None. | ||
| headers (dict, optional): Additional headers to include in the request. Defaults to None. | ||
| json_data (dict, optional): JSON data to include in the request body. Defaults to None. | ||
| params (dict, optional): Query parameters to include in the request URL. Defaults to None. | ||
| auth (bool, optional): Whether to include authentication headers. Defaults to True. | ||
|
|
||
| Returns: | ||
| dict: The response from the HTTP request. | ||
|
|
||
| Raises: | ||
| Exception: If the request fails. | ||
|
|
||
| """ | ||
| if auth: | ||
| headers = headers or {} | ||
| headers['Authorization'] = f'Bearer {self._auth_token}' | ||
| try: | ||
| response = super()._http_request( | ||
| method=method, | ||
| url_suffix=url_suffix, | ||
| full_url=full_url, | ||
| headers=headers, | ||
| json_data=json_data, | ||
| params=params, | ||
| raise_on_status=True | ||
| ) | ||
| demisto.debug(f"Got response: {response}") | ||
| return response | ||
| except DemistoException as e: | ||
| demisto.error(f"Failed to execute {method} request to {url_suffix}. Error: {str(e)}") | ||
| raise Exception(f"Failed to execute {method} request to {url_suffix}. Error: {str(e)}") |
There was a problem hiding this comment.
you can use http_request from the CommonServerPython http_request and to add error handler which outputs the new formatted error message.
There was a problem hiding this comment.
I'm using this function to add the Authorization header to the request
There was a problem hiding this comment.
@liormgem I still recommend using the 'http_request' function from CommonServerPython.
It's supported within XSOAR, ensuring compatibility, and we actively maintain it, promptly adapting it to any changes that may arise.
Additionally, I'm available to assist you in integrating it into your code if needed.
However, if you still prefer to use your own 'http_request', you can proceed with that choice and resolve this note.
Packs/Gem/Integrations/Gem/Gem.py
Outdated
| if not time_start: | ||
| raise DemistoException('Start time is a required parameter.') | ||
|
|
||
| if not time_end: | ||
| raise DemistoException('End time is a required parameter.') |
There was a problem hiding this comment.
If start_time and end_time is required parameters then the user can't run the command without them.
Packs/Gem/Integrations/Gem/Gem.py
Outdated
| if not entity_id: | ||
| raise DemistoException('Entity ID is a required parameter.') | ||
|
|
||
| if not entity_type: | ||
| raise DemistoException('Entity Type is a required parameter.') | ||
|
|
||
| if not start_time: | ||
| raise DemistoException('Start time is a required parameter.') | ||
|
|
||
| if not end_time: | ||
| raise DemistoException('End time is a required parameter.') |
Packs/Gem/Integrations/Gem/Gem.py
Outdated
| if not action: | ||
| raise DemistoException('Action is a required parameter.') | ||
| if not entity_id: | ||
| raise DemistoException('Entity ID is a required parameter.') | ||
| if not entity_type: | ||
| raise DemistoException('Entity type is a required parameter.') | ||
| if not alert_id: | ||
| raise DemistoException('Alert ID is a required parameter.') | ||
| if not resource_id: | ||
| raise DemistoException('Resource ID is a required parameter.') |
There was a problem hiding this comment.
Please make sure those required in the command as well
Packs/Gem/Integrations/Gem/Gem.py
Outdated
| if not threat_id: | ||
| raise DemistoException('Threat ID is a required parameter.') | ||
| if not comment: | ||
| raise DemistoException('Comment is a required parameter.') |
There was a problem hiding this comment.
Please make sure those required in the command as well
|
Hi @maimorag, I'm working on uploading the example video to the demisto-assets repo and then I'll submit the Contribution form. Lior |
There was a problem hiding this comment.
Hey @liormgem
A couple of notes from my side.
- Note that I usually suggest not using the exact brand
GEMin the commands. In case a customer would like to copy your integration, he will need to change all the OOTB playbooks as well (there is an option to choose the non-brand command from the PB itself). This isn't required, however, a tip from my experience.
- Usually we add a
Donetask at the end. Just a cosmetic tip.
- Note that you don't have a playbook associated with your incident type. Once the investigation started the user would need to choose how to handle it automatically.
- Extracting all indicators from all fields might impact server performance, hence, suggesting to extract indicators from only necessary fields (basically the ones that you mapped).
Let me know if you need me to elaborate more on a point or help with anything else.
Cheers!
There was a problem hiding this comment.
Would like to suggest moving some of the fields under the case details to the investigation data (or even expanding the case details section), since if all the fields may be mapped, the user might miss that info.
Not mandatory but nice to have (of course analysts will appreciate it :) ).
There was a problem hiding this comment.
Hi @ssokolovich,
Thank you for the review.
- I think that would be fine with the amount of content we have.
- Will be Added
- Customers we consulted prefer to not have a default playbook
- There are indicators in other parts of the incident, not just in the mapped fields and we would like it to be processed. The number of incidents is low enough (not less than 10 a day) to not have an impact.
- We will leave it as it is, for now, it's extra information.
Lior
There was a problem hiding this comment.
Cool! so just let me know when you fix # 2 and also please remember to update the playbook images.
Then waiting for your update @liormgem.
Cheers!
There was a problem hiding this comment.
Great, done.
@ssokolovich
maimorag
added
the
ready-for-instance-test
maimorag
added
pending-demo
maimorag
merged commit fa6cb54
into
demisto:contrib/Gem-Security_gem_security_pack
|
Thank you for your contribution. Your external PR has been merged and the changes are now included in an internal PR for further review. The internal PR will be merged to the master branch within 3 business days. |
* Gem Security pack Commit Pack includes: 1 Automation 3 Classifiers 16 Incident Fields 1 Incident Type 1 Integration 1 Layout 3 Playbooks 1 Pre-process Rule * Ci fix (#5) * Fix post commit validation issues * Fix tests coverage * Fix indent (#6) * Cr fix (#7) * Fix playbooks * Format playbooks and fix Gem Alert Classifier * Fix blank space in Gem Layout * Fix incident fields * Fix cr (#8) * Fix double line * Update playbooks (#9) Co-authored-by: Lior Maman <155369912+liormgem@users.noreply.github.com>
* Gem Security pack Commit Pack includes: 1 Automation 3 Classifiers 16 Incident Fields 1 Incident Type 1 Integration 1 Layout 3 Playbooks 1 Pre-process Rule * Ci fix (demisto#5) * Fix post commit validation issues * Fix tests coverage * Fix indent (demisto#6) * Cr fix (demisto#7) * Fix playbooks * Format playbooks and fix Gem Alert Classifier * Fix blank space in Gem Layout * Fix incident fields * Fix cr (#8) * Fix double line * Update playbooks (demisto#9) Co-authored-by: Lior Maman <155369912+liormgem@users.noreply.github.com>
Contributing to Cortex XSOAR Content
Make sure to register your contribution by filling the contribution registration form
The Pull Request will be reviewed only after the contribution registration form is filled.
Status
Related Issues
fixes: link to the issue
Description
Pack Gem integrates with the Gem Security platform.
Pack includes:
Must have