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

new commands and playbook added #25253

Merged
merged 9 commits into from Mar 22, 2023
Merged

Conversation

pk-druva
Copy link
Contributor

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

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: link to the issue

Description

A few sentences describing the overall goals of the pull request's commits.

Screenshots

Paste here any images that will help the reviewer

Minimum version of Cortex XSOAR

  • 6.0.0
  • 6.1.0
  • 6.2.0
  • 6.5.0

Does it break backward compatibility?

  • Yes
    • Further details:
  • No

Must have

  • Tests
  • Documentation

@CLAassistant
Copy link

CLAassistant commented Mar 14, 2023

CLA assistant check
All committers have signed the CLA.

@content-bot content-bot added the Contribution Thank you! Contributions are always welcome! label Mar 14, 2023
@content-bot content-bot changed the base branch from master to contrib/pk-druva_Druva March 14, 2023 05:28
@content-bot
Copy link
Collaborator

Thank you for your contribution. Your generosity and caring are unrivaled! Make sure to register your contribution by filling the Contribution Registration form, so our content wizard @yucohen will know the proposed changes are ready to bereviewed.

@kgal-pan kgal-pan self-assigned this Mar 14, 2023
@content-bot content-bot added Contribution Form Filled Whether contribution form filled or not. Partner labels Mar 14, 2023


def post_quarantineResource(self,org_id,resource_id, resource_type, from_date, to_date):
if org_id is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we expecting org_id to be defined? What happens if org_id is None? Currently, we still send it in json_data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case of Backupset that is FS,NAS or VMware OrgId is mandatory and in other resource types we don't need to give orgid

org_id=int(org_id)
json_data = {'orgID':org_id,'resourceType': resource_type, 'fromDate': from_date, 'toDate': to_date}

url_suffix = '/realize/ransomwarerecovery/v1/quarantineranges/resource/' + str(resource_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

We can utilize format strings in this. Looks cleaner IMO.

Suggested change
url_suffix = '/realize/ransomwarerecovery/v1/quarantineranges/resource/' + str(resource_id)
url_suffix = f"/realize/ransomwarerecovery/v1/quarantineranges/resource/{resource_id}"

Feel free to change in all other client method url_suffixs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

url_suffixs are now changed in all other client method

return (readable_output, outputs, raw_response)

else:
raise RuntimeError('Error: ' + str(response.status_code))
Copy link
Contributor

Choose a reason for hiding this comment

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

If there's an error in the request (statusCode != 200), does the endpoint provide an error message back (usually in response.content or response.json())?

I think that providing the user with Error: {statusCode} doesn't contain enough information for the user to know what's wrong and how to fix their issue. Might be helpful to add the response content when you raise the RuntimeError in all *_Command methods.

Comment on lines 433 to 434

if command == 'hi-command':
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove. Also, remove all other commented out code you have in the file, for example:

# return_outputs(*Druva_ListQuarantine_Snapshots_Command(clientObj, resource_id, range_id))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Commented code is removed

@efelmandar
Copy link
Contributor

Hi @pk-druva thank you very much for your contribution it is highly appreciated, I reviewed the playbook you added and everything looks great. just a couple of things to consider:

  1. Please add some descriptions to the playbook itself, playbook inputs, and playbook tasks to help the users better understand and use your content.
  2. The playbook begins with !DeleteContext all=yes which will delete all of the context every time the playbooks run, which can result in some unwanted behavior such as deleting other integration's data of an existing incident. please consider removing it if not strictly necessary.

Copy link
Contributor

@yucohen yucohen left a comment

Choose a reason for hiding this comment

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

@pk-druva Thanks for the contribution! great work!
Please see my comments :)

readable_output = tableToMarkdown('Found Druva users', responseJson['users'])
outputs = {"Druva.User(val.userID == obj.userID)": responseJson['users']}
raw_response = responseJson
return (readable_output, outputs, raw_response)
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets change this to a commandResults object. You can see this article. Relevant to all the new commands added

Copy link
Contributor Author

Choose a reason for hiding this comment

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

commandResults object is used in all commands.

