Skip to content

Conversation

@collinpreston
Copy link
Contributor

@collinpreston collinpreston commented Aug 30, 2023

Pre-Flight checklist

  • Testing
    • Changes have been manually tested

What are the relevant tickets?

#2

What's this PR do?

  1. Removes all Reddit related functionality (praw, posts, channels, comments, notifications, etc).
  2. Removes Moira related functionality.
  3. Removes some remaining auth functionality that will be replaced by Keycloak.
  4. Removes unused environment variables.
  5. Removes Akismet which was used for mitigating spam comments in Reddit.

How should this be manually tested?

  1. All tests should pass.
  2. All MIT Open related functionality perform the same as the current.
  3. Review modified to code and management commands.

Questions for reviewer

  1. Should ckeditor remain or be removed?
  2. Should uploading to S3 functionality remain or be removed?

@collinpreston collinpreston linked an issue Aug 30, 2023 that may be closed by this pull request
@gitguardian
Copy link

gitguardian bot commented Aug 30, 2023

⚠️ GitGuardian has uncovered 156 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id Secret Commit Filename
7518733 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_channel_forbidden.json View secret
7518854 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_delete_comment.json View secret
7518741 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_get_channel_not_found.json View secret
7518333 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_update_title_non_superuser_forbidden[put].json View secret
7518333 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_update_title_non_superuser_forbidden[put].json View secret
7518333 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_update_title_non_superuser_forbidden[put].json View secret
7946742 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_get_comment_404.json View secret
7946742 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_get_comment_404.json View secret
7946743 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_list_channels.json View secret
7946743 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_list_channels.json View secret
7946744 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_get_channel.json View secret
7946744 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_get_channel.json View secret
7946745 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_create_channel_already_exists.json View secret
7946746 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_get_comment_404.json View secret
7946746 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_get_comment_404.json View secret
7946747 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params1-extra_expected1-0].json View secret
7946747 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params1-extra_expected1-0].json View secret
7946747 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params1-extra_expected1-0].json View secret
7946747 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params1-extra_expected1-0].json View secret
7946747 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params1-extra_expected1-0].json View secret
7946747 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params1-extra_expected1-0].json View secret
7946748 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params1-extra_expected1-0].json View secret
7946748 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params1-extra_expected1-0].json View secret
7946749 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_channel_validate_image[banner].json View secret
7518396 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_list_channels_ordered.json View secret
7518396 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_list_channels_ordered.json View secret
7518396 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_list_channels_ordered.json View secret
7518396 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_list_channels_ordered.json View secret
7518396 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_list_channels_ordered.json View secret
7518396 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_list_channels_ordered.json View secret
7946750 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment_reply_to_comment.json View secret
7946750 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment_reply_to_comment.json View secret
7946751 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params2-extra_expected2--1].json View secret
7946751 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params2-extra_expected2--1].json View secret
7518416 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_title_non_superuser_forbidden.json View secret
7518416 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_title_non_superuser_forbidden.json View secret
7518416 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_title_non_superuser_forbidden.json View secret
7518416 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_title_non_superuser_forbidden.json View secret
7946752 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_get_comment.json View secret
7946752 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_get_comment.json View secret
7946752 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_get_comment.json View secret
7946752 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_get_comment.json View secret
7946752 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_get_comment.json View secret
7946752 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_get_comment.json View secret
7946753 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_get_channel_anonymous.json View secret
7518425 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_get_channel.json View secret
7518425 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_get_channel.json View secret
7518183 Base64 Basic Authentication 1012d83 cassettes/channels.views.comments_test.test_delete_comment.json View secret
7518692 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_channel_nonstaff.json View secret
7518443 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_create_channel_no_descriptions.json View secret
7518443 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_create_channel_no_descriptions.json View secret
7518443 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_create_channel_no_descriptions.json View secret
7518786 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_channel_not_found.json View secret
7518193 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params0-extra_expected0-1].json View secret
7518193 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params0-extra_expected0-1].json View secret
7518193 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params0-extra_expected0-1].json View secret
7518193 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params0-extra_expected0-1].json View secret
7518193 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment[extra_params0-extra_expected0-1].json View secret
7518454 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_update_title_non_superuser_forbidden[patch].json View secret
7518454 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_update_title_non_superuser_forbidden[patch].json View secret
7518454 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_update_title_non_superuser_forbidden[patch].json View secret
7518454 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_update_title_non_superuser_forbidden[patch].json View secret
7518196 Base64 Basic Authentication 1012d83 cassettes/channels.views.channels_test.test_list_channels_anonymous.json View secret
7518196 Base64 Basic Authentication 1012d83 cassettes/channels.views.channels_test.test_get_channel_anonymous.json View secret
7946754 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_channel_moderator.json View secret
7946754 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_channel_moderator.json View secret
7946754 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_channel_moderator.json View secret
7946754 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_channel_moderator.json View secret
7946754 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_patch_channel_moderator.json View secret
7946755 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_get_channel_with_avatar_banner.json View secret
7946755 Bearer Token 1012d83 cassettes/channels.views.channels_test.test_get_channel_with_avatar_banner.json View secret
7946756 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment_reply_to_deleted_comment.json View secret
7946756 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment_reply_to_deleted_comment.json View secret
7946756 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment_reply_to_deleted_comment.json View secret
7946756 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment_reply_to_deleted_comment.json View secret
7946756 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment_reply_to_deleted_comment.json View secret
7946756 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment_reply_to_deleted_comment.json View secret
7946757 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment_moderator.json View secret
7946757 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment_moderator.json View secret
7946757 Bearer Token 1012d83 cassettes/channels.views.comments_test.test_create_comment_moderator.json View secret

