-
Notifications
You must be signed in to change notification settings - Fork 422
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
[entityanalytics_okta] Initial Release for the Entity Analytics Okta #6911
[entityanalytics_okta] Initial Release for the Entity Analytics Okta #6911
Conversation
🌐 Coverage report
|
responses: | ||
- status_code: 200 | ||
body: | | ||
[{"id":"00ub0oNGTSWTBKOLGLNR","status":"ACTIVE","created":"2013-06-24T16:39:18.000Z","activated":"2013-06-24T16:39:19.000Z","statusChanged":"2013-06-24T16:39:19.000Z","lastLogin":"2013-06-24T17:39:19.000Z","lastUpdated":"2013-07-02T21:36:25.344Z","passwordChanged":"2013-07-02T21:36:25.344Z","profile":{"firstName":"Isaac","lastName":"Brock","email":"isaac.brock@example.com","login":"isaac.brock@example.com","mobilePhone":"555-415-1337"},"credentials":{"password":{},"recovery_question":{"question":"Who's a major player in the cowboy scene?"},"provider":{"type":"OKTA","name":"OKTA"}}}] |
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.
In the password
field, please add subfields, "value": "tlpWENT2m"
and to recovery_question
please add "answer": "Annie Oakley"
. These will be stripped by the provider so the change will test that this is the case and we are not retaining secrets.
[{"id":"00ub0oNGTSWTBKOLGLNR","status":"ACTIVE","created":"2013-06-24T16:39:18.000Z","activated":"2013-06-24T16:39:19.000Z","statusChanged":"2013-06-24T16:39:19.000Z","lastLogin":"2013-06-24T17:39:19.000Z","lastUpdated":"2013-07-02T21:36:25.344Z","passwordChanged":"2013-07-02T21:36:25.344Z","profile":{"firstName":"Isaac","lastName":"Brock","email":"isaac.brock@example.com","login":"isaac.brock@example.com","mobilePhone":"555-415-1337"},"credentials":{"password":{},"recovery_question":{"question":"Who's a major player in the cowboy scene?"},"provider":{"type":"OKTA","name":"OKTA"}}}] | |
[{"id":"00ub0oNGTSWTBKOLGLNR","status":"ACTIVE","created":"2013-06-24T16:39:18.000Z","activated":"2013-06-24T16:39:19.000Z","statusChanged":"2013-06-24T16:39:19.000Z","lastLogin":"2013-06-24T17:39:19.000Z","lastUpdated":"2013-07-02T21:36:25.344Z","passwordChanged":"2013-07-02T21:36:25.344Z","profile":{"firstName":"Isaac","lastName":"Brock","email":"isaac.brock@example.com","login":"isaac.brock@example.com","mobilePhone":"555-415-1337"},"credentials":{"password":{"value": "tlpWENT2m"},"recovery_question":{"question":"Who's a major player in the cowboy scene?","answer": "Annie Oakley"},"provider":{"type":"OKTA","name":"OKTA"}}}] |
"credentials": { | ||
"password": { | ||
"hash": { | ||
"algorithm": "SHA-1", | ||
"salt": "UEO3wsAsgzQ=", | ||
"saltOrder": "POSTFIX", | ||
"value": "xjrauE6J6kbjcvMjWSSc+PsBBls=" | ||
} | ||
}, | ||
"recovery_question": { | ||
"question": "Who's a major player in the cowboy scene?" | ||
}, | ||
"provider": { | ||
"type": "OKTA", | ||
"name": "OKTA" | ||
} | ||
} |
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 provider does not fill password or recovery_question. If you are finding that it does, this is a defect, so please don't have pipeline tests that include these fields (or if you do, ensure that the secrets are deleted from the document before ingest).
Looking at sample_event.json, this appears to be true, happily.
- name: credentials | ||
type: group | ||
fields: | ||
- name: password |
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.
This group should never be present.
type: keyword | ||
- name: type | ||
type: keyword | ||
- name: recovery_question |
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.
This group should never be present.
- name: sync_interval | ||
type: text | ||
title: Sync Interval | ||
description: How often full synchronizations should occur. Must be greater than Update Interval. Expected value is a duration string (15m, 1h, 1m30, etc), defaults to 24h. NOTE:- Supported units for this parameter are h/m/s. |
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.
description: How often full synchronizations should occur. Must be greater than Update Interval. Expected value is a duration string (15m, 1h, 1m30, etc), defaults to 24h. NOTE:- Supported units for this parameter are h/m/s. | |
description: How often full synchronizations should occur. Must be greater than Update Interval. Expected value is a duration string (15m, 1h, 1m30, etc), defaults to 24h. Supported units for this parameter are h/m/s. |
- name: update_interval | ||
type: text | ||
title: Update Interval | ||
description: How often incremental updates should occur. Must be less than Sync Interval. Expected value is a duration string (15m, 1h, 1m30, etc), defaults to 15m. NOTE:- Supported units for this parameter are h/m/s. |
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.
description: How often incremental updates should occur. Must be less than Sync Interval. Expected value is a duration string (15m, 1h, 1m30, etc), defaults to 15m. NOTE:- Supported units for this parameter are h/m/s. | |
description: How often incremental updates should occur. Must be less than Sync Interval. Expected value is a duration string (15m, 1h, 1m30, etc), defaults to 15m. Supported units for this parameter are h/m/s. |
- name: http_client_timeout | ||
type: text | ||
title: HTTP Client Timeout | ||
description: "Duration before declaring that the HTTP client connection has timed out. NOTE: Valid time units are ns, us, ms, s, m, h." |
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.
description: "Duration before declaring that the HTTP client connection has timed out. NOTE: Valid time units are ns, us, ms, s, m, h." | |
description: "Duration before declaring that the HTTP client connection has timed out. Valid time units are ns, us, ms, s, m, h." |
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.
LGTM after clarifications.
@@ -0,0 +1,234 @@ | |||
- name: asset |
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.
Is the asset
group formally approved?
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.
We've added the asset
and user
group objects to the fields.yml
because these fields are coming to the upcoming ECS Schema. (@jamiehynds told us to follow the PR and map this field to the upcoming proposed ECS fields.)
I've one concern: if in the future (let's say 8.9) this proposed field comes in ECS, then it may be a conflict that we've added that in fields.yml
and also that it will be added by import mappings (from the build.yml). (Again, that scenario may arise when we have the same fields in agent.yml
and ecs.yml
)
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.
Thanks. I had not seen that ECS RFC. When the fields are in we can clean up any issues that arise.
- name: type | ||
type: object | ||
description: user type that determines the schema for the user's profile. | ||
- name: labels |
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.
Same query for labels
.
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.
For labels
, it is a necessary field that I have to add in fields.yml
in order to execute a successful system test. If we don't provide that, then it results in labels.identity_source is undefined
error. even if the labels
field is added by the import_mapping feature
.
So, to resolve this issue, I've added labels.identity_source
in fields.yml
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.
Since labels is an ECS field, please add this to the ecs.yml, its totally fine to define ECS fields in ecs.yml even though dynamic mappings are enabled, especially if they are not covered by it. I will take a note to update the dynamic template.
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.
@P1llus, should we add upcoming proposed ECS fields in ecs.yml too ?
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.
@brijesh-elastic sure that would be great!
Package entityanalytics_okta - 0.1.0 containing this change is available at https://epr.elastic.co/search?package=entityanalytics_okta |
…6911) * Initial Release for the Entity Analytics Okta * Update the changelog entry * Resolve the comments * Resolve the comments
What does this PR do?
Integration release checklist
This checklist is intended for integrations maintainers to ensure consistency
when creating or updating a Package, Module or Dataset for an Integration.
All changes
New Package
Dashboards changes
Log dataset changes
How to test this PR locally
Screenshots
Automated Test