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

Sanitize inputs to handle NULL characters when parsing a string literal #11660

Merged
merged 3 commits into from
Dec 19, 2023

Conversation

KshitijThareja
Copy link
Contributor

Summary

This PR aims to sanitize the username and facility inputs from Create Account page by passing them through a serializer so as to avoid NULL characters and use the validated data from the serializer.

References

#10505


Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@github-actions github-actions bot added DEV: backend Python, databases, networking, filesystem... SIZE: small labels Dec 19, 2023

serializer = SanitizeInputsSerializer(data=request.data)
serializer.is_valid(raise_exception=True)
if not serializer.is_valid():
Copy link
Member

Choose a reason for hiding this comment

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

This second validity check for the serializer can be removed. If the validation fails, it will automatically raise an exception due to serializer.is_valid(raise_exception=True), so the code inside this if block would not be executed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure @LianaHarris360. I'll update it.

@LianaHarris360 LianaHarris360 linked an issue Dec 19, 2023 that may be closed by this pull request
Copy link
Member

@LianaHarris360 LianaHarris360 left a comment

Choose a reason for hiding this comment

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

Thank you for working on this @KshitijThareja! The changes are as requested and now sanitize and validate the inputs before checking if the username already exists.

@LianaHarris360 LianaHarris360 merged commit 6336620 into learningequality:release-v0.16.x Dec 19, 2023
33 checks passed
@KshitijThareja
Copy link
Contributor Author

Thanks @LianaHarris360. I'm glad that the changes were as expected and helped solve the issue 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DEV: backend Python, databases, networking, filesystem... SIZE: small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle NULL characters when parsing a string literal
2 participants