- Notifications
You must be signed in to change notification settings - Fork545
cisco_duo: fix 400 and 401 errors in Auth and Activity#11670
Merged
chemamartinez merged 8 commits intoelastic:mainfromNov 13, 2024
Merged
cisco_duo: fix 400 and 401 errors in Auth and Activity#11670chemamartinez merged 8 commits intoelastic:mainfrom
chemamartinez merged 8 commits intoelastic:mainfrom
Conversation
elasticmachine commentedNov 8, 2024
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
🚀 Benchmarks reportTo see the full report comment with |
chrisberkhout approved these changesNov 8, 2024
Contributor
chrisberkhout left a comment
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.
after merge of#11640
Uh oh!
There was an error while loading.Please reload this page.
Comment on lines +38 to +41
| string(int([ | ||
| int(timestamp(int(state.mintime)/1000) + duration("4320h")), | ||
| int(now - duration("2m")) | ||
| ].min()) * 1000) |
Contributor
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 allocates an ephemeral array. Ideally we would not need to do this, but the expansion of code that would be required to not do this I think, on balance, would be worse. Maybe we need amin(a, b)/max(a, b) as well as the overloads that we currently have.
Uh oh!
There was an error while loading.Please reload this page.
elasticmachine commentedNov 12, 2024
💚 Build Succeeded
History
|
efd6 approved these changesNov 13, 2024
Package cisco_duo - 2.2.3 containing this change is available athttps://epr.elastic.co/package/cisco_duo/2.2.3/ |
5 tasks
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull requestFeb 4, 2025
- For Auth and Activity, fixes 40103 by creating Date header during each CEL request.- For Auth and Activity, preemptively fixes the 400 error by checking duration between mintime and maxtime doesn't cross180d.- For Auth, enable it by default to avoid it getting disabled during upgrades.
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull requestFeb 4, 2025
…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 timestampvalue. This was changed to use RFC3339 formatted times in laterPRs[3][4] (with the corresponding read-time parsing added in [5]).Users who didn't create a new policy may have the current parsing logicfail 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 thatobsolete 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
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull requestFeb 5, 2025
- For Auth and Activity, fixes 40103 by creating Date header during each CEL request.- For Auth and Activity, preemptively fixes the 400 error by checking duration between mintime and maxtime doesn't cross180d.- For Auth, enable it by default to avoid it getting disabled during upgrades.
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull requestFeb 5, 2025
…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 timestampvalue. This was changed to use RFC3339 formatted times in laterPRs[3][4] (with the corresponding read-time parsing added in [5]).Users who didn't create a new policy may have the current parsing logicfail 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 thatobsolete 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
bugfixPull request that fixes a bug issue Integration:cisco_duoCisco Duo Team:Security-Service IntegrationsSecurity Service Integrations team [elastic/security-service-integrations]
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed commit message
Cisco API documentation mentions that the
Dateheader should be equal to the current timestamp. In the current CEL program, the previous execution'sstate.dateis being captured and used in pagination and subsequent requests leading to possible40103errors. In a related issue users also observed the error400:mintime must be within the past 180 days.This PR:
40103by creatingDateheader during each CEL request.400error by checkingdurationbetweenmintimeandmaxtimedoesn't cross180d.Checklist
changelog.ymlfile.Related issues
2.2.1to2.2.3because this will only be merged after the PR mentioned above ismerged, and that already increments the version to
2.2.2.