Auth updates for formplayer to allow automated app execution #34441
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
snopoke
added
awaiting QA
dimagimon
removed
the
Risk: Medium
snopoke
commented
Apr 16, 2024
| ) | ||
|
|
||
|
|
||
| @handle_401_response |
There was a problem hiding this comment.
this isn't necessary on this view since the decorator only support basic auth
esoergel
approved these changes
Apr 17, 2024
| @@ -405,23 +346,56 @@ def _secure_post_api_key(request, domain, app_id=None): | |||
| @set_request_duration_reporting_threshold(60) | |||
There was a problem hiding this comment.
I notice this decorator is on both the parent and the child views. Is it perhaps that one of the decorators doesn't call functools.wraps and so the wrapped fn doesn't have the proper attribute set?
Comment on lines
+79
to
+80
| # do this after header checks since it may read the request body which interferes with | ||
| # other auth methods e.g. HMAC |
4 tasks
esoergel
approved these changes
Apr 24, 2024
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See also: dimagi/formplayer#1575
Product Description
Allow Formplayer to submit forms using HMAC auth to allow automatic submissions of forms from FP (when a user isn't making the request).
Technical Summary
The primary change is to allow 'formplayer auth' for form submissions. When requests to FP are made from CommCare use use HMAC auth instead of logging in as a user and using session auth. When this was implemented we did not enable it for form submissions since they required a POST request and there wasn't a need to add that functionality to Formplayer (computing HMAC auth for POST requests).
Some upcoming changes to HQ will require the ability to submit forms from FP to HQ using HMAC auth.
As part of this change the form submission view is refactored (bff809f) to remove the per auth fake views whose only function was to support the different auth decorators. The new approach is similar to what's done in get_auth_decorator_map. I decided not to use that function since there are other decorators that need to be added as well.
Safety Assurance
Safety story
Form submission refactor
This is probably the most risky part of the change however it can be inspected manually to confirm that the changes are equivalent to the previous fake views. The approach to dynamically applying decorators is also not new as mentioned above.
Form submission auth change
This permits and existing auth mechanism to be used for the form submission API. We already use this auth on the 'ota' views. Permitting it for form submissions does increase the scope that this auth has, specifically that this is the first WRITE api to be permitted. I think this is safe.
Changing the order in which auth type is determined
(fe3f2db) This is a small change which should not have any impact. The comment in the code indicates the reason for the change (if it runs before HMAC auth then HMAC auth fails).
Automated test coverage
No additional tests were added. These changes are relying on the existing test suite.
QA Plan
This can be run on staging along with corresponding FP changes (still to come).
Rollback instructions
Labels & Review