Upgrade Django to 4.2.10

[kiblik]

Upgrade to Django 5.x https://github.com/DefectDojo/django-DefectDojo/pull/9258 might be quite complated but we need to go to at least 4.2.10 because 4.1.x is not supported anymore (check https://www.djangoproject.com/download/)

Needs to be checked/considered:

• [ ] https://docs.djangoproject.com/en/4.2/howto/upgrade-version/#deployment
• [ ] https://docs.djangoproject.com/en/5.0/releases/4.2/ - Might be handy: CharField.max_length is no longer required to be set on PostgreSQL, which supports unlimited VARCHAR columns. - We might need to change some lines for makemigrations: The makemigrations --check option no longer creates missing migration files.

[dryrunsecurity[bot]]

Contextual Security Analysis

As DryRun Security performs checks, we’ll summarize them here. You can always dive into the detailed results in the section below for checks.

Status	DryRun Security Check
✅	Sensitive Functions Analyzer
❌	Configured Sensitive Files Analyzer
✅	Sensitive Files Analyzer

Chat with your AI-powered Security Buddy by typing @dryrunsecurity followed by your question into a comment. Example: @dryrunsecurity What are common security issues with web application cookies?

Install and configure more repositories at DryRun Security

[Maffooch]

I think the best approach here is to remove tests for MySQL by removing the following entry from the integration tests and then replace all the mysql references with Postgres

[kiblik]

@Maffooch, noticed that you implemented https://github.com/DefectDojo/django-DefectDojo/pull/5899. Now errors.append(ValidationError('Query "{}" has invalid format - It contains the NULL character. The following action was taken: {}'.format(old_value, action_string))) breaks test. If correction (query.replace(remove_str, '%00')) was performed, is it still necessary to raise ValidationError? I'm considering change it to logger.error.

[Maffooch]

I guess it doesn't really make sense to raise a validation error on a successful correction.. Changing to a logger statement makes sense. Good call!

[kiblik]

Help needed

I'm facing unusual behavior. Based on unittests, in test_rest_framework.py I adjusted results for deleted_objects. For Language_Type from 2 to 1 and for TestType from 18 to 5. At first, this kind of adjustment should not be done (if there is no change in the data of the algorithm). But I wanted to see the results.

Now, the complicated part:

• Why it happen?
• Why unittests for Alpine pass but for Debian fail (with a request to roll back from 5 to 18 for TestsTest and not mentioning Language_Type)

[Maffooch]

@kiblik this is a very strange error, and I do not understand why it is happening. I am hoping to get some time in the near future to pull this PR down and play around with it

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions[bot]]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[dryrunsecurity[bot]]

Hi there :wave:, @dryrunsecurity here, below is a summary of our analysis and findings.

DryRun Security	Status	Findings
Authn/Authz Analyzer	:white_check_mark:	0 findings
Configured Codepaths Analyzer	:white_check_mark:	0 findings
Sensitive Files Analyzer	:grey_exclamation:	1 finding
AppSec Analyzer	:white_check_mark:	0 findings
Secrets Analyzer	:white_check_mark:	0 findings

[!Note] :green_circle: Risk threshold not exceeded.

Change Summary (click to expand)

The following is a summary of changes in this pull request made by me, your security buddy :robot:. Note that this summary is auto-generated and not meant to be a definitive list of security issues but rather a helpful summary from a security perspective.

Summary:

The code changes in this pull request focus on several key areas that have a direct impact on the security and reliability of the application:

1. Dependency Updates: The changes update the Django version from 4.1.13 to 4.2.13, which is a positive security improvement as it likely addresses known vulnerabilities in the previous version. The changes also update the dependencies of the DefectDojo application, including security-related libraries like cryptography and argon2-cffi, which is a good security practice.

2. Integration Testing Workflow: The changes to the GitHub Actions workflow for running integration tests introduce several security-relevant updates, such as the switch from MySQL to PostgreSQL as the database backend, the change from RabbitMQ to Redis as the message queue, and the phased startup approach to ensure the application's dependencies are properly configured before running the tests. These changes help improve the overall security and reliability of the testing process.

3. User Authorization Tests: The changes to the unit tests for the get_authorized_users function in the dojo.user.queries module focus on validating the correct authorization and access control mechanisms for different user roles and permissions. This is a good security practice to ensure the application's authorization logic is working as expected.

4. Remote User Authentication: The changes to the test_remote_user.py file demonstrate a well-designed and secure implementation of the remote user authentication functionality, with a comprehensive set of test cases covering various scenarios, such as trusted/untrusted proxy handling and API schema visibility.

Overall, the changes in this pull request appear to be focused on improving the security and reliability of the application by updating dependencies, enhancing the testing workflow, validating authorization logic, and implementing secure remote user authentication. These are all important aspects of maintaining a secure and well-functioning application.

Files Changed:

1. requirements.txt: The changes update the Django version from 4.1.13 to 4.2.13, which is a positive security improvement.
2. .github/workflows/integration-tests.yml: The changes update the database configuration from MySQL to PostgreSQL and the message queue configuration from RabbitMQ to Redis, which could have security implications that should be reviewed. The changes also introduce a phased startup approach and improve the handling of exit codes for the integration tests.
3. unittests/test_user_queries.py: The changes focus on improving the unit tests for the get_authorized_users function, which is important for validating the application's authorization and access control mechanisms.
4. unittests/test_remote_user.py: The changes demonstrate a well-designed and secure implementation of the remote user authentication functionality, with a comprehensive set of test cases covering various scenarios.

Powered by DryRun Security

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[kiblik]

@Maffooch, it looks like the mentioned issue is not connected to the upgrade to Django 4.2 but to migration tests from MySQL to Postgres This fails in the same way https://github.com/DefectDojo/django-DefectDojo/pull/9885

[github-actions[bot]]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[kiblik]

I'm happy that we are ready, just be careful with timing:

Dropped support for MySQL 5.7 Upstream support for MySQL 5.7 ends in October 2023. Django 4.2 supports MySQL 8 and higher. Source: https://docs.djangoproject.com/en/5.0/releases/4.2/#dropped-support-for-mysql-5-7

Docker-compose.yml uses mysql:5.7.44 right now.

[Maffooch]

@kiblik I am glad you mentioned this in this PR :) this is a good opportunity to link to the discussion where this change is announced by @mtesauro

https://github.com/DefectDojo/django-DefectDojo/discussions/9690

[cneill]

Since our version of MySQL isn't supported by the version of Django we're moving to and it's our intention to deprecate MySQL with the next release, should we go ahead and remove it entirely as part of this PR?

[mtesauro]

Since our version of MySQL isn't supported by the version of Django we're moving to and it's our intention to deprecate MySQL with the next release, should we go ahead and remove it entirely as part of this PR?

I'd prefer to keep this PR only about the Django update and have a separate PR to remove MySQL from the various places. This PR is complicated enough without adding to it's scope.

FWIW, I'll be working on a PR to remove MySQL & RabbitMQ from the compose files over the weekend.