Skip to content
This repository has been archived by the owner on Dec 12, 2023. It is now read-only.

Atlassian conversion #6

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Atlassian conversion #6

wants to merge 7 commits into from

Conversation

nkulig
Copy link

@nkulig nkulig commented Feb 2, 2023

Background

This branch contains the changes necessary to bring the Atlassian rules from panther-analysis over to panther-detections.

Changes

An Atlassian folder was added under the Providers folder that contains the folder structure necessary to move forward with additional rules in the future. For now, there is only one rule to convert, and that has been added.

Testing

Tests are included under tests/providers/atlassian and I verified that the rule works correctly by running: pipenv run panther_analysis_tool --debug sdk test

@nkulig nkulig requested review from a team February 2, 2023 22:53
Copy link
Contributor

@maxrichie5 maxrichie5 left a comment

Choose a reason for hiding this comment

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

Awesome!!!! Thanks for doing this. Just a few minor comments

from typing import Literal

from . import queries, rules, sample_logs
from ._shared import *
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove this from here as the _ prefix implies it should not be exported

return {
"Timestamp": event.deep_get("attributes", "time", default="<unknown-time>"),
"Actor": event.deep_get("attributes", "actor", "email", default="<unknown-actor-email>"),
"Impersonated user": event.deep_get("attributes", "context", default=[{}])[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Does deep get not work with list indexes?

overrides=detection.RuleOverrides(name=name_override)
)

self.assertIsInstance(rule, detection.Rule)
Copy link
Contributor

Choose a reason for hiding this comment

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

This test will fail won't it?

It would also be good to add a test for the title function 😄

Copy link

@darwayne darwayne left a comment

Choose a reason for hiding this comment

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

Nice work! I think this looks good once Max's comments are addressed

overrides=overrides,
name="Atlassian user logged in as user",
rule_id="Atlassian.User.LoggedInAsUser",
log_types=[SYSTEM_LOG_TYPE],
Copy link
Contributor

Choose a reason for hiding this comment

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

with the latest version of SDK, we can now use schema.LogTypeAtlassianAudit instead.

@maxrichie5
Copy link
Contributor

There have been some updates to the standard of how to write detections in this repo. Mainly 2:

  • Log types have constants we can use be be consistant
  • pre-filters are now replaced with extensions that work the same way overrides do. No more need for pre-filter stuff

Please make sure to pull in main to follow this pattern

Thanks again for all your work on this!!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants