From d5bf41ab998f3a2f46df8f9c7f027bdb729e6ff1 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 22 Aug 2024 15:17:02 +0200 Subject: [PATCH 1/3] add 'include_client_id' and 'scope' field to ConnectionSettings --- corehq/motech/auth.py | 16 +++++++++++-- corehq/motech/forms.py | 17 +++++++++++++- ...tionsettings_include_client_id_and_more.py | 23 +++++++++++++++++++ corehq/motech/models.py | 6 +++++ .../motech/js/connection_settings_detail.js | 2 ++ corehq/motech/utils.py | 1 - migrations.lock | 1 + 7 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 corehq/motech/migrations/0016_connectionsettings_include_client_id_and_more.py diff --git a/corehq/motech/auth.py b/corehq/motech/auth.py index 37fd35dcaa86..47af12ec3777 100644 --- a/corehq/motech/auth.py +++ b/corehq/motech/auth.py @@ -185,6 +185,8 @@ def __init__( token_url: str, refresh_url: str, pass_credentials_in_header: bool, + include_client_id: bool, + scope: str, connection_settings: 'ConnectionSettings', ): self.base_url = base_url @@ -193,6 +195,8 @@ def __init__( self.token_url = token_url self.refresh_url = refresh_url self.pass_credentials_in_header = pass_credentials_in_header + self.include_client_id = include_client_id + self.scope = scope self.connection_settings = connection_settings @property @@ -216,19 +220,21 @@ def set_last_token(token): self.last_token = token if not self.last_token or self.last_token.get('refresh_token') is None: - client = BackendApplicationClient(client_id=self.client_id) + client = BackendApplicationClient(client_id=self.client_id, scope=self.scope) session = OAuth2Session(client=client) if self.pass_credentials_in_header: auth = HTTPBasicAuth(self.client_id, self.client_secret) self.last_token = session.fetch_token( token_url=self.token_url, auth=auth, + include_client_id=self.include_client_id, ) else: self.last_token = session.fetch_token( token_url=self.token_url, client_id=self.client_id, client_secret=self.client_secret, + include_client_id=self.include_client_id, ) refresh_kwargs = { @@ -266,6 +272,8 @@ def __init__( token_url: str, refresh_url: str, pass_credentials_in_header: bool, + include_client_id: bool, + scope: str, connection_settings: 'ConnectionSettings', ): self.base_url = base_url @@ -276,6 +284,8 @@ def __init__( self.token_url = token_url self.refresh_url = refresh_url self.pass_credentials_in_header = pass_credentials_in_header + self.include_client_id = include_client_id + self.scope = scope self.connection_settings = connection_settings @property @@ -303,7 +313,7 @@ def set_last_token(token): # without error, or refactoring the way sessions are used across # all repeaters. if not self.last_token or self.last_token.get('refresh_token') is None: - client = LegacyApplicationClient(client_id=self.client_id) + client = LegacyApplicationClient(client_id=self.client_id, scope=self.scope) session = OAuth2Session(client=client) if self.pass_credentials_in_header: auth = HTTPBasicAuth(self.client_id, self.client_secret) @@ -312,6 +322,7 @@ def set_last_token(token): username=self.username, password=self.password, auth=auth, + include_client_id=self.include_client_id, ) else: self.last_token = session.fetch_token( @@ -320,6 +331,7 @@ def set_last_token(token): password=self.password, client_id=self.client_id, client_secret=self.client_secret, + include_client_id=self.include_client_id, ) # Return session that refreshes token automatically diff --git a/corehq/motech/forms.py b/corehq/motech/forms.py index c921940a0c28..ca7f7dd375c9 100644 --- a/corehq/motech/forms.py +++ b/corehq/motech/forms.py @@ -59,6 +59,16 @@ class ConnectionSettingsForm(forms.ModelForm): help_text=_('Pass credentials in Basic Auth header when requesting a token'), required=False, ) + include_client_id = forms.BooleanField( + label=_('Include client ID'), + help_text=_('Send the client ID in the body of the token request'), + required=False, + ) + scope = forms.CharField( + label=_('Scope'), + help_text=_('Space-separated list of scopes e.g. "read write"'), + required=False, + ) skip_cert_verify = forms.BooleanField( label="", help_text=_('Do not use in a production environment'), @@ -92,6 +102,8 @@ class Meta: 'plaintext_password', 'client_id', 'plaintext_client_secret', + 'include_client_id', + 'scope', 'skip_cert_verify', 'notify_addresses_str', 'token_url', @@ -132,6 +144,7 @@ def helper(self): from corehq.motech.views import ConnectionSettingsListView helper = hqcrispy.HQFormHelper() + helper.form_class = "form-horizontal" helper.layout = crispy.Layout( crispy.Field('name'), crispy.Field('notify_addresses_str'), @@ -147,7 +160,9 @@ def helper(self): crispy.Field('auth_preset'), crispy.Field('token_url'), crispy.Field('refresh_url'), - twbscrispy.PrependedText('pass_credentials_in_header', ''), + crispy.Field('pass_credentials_in_header'), + crispy.Field('include_client_id'), + crispy.Field('scope'), ), id="div_id_oauth_settings", ), diff --git a/corehq/motech/migrations/0016_connectionsettings_include_client_id_and_more.py b/corehq/motech/migrations/0016_connectionsettings_include_client_id_and_more.py new file mode 100644 index 000000000000..1dfe7a21ecc7 --- /dev/null +++ b/corehq/motech/migrations/0016_connectionsettings_include_client_id_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.14 on 2024-08-22 12:13 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('motech', '0015_requestlog_duration'), + ] + + operations = [ + migrations.AddField( + model_name='connectionsettings', + name='include_client_id', + field=models.BooleanField(default=None, null=True), + ), + migrations.AddField( + model_name='connectionsettings', + name='scope', + field=models.TextField(blank=True, null=True), + ), + ] diff --git a/corehq/motech/models.py b/corehq/motech/models.py index 78ba3902ef99..a2dbc6061a0c 100644 --- a/corehq/motech/models.py +++ b/corehq/motech/models.py @@ -103,6 +103,8 @@ class ConnectionSettings(models.Model): token_url = models.CharField(max_length=255, blank=True, null=True) refresh_url = models.CharField(max_length=255, blank=True, null=True) pass_credentials_in_header = models.BooleanField(default=None, null=True) + include_client_id = models.BooleanField(default=None, null=True) + scope = models.TextField(null=True, blank=True) notify_addresses_str = models.CharField(max_length=255, default="") # last_token is stored encrypted because it can contain secrets last_token_aes = models.TextField(blank=True, default="") @@ -204,6 +206,8 @@ def get_auth_manager(self): token_url=self.token_url, refresh_url=self.refresh_url, pass_credentials_in_header=self.pass_credentials_in_header, + include_client_id=self.include_client_id, + scope=self.scope or None, connection_settings=self, ) @@ -237,6 +241,8 @@ def get_auth_manager(self): token_url=self.token_url, refresh_url=self.refresh_url, pass_credentials_in_header=self.pass_credentials_in_header, + include_client_id=self.include_client_id, + scope=self.scope or None, connection_settings=self, ) raise ValueError(f'Unknown auth type {self.auth_type!r}') diff --git a/corehq/motech/static/motech/js/connection_settings_detail.js b/corehq/motech/static/motech/js/connection_settings_detail.js index 72d4acf80282..5ed963d61b96 100644 --- a/corehq/motech/static/motech/js/connection_settings_detail.js +++ b/corehq/motech/static/motech/js/connection_settings_detail.js @@ -20,6 +20,8 @@ hqDefine("motech/js/connection_settings_detail", [ 'token_url', 'refresh_url', 'pass_credentials_in_header', + 'include_client_id', + 'scope', ]; if (authPreset === 'CUSTOM') { _.each(customAuthPresetFields, function (field) { diff --git a/corehq/motech/utils.py b/corehq/motech/utils.py index 4da790c08502..9d0cddd9fd38 100644 --- a/corehq/motech/utils.py +++ b/corehq/motech/utils.py @@ -212,7 +212,6 @@ def split_url(url): return url.split(connection.url.rstrip('/'))[1] except IndexError: return None - for preset_slug, preset in AUTH_PRESETS.items(): if ( split_url(connection.token_url) == preset.token_endpoint diff --git a/migrations.lock b/migrations.lock index da2e14faf362..772001d36acd 100644 --- a/migrations.lock +++ b/migrations.lock @@ -710,6 +710,7 @@ motech 0013_alter_connectionsettings_auth_type 0014_alter_connectionsettings_password 0015_requestlog_duration + 0016_connectionsettings_include_client_id_and_more notifications 0001_squashed_0003_auto_20160504_2049 (3 squashed migrations) 0002_auto_20160505_2058 From 027e65420e5c0152e38816f4bc4ef16b7be38078 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 22 Aug 2024 15:19:03 +0200 Subject: [PATCH 2/3] update 'test connection' workflow for oauth clients --- .../static/motech/js/connection_settings_detail.js | 5 +++++ corehq/motech/views.py | 11 +++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/corehq/motech/static/motech/js/connection_settings_detail.js b/corehq/motech/static/motech/js/connection_settings_detail.js index 5ed963d61b96..704b38458a3c 100644 --- a/corehq/motech/static/motech/js/connection_settings_detail.js +++ b/corehq/motech/static/motech/js/connection_settings_detail.js @@ -143,6 +143,11 @@ hqDefine("motech/js/connection_settings_detail", [ plaintext_password: $('#id_plaintext_password').val(), client_id: $('#id_client_id').val(), plaintext_client_secret: $('#id_plaintext_client_secret').val(), + pass_credentials_in_header: $('#id_pass_credentials_in_header').prop('checked'), + include_client_id: $('#id_include_client_id').prop('checked'), + scope: $('#id_scope').val(), + token_url: $('#id_token_url').val(), + auth_preset: $('#id_auth_preset').val(), skip_cert_verify: $('#id_skip_cert_verify').prop('checked'), }; $testConnectionButton.disableButton(); diff --git a/corehq/motech/views.py b/corehq/motech/views.py index 3f5f175b7add..a1fd373042a6 100644 --- a/corehq/motech/views.py +++ b/corehq/motech/views.py @@ -24,7 +24,7 @@ from corehq.apps.hqwebapp.views import CRUDPaginatedViewMixin from corehq.apps.users.decorators import require_permission from corehq.apps.users.models import HqPermissions -from corehq.motech.const import PASSWORD_PLACEHOLDER +from corehq.motech.const import PASSWORD_PLACEHOLDER, OAUTH2_CLIENT, OAUTH2_PWD from corehq.motech.forms import ConnectionSettingsForm, UnrecognizedHost from corehq.motech.models import ConnectionSettings, RequestLog from corehq.util.urlvalidate.urlvalidate import PossibleSSRFAttempt @@ -308,7 +308,14 @@ def test_connection_settings(request, domain): raise Http404 # If auth_type is set to None, we ignore this check - if request.POST.get('plaintext_password') == PASSWORD_PLACEHOLDER and request.POST.get('auth_type'): + auth_type = request.POST.get('auth_type') + client_secret = request.POST.get('plaintext_client_secret') + if auth_type in (OAUTH2_CLIENT, OAUTH2_PWD) and client_secret == PASSWORD_PLACEHOLDER: + return JsonResponse({ + "success": False, + "response": _("Please enter client secret again."), + }) + if auth_type and request.POST.get('plaintext_password') == PASSWORD_PLACEHOLDER: # The user is editing an existing instance, and the form is # showing the password placeholder. (We don't tell the user what # the API password is.) From feb6730a41c0f27e1a081502afb254e6cced702d Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 22 Aug 2024 15:51:27 +0200 Subject: [PATCH 3/3] use normal field for 'skip_cert_verify' --- corehq/motech/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/corehq/motech/forms.py b/corehq/motech/forms.py index ca7f7dd375c9..30e92b2773aa 100644 --- a/corehq/motech/forms.py +++ b/corehq/motech/forms.py @@ -166,7 +166,7 @@ def helper(self): ), id="div_id_oauth_settings", ), - twbscrispy.PrependedText('skip_cert_verify', ''), + crispy.Field('skip_cert_verify'), self.test_connection_button, twbscrispy.StrictButton(