and 76 others.

🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@codecov
Copy link

codecov bot commented Aug 30, 2023

Codecov Report

Merging #64 (7be7b7f) into main (161295b) will decrease coverage by 1.70%.
Report is 7 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main      #64      +/-   ##
==========================================
- Coverage   86.25%   84.56%   -1.70%     
==========================================
  Files         399      316      -83     
  Lines       16251    11929    -4322     
  Branches     2552     1875     -677     
==========================================
- Hits        14018    10088    -3930     
+ Misses       1920     1583     -337     
+ Partials      313      258      -55     

see 83 files with indirect coverage changes

@collinpreston collinpreston marked this pull request as ready for review August 30, 2023 21:32
@collinpreston collinpreston added the Needs Review An open Pull Request that is ready for review label Aug 30, 2023
@mbertrand mbertrand self-assigned this Sep 6, 2023
Copy link
Member

@mbertrand mbertrand left a comment

Choose a reason for hiding this comment

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

Should ckeditor remain or be removed?

I'd defer to @ChristopherChudzicki on that, but I think there's a chance we might want it for field channels and/or their widgets?

Should uploading to S3 functionality remain or be removed?

Are you referring to MITOPEN_USE_S3 and DEFAULT_FILE_STORAGE = "storages.backends.s3boto3.S3Boto3Storage"? I think we should keep that, for when FieldChannel images are uploaded, and maybe user profile images too (unless keycloak will be replacing that part).

@@ -1,15 +1,33 @@
"""Betamax fixtures"""
Copy link
Member

Choose a reason for hiding this comment

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

I think you should be able to delete this entire file and remove references to these fixtures elsewhere (like users.py)

from betamax.fixtures.pytest import _casette_name

from channels.test_utils import no_ssl_verification
from open_discussions.betamax_config import setup_betamax
Copy link
Member

Choose a reason for hiding this comment

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

You can probably remove the the file open_discussions.betamax_config.py completely and any usages of its methods

@@ -6,8 +6,7 @@

from django.conf import settings

from channels.models import LinkMeta
from embedly.api import get_embedly_summary, THUMBNAIL_URL
Copy link
Member

Choose a reason for hiding this comment

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

You can probably delete the constant THUMBNAIL_URL from embedly.api

@@ -47,7 +47,7 @@ def add_arguments(self, parser):
super().add_arguments(parser)

def handle(self, *args, **options):
"""Index the comments and posts for the channels the user is subscribed to"""
"""Index Courses"""
Copy link
Member

Choose a reason for hiding this comment

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

"""Index all learning resources"""?

@mbertrand mbertrand added Waiting on author and removed Needs Review An open Pull Request that is ready for review labels Sep 6, 2023
@ChristopherChudzicki
Copy link
Contributor

Should ckeditor remain or be removed?

I'd defer to @ChristopherChudzicki on that, but I think there's a chance we might want it for field channels and/or their widgets?

I would keep it. The API endpoint gives us a key for CKEditor's image upload service, which we may well continue to use.

@collinpreston collinpreston added Needs Review An open Pull Request that is ready for review and removed Waiting on author labels Sep 7, 2023
Copy link
Member

@mbertrand mbertrand left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@mbertrand mbertrand added Waiting on author and removed Needs Review An open Pull Request that is ready for review labels Sep 7, 2023
@collinpreston collinpreston merged commit 885cdd7 into main Sep 7, 2023
@collinpreston collinpreston deleted the 2-remove-discussions-backend-code-2 branch September 7, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove discussions backend code

4 participants