-
Notifications
You must be signed in to change notification settings - Fork 539
[microsoft-sentinel-intel] Update connector to be "manager_supported" #5339
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
Conversation
| 4. Default values | ||
| """ | ||
| if Path(settings_cls.model_config["yaml_file"] or "").is_file(): # type: ignore | ||
| if Path(settings_cls.model_config["yaml_file"] or "").is_file(): |
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.
suggestion
| if Path(settings_cls.model_config["yaml_file"] or "").is_file(): | |
| if Path(settings_cls.model_config.get("yaml_file", "")).is_file(): |
| if Path(settings_cls.model_config["yaml_file"] or "").is_file(): | ||
| return (YamlConfigSettingsSource(settings_cls),) | ||
| if Path(settings_cls.model_config["env_file"] or "").is_file(): # type: ignore | ||
| if Path(settings_cls.model_config["env_file"] or "").is_file(): |
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.
suggestion
| if Path(settings_cls.model_config["env_file"] or "").is_file(): | |
| if Path(settings_cls.model_config.get("env_file", "")).is_file(): |
64f1d5d to
ac06a00
Compare
3dbe9e1 to
c46b46f
Compare
jabesq
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.
See comments, I will push to fix them
| ) | ||
| scope: ListFromString = Field( | ||
| description="The scope of the stream connector.", | ||
| default="sentinel", |
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.
| ) | ||
| live_stream_id: str = Field( | ||
| description="The ID of the live stream to connect to.", | ||
| default="live", # listen the global stream (not filtered) |
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.
A stream ID must be explicitly set with appropriate filters.
Using "live" as the default is risky, as it would collect all events from every action on the OpenCTI platform.
Suggestion: remove default
| description="Whether to listen for delete events in the live stream.", | ||
| default=True, | ||
| ) | ||
| live_stream_no_dependencies: bool = Field( |
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.
suggestion: live_stream_no_dependencies and live_stream_listen_delete are already defined in BaseStreamConnectorConfig class => To be removed here
|
I still have UnauthorizedAccess error when using this connector with the given credentials: |
f1d1155 to
bb721af
Compare
a35f198 to
34b8e57
Compare
…e_stream_no_dependencies`
bb721af to
f58fd59
Compare
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.
No permission issue anymore, discussed with @jabesq
All good to me :)


Proposed changes
settings.pyconnector.pymain.pyor__main__.pyRelated issues
Checklist
Further comments
The code needs to be reviewed by two people: one must fix any issue, the other one review the final commits.