django-DefectDojo icon indicating copy to clipboard operation
django-DefectDojo copied to clipboard

SAML2 group mapping

Open fopina opened this issue 3 months ago • 4 comments

Description

This is the second take of https://github.com/DefectDojo/django-DefectDojo/pull/11273/

While SAML2 authentication works well as it is, it's not very useful for authorization.

This PR introduces the ability to map SAML2 IdP groups (or some other list attribute) to Dojo groups. And its configuration is similar to Azure AD group matching:

  • DD_SAML2_GROUPS_ATTRIBUTE defines which attribute holds the group list - and if unset (default), group processing is disabled (just authentication, current behavior), similar to DD_SOCIAL_AUTH_AZUREAD_TENANT_OAUTH2_GET_GROUPS
  • Take all groups from IdP that match the configurable regexp DD_SAML2_GROUPS_FILTER - and if unset, every group will match, similar to DD_SOCIAL_AUTH_AZUREAD_TENANT_OAUTH2_GROUPS_FILTER
  • Take all groups from the user on Dojo that match the same regexp
    • Add the missing ones from previous step
    • Remove the ones no longer present in previous

Note

I've updated the condition to bypass assign current user as owner of the new group in dojo/group/utils.py Initially I simply added an extra condition not settings.SAML2_GROUPS_ATTRIBUTE: similar to AzureAD, skip it if SAML2 group matching was enabled.
But then realized that condition (even the existing one) is not ideal, as groups can still be manually created via the UI (and without any "social_provider"), even when AzureAD (or other) is enabled.
I changed the condition to simply check if social_provider is set for 2 reasons:

  • technically assured it comes from those steps as social_provider is not exposed via UI
  • makes business sense (IMO) that a group from a social provider does not have a owner user, as it's owned by someone else outside DefectDojo

Testing

Auth0 was used to test the setup as it support SAML2

  • Create new application, Regular Web Application
  • Under application addons, enable SAML2 web-app
    • Use settings
    {
     "mappings": {
       "user_id":     "urn:oid:0.9.2342.19200300.100.1.1",
       "email":       "urn:oid:1.2.840.113549.1.9.1.1",
       "given_name":  "urn:oid:2.5.4.3",
       "family_name": "urn:oid:2.5.4.4",
       "groups":      "http://schemas.xmlsoap.org/claims/Group"
     },
     "passthroughClaimsWithNoMapping": false
    }
    
  • Add http://localhost:8080/saml/acs/ as callback url and click Enable

As there are no groups in Auth0 and roles are not shared via SAML assertions, we can push them (roles) into a custom assertion (based on this and this links

  • Go to Actions choose Trigger
  • Choose postlogin trigger
/**
* Handler that will be called during the execution of a PostLogin flow.
*
* @param {Event} event - Details about the user and the context in which they are logging in.
* @param {PostLoginAPI} api - Interface whose methods can be used to change the behavior of the login.
*/
exports.onExecutePostLogin = async (event, api) => {
  var roles = event.authorization.roles;
	api.samlResponse.setAttribute('urn:oid:2.5.4.33', roles);
};
  • Back to Dojo, set the following extra configuration (add to docker-compose.yml, under uwsgi service):
DD_SAML2_ENABLED: "true"
DD_SAML2_METADATA_AUTO_CONF_URL: <METADATA URL FROM AUTH0 SAML2 ADDON>
DD_SITE_URL: http://localhost:8080
DD_SOCIAL_LOGIN_AUTO_REDIRECT: "true"
DD_SOCIAL_AUTH_SHOW_LOGIN_FORM: "false"
DD_SAML2_CREATE_USER: "true"
DD_SAML2_ATTRIBUTES_MAP: email=email,uid=username,cn=first_name,sn=last_name
DD_SAML2_GROUPS_FILTER: ".*"
DD_SAML2_GROUPS_ATTRIBUTE: roleOccupant
  • Now create some roles (eg: DojoUser) and add them to the test user in Auth0.
  • Create Dojo groups with the exame same name (eg: DojoUser)
  • Login using that test user and the user should be part of the mapped Dojo groups
    • You can verify in logs that those groups were deteted

fopina avatar Oct 05 '25 12:10 fopina

Converted to draft as you mentioned it's draft :-)

valentijnscholten avatar Oct 05 '25 14:10 valentijnscholten

DryRun Security

:red_circle: Risk threshold exceeded.

This pull request introduces sensitive edits to multiple files (dojo/db_migrations/0245_alter_dojo_group_social_provider.py, dojo/group/utils.py, dojo/models.py) and contains changes that unconditionally add group members (authorization bypass and potential ValueError when user is None), a ReDoS risk in SAML group filtering (regex match without protections), and an uncontrolled resource consumption risk by automatically creating groups from SAML attributes with no limits.

:red_circle: Configured Codepaths Edit in dojo/db_migrations/0245_alter_dojo_group_social_provider.py
Vulnerability Configured Codepaths Edit
Description Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.
:red_circle: Configured Codepaths Edit in dojo/group/utils.py
Vulnerability Configured Codepaths Edit
Description Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.
:red_circle: Configured Codepaths Edit in dojo/models.py
Vulnerability Configured Codepaths Edit
Description Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.
Authorization Bypass and Data Integrity Issue in dojo/group/utils.py
Vulnerability Authorization Bypass and Data Integrity Issue
Description The indentation change in dojo/group/utils.py causes the code responsible for adding the current user as a group owner to execute unconditionally. Previously, this action was guarded by if user and not group.social_provider:, ensuring it only applied to locally managed groups and when a user was present. With the change, Dojo_Group_Member objects are now created for groups managed by social providers (like SAML2 or AzureAD), which should be handled by their respective backends. This can lead to conflicting group memberships and an authorization bypass, as users might be incorrectly added to groups. Furthermore, if get_current_user() returns None, the code attempts to assign None to member.user, which is a non-nullable field, causing a ValueError and preventing group creation.

https://github.com/DefectDojo/django-DefectDojo/blob/49eb105d96919e71e93ad3400dda10c44f6605c6/dojo/group/utils.py#L34-L37

ReDoS in SAML Group Filtering in dojo/backends.py
Vulnerability ReDoS in SAML Group Filtering
Description The Dojo_Group model in DefectDojo is susceptible to Regular Expression Denial of Service (ReDoS) due to the use of re.compile() with a potentially vulnerable regex pattern (DD_SAML2_GROUPS_FILTER) without any explicit mitigations like timeouts or a ReDoS-resistant regex engine. This regex is used to match incoming SAML group names via self.group_re.match(group_name).

https://github.com/DefectDojo/django-DefectDojo/blob/49eb105d96919e71e93ad3400dda10c44f6605c6/dojo/backends.py#L57-L60

Uncontrolled Resource Consumption via Group Creation in dojo/backends.py
Vulnerability Uncontrolled Resource Consumption via Group Creation
Description The application automatically creates new Dojo_Group objects based on SAML attributes provided during user authentication. If a SAML assertion contains a large number of unique group names that do not already exist in the system, the application will attempt to create a corresponding Dojo_Group and Dojo_Group_Member for each, potentially leading to excessive database writes and storage consumption. There are no apparent limits or controls in place to prevent an attacker with control over SAML attributes (e.g., via a compromised Identity Provider) from causing a denial of service through database bloat.

https://github.com/DefectDojo/django-DefectDojo/blob/49eb105d96919e71e93ad3400dda10c44f6605c6/dojo/backends.py#L83-L86

We've notified @mtesauro.


All finding details can be found in the DryRun Security Dashboard.

dryrunsecurity[bot] avatar Oct 05 '25 21:10 dryrunsecurity[bot]

Sorry about that @valentijnscholten, I meant to make it draft and must have missed it in the end. It's ready now!

@kiblik as you were reviewing the previous "take", let me know what you think of it like this - closer to AzureAD for consistency, I hope :)

fopina avatar Oct 05 '25 21:10 fopina

@Maffooch any feedback on this one? I hope adding group support to more auth backends is a welcome change 🙏

fopina avatar Nov 05 '25 06:11 fopina