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

Allow SiteID and/or AccountID in CommandLine #79

Merged
merged 18 commits into from
Nov 9, 2022

Conversation

rc-csmith
Copy link
Contributor

@rc-csmith rc-csmith commented Oct 4, 2022

Changes

  • Modify order of requirement checks so both cmdline and config file site_ids and account_ids are processed before checking for missing data
    • Have cmdline input take precedence over config file settings
      • If site IDs/account IDs/account names are provided as cmdline parameters, ignore config file options
    • Process account_ids and account_name before site_ids
      • If a site_id is already covered by an account_id, only use account_id
    • Log a warning if account_id, account_name, or site_id is not found
    • Save both account_id and site_id in _get_default_body()
    • Remove SiteID from merged query since that is already defined in _get_default_body()
    • Remove GET request to /web/api/v2.1/agents/count since API key validation naturally occurs in _get_site_ids()
  • This reordering will also account for the default profile issue

Closes #75 and #76

@rc-csmith rc-csmith self-assigned this Oct 4, 2022
@rc-csmith rc-csmith linked an issue Oct 4, 2022 that may be closed by this pull request
Copy link
Contributor

@xC0uNt3r7hr34t xC0uNt3r7hr34t left a comment

Choose a reason for hiding this comment

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

In addition to the suggested changes, it would be nice to prevent site_ids from being added to the DV query if they reach a certain number. For example when trying to query an account with 50+ siteIds, the query can reach a maximum limitation in the API query.

I recommend line 449 be changed to check if the length is greater than 50 and if so, do not add the siteIds to the query.

products/sentinel_one.py Show resolved Hide resolved
products/sentinel_one.py Outdated Show resolved Hide resolved
products/sentinel_one.py Outdated Show resolved Hide resolved
products/sentinel_one.py Outdated Show resolved Hide resolved
products/sentinel_one.py Outdated Show resolved Hide resolved
rc-csmith and others added 7 commits October 5, 2022 14:49
remove brackets around self._account_ids

Co-authored-by: xC0uNt3r7hr34t <61033168+xC0uNt3r7hr34t@users.noreply.github.com>
allow multiple account_ids in config file

Co-authored-by: xC0uNt3r7hr34t <61033168+xC0uNt3r7hr34t@users.noreply.github.com>
allow multiple site_ids in config file

Co-authored-by: xC0uNt3r7hr34t <61033168+xC0uNt3r7hr34t@users.noreply.github.com>
allow multiple account_names in config file

Co-authored-by: xC0uNt3r7hr34t <61033168+xC0uNt3r7hr34t@users.noreply.github.com>
products/sentinel_one.py Outdated Show resolved Hide resolved
products/sentinel_one.py Outdated Show resolved Hide resolved
rc-csmith and others added 2 commits October 7, 2022 08:52
Co-authored-by: xC0uNt3r7hr34t <61033168+xC0uNt3r7hr34t@users.noreply.github.com>
products/sentinel_one.py Outdated Show resolved Hide resolved
products/sentinel_one.py Outdated Show resolved Hide resolved
products/sentinel_one.py Outdated Show resolved Hide resolved
rc-csmith and others added 6 commits November 3, 2022 08:27
Co-authored-by: xC0uNt3r7hr34t <61033168+xC0uNt3r7hr34t@users.noreply.github.com>
Co-authored-by: xC0uNt3r7hr34t <61033168+xC0uNt3r7hr34t@users.noreply.github.com>
Co-authored-by: xC0uNt3r7hr34t <61033168+xC0uNt3r7hr34t@users.noreply.github.com>
Copy link
Contributor

@xC0uNt3r7hr34t xC0uNt3r7hr34t left a comment

Choose a reason for hiding this comment

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

Tested and verified siteID and accountID filtering works correctly. Ready to be merged.

@rc-csmith rc-csmith merged commit 6237ce3 into redcanaryco:master Nov 9, 2022
@rc-csmith rc-csmith deleted the 75_accountid_siteid_issue branch November 9, 2022 15:51
xC0uNt3r7hr34t pushed a commit to xC0uNt3r7hr34t/surveyor that referenced this pull request Jun 30, 2023
…issue

Allow SiteID and/or AccountID in CommandLine
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants