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.

Sign up for GitHub

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

Jump to bottom

Secure file fields by adding random uuid in the file name. #2256

Merged
merged 4 commits into from
Nov 7, 2024
Merged

Secure file fields by adding random uuid in the file name. #2256

merged 4 commits into from
Nov 7, 2024

Conversation

Rup-Narayan-Rajbanshi
Copy link
Collaborator

@Rup-Narayan-Rajbanshi Rup-Narayan-Rajbanshi commented Sep 4, 2024 •
edited
Loading

Address

Changes

  • Add adding random uuid in the file name for uploaded files.

Checklist

Things that should succeed before merging.

  • Updated/ran unit tests
  • Updated CHANGELOG.md

Release

If there is a version update, make sure to tag the repository with the latest version.

susilnem
susilnem requested changes Sep 4, 2024
View reviewed changes
flash_update/models.py Show resolved Hide resolved
@Rup-Narayan-Rajbanshi Rup-Narayan-Rajbanshi force-pushed the feature/secure-files branch 4 times, most recently from 25f9eed to 1986d35 Compare September 10, 2024 11:16
thenav56
thenav56 requested changes Sep 11, 2024
View reviewed changes
Copy link
Member

@thenav56 thenav56 left a comment

Choose a reason for hiding this comment

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

Let's run makemigrations at the end to avoid merge migrations and conflicts

main/base_class.py Outdated Show resolved Hide resolved
@Rup-Narayan-Rajbanshi Rup-Narayan-Rajbanshi force-pushed the feature/secure-files branch 6 times, most recently from 737e3c0 to 49607d9 Compare September 18, 2024 04:14
thenav56
thenav56 requested changes Oct 7, 2024
View reviewed changes
Copy link
Member

@thenav56 thenav56 left a comment

Choose a reason for hiding this comment

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

Let's add some unit test cases as well.

main/fields.py Outdated Show resolved Hide resolved
@szabozoltan69
Copy link
Contributor

Maybe this change could be linked to this IFRCGo/go-web-app#750 logging requirement also.

@Rup-Narayan-Rajbanshi Rup-Narayan-Rajbanshi force-pushed the feature/secure-files branch 3 times, most recently from 1b78e37 to d9b2b0a Compare October 17, 2024 06:17
susilnem
susilnem approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@susilnem susilnem left a comment

Choose a reason for hiding this comment

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

Looks good!

@Rup-Narayan-Rajbanshi Rup-Narayan-Rajbanshi force-pushed the feature/secure-files branch 3 times, most recently from 35a2aeb to 7428495 Compare October 29, 2024 08:57
@Rup-Narayan-Rajbanshi Rup-Narayan-Rajbanshi marked this pull request as ready for review November 7, 2024 04:32
@Rup-Narayan-Rajbanshi
Copy link
Collaborator Author

@szabozoltan69 lets merge this branch.

@szabozoltan69 szabozoltan69 merged commit 96f724f into develop Nov 7, 2024
3 checks passed
@szabozoltan69 szabozoltan69 deleted the feature/secure-files branch November 7, 2024 06:06
@szabozoltan69
Copy link
Contributor

Should I deploy it to Staging?

@Rup-Narayan-Rajbanshi
Copy link
Collaborator Author

Should I deploy it to Staging?

Yes , you may.

@szabozoltan69
Copy link
Contributor

Done. Migrations are ok:

Running migrations:
  Applying api.0215_alter_generaldocument_document_and_more... OK
  Applying country_plan.0008_alter_countryplan_internal_plan_file... OK
  Applying dref.0075_alter_dref_budget_file_preview_alter_dreffile_file_and_more... OK
  Applying flash_update.0013_alter_flashgraphicmap_file_and_more... OK
  Applying per.0123_alter_perdocumentupload_file_alter_perfile_file... OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@susilnem susilnem susilnem approved these changes

@thenav56 thenav56 thenav56 approved these changes

Assignees

@thenav56 thenav56

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Rup-Narayan-Rajbanshi @szabozoltan69 @thenav56 @susilnem @samshara