feat(pacer): Link pacer sessions to proxies #4192

ERosendo · 2024-07-10T20:56:59Z

This PR introduces a new approach to caching PACER sessions and updates the code of the ProxyPacerSession class to pick one of the proxy connection strings from the settings.

Key Changes:

Introduces a new environment variable (EGRESS_PROXY_HOSTS) to manage a list of proxies.
Implements new logic in the ProxyPacerSession to choose a proxy connection string from the settings, enabling dynamic proxy selection.
Updates the log_into_pacer method to return a tuple containing both the user's cookie and the used proxy address used for login
Tweaks the get_or_cache_pacer_cookies to handle both the new return format and potentially existing cached data in the old format, ensuring backward compatibility.
Updates tasks from corpus_importer and recap modules to accommodate the new PACER user cookie format.
Adapts commands in the corpus_importer module that use cookies to handle the new format.
Introduces new tests for pacer session utils.

This PR closes #4087

This commit updates the `ProxyPacerSession` class to enable selection of a proxy connection string from the application settings.

This commit updates the `log_into_pacer` method to return a tuple containing the user's cookie and the proxy address used for login (if applicable). This improvement provides more context about the login session, facilitating further actions requiring both cookies and potential proxy information.

This commit tweaks the `get_or_cache_pacer_cookies` function to handle the new return format of `log_into_pacer`. It ensures compatibility with both the updated function and any existing cached data that might be in the old format.

… cookie handling - Updates tasks logic to accommodate the new format for PACER user cookies. - Updates function signature to reflect the change in cookie data format.

- Adapts update_docket_info_iquery task to handle the updated output format of get_or_cache_pacer_cookies. - Enhances task functionality by passing the proxy parameter to the ProxyPacerSession class

semgrep-app · 2024-07-10T22:45:48Z

Semgrep found 4 avoid-pickle findings:

cl/lib/tests.py
- L101 - Triage
- L106 - Triage
- L111 - Triage
- L116 - Triage

Avoid using pickle, which is known to lead to code execution vulnerabilities. When unpickling, the serialized data could be manipulated to run arbitrary code. Instead, consider serializing the relevant data as JSON or a similar text-based serialization format.

_{Ignore this finding from avoid-pickle.}

mlissner

Looks like you've got an effective solution here, and that you've thought carefully about how to roll it out without queued tasks failing. Nice.

I think I would change something though. Having to work with tuples and pass them around is never much fun. They're kind of ugly, it's hard to remember what order the tuple is in ("Was it cookies then proxy or proxy then cookies?"), and so forth.

Since I think the values we need are getting added to the PACER session object anyway, did you consider just passing that around? Even log_into_pacer could just return s, I think?

cl/lib/pacer_session.py

mlissner · 2024-07-11T13:45:55Z

cl/lib/pacer_session.py

-        return cookies
+        if isinstance(cookies_data, tuple):
+            return cookies_data
+        return cookies_data, settings.EGRESS_PROXY_HOST


Let's drop this variable in the code now so we don't have to think about it again. I see you're doing backwards compat here, but it'd be better to just hardcode EGRESS_PROXY_HOSTS[0], say. Let's also make a comment about how this code can be removed the next time anybody is in this area and that it's only here for transitioning.

cl/recap/tasks.py

cl/corpus_importer/tasks.py

…_title helper

…t class

semgrep-app · 2024-07-30T02:26:15Z

cl/corpus_importer/management/commands/legal_robot.py

@@ -7,7 +7,7 @@
 from cl.corpus_importer.tasks import add_tags, get_pacer_doc_by_rd
 from cl.lib.celery_utils import CeleryThrottle
 from cl.lib.command_utils import VerboseCommand, logger
-from cl.lib.pacer_session import ProxyPacerSession
+from cl.lib.pacer_session import ProxyPacerSession, SessionData
 from cl.lib.scorched_utils import ExtraSolrInterface
 from cl.lib.search_utils import build_main_query_from_query_string
 from cl.scrapers.tasks import extract_recap_pdf


You are using environment variables inside django app. Use django-environ as it a better alternative for deployment.
_{Ignore this finding from use-django-environ.}

semgrep-app · 2024-07-30T02:26:16Z

cl/corpus_importer/management/commands/invoice_project.py

@@ -14,7 +14,7 @@
 )
 from cl.lib.celery_utils import CeleryThrottle
 from cl.lib.command_utils import VerboseCommand, logger
-from cl.lib.pacer_session import ProxyPacerSession
+from cl.lib.pacer_session import ProxyPacerSession, SessionData
 from cl.lib.scorched_utils import ExtraSolrInterface
 from cl.lib.search_utils import build_main_query_from_query_string
 from cl.recap.tasks import process_recap_attachment


You are using environment variables inside django app. Use django-environ as it a better alternative for deployment.
_{Ignore this finding from use-django-environ.}

semgrep-app · 2024-07-30T02:26:17Z

Semgrep found 5 avoid-pickle findings:

cl/lib/pacer_session.py
- L177 - Triage
cl/lib/tests.py
- L110 - Triage
- L115-117 - Triage
- L122 - Triage
- L127-129 - Triage

Avoid using pickle, which is known to lead to code execution vulnerabilities. When unpickling, the serialized data could be manipulated to run arbitrary code. Instead, consider serializing the relevant data as JSON or a similar text-based serialization format.

_{Ignore this finding from avoid-pickle.}

ERosendo · 2024-07-30T13:41:06Z

@mlissner I've implemented your suggestions. I've removed the EGRESS_PROXY_HOST environment variable from settings and replaced tuples with a dedicated dataclass named SessionData for better structure and readability.

Let me know what you think.

mlissner

Great! This is a lot easier to reason about. Thanks for making the little helper class.

Can you make an issue with details about deployment and the follow up tasks? E.g.:

Need the variables set before deployment
Need to remove the old proxies after a few hours or maybe a day
Need to remove shim code after a few hours or maybe a day

Anything else?

mlissner · 2024-07-30T14:13:48Z

This is approved, but can you work with Ramiro to make sure that we've got the variables and k8s deployments in place before we merge and deploy?

ERosendo · 2024-07-30T14:14:40Z

Sure!

ERosendo · 2024-07-30T16:08:00Z

@mlissner Ramiro confirmed the environment variable was set up correctly and that CL can successfully reach the newly deployed proxies.

Let's merge this PR.

mlissner · 2024-07-30T16:10:44Z

Auto-merge is set. Thanks all!

mlissner · 2024-08-12T17:00:53Z

@ERosendo Now that this has been live for a couple weeks, is there code in here we want to refactor to simplify things?

ERosendo · 2024-08-12T17:08:24Z

@mlissner Yes, We used several ternary if statements to manage legacy cached data. I believe we can safely remove this code. Here are examples:

courtlistener/cl/recap/tasks.py

Lines 1658 to 1660 in 5716284

    
           session_data = ( 
        
               cookies if isinstance(cookies, SessionData) else SessionData(cookies) 
        
           )

courtlistener/cl/recap/tasks.py

Lines 2416 to 2421 in 5716284

    
           # Ensures session data is a `SessionData` instance for consistent handling. 
        
           # This approach prevents disruptions during processing of enqueued data 
        
           # after deployment. 
        
           session_data = ( 
        
               cookies if isinstance(cookies, SessionData) else SessionData(cookies) 
        
           )

courtlistener/cl/lib/pacer_session.py

Lines 165 to 169 in 5716284

    
           if cookies_data and ttl_seconds >= 300 and not refresh: 
        
               # cookies were found in cache and ttl >= 5 minutes, return them 
        
               if isinstance(cookies_data, SessionData): 
        
                   return cookies_data 
        
               return SessionData(cookies_data)

freelawproject deleted a comment from semgrep-app bot Jul 10, 2024

ERosendo force-pushed the 4087-feat-link-sessions-to-proxy branch from 6cebd2b to 4579385 Compare July 10, 2024 21:07

ERosendo marked this pull request as ready for review July 10, 2024 22:43

ERosendo added 9 commits July 10, 2024 18:43

feat(setting): Adds new setting for managing a list of proxies

ce3bba1

feat(lib): Adds proxy selection logic for the ProxyPacerSession class

f31ca58

This commit updates the `ProxyPacerSession` class to enable selection of a proxy connection string from the application settings.

feat(corpus_importer): Updates tasks logic and signature for improved…

8b3417e

… cookie handling - Updates tasks logic to accommodate the new format for PACER user cookies. - Updates function signature to reflect the change in cookie data format.

feat(recap): Tweaks tasks to handle the new format for user cookies.

588818d

feat(scrapers): Updates update_docket_info_iquery task

8489419

- Adapts update_docket_info_iquery task to handle the updated output format of get_or_cache_pacer_cookies. - Enhances task functionality by passing the proxy parameter to the ProxyPacerSession class

feat(corpus_importer): Updates commands to use the new cookie format

b93fd90

feat(lib): Adds tests for pacer session utils

693552d

ERosendo force-pushed the 4087-feat-link-sessions-to-proxy branch from a1c1746 to 693552d Compare July 10, 2024 22:43

ERosendo requested a review from mlissner July 10, 2024 22:43

mlissner requested changes Jul 11, 2024

View reviewed changes

ERosendo force-pushed the 4087-feat-link-sessions-to-proxy branch from 2954d38 to b4cc6df Compare July 12, 2024 16:23

feat(pacer_session): Updates logic to pick a proxy connection str

5a47599

ERosendo force-pushed the 4087-feat-link-sessions-to-proxy branch from b4cc6df to 5a47599 Compare July 12, 2024 16:37

ERosendo added 6 commits July 12, 2024 13:52

feat(corpus_importer): Adds an exception to the get_pacer_case_id_and…

b0b6eb3

…_title helper

feat(test): Override PROXY_HOSTS setting for tests

02141d9

Merge branch 'main' into 4087-feat-link-sessions-to-proxy

3d2af40

fix(tests): Override PROXY_HOSTS setting for the ScrapeIqueryPagesTes…

f997bcc

…t class

Merge branch 'main' into 4087-feat-link-sessions-to-proxy

ccfcbd2

feat(settings): Removes EGRESS_PROXY_HOST env variable

d1058d0

semgrep-app bot reviewed Jul 30, 2024

View reviewed changes

feat(pacer): Adds dataclass for storing PACER session data

e34f3d5

ERosendo force-pushed the 4087-feat-link-sessions-to-proxy branch from 81cd87d to e34f3d5 Compare July 30, 2024 02:52

ERosendo added 2 commits July 29, 2024 23:09

Merge branch 'main' into 4087-feat-link-sessions-to-proxy

78dcb23

fix(corpus importer): Use correct parameter name for session data

ea91f42

ERosendo requested a review from mlissner July 30, 2024 13:23

mlissner approved these changes Jul 30, 2024

View reviewed changes

Merge branch 'main' into 4087-feat-link-sessions-to-proxy

6f31ed8

mlissner enabled auto-merge July 30, 2024 16:10

mlissner merged commit 723b7ec into main Jul 30, 2024
13 checks passed

mlissner deleted the 4087-feat-link-sessions-to-proxy branch July 30, 2024 16:20

mlissner mentioned this pull request Aug 12, 2024

Clean up old fetch proxy shim code #4298

Closed

ERosendo mentioned this pull request Aug 14, 2024

feat(proxy): Removes logic to handle legacy cached session data #4309

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pacer): Link pacer sessions to proxies #4192

feat(pacer): Link pacer sessions to proxies #4192

ERosendo commented Jul 10, 2024 •

edited

Loading

semgrep-app bot commented Jul 10, 2024

mlissner left a comment

mlissner Jul 11, 2024

semgrep-app bot Jul 30, 2024

semgrep-app bot Jul 30, 2024

semgrep-app bot commented Jul 30, 2024

ERosendo commented Jul 30, 2024

mlissner left a comment

mlissner commented Jul 30, 2024

ERosendo commented Jul 30, 2024

ERosendo commented Jul 30, 2024

mlissner commented Jul 30, 2024

mlissner commented Aug 12, 2024

ERosendo commented Aug 12, 2024

feat(pacer): Link pacer sessions to proxies #4192

feat(pacer): Link pacer sessions to proxies #4192

Conversation

ERosendo commented Jul 10, 2024 • edited Loading

semgrep-app bot commented Jul 10, 2024

mlissner left a comment

Choose a reason for hiding this comment

mlissner Jul 11, 2024

Choose a reason for hiding this comment

semgrep-app bot Jul 30, 2024

Choose a reason for hiding this comment

semgrep-app bot Jul 30, 2024

Choose a reason for hiding this comment

semgrep-app bot commented Jul 30, 2024

ERosendo commented Jul 30, 2024

mlissner left a comment

Choose a reason for hiding this comment

mlissner commented Jul 30, 2024

ERosendo commented Jul 30, 2024

ERosendo commented Jul 30, 2024

mlissner commented Jul 30, 2024

mlissner commented Aug 12, 2024

ERosendo commented Aug 12, 2024

ERosendo commented Jul 10, 2024 •

edited

Loading