@@ -282,12 +355,29 @@ def main():
search_string = demisto.args().get('search_string')
return_outputs(*Druva_FindDevice_Command(clientObj, search_string))

if command == 'druva-find-user':
user_string=demisto.args().get('user_string')
return_outputs(*Druva_FindUser_Command(clientObj,user_string))
Copy link
Contributor

Choose a reason for hiding this comment

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

After changing to ComandResults, lets also change the return outputs to return_results

Copy link
Contributor Author

Choose a reason for hiding this comment

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

return_outputs is replaced with return_results

@@ -15,10 +15,67 @@ configuration:
name: clientId
type: 4
required: true
defaultvalue: K7QYkjixOag7f2P0J/t2sgXq9NMsbzj/
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this default value should work for some users? If not, lets remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

default value is removed

- display: Secret Key
name: secretKey
type: 4
required: true
defaultvalue: 7AHzEWu9yYgjeB4GdWVs9QopoWRlIGc3
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

runonce: false
subtype: python3
longRunningPort: true
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a long running integration? if not, lets remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed long running integration.

Comment on lines 100 to 122
#### Command Example
```!druva-list-quarantine-ranges```

#### Context Example
```
{
"Druva": {
"activeQuarantineRanges": {
"fromDate": "2020-07-13",
"orgID": -1,
"rangeID": 415,
"recoveryStatus": "None",
"resourceID": 4497505,
"resourceName": "SahilG-MBP",
"resourceParent": "Druva Integrations",
"resourcePlatform": "darwin",
"resourceType": "Endpoint",
"toDate": "2020-07-15",
"workload": "endpoints"
}
}
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

why did we remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added Command Example


##### Druva Ransomware Response

- %%UPDATE_RN%%
Copy link
Contributor

Choose a reason for hiding this comment

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

please update the relevant change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated relevant change in 1_1_2.md

@pk-druva pk-druva requested review from kgal-pan and yucohen and removed request for efelmandar, kgal-pan and yucohen March 17, 2023 09:35
@yucohen yucohen requested a review from efelmandar March 19, 2023 10:25
@efelmandar
Copy link
Contributor

@pk-druva changes now look excellent! Thank you again for making this contribution!
@yucohen everything is done from PB side

Comment on lines 22 to 76
- display: Fetch indicators
name: feed
type: 8
required: false
- display: Indicator Verdict
name: feedReputation
type: 18
required: false
options:
- Unknown
- None
- Benign
- Good
- Suspicious
- Malicious
- Bad
additionalinfo: Indicators from this integration instance will be marked with this verdict
- display: Source Reliability
name: feedReliability
defaultvalue: F - Reliability cannot be judged
type: 15
required: true
options:
- A - Completely reliable
- B - Usually reliable
- C - Fairly reliable
- D - Not usually reliable
- E - Unreliable
- F - Reliability cannot be judged
additionalinfo: Reliability of the source providing the intelligence data
- display: ""
name: feedExpirationPolicy
defaultvalue: indicatorType
type: 17
required: false
options:
- never
- interval
- indicatorType
- suddenDeath
- display: ""
name: feedExpirationInterval
defaultvalue: "20160"
type: 1
required: false
- display: Feed Fetch Interval
name: feedFetchInterval
defaultvalue: "240"
type: 19
required: false
- display: Bypass exclusion list
name: feedBypassExclusionList
type: 8
required: false
additionalinfo: When selected, the exclusion list is ignored for indicators from this feed. This means that if an indicator from this feed is on the exclusion list, the indicator might still be added to the system.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the integration feed? If not, those parameters can be remoed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed these parameters.

@yucohen yucohen merged commit a2295ad into demisto:contrib/pk-druva_Druva Mar 22, 2023
17 of 19 checks passed
yucohen added a commit that referenced this pull request Mar 23, 2023
* new command added

* suggested changes have been made

* Feed parameters removed

* fixed lint

* fixed validate

* fixed validate

* fixed validate

---------

Co-authored-by: pk-druva <126672043+pk-druva@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contribution Form Filled Whether contribution form filled or not. Contribution Thank you! Contributions are always welcome! docs-approved Partner
Projects
None yet
6 participants