-
Notifications
You must be signed in to change notification settings - Fork 2k
feat(m365): add defender_safelinks_policy_enabled security check #9832
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
base: master
Are you sure you want to change the base?
Conversation
Add new security check defender_safelinks_policy_enabled for m365 provider. Includes check implementation, metadata, and unit tests. Related: https://prowlerpro.atlassian.net/browse/PROWLER-707
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
|
✅ All necessary |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9832 +/- ##
===========================================
- Coverage 86.60% 76.56% -10.04%
===========================================
Files 222 177 -45
Lines 5645 9632 +3987
===========================================
+ Hits 4889 7375 +2486
- Misses 756 2257 +1501
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
🔒 Container Security ScanImage: 📊 Vulnerability Summary
3 package(s) affected
|
HugoPBrito
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.
Please also map the check with the corresponding compliances.
prowler/lib/powershell/powershell.py
Outdated
| f"or insufficient permissions. Related checks will be skipped." | ||
| ) | ||
| else: | ||
| logger.error(f"PowerShell error output: {error_result}") |
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 this is an error for a Microsoft specific module, I would add this logic to m365_powershell.py and I would leave this file untouched. We could even add it to all calls having a method that does that.
It returns the original PowerShell error (as it is intended to). Specific integration messages should be in the inteegration file m365_powershell.py.
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.
Done
Address PR #9832 review feedback: - Update CHANGELOG entry to 5.18.0 section - Revert powershell.py to simple error logging - Override read_output() in M365PowerShell with M365-specific cmdlet error handling for missing licensing scenarios
| result = default | ||
|
|
||
| if error_result: | ||
| self._process_m365_error(error_result) |
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.
Please, recreate this method (_process_m365_error) in the original powerhsell.py file, keeping the original handling.
Then, create an @override to that function here. That way we enhance original integration scalability and avoid repeating a lot of code, ensuring best practices.
…override pattern - Create _process_error() method in base PowerShellSession class with generic error messaging for missing cmdlets - Add @OverRide _process_error() in M365PowerShell class with M365-specific messaging about Microsoft Defender for Office 365 licensing - Update changelog with defender_safelinks_policy_enabled entry in v5.18.0
0b978d0 to
2b37a20
Compare
| @@ -0,0 +1,40 @@ | |||
| { | |||
| "Provider": "m365", | |||
| "CheckID": "defender_safelinks_policy_enabled", | |||
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.
Please map the check with its compliance requirements.
|
|
||
| findings.append(report) | ||
|
|
||
| # Multiple Safe Links Policies |
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 thing as #9833 (comment). We should handle with distinction between default, non default, its coverage and priority.
Context
Adds a new security check
defender_safelinks_policy_enabledfor the m365 provider.Description
This PR adds a new security check for m365:
defender_safelinks_policy_enabledSteps to review
prowler/providers/m365/services/defender/defender_safelinks_policy_enabled/poetry run pytest tests/providers/m365/services/defender/defender_safelinks_policy_enabled/ -vChecklist
Community Checklist
SDK/CLI
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.