-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add support for API 2.0 in Mimecast Event Collector #42498
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
base: master
Are you sure you want to change the base?
Conversation
Coverage Report
|
||||||||||||||||||||||||||||||
|
Validate summary Verdict: PR can be force merged from validate perspective? ✅ |
🤖 Content-bot Review DisclaimerThis review was generated by an AI-powered tool and may contain inaccuracies. Please be advised, and we extend our sincere apologies for any inconvenience this may cause. |
content-bot
left a comment
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.
Hi there! Thanks for the update to the Mimecast pack. I've reviewed the changes and have a few points to discuss, primarily regarding the proxy implementation and test mocking.
Please ensure proxy arguments are passed correctly to the request methods rather than the aiohttp session, and update the unit tests to use AsyncMock for asynchronous methods. It would also be helpful to clarify the API 2.0 credential steps in the documentation and consolidate the breaking changes in the release notes.
Great work so far
@richardbluestone, @JasBeilin please review and approve the results generated by the AI Reviewer by responding 👍 on this comment.
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "breakingChanges": true, | |||
| "breakingChangesNotes": "Migrated from the legacy API 1.0 authentication method to OAuth2 client credentials flow for Mimecast API 2.0. Integration instances must be re-configured using OAuth2 Client ID and Client Secret." | |||
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.
Consider adding the removal of the 'First fetch timestamp' parameter to the breaking changes notes.
| import aiohttp | ||
| from http import HTTPStatus | ||
| import asyncio | ||
| from typing import Any |
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.
Missing imports cast and traceback.
| self._proxy_url = handle_proxy().get("http", "") if proxy else None | ||
|
|
||
| async def __aenter__(self): | ||
| self._session = aiohttp.ClientSession(connector=aiohttp.TCPConnector(ssl=self._verify), proxy=self._proxy_url) |
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.
aiohttp.ClientSession does not accept a proxy argument.
| raise DemistoException(f"There was an error with siem events call {fail_reason}") | ||
| while retry_count <= max_retries: | ||
| try: | ||
| async with self._session.request(method=method, url=url, headers=headers, **request_kwargs) as response: |
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.
Proxy configuration is missing in the request.
| demisto.debug(f"[{event_type}] Received HTTP 401 Unauthorized. Generating new token and retrying...") | ||
| headers["Authorization"] = await self.get_authorization_header(force_generate_new_token=True) | ||
| # Retry immediately with new token | ||
| async with self._session.request(method=method, url=url, headers=headers, **request_kwargs) as response: |
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.
Proxy configuration is missing in the retry request.
| | Base URL | Use `https://api.services.mimecast.com` for the Global region or review the [Mimecast guide on per-region Base URLs](https://integrations.mimecast.com/documentation/api-overview/global-base-urls/) to find the suitable Base URL. | True | | ||
| | Client ID | Refer to the help section for instructions on how to obtain API 2.0 OAuth2 credentials. | True | | ||
| | Client secret | Refer to the help section for instructions on how to obtain API 2.0 OAuth2 credentials. | True | | ||
| | Fetch events | | False | |
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.
Add a description for the "Fetch events" parameter.
| | **Argument Name** | **Description** | **Required** | | ||
| | --- | --- | --- | | ||
| | should_push_events | Set this argument to True in order to create events, otherwise the command will only display them. Possible values are: True, False. Default is False. | Required | | ||
| | should_push_events | If True, the command will push the events to the Cortex XSIAM dataset; otherwise, it will only display them. Default is False. | Required | |
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.
Clarify if this argument is Required or Optional.
| Note: The supported time format is yyyy-MM-ddThh:mm:ss+|-nnnn (2021-12-08T10:00:00-0400). The relevant fields are "datetime" or "eventTime" | ||
| # Mimecast | ||
|
|
||
| Mimecast is a cloud-based email security and management platform that provides comprehensive protection against email-borne threats, data leaks, and ensures business continuity through email archiving and continuity services. |
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.
Grammar suggestion: Insert 'and' between 'email-borne threats' and 'data leaks' for better sentence structure.
|
|
||
| ##### Mimecast Event Collector | ||
|
|
||
| - **Breaking Changes**: Migrated from the legacy API 1.0 authentication method to OAuth2 client credentials flow for Mimecast API 2.0. |
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.
Consolidate all breaking changes under a single "Breaking Changes" bullet point for better readability and adherence to standards.
|
|
||
| - Added support for the ***Fetch event types*** parameter that denotes the types of audit and SIEM events to fetch. | ||
|
|
||
| - Added support for the `siem_event_types`, `start_date`, and `end_date` arguments in the ***!mimecast-get-events*** command. |
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.
The argument name siem_event_types does not match the integration YAML. It should be event_types.
@julieschwartz18 Can you do this? |
Related Issues
fixes: CIAC-14914
Description
Add support for Mimecast API 2.0 and implement async event fetching logic.
Version release notes: