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

feat(entra): add new check entra_admin_consent_workflow_enabled #7110

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danibarranqueroo
Copy link
Member

Context

This PR introduces a new check for Microsoft365 Entra. This check verifies that the admin consent workflow is enabled to manage user requests for application permissions. The recommendation for this check is to have that enabled and also to have notifications to the reviewers enabled.
We could add more recommendations like the number of days, notifications before the end of the consent period, number of reviewers,... but I think those parameters depend on the organization and should not be an evaluable part of the check.

Enabling this workflow ensures that administrators review and approve all permission requests, preventing unauthorized applications from accessing organizational resources and improving overall security governance.

Description

Added new check entra_admin_consent_workflow_enabled, modified entra service and add unit tests.

Checklist

API

  • Verify if API specs need to be regenerated.
  • Check if version updates are required (e.g., specs, Poetry, etc.).
  • Ensure new entries are added to CHANGELOG.md, if applicable.

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@danibarranqueroo danibarranqueroo requested review from a team as code owners March 4, 2025 12:27
Copy link

codecov bot commented Mar 4, 2025

Codecov Report

Attention: Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.76%. Comparing base (f7a9187) to head (3f59299).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7110      +/-   ##
==========================================
+ Coverage   88.75%   88.76%   +0.01%     
==========================================
  Files        1207     1208       +1     
  Lines       35041    35076      +35     
==========================================
+ Hits        31099    31135      +36     
+ Misses       3942     3941       -1     
Flag Coverage Δ
prowler 88.76% <97.14%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
prowler 88.76% <97.14%> (+0.01%) ⬆️
api ∅ <ø> (∅)

attributes = loop.run_until_complete(
gather(
self._get_authorization_policy(),
self._get_admin_consent_poolicy(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self._get_admin_consent_poolicy(),
self._get_admin_consent_policy(),

prob will need some search&replace, as it spread across the file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants