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
x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher: add device handling #35807
Conversation
71e0e07
to
9d1adf7
Compare
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
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.
Looks Good.
Just a thought though : There are instances where a lot of code is duplicated between users
groups
and devices
. Not sure if we can think of a solution that helps to add more entities in future in a simple way?
x-pack/filebeat/input/entityanalytics/provider/azuread/azure.go
Outdated
Show resolved
Hide resolved
"github.com/elastic/elastic-agent-libs/mapstr" | ||
) | ||
|
||
// TODO: Implement fetchers for the registered owners and users |
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 github issue link to the TODO ?
x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/device.go
Show resolved
Hide resolved
x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/device.go
Show resolved
Hide resolved
This pull request is now in conflicts. Could you fix it? 🙏
|
x-pack/filebeat/input/entityanalytics/provider/azuread/azure.go
Outdated
Show resolved
Hide resolved
x-pack/filebeat/input/entityanalytics/provider/azuread/azure.go
Outdated
Show resolved
Hide resolved
@@ -165,6 +165,21 @@ func (p *azure) runFullSync(inputCtx v2.Context, store *kvstore.Store, client be | |||
tracker.Wait() | |||
} | |||
|
|||
if len(state.devices) != 0 { | |||
tracker := kvstore.NewTxTracker(ctx) | |||
|
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.
I'm wondering, do we want a single set of writer markers to bound both users and devices? If we want them separate, do we want to attach any sort of field to indicate what the write markers are associated with? (device or user).
Thoughts, @andrewkroh?
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.
Good point, either is acceptable to me. If it is two separate marks then someone who is querying would need to be know if it was associated to the users or devices.
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.
ISTM a single set makes more sense. devices are users are really pretty much the same kind of entity (at the moment more so, with the TODO additions less so).
x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/device.go
Show resolved
Hide resolved
|
||
return nil | ||
}); err != nil && !errIsItemNotFound(err) { | ||
return nil, fmt.Errorf("unable to get users from state: %w", err) |
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.
s/users/devices
9d1adf7
to
267f68a
Compare
This pull request is now in conflicts. Could you fix it? 🙏
|
Wrap update and sync publication of users and devices together into single tracker/write marker operations.
bda997c
to
6f4faa5
Compare
What does this PR do?
Adds device support to the Azure AD entity analytics input provider.
Why is it important?
Milestone task needed for security solution.
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs