Skip to content

Update verbose name + help text for JIRA username and password fields #12261

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

Merged
merged 2 commits into from
Apr 28, 2025

Conversation

valentijnscholten
Copy link
Member

@valentijnscholten valentijnscholten commented Apr 18, 2025

To accompany #12250 (but also good on itself)

EDIT1:

Because these changes are only to textual Django fields that are not affecting the database sql schema it's safe to edit the existing migration that introduced these fields. Initially I had created a migration but it would conflict with other PRs so this is more efficient.

Tested upgrade from bugfix to code with this PR included locally. It works and passes the "no db model changes check" on startup and also passes docker compose exec uwsgi bash -c "python manage.py makemigrations"

EDIT2:
Turns out the help_text and verbose_name were not shown on the JIRA forms as they got hidden because of redeclaring the password field in the form classes. I linked to the texts there as well.

@github-actions github-actions bot added the New Migration Adding a new migration file. Take care when merging. label Apr 18, 2025
@valentijnscholten valentijnscholten changed the title Update verbose name + help text for JIRA password field Update verbose name + help text for JIRA username and password fields Apr 18, 2025
@valentijnscholten valentijnscholten marked this pull request as ready for review April 21, 2025 17:36
Copy link

dryrunsecurity bot commented Apr 21, 2025

DryRun Security

This pull request suggests a potential input validation concern with increasing username and password field lengths to 2000 characters, which might require additional validation to prevent potential input abuse.

💭 Unconfirmed Findings (1)
Vulnerability Potential Input Validation Concern
Description Increasing maximum field length to 2000 characters for username and password fields could allow overly long inputs, which may require careful input validation to prevent potential abuse

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

Copy link
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

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

Approved

Copy link
Contributor

@Maffooch Maffooch left a comment

Choose a reason for hiding this comment

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

@valentijnscholten I apologize for the flip flop here, but can this PR go against dev instead? There is already a 226 migration over there, so this one would complicate the release process a little bit

@valentijnscholten valentijnscholten force-pushed the valentijnscholten-patch-8 branch from c8c1f6b to f90d6da Compare April 23, 2025 07:40
@valentijnscholten valentijnscholten marked this pull request as draft April 23, 2025 07:57
@valentijnscholten valentijnscholten modified the milestones: 2.46.0, 2.45.3 Apr 23, 2025
@valentijnscholten
Copy link
Member Author

@Maffooch I made two changes, see EDIT1 and EDIT2 above.
(re-)Requested reviews because of the changes.

@valentijnscholten valentijnscholten marked this pull request as ready for review April 23, 2025 08:17
Copy link
Contributor

@Maffooch Maffooch left a comment

Choose a reason for hiding this comment

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

There has been a recent rule to not allow changes to existing migrations any longer. Can you please make new migrations, and then base this PR against the dev branch?

@valentijnscholten valentijnscholten modified the milestones: 2.45.3, 2.46.0 Apr 25, 2025
@valentijnscholten valentijnscholten force-pushed the valentijnscholten-patch-8 branch from 83486e5 to c364210 Compare April 27, 2025 06:46
Copy link
Contributor

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

@github-actions github-actions bot added conflicts-detected and removed conflicts-detected docker New Migration Adding a new migration file. Take care when merging. settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR apiv2 docs unittests integration_tests ui parser helm lint labels Apr 27, 2025
@github-actions github-actions bot added the New Migration Adding a new migration file. Take care when merging. label Apr 27, 2025
@Maffooch Maffooch merged commit 2f60528 into dev Apr 28, 2025
79 checks passed
@Maffooch Maffooch deleted the valentijnscholten-patch-8 branch April 28, 2025 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Migration Adding a new migration file. Take care when merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants