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

x-pack/filebeat/input/entityanalytics/provider/internal/okta: relax profile shape #40359

Merged
merged 4 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ https:/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]
- Fix handling of deeply nested numeric values in HTTP Endpoint CEL programs. {pull}40115[40115]
- Prevent panic in CEL and salesforce inputs when github.com/hashicorp/go-retryablehttp exceeds maximum retries. {pull}40144[40144]
- Fix bug in CEL input rate limit logic. {issue}40106[40106] {pull}40270[40270]
- Relax requirements in Okta entity analytics provider user and device profile data shape. {pull}40359[40359]

*Heartbeat*

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,49 +41,12 @@
PasswordChanged *time.Time `json:"passwordChanged,omitempty"`
Type map[string]any `json:"type"`
TransitioningToStatus *string `json:"transitioningToStatus,omitempty"`
Profile Profile `json:"profile"`
Profile map[string]any `json:"profile"`
Credentials *Credentials `json:"credentials,omitempty"`
Copy link
Member

@andrewkroh andrewkroh Jul 31, 2024

Choose a reason for hiding this comment

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

Can you add the recovery_question.question to the credentials object please. That is field also part of a feature request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add it, but I think this is not a safe change. I would like to get input from infosec on their views on its inclusion given that knowledge of the question will give significant information to an attacker that has details about a victim.

Copy link
Member

Choose a reason for hiding this comment

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

I'm checking with the user to understand what their use case is for having this information is. If they actually do want it and understand the risk, then maybe this should be opt-in in some manner.

Links HAL `json:"_links,omitempty"` // See https://developer.okta.com/docs/reference/api/users/#links-object for details.
Embedded HAL `json:"_embedded,omitempty"`
}

// Profile is an Okta user's profile.
//
// See https://developer.okta.com/docs/reference/api/users/#profile-object for details.
type Profile struct {
Login string `json:"login"`
Email string `json:"email"`
SecondEmail *string `json:"secondEmail,omitempty"`
FirstName *string `json:"firstName,omitempty"`
LastName *string `json:"lastName,omitempty"`
MiddleName *string `json:"middleName,omitempty"`
HonorificPrefix *string `json:"honorificPrefix,omitempty"`
HonorificSuffix *string `json:"honorificSuffix,omitempty"`
Title *string `json:"title,omitempty"`
DisplayName *string `json:"displayName,omitempty"`
NickName *string `json:"nickName,omitempty"`
ProfileUrl *string `json:"profileUrl,omitempty"`
PrimaryPhone *string `json:"primaryPhone,omitempty"`
MobilePhone *string `json:"mobilePhone,omitempty"`
StreetAddress *string `json:"streetAddress,omitempty"`
City *string `json:"city,omitempty"`
State *string `json:"state,omitempty"`
ZipCode *string `json:"zipCode,omitempty"`
CountryCode *string `json:"countryCode,omitempty"`
PostalAddress *string `json:"postalAddress,omitempty"`
PreferredLanguage *string `json:"preferredLanguage,omitempty"`
Locale *string `json:"locale,omitempty"`
Timezone *string `json:"timezone,omitempty"`
UserType *string `json:"userType,omitempty"`
EmployeeNumber *string `json:"employeeNumber,omitempty"`
CostCenter *string `json:"costCenter,omitempty"`
Organization *string `json:"organization,omitempty"`
Division *string `json:"division,omitempty"`
Department *string `json:"department,omitempty"`
ManagerId *string `json:"managerId,omitempty"`
Manager *string `json:"manager,omitempty"`
}

// Credentials is a redacted Okta user's credential details. Only the credential provider is retained.
//
// See https://developer.okta.com/docs/reference/api/users/#credentials-object for details.
Expand Down Expand Up @@ -116,7 +79,7 @@
Created time.Time `json:"created"`
ID string `json:"id"`
LastUpdated time.Time `json:"lastUpdated"`
Profile DeviceProfile `json:"profile"`
Profile map[string]any `json:"profile"`
ResourceAlternateID string `json:"resourceAlternateID"`
ResourceDisplayName DeviceDisplayName `json:"resourceDisplayName"`
ResourceID string `json:"resourceID"`
Expand All @@ -130,27 +93,6 @@
Users []User `json:"users,omitempty"`
}

// DeviceProfile is an Okta device's hardware and security profile.
//
// See https://developer.okta.com/docs/api/openapi/okta-management/management/tag/Device/#tag/Device/operation/listDevices for details
type DeviceProfile struct {
DiskEncryptionType *string `json:"diskEncryptionType,omitempty"`
DisplayName string `json:"displayName"`
IMEI *string `json:"imei,omitempty"`
IntegrityJailBreak *bool `json:"integrityJailBreak,omitempty"`
Manufacturer *string `json:"manufacturer,omitempty"`
MEID *string `json:"meid,omitempty"`
Model *string `json:"model,omitempty"`
OSVersion *string `json:"osVersion,omitempty"`
Platform string `json:"platform"`
Registered bool `json:"registered"`
SecureHardwarePresent *bool `json:"secureHardwarePresent,omitempty"`
SerialNumber *string `json:"serialNumber,omitempty"`
SID *string `json:"sid,omitempty"`
TPMPublicKeyHash *string `json:"tpmPublicKeyHash,omitempty"`
UDID *string `json:"udid,omitempty"`
}

// DeviceDisplayName is an Okta device's annotated display name.
//
// See https://developer.okta.com/docs/api/openapi/okta-management/management/tag/Device/#tag/Device/operation/listDevices for details
Expand Down Expand Up @@ -346,7 +288,7 @@
defer resp.Body.Close()
err = oktaRateLimit(resp.Header, window, lim, log)
if err != nil {
io.Copy(io.Discard, resp.Body)

Check failure on line 291 in x-pack/filebeat/input/entityanalytics/provider/okta/internal/okta/okta.go

View workflow job for this annotation

GitHub Actions / lint (windows)

Error return value of `io.Copy` is not checked (errcheck)
return nil, nil, err
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,22 +117,23 @@
})

t.Run("user", func(t *testing.T) {
if me.Profile.Login == "" {
login, _ := me.Profile["login"].(string)
if login == "" {
b, _ := json.Marshal(me)
t.Skipf("cannot run user test without profile.login field set: %s", b)
}

query := make(url.Values)
query.Set("limit", "200")
users, _, err := GetUserDetails(context.Background(), http.DefaultClient, host, key, me.Profile.Login, query, omit, limiter, window, logger)
users, _, err := GetUserDetails(context.Background(), http.DefaultClient, host, key, login, query, omit, limiter, window, logger)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(users) != 1 {
t.Fatalf("unexpected len(users): got:%d want:1", len(users))
}
if !cmp.Equal(me, users[0]) {
t.Errorf("unexpected result:\n-'me'\n+'%s'\n%s", me.Profile.Login, cmp.Diff(me, users[0]))
t.Errorf("unexpected result:\n-'me'\n+'%s'\n%s", login, cmp.Diff(me, users[0]))
}
})

Expand Down Expand Up @@ -218,7 +219,7 @@
{
// Test case constructed from API-returned value with details anonymised.
name: "users",
msg: `[{"id":"userid","status":"STATUS","created":"2023-05-14T13:37:20.000Z","activated":null,"statusChanged":"2023-05-15T01:50:30.000Z","lastLogin":"2023-05-15T01:59:20.000Z","lastUpdated":"2023-05-15T01:50:32.000Z","passwordChanged":"2023-05-15T01:50:32.000Z","type":{"id":"typeid"},"profile":{"firstName":"name","lastName":"surname","mobilePhone":null,"secondEmail":null,"login":"[email protected]","email":"[email protected]"},"credentials":{"password":{"value":"secret"},"emails":[{"value":"[email protected]","status":"VERIFIED","type":"PRIMARY"}],"provider":{"type":"OKTA","name":"OKTA"}},"_links":{"self":{"href":"https://localhost/api/v1/users/userid"}}}]`,
msg: `[{"id":"userid","status":"STATUS","created":"2023-05-14T13:37:20.000Z","activated":null,"statusChanged":"2023-05-15T01:50:30.000Z","lastLogin":"2023-05-15T01:59:20.000Z","lastUpdated":"2023-05-15T01:50:32.000Z","passwordChanged":"2023-05-15T01:50:32.000Z","recovery_question":{"question":"Who's a major player in the cowboy scene?","answer":"Annie Oakley"},"type":{"id":"typeid"},"profile":{"firstName":"name","lastName":"surname","mobilePhone":null,"secondEmail":null,"login":"[email protected]","email":"[email protected]"},"credentials":{"password":{"value":"secret"},"emails":[{"value":"[email protected]","status":"VERIFIED","type":"PRIMARY"}],"provider":{"type":"OKTA","name":"OKTA"}},"_links":{"self":{"href":"https://localhost/api/v1/users/userid"}}}]`,
fn: func(ctx context.Context, cli *http.Client, host, key, user string, query url.Values, lim *rate.Limiter, window time.Duration, log *logp.Logger) (any, http.Header, error) {
return GetUserDetails(context.Background(), cli, host, key, user, query, OmitNone, lim, window, log)
},
Expand All @@ -236,7 +237,7 @@
{
// Test case constructed from API-returned value with details anonymised.
name: "devices_users",
msg: `[{"created":"2023-08-07T21:48:27.000Z","managementStatus":"NOT_MANAGED","user":{"id":"userid","status":"STATUS","created":"2023-05-14T13:37:20.000Z","activated":null,"statusChanged":"2023-05-15T01:50:30.000Z","lastLogin":"2023-05-15T01:59:20.000Z","lastUpdated":"2023-05-15T01:50:32.000Z","passwordChanged":"2023-05-15T01:50:32.000Z","type":{"id":"typeid"},"profile":{"firstName":"name","lastName":"surname","mobilePhone":null,"secondEmail":null,"login":"[email protected]","email":"[email protected]"},"credentials":{"password":{"value":"secret"},"emails":[{"value":"[email protected]","status":"VERIFIED","type":"PRIMARY"}],"provider":{"type":"OKTA","name":"OKTA"}},"_links":{"self":{"href":"https://localhost/api/v1/users/userid"}}}}]`,
msg: `[{"created":"2023-08-07T21:48:27.000Z","managementStatus":"NOT_MANAGED","user":{"id":"userid","status":"STATUS","created":"2023-05-14T13:37:20.000Z","activated":null,"statusChanged":"2023-05-15T01:50:30.000Z","lastLogin":"2023-05-15T01:59:20.000Z","lastUpdated":"2023-05-15T01:50:32.000Z","passwordChanged":"2023-05-15T01:50:32.000Z","type":{"id":"typeid"},"profile":{"firstName":"name","lastName":"surname","mobilePhone":null,"secondEmail":null,"login":"[email protected]","email":"[email protected]"},"credentials":{"password":{"value":"secret"},"recovery_question":{"question":"Who's a major player in the cowboy scene?","answer":"Annie Oakley"},"emails":[{"value":"[email protected]","status":"VERIFIED","type":"PRIMARY"}],"provider":{"type":"OKTA","name":"OKTA"}},"_links":{"self":{"href":"https://localhost/api/v1/users/userid"}}}}]`,
id: "devid",
fn: func(ctx context.Context, cli *http.Client, host, key, device string, query url.Values, lim *rate.Limiter, window time.Duration, log *logp.Logger) (any, http.Header, error) {
return GetDeviceUsers(context.Background(), cli, host, key, device, query, OmitNone, lim, window, log)
Expand Down Expand Up @@ -386,7 +387,7 @@
func TestNext(t *testing.T) {
for i, test := range nextTests {
got, err := Next(test.header)
if err != test.wantErr {

Check failure on line 390 in x-pack/filebeat/input/entityanalytics/provider/okta/internal/okta/okta_test.go

View workflow job for this annotation

GitHub Actions / lint (windows)

comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
t.Errorf("unexpected ok result for %d: got:%v want:%v", i, err, test.wantErr)
}
if got.Encode() != test.want {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,15 @@ func TestStateStore(t *testing.T) {
Type: map[string]interface{}{
"id": "typeid",
},
Profile: okta.Profile{
Login: "[email protected]",
Email: "[email protected]",
FirstName: ptr("name"),
LastName: ptr("surname"),
Profile: map[string]interface{}{
"login": "[email protected]",
"email": "[email protected]",
"firstName": "name",
"lastName": "surname",
},
Credentials: &okta.Credentials{
Password: &struct{}{}, // Had a password: not retained.
Password: &struct{}{}, // Had a password: not retained.
RecoveryQuestion: &struct{}{}, // Had a question: not retained.
Provider: okta.Provider{
Type: "OKTA",
Name: ptr("OKTA"),
Expand Down
Loading