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

[Zscaler] Stream Connector #3007

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

MohamedMerimi
Copy link

@MohamedMerimi MohamedMerimi commented Nov 21, 2024

Hello the zscaler connector is finished, the fonctions of the connector are :
  • Authenticate with zscaler (obfuscation api key, JSESSIONID)
  • Detection of event creation and deletion (indicator)
  • Sending of domain type indicators to the black Zscaler list.
  • Use the “lookup” API request to check URL classification (domain name type indicator)
  • Verification of the domain type indicator if it is already in the Zscaler list, and if the indicator has a correct url format with the validator library.
  • Handling the flow limit of zcaler api requests
  • Apply change activation after sending domain names to zscaler's black list

Proposed changes

Related issues

Checklist

  • I consider the submitted work as finished
  • I tested the code for its functionality using different use cases
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality

Further comments

@yassine-ouaamou yassine-ouaamou added the community use to identify PR from community label Nov 21, 2024
@yassine-ouaamou yassine-ouaamou added partner used to identify PR from patner and removed community use to identify PR from community labels Dec 5, 2024
@Megafredo
Copy link
Member

Megafredo commented Dec 6, 2024

Hello @yassine-ouaamou, and @MohamedMerimi,
Since we don't have any identifying information to test this partner connector, we can't test the quality of data ingestion from this connector, so we'll just skim the connector code to highlight best practices.

Suggestions for Code Improvement

While reviewing the code for this PR, I have provided some recommendations for best practices and improvements that align with our connector development guidelines. Please refer to the Stream Connector template for further guidance.

1. Readme

  • Remove the duration_period environment variable from the README. This is unnecessary for stream connectors.
  • The provided Docker Compose example contains several non-existent or obsolete variables; consider removing it entirely.
  • Add more context about the connector's functionality to the README:
    • Scope: For example, explain what data is being ingested (e.g., indicators, STIX objects of type domain-name).
    • Stream Configuration Best Practices: Recommend filtering based on specific types (e.g., "indicators"), formats (e.g., "STIX") and types (e.g., "domain-name")
    • Added that the stream connector currently listens only for creation and deletion events but does not process updates.

2. Docker Compose

  • Remove the duration_period environment variable as it is not used in stream connectors.

3. requirements.txt

  • Explicitly pin dependency versions to ensure compatibility and stability:
    • Example: pycti==6.4.2, requests==2.32.2.
  • This practice prevents issues caused by unexpected updates in third-party libraries.

4. Add Traceback for Exception Handling

  • To improve exception handling in the main input of the script, add a traceback to the main block, see template example
import traceback

from stream_connector import ConnectorTemplate

if __name__ == "__main__":
    """
    Entry point of the script

    - traceback.print_exc(): This function prints the traceback of the exception to the standard error (stderr).
    The traceback includes information about the point in the program where the exception occurred,
    which is very useful for debugging purposes.
    - exit(1): effective way to terminate a Python program when an error is encountered.
    It signals to the operating system and any calling processes that the program did not complete successfully.
    """
    try:
        connector = ConnectorTemplate()
        connector.run()
    except Exception:
        traceback.print_exc()
        exit(1)

5. Logger

  • Leverage the built-in logger from the helper for better traceability and standardized log formatting
self.helper.connector_logger.info("Info message", {"key1": value1, "key2": value2})
self.helper.connector_logger.debug("Debugging message", {"key1": value1, "key2": value2})
self.helper.connector_logger.error("Error message", {"key1": value1, "key2": value2})

6. Add config.yml.sample and env.sample

  • Include sample configuration files to guide users, config.yml.sample or env.sample

7. Use get_config_variable()

  • Replace custom environment variable parsing logic in config_variables.py with use the helper.get_config_variable() method. (Like what you did for Comlaude)

    • This allows users to configure variables via config.yml, env files, or direct environment variables when using Docker.
    • Example see template :
self.api_base_url = get_config_variable(  
    "CONNECTOR_TEMPLATE_API_BASE_URL",  
    ["connector_template", "api_base_url"],  
    self.load,  
)

8. Tests Folder

  • If you do not plan to implement tests, consider removing the tests folder to reduce unnecessary clutter.

@MohamedMerimi
Copy link
Author

MohamedMerimi commented Dec 24, 2024

Hello @Megafredo,

  1. Readme : I modified the Readme, I have added some explanations of the zscaler connector, detailing some points
  2. Docker Compose : I deleted "duration_period"
  3. requirements.txt : I added the version of dependencies (requests~=2.32.2 , validators==0.33.0)
  4. Add Traceback for Exception Handling : I added Traceback in import and i added also "traceback.print_exc()
    exit(1)"
  5. Logger : I added "self.helper.connector_logger.info" in the scripts and delete the version obsolete of logs
  6. **Add config.yml.sample and env.sample : i added config.yml.sample in "src" directory
  7. Use get_config_variable() : I added "get_config_variable" like comlaude in the script "config_variables.py"
  8. Tests Folder : I deleted the test file

I did a few modifications to the connector (main.py , connector.py and config_variables.py) to improve the connector.

@romain-filigran romain-filigran added feature use for describing a new feature to develop and removed feature use for describing a new feature to develop labels Jan 7, 2025
@romain-filigran romain-filigran added this to the PRs backlog milestone Jan 7, 2025
Copy link
Member

@Megafredo Megafredo left a comment

Choose a reason for hiding this comment

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

That's fine, thanks again for the corrections and for your contribution ;)

PS: Hi @MohamedMerimi, unfortunately, there's a bit of a problem: all your commits need to be signed. We cannot merge without them.

PS2: Hi @MohamedMerimi, I've also just seen that in your docker-compose there's a variable you're not using:
- ZSCALER_API_BASE_URL=ChangeMe, can you take advantage of this to delete it too? thanks

Copy link
Member

@Megafredo Megafredo left a comment

Choose a reason for hiding this comment

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

That's fine, thanks again for the corrections and for your contribution ;)

PS: Hi @MohamedMerimi, unfortunately, there's a bit of a problem: all your commits need to be signed. We cannot merge without them.

PS2: Hi @MohamedMerimi, I've also just seen that in your docker-compose there's a variable you're not using:

  • ZSCALER_API_BASE_URL=ChangeMe, can you take advantage of this to delete it too? thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
partner used to identify PR from patner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants