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.

Sign up for GitHub

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

Jump to bottom

automation & sequence: fix use of default policies and set Sequence as default sequence policy #5897

Merged
merged 2 commits into from
Nov 15, 2024
Merged

automation & sequence: fix use of default policies and set Sequence as default sequence policy #5897

merged 2 commits into from
Nov 15, 2024

Conversation

kingthorin
Copy link
Member

@kingthorin kingthorin commented Nov 7, 2024 •
edited
Loading

Overview

Modify automation and sequence to properly build and use a policy from policyDefinition if present in the plan. If neither policy nor policyDefinition are defined fall back to Default Policy or Sequence respectively.

Related Issues

n/a

Checklist

  • Update help
  • Update changelog
  • Run ./gradlew spotlessApply for code formatting
  • Write tests
  • Check code coverage
  • Sign-off commits
  • Squash commits
  • Use a descriptive title

@psiinon psiinon mentioned this pull request Nov 7, 2024
Merged
8 tasks
@kingthorin kingthorin force-pushed the seq-policy2 branch 4 times, most recently from 5d954f9 to a4deb50 Compare November 7, 2024 19:39
@kingthorin
Copy link
Member Author

I had to rename one of the UnitTest classes so that Eclipse would properly find it and run it as a JUnit test.

@kingthorin kingthorin marked this pull request as ready for review November 7, 2024 20:03
@kingthorin
Copy link
Member Author

Still needs changelog and help updates I guess

@thc202 thc202 changed the title automation & sequence: Revise scan policy handling & try to use defaults automation & sequence: fix use of default policies and set Sequence as default sequence policy Nov 7, 2024
@kingthorin kingthorin force-pushed the seq-policy2 branch 2 times, most recently from ebd5ac4 to 02430c9 Compare November 7, 2024 22:27
@kingthorin

This comment was marked as resolved.

Sign in to view
@kingthorin kingthorin force-pushed the seq-policy2 branch 4 times, most recently from a14df14 to d2a97d4 Compare November 8, 2024 15:28
@kingthorin kingthorin force-pushed the seq-policy2 branch 3 times, most recently from 9850833 to 9f85171 Compare November 8, 2024 15:43
@kingthorin
Copy link
Member Author

Okay I think I'm caught up to all the feedback now 😉

@kingthorin
Copy link
Member Author

kingthorin commented Nov 14, 2024 •
edited
Loading

Call parse from verify?

@kingthorin
Copy link
Member Author

kingthorin commented Nov 14, 2024 •
edited
Loading

#5897 (comment)

It can be done in verifyParameters when policyDefinition is not present. (The policy should not matter for this case.) It's the same handling for both jobs. It might be better to call always the parsePolicyDefinition and keep the null strength handling internally.

Then I believe it's already covered. Both activeScanJob and sequenceActiveScanJob call parse in verifyParameters.

@kingthorin kingthorin force-pushed the seq-policy2 branch 3 times, most recently from 8734388 to 454ec06 Compare November 14, 2024 17:11
@thc202
Copy link
Member

thc202 commented Nov 14, 2024

They need to call it always, currently when not present that method is not called and the null strength is not set.

@kingthorin
Copy link
Member Author

Done

@kingthorin
Copy link
Member Author

Done x4

@kingthorin
Copy link
Member Author

Fixed....I dunno why I over complicated it 👍

@thc202
Copy link
Member

thc202 commented Nov 15, 2024

Thank you!

@kingthorin kingthorin requested a review from psiinon November 15, 2024 14:31
@psiinon
Copy link
Member

psiinon commented Nov 15, 2024

Now has conflicts

@kingthorin
automation & sequence: Revise scan policy handling & try to use defaults
381c948 
Signed-off-by: kingthorin <kingthorin@users.noreply.github.com>
# Conflicts:
#	addOns/sequence/CHANGELOG.md
@kingthorin
PolicyDefinition make parseDefinition non-static and adjust accesses
e79bd6f 
Signed-off-by: kingthorin <kingthorin@users.noreply.github.com>
@kingthorin
Copy link
Member Author

Tweaked

@thc202 thc202 enabled auto-merge November 15, 2024 16:00
@thc202 thc202 merged commit e26cfb0 into zaproxy:main Nov 15, 2024
10 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 15, 2024
@kingthorin kingthorin deleted the seq-policy2 branch November 15, 2024 16:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@thc202 thc202 thc202 approved these changes

@psiinon psiinon psiinon approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kingthorin @thc202 @psiinon