Refactor authentication for requests to CommCare

[snopoke]

Technical Summary

This consolidates and standardises the auth and common headers which get added to requests that are made to CommCare.

Previously this code was in RestoreFactory but it does not belong there.

Prior setup

• Requests are annotated with a '@UserRestore' annotation and the request methods have a 'cookie' arguemnt.
• The UserRestoreAspect grabs the user data from the request and the session cookie value and passes them to RestoreFactory
• Requests from FP to HQ call restoreFactory.getHeaders which adds in the auth headers using the session cookie value (+ other headers).

Change in new setup

• Remove the 'cookie' param in requests
• Remove the auth details from RestoreFactory
• Auth headers are added automatically by request interceptors which are configured in the RestTemplate
• For session auth, the session key is taken from the current user details (associated with the request)

Other changes

• Support HMAC headers for POST requests (this previously wasn't done because I didn't know how to get access to the raw request body)

Safety Assurance

Safety story

There are a decent set of tests for this and I have tested locally as well. I also plan to put this on staging for some time along with dimagi/commcare-hq#34441

Automated test coverage

Added / updated in the PR

QA Plan

QA on staging (regression tests): https://dimagi.atlassian.net/browse/QA-6505

Special deploy instructions

• This PR can be deployed after merge with no further considerations.

Rollback instructions

• This PR can be reverted after deploy with no further considerations.

Review

• The set of people pinged as reviewers is appropriate for the level of risk of the change.

[codecov]

Codecov Report

Attention: Patch coverage is 85.14851% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 70.90%. Comparing base (e47524e) to head (55be2b5).

Files	Patch %	Lines
...are/formplayer/web/client/HmacAuthInterceptor.java	75.75%	4 Missing and 4 partials ⚠️
...commcare/formplayer/aspects/UserRestoreAspect.java	0.00%	5 Missing ⚠️
.../formplayer/web/client/CommCareDefaultHeaders.java	87.50%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1575      +/-   ##
============================================
+ Coverage     70.49%   70.90%   +0.40%     
- Complexity     2025     2048      +23     
============================================
  Files           252      256       +4     
  Lines          7833     7856      +23     
  Branches        722      722              
============================================
+ Hits           5522     5570      +48     
+ Misses         2039     2012      -27     
- Partials        272      274       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[shubham1g5]

Looks great!

[shubham1g5]

maybe wrap all the request param logic in a parent if(userDetails.isPresent() block as we simply want to return the request as it is otherwise

[snopoke]

I left this as is because it's a dual condition: missing param AND user details present. We also expect that user details will always be present, the check is just a safeguard.

[esoergel]

@snopoke I haven't yet confirmed, but I suspect this is triggering an odd bug on staging (jira ticket). If this adds authentication credentials to all requests from formplayer to HQ, then it might affect this behavior:

https://github.com/dimagi/commcare-hq/blob/master/corehq/apps/locations/middleware.py#L29-L32

At present, that exempts requests without auth from location restrictions, assuming that this is outside the scope of its concern (non-domain requests, pre-login, etc). I've long been uncomfortable with this code (which I wrote), as it's easy for stuff to fall through the cracks if you default to permissiveness, which is I think essentially what's happened here.

It looks like the DownloadCCZ view isn't decorated as location-safe, but being a mobile endpoint, and one which doesn't require auth at all, it should be marked location_safe_bypass, I believe. There might be other views that need the same treatment. I'll comment on the QA ticket as well

Aside: It'd be nice to explicitly mark views that don't require auth, both the location middleware can automatically exempt them, and to make it clear when no auth is needed, versus the current world, where it's implied only by the absence of auth decorators, which can be hard to see.

[snopoke]

@esoergel thanks for identifying this. I've tracked down two other places that could be an issue as well.

• App download
  • This hits the download_ccz endpoint
• New Form
  • I don't know when this is called or where the URLs come from but it could be an issue if it hits an endpoint like app_download_file or download_xform
• Formatted Questions
  • This hits the readable_questions endpoint

I'll have to look into what can be done make the auth opt-in instead of opt-out