[Cisco Duo] - Added fix for 400 and 401 errors in telephony v2 data stream by ShourieG · Pull Request #11640 · elastic/integrations

ShourieG · 2024-11-05T12:10:42Z

Type of change

Bug

Proposed commit message

Cisco API documentation mentions that the Date header should be equal to the current timestamp. In the current CEL program, the previous execution's state.date is being captured and used in pagination and subsequent requests leading to possible 40103 errors. In a related issue users also observed the error 400: mintime must be within the past 180 days.

This PR:

Fixes 40103 by creating Date header during each CEL request.
Preemptively fixes the 400 error by checking duration between mintime and maxtime doesn't cross
180d, based on a similar issue & PR.

Note:

This PR was derived from a similar PR here
The version bump here is from 2.2.0 to 2.2.2 because this will only be merged after the PR mentioned above is
merged, and that already increments the version to 2.2.1.

Checklist

I have reviewed tips for building integrations and this pull request is aligned with them.
I have verified that all data streams collect metrics or logs.
I have added an entry to my package's changelog.yml file.
I have verified that Kibana version constraints are current according to guidelines.
I have verified that any added dashboard complies with Kibana's Dashboard good practices

Author's Checklist

[ ]

How to test this PR locally

Related issues

Relates cisco_duo.trust_monitor: Fix 400 and 401 errors in Trust Monitor #11621

Screenshots

elasticmachine · 2024-11-05T12:14:44Z

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

elastic-vault-github-plugin-prod · 2024-11-05T12:34:42Z

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

efd6 · 2024-11-06T10:18:33Z

+        "maxtime": (int(now - duration("2m"))).as(mx,
+            mx < int(timestamp(int(state.mintime) / 1000) + duration("4320h")) ? string(mx * 1000) : string(int(timestamp(int(state.mintime) / 1000) + duration("4320h") - duration("2m")) * 1000)
+        )


Suggested change

"maxtime": (int(now - duration("2m"))).as(mx,

mx < int(timestamp(int(state.mintime) / 1000) + duration("4320h")) ? string(mx * 1000) : string(int(timestamp(int(state.mintime) / 1000) + duration("4320h") - duration("2m")) * 1000)

)

"maxtime":int(now - duration("2m"))).as(mx,

mx < int(timestamp(state.mintime / 1000) + duration("4320h")) ?

mx

:

int(timestamp(state.mintime / 1000) + duration("4320h") - duration("2m"))

) * 1000

Why is the 2m subtracted from the false branch?

Since this PR is same as #11621, I am also addressing the comments.
@ShourieG, please make similar change if @efd6 agrees to the changes.

Why is the 2m subtracted from the false branch?

Makes sense. Addressed in edcac5a.

Did not remove the string( conversion due to reasons in #11640 (comment)

efd6 · 2024-11-06T10:19:25Z

@@ -31,114 +31,108 @@ program: |
            state


What happens with the calculation below when this path is taken? I don't see a place where this would have mintime, but it's expected below. what am I missing?

In the case of want_more: true i.e., pagination requests, since we are wrapping the state using .with, it should already contain the mintime from its non-pagination request.

efd6 · 2024-11-06T10:20:05Z

-                "mintime": state.?cursor.last_published.orValue(int(now - duration(state.initial_interval)) * 1000),
-                "maxtime": int(now - duration("2m")) * 1000,
-                "date": now.format(time_layout.RFC1123Z),
+                "mintime": string(state.?cursor.last_published.orValue(int(now - duration(state.initial_interval)) * 1000)),


Suggested change

"mintime": string(state.?cursor.last_published.orValue(int(now - duration(state.initial_interval)) * 1000)),

"mintime": state.?cursor.last_published.orValue(int(now - duration(state.initial_interval)) * 1000),

When string( is removed from state's mintime and maxtime, CEL gives compile time errors: failed to check program: failed compilation: ERROR: <input>:24:41: found no matching overload for 'trim_right' applied to 'int.(string)'\n | \"url_base\": state.url.trim_right(\"/\"),\n | ........................................^\nERROR: <input>:38:35: found no matching overload for '_+_' applied to '(int, string)'\n | state.integration_key + \":\" + (\n | ..................................^\nERROR: <input>:46:36: found no matching overload for 'bytes' applied to '(int)'\n | .hmac(\"sha1\", bytes(state.secret_key))\n | ...................................^ accessing config.

Essentially, I think whatever is the first value getting assigned inside state (in this case mintime) should be of type string. If not, the rest of the values, such as url, integration_key, etc. are also being treated as the datatype of first assigned value (in this case int). Not sure if this is because of adding dyn(in few places, because it wasn't the issue in current version of the code (which usesmintimeandmaxtimeasint`).

ShourieG · 2024-11-07T08:24:30Z

@efd6, I've updated the PR as per the suggestions made. Let me know if @kcreddy's suggestions are ok atm or if they require further changes.

ShourieG · 2024-11-07T13:10:45Z

@chrisberkhout, I've updated the PR with suggestions made here: #11621

chrisberkhout · 2024-11-07T17:26:34Z

@chrisberkhout, I've updated the PR with suggestions made here: #11621

Thanks. Let's wait for that one to merge and then I'll take a final look at this one.

elasticmachine · 2024-11-08T09:05:24Z

💚 Build Succeeded

Buildkite Build
Commit: 57ead65

History

💚 Build #18048 succeeded 77ecc6e
💚 Build #18016 succeeded 163d780
💚 Build #18001 succeeded 958bc35
💚 Build #17996 succeeded ee0abcf
💚 Build #17918 succeeded 961fda2

cc @ShourieG

elastic-sonarqube · 2024-11-08T09:05:27Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

ShourieG · 2024-11-08T09:39:22Z

@chrisberkhout, could you take a final look at the PR since @kcreddy's PR has now been merged

elastic-vault-github-plugin-prod · 2024-11-08T18:51:09Z

Package cisco_duo - 2.2.2 containing this change is available at https://epr.elastic.co/search?package=cisco_duo

…tream (elastic#11640) * added fix in telephony_v2 data stream for 400 & 401 errors

…stic#12492) In an earlier version of the CEL code for `activity`[1] and `telephony_v2`[2], `cursor.last_published` was set to a UNIX timestamp value. This was changed to use RFC3339 formatted times in later PRs[3][4] (with the corresponding read-time parsing added in [5]). Users who didn't create a new policy may have the current parsing logic fail when it encounters an old UNIX timestamp value in `cursor.last_published`. This PR addresses that issue by renaming `cursor.last_published` to `cursor.last_response_ts`. That effectively clears the cursor so that obsolete values will not be seen. [1]: https://github.com/elastic/integrations/blob/2ea993/packages/cisco_duo/data_stream/activity/agent/stream/cel.yml.hbs#L111-L114 [2]: https://github.com/elastic/integrations/blob/2ea993/packages/cisco_duo/data_stream/telephony_v2/agent/stream/cel.yml.hbs#L112-L115 [3]: elastic#11640 [4]: elastic#11670 [5]: elastic#11772

…tream (elastic#11640) * added fix in telephony_v2 data stream for 400 & 401 errors

…stic#12492) In an earlier version of the CEL code for `activity`[1] and `telephony_v2`[2], `cursor.last_published` was set to a UNIX timestamp value. This was changed to use RFC3339 formatted times in later PRs[3][4] (with the corresponding read-time parsing added in [5]). Users who didn't create a new policy may have the current parsing logic fail when it encounters an old UNIX timestamp value in `cursor.last_published`. This PR addresses that issue by renaming `cursor.last_published` to `cursor.last_response_ts`. That effectively clears the cursor so that obsolete values will not be seen. [1]: https://github.com/elastic/integrations/blob/2ea993/packages/cisco_duo/data_stream/activity/agent/stream/cel.yml.hbs#L111-L114 [2]: https://github.com/elastic/integrations/blob/2ea993/packages/cisco_duo/data_stream/telephony_v2/agent/stream/cel.yml.hbs#L112-L115 [3]: elastic#11640 [4]: elastic#11670 [5]: elastic#11772

added fix in telephony_v2 data stream for 400 & 401 errors

e59bfdc

ShourieG requested a review from a team as a code owner November 5, 2024 12:10

updated changelog

961fda2

ShourieG self-assigned this Nov 5, 2024

ShourieG added integration Label used for meta issues tracking each integration Integration:cisco_duo Cisco Duo bugfix Pull request that fixes a bug issue Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] labels Nov 5, 2024

efd6 reviewed Nov 6, 2024

View reviewed changes

kcreddy mentioned this pull request Nov 6, 2024

cisco_duo.trust_monitor: Fix 400 and 401 errors in Trust Monitor #11621

Merged

5 tasks

addressed PR suggestions

ee0abcf

chrisberkhout reviewed Nov 7, 2024

View reviewed changes

Comment thread packages/cisco_duo/data_stream/telephony_v2/agent/stream/cel.yml.hbs Outdated

ShourieG added 3 commits November 7, 2024 14:48

updated as per PR suggestions

958bc35

updated maxtime calculation logic based on suggestions on a similar PR

12af8bf

updated mintime calculation logic based on suggestions on a similar PR

163d780

resolved merge conflicts

77ecc6e

chemamartinez reviewed Nov 8, 2024

View reviewed changes

Comment thread packages/cisco_duo/data_stream/telephony_v2/agent/stream/cel.yml.hbs

removed next_offset_timestamp definition

57ead65

chemamartinez mentioned this pull request Nov 8, 2024

cisco_duo: fix 400 and 401 errors in Auth and Activity #11670

Merged

5 tasks

chrisberkhout approved these changes Nov 8, 2024

View reviewed changes

ShourieG merged commit f4550b7 into elastic:main Nov 8, 2024

ShourieG deleted the cisco_duo/fix_sdh_5334 branch November 8, 2024 18:33

chrisberkhout mentioned this pull request Jan 28, 2025

[cisco_duo] Avoid obsolete cursor data in activity, telephony_v2 #12492

Merged

5 tasks

harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 4, 2025

[Cisco Duo] - Added fix for 400 and 401 errors in telephony v2 data s…

7ffeb68

…tream (elastic#11640) * added fix in telephony_v2 data stream for 400 & 401 errors

harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 5, 2025

[Cisco Duo] - Added fix for 400 and 401 errors in telephony v2 data s…

a2fd207

…tream (elastic#11640) * added fix in telephony_v2 data stream for 400 & 401 errors

Navigation Menu

Uh oh!

[Cisco Duo] - Added fix for 400 and 401 errors in telephony v2 data stream - #11640

[Cisco Duo] - Added fix for 400 and 401 errors in telephony v2 data stream#11640
ShourieG merged 8 commits into
elastic:mainfrom
ShourieG:cisco_duo/fix_sdh_5334

ShourieG commented Nov 5, 2024 •

edited

Loading

elasticmachine commented Nov 5, 2024

elastic-vault-github-plugin-prod Bot commented Nov 5, 2024 •

edited

Loading

efd6 Nov 6, 2024

kcreddy Nov 6, 2024 •

edited

Loading

efd6 Nov 6, 2024

kcreddy Nov 6, 2024

efd6 Nov 6, 2024

kcreddy Nov 6, 2024

Uh oh!

ShourieG commented Nov 7, 2024

Uh oh!

ShourieG commented Nov 7, 2024

chrisberkhout commented Nov 7, 2024

Uh oh!

elasticmachine commented Nov 8, 2024

elastic-sonarqube Bot commented Nov 8, 2024

ShourieG commented Nov 8, 2024

elastic-vault-github-plugin-prod Bot commented Nov 8, 2024

Labels

6 participants

	"mintime": string(state.?cursor.last_published.orValue(int(now - duration(state.initial_interval)) * 1000)),
	"mintime": state.?cursor.last_published.orValue(int(now - duration(state.initial_interval)) * 1000),

Uh oh!

Conversation

ShourieG commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change

Proposed commit message

Note:

Checklist

Author's Checklist

How to test this PR locally

Related issues

Screenshots

elasticmachine commented Nov 5, 2024

elastic-vault-github-plugin-prod Bot commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚀 Benchmarks report

efd6 Nov 6, 2024

Choose a reason for hiding this comment

kcreddy Nov 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

efd6 Nov 6, 2024

Choose a reason for hiding this comment

kcreddy Nov 6, 2024

Choose a reason for hiding this comment

efd6 Nov 6, 2024

Choose a reason for hiding this comment

kcreddy Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

ShourieG commented Nov 7, 2024

Uh oh!

ShourieG commented Nov 7, 2024

chrisberkhout commented Nov 7, 2024

Uh oh!

elasticmachine commented Nov 8, 2024

💚 Build Succeeded

History

elastic-sonarqube Bot commented Nov 8, 2024

Quality Gate passed

ShourieG commented Nov 8, 2024

elastic-vault-github-plugin-prod Bot commented Nov 8, 2024

Labels

6 participants

ShourieG commented Nov 5, 2024 •

edited

Loading

elastic-vault-github-plugin-prod Bot commented Nov 5, 2024 •

edited

Loading

kcreddy Nov 6, 2024 •

edited

Loading