From 1884e628e8fb0152dabe10a4ffd890a5b31df59d Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 13 Nov 2018 12:15:17 +0100 Subject: [PATCH 01/17] Use project detail endpoint with admin serializer Add the `environment_variables` field to this serializer that will be returned only when the user is admin. --- readthedocs/restapi/serializers.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/readthedocs/restapi/serializers.py b/readthedocs/restapi/serializers.py index 1d473a9ea..264955948 100644 --- a/readthedocs/restapi/serializers.py +++ b/readthedocs/restapi/serializers.py @@ -43,6 +43,12 @@ class ProjectAdminSerializer(ProjectSerializer): slug_field='feature_id', ) + def get_environment_variables(self, obj): + return { + variable.name: variable.value + for variable in obj.environmentvariable_set.all() + } + class Meta(ProjectSerializer.Meta): fields = ProjectSerializer.Meta.fields + ( 'enable_epub_build', From ac572ba96ed529a8f5ae550deecae90325d4d618 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 13 Nov 2018 12:40:43 +0100 Subject: [PATCH 02/17] Form and views for EnvironmentVariable CRUD from Project's Admin --- readthedocs/projects/forms.py | 42 +++++++++++++++++++- readthedocs/projects/urls/private.py | 55 ++++++++++++++++++++++----- readthedocs/projects/views/private.py | 33 ++++++++++++++++ 3 files changed, 119 insertions(+), 11 deletions(-) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index e8bff7505..e3eb9b10d 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- + """Project forms.""" from __future__ import ( @@ -27,11 +28,11 @@ from readthedocs.core.utils.extend import SettingsOverrideObject from readthedocs.integrations.models import Integration from readthedocs.oauth.models import RemoteRepository from readthedocs.projects import constants -from readthedocs.projects.constants import PUBLIC from readthedocs.projects.exceptions import ProjectSpamError from readthedocs.projects.models import ( Domain, EmailHook, + EnvironmentVariable, Feature, Project, ProjectRelationship, @@ -39,7 +40,6 @@ from readthedocs.projects.models import ( ) from readthedocs.redirects.models import Redirect - class ProjectForm(forms.ModelForm): """ @@ -758,3 +758,41 @@ class FeatureForm(forms.ModelForm): def __init__(self, *args, **kwargs): super(FeatureForm, self).__init__(*args, **kwargs) self.fields['feature_id'].choices = Feature.FEATURES + + +class EnvironmentVariableForm(forms.ModelForm): + + """ + Form to add an EnvironmentVariable to a Project. + + This limits the name of the variable. + """ + + project = forms.CharField(widget=forms.HiddenInput(), required=True) + + class Meta(object): + model = EnvironmentVariable + fields = ('name', 'value', 'project') + + def __init__(self, *args, **kwargs): + self.project = kwargs.pop('project', None) + super(EnvironmentVariableForm, self).__init__(*args, **kwargs) + + def clean_project(self): + return self.project + + def clean_name(self): + name = self.cleaned_data['name'] + if name.startswith('__'): + raise forms.ValidationError( + _("Variable name can't start with __ (double underscore)"), + ) + elif name.startswith('READTHEDOCS'): + raise forms.ValidationError( + _("Variable name can't start with READTHEDOCS"), + ) + elif self.project.environmentvariable_set.filter(name=name).exists(): + raise forms.ValidationError( + _('There is already a variable with this name for this project'), + ) + return name diff --git a/readthedocs/projects/urls/private.py b/readthedocs/projects/urls/private.py index 1cdc29441..1cc8cd37f 100644 --- a/readthedocs/projects/urls/private.py +++ b/readthedocs/projects/urls/private.py @@ -1,18 +1,38 @@ -"""Project URLs for authenticated users""" +"""Project URLs for authenticated users.""" + +from __future__ import ( + absolute_import, + division, + print_function, + unicode_literals, +) -from __future__ import absolute_import from django.conf.urls import url from readthedocs.constants import pattern_opts +from readthedocs.projects.backends.views import ImportDemoView, ImportWizardView from readthedocs.projects.views import private from readthedocs.projects.views.private import ( - ProjectDashboard, ImportView, - ProjectUpdate, ProjectAdvancedUpdate, - DomainList, DomainCreate, DomainDelete, DomainUpdate, - IntegrationList, IntegrationCreate, IntegrationDetail, IntegrationDelete, - IntegrationExchangeDetail, IntegrationWebhookSync, ProjectAdvertisingUpdate) -from readthedocs.projects.backends.views import ImportWizardView, ImportDemoView - + DomainCreate, + DomainDelete, + DomainList, + DomainUpdate, + EnvironmentVariableCreate, + EnvironmentVariableDelete, + EnvironmentVariableList, + EnvironmentVariableUpdate, + ImportView, + IntegrationCreate, + IntegrationDelete, + IntegrationDetail, + IntegrationExchangeDetail, + IntegrationList, + IntegrationWebhookSync, + ProjectAdvancedUpdate, + ProjectAdvertisingUpdate, + ProjectDashboard, + ProjectUpdate, +) urlpatterns = [ url(r'^$', @@ -171,3 +191,20 @@ subproject_urls = [ ] urlpatterns += subproject_urls + +environmentvariable_urls = [ + url(r'^(?P[-\w]+)/environmentvariables/$', + EnvironmentVariableList.as_view(), + name='projects_environmentvariables'), + url(r'^(?P[-\w]+)/environmentvariables/create/$', + EnvironmentVariableCreate.as_view(), + name='projects_environmentvariables_create'), + url(r'^(?P[-\w]+)/environmentvariables/(?P[-\w]+)/edit/$', + EnvironmentVariableUpdate.as_view(), + name='projects_environmentvariables_edit'), + url(r'^(?P[-\w]+)/environmentvariables/(?P[-\w]+)/delete/$', + EnvironmentVariableDelete.as_view(), + name='projects_environmentvariables_delete'), +] + +urlpatterns += environmentvariable_urls diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 330c22b5b..a11fc3a16 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -43,6 +43,7 @@ from readthedocs.projects import tasks from readthedocs.projects.forms import ( DomainForm, EmailHookForm, + EnvironmentVariableForm, IntegrationForm, ProjectAdvancedForm, ProjectAdvertisingForm, @@ -59,6 +60,7 @@ from readthedocs.projects.forms import ( from readthedocs.projects.models import ( Domain, EmailHook, + EnvironmentVariable, Project, ProjectRelationship, WebHook, @@ -875,3 +877,34 @@ class ProjectAdvertisingUpdate(PrivateViewMixin, UpdateView): def get_success_url(self): return reverse('projects_advertising', args=[self.object.slug]) + + +class EnvironmentVariableMixin(ProjectAdminMixin, PrivateViewMixin): + + """Environment Variables to be added when building the Project.""" + + model = EnvironmentVariable + form_class = EnvironmentVariableForm + lookup_url_kwarg = 'environmentvariable_pk' + + def get_success_url(self): + return reverse( + 'projects_environment_variables', + args=[self.get_project().slug], + ) + + +class EnvironmentVariableList(EnvironmentVariableMixin, ListViewWithForm): + pass + + +class EnvironmentVariableCreate(EnvironmentVariableMixin, CreateView): + pass + + +class EnvironmentVariableUpdate(EnvironmentVariableMixin, UpdateView): + pass + + +class EnvironmentVariableDelete(EnvironmentVariableMixin, DeleteView): + pass From f01bbbe93e521964a62b6a247d52d720cf485eba Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 4 Dec 2018 17:26:17 +0100 Subject: [PATCH 03/17] Admin tab menu to List, Create and Delete --- readthedocs/projects/forms.py | 2 +- readthedocs/projects/urls/private.py | 8 ++-- readthedocs/projects/views/private.py | 6 +-- .../projects/environmentvariable_detail.html | 23 ++++++++++ .../projects/environmentvariable_form.html | 28 ++++++++++++ .../projects/environmentvariable_list.html | 45 +++++++++++++++++++ .../templates/projects/project_edit_base.html | 1 + 7 files changed, 105 insertions(+), 8 deletions(-) create mode 100644 readthedocs/templates/projects/environmentvariable_detail.html create mode 100644 readthedocs/templates/projects/environmentvariable_form.html create mode 100644 readthedocs/templates/projects/environmentvariable_list.html diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index e3eb9b10d..32026059b 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -768,7 +768,7 @@ class EnvironmentVariableForm(forms.ModelForm): This limits the name of the variable. """ - project = forms.CharField(widget=forms.HiddenInput(), required=True) + project = forms.CharField(widget=forms.HiddenInput(), required=False) class Meta(object): model = EnvironmentVariable diff --git a/readthedocs/projects/urls/private.py b/readthedocs/projects/urls/private.py index 1cc8cd37f..5dc7b649d 100644 --- a/readthedocs/projects/urls/private.py +++ b/readthedocs/projects/urls/private.py @@ -20,7 +20,7 @@ from readthedocs.projects.views.private import ( EnvironmentVariableCreate, EnvironmentVariableDelete, EnvironmentVariableList, - EnvironmentVariableUpdate, + EnvironmentVariableDetail, ImportView, IntegrationCreate, IntegrationDelete, @@ -199,9 +199,9 @@ environmentvariable_urls = [ url(r'^(?P[-\w]+)/environmentvariables/create/$', EnvironmentVariableCreate.as_view(), name='projects_environmentvariables_create'), - url(r'^(?P[-\w]+)/environmentvariables/(?P[-\w]+)/edit/$', - EnvironmentVariableUpdate.as_view(), - name='projects_environmentvariables_edit'), + url(r'^(?P[-\w]+)/environmentvariables/(?P[-\w]+)/$', + EnvironmentVariableDetail.as_view(), + name='projects_environmentvariables_detail'), url(r'^(?P[-\w]+)/environmentvariables/(?P[-\w]+)/delete/$', EnvironmentVariableDelete.as_view(), name='projects_environmentvariables_delete'), diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index a11fc3a16..d753cf9a9 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -889,12 +889,12 @@ class EnvironmentVariableMixin(ProjectAdminMixin, PrivateViewMixin): def get_success_url(self): return reverse( - 'projects_environment_variables', + 'projects_environmentvariables', args=[self.get_project().slug], ) -class EnvironmentVariableList(EnvironmentVariableMixin, ListViewWithForm): +class EnvironmentVariableList(EnvironmentVariableMixin, ListView): pass @@ -902,7 +902,7 @@ class EnvironmentVariableCreate(EnvironmentVariableMixin, CreateView): pass -class EnvironmentVariableUpdate(EnvironmentVariableMixin, UpdateView): +class EnvironmentVariableDetail(EnvironmentVariableMixin, DetailView): pass diff --git a/readthedocs/templates/projects/environmentvariable_detail.html b/readthedocs/templates/projects/environmentvariable_detail.html new file mode 100644 index 000000000..289eaaeba --- /dev/null +++ b/readthedocs/templates/projects/environmentvariable_detail.html @@ -0,0 +1,23 @@ +{% extends "projects/project_edit_base.html" %} + +{% load i18n %} + +{% block title %}{% trans "Environment Variables" %}{% endblock %} + +{% block nav-dashboard %} class="active"{% endblock %} + +{% block editing-option-edit-environment-variables %}class="active"{% endblock %} + +{% block project-environment-variables-active %}active{% endblock %} +{% block project_edit_content_header %} + {% blocktrans trimmed with name=environmentvariable.name %} + Environment Variable: {{ name }} + {% endblocktrans %} +{% endblock %} + +{% block project_edit_content %} +
+ {% csrf_token %} + +
+{% endblock %} diff --git a/readthedocs/templates/projects/environmentvariable_form.html b/readthedocs/templates/projects/environmentvariable_form.html new file mode 100644 index 000000000..c1587bc8f --- /dev/null +++ b/readthedocs/templates/projects/environmentvariable_form.html @@ -0,0 +1,28 @@ +{% extends "projects/project_edit_base.html" %} + +{% load i18n %} + +{% block title %}{% trans "Environment Variables" %}{% endblock %} + +{% block nav-dashboard %} class="active"{% endblock %} + +{% block editing-option-edit-environment-variables %}class="active"{% endblock %} + +{% block project-environment-variables-active %}active{% endblock %} +{% block project_edit_content_header %}{% trans "Environment Variables" %}{% endblock %} + +{% block project_edit_content %} +

+ {% blocktrans trimmed %} + Notice that the values are not escaped when your builds are executed. + Special characters (for bash) should be escaped accordingly. + {% endblocktrans %} +

+
+ {% csrf_token %} + {{ form.as_p }} + +
+{% endblock %} diff --git a/readthedocs/templates/projects/environmentvariable_list.html b/readthedocs/templates/projects/environmentvariable_list.html new file mode 100644 index 000000000..50123b04f --- /dev/null +++ b/readthedocs/templates/projects/environmentvariable_list.html @@ -0,0 +1,45 @@ +{% extends "projects/project_edit_base.html" %} + +{% load i18n %} + +{% block title %}{% trans "Environment Variables" %}{% endblock %} + +{% block nav-dashboard %} class="active"{% endblock %} + +{% block editing-option-edit-environment-variables %}class="active"{% endblock %} + +{% block project-environment-variables-active %}active{% endblock %} +{% block project_edit_content_header %}{% trans "Environment Variables" %}{% endblock %} + +{% block project_edit_content %} +
+ +
+ +
+
+
    + {% for environmentvariable in object_list %} +
  • + + {{ environmentvariable.name }} + +
    • + {% empty %} +
  • +

    + {% trans 'No environment variables are currently configured.' %} +

    +
    • + {% endfor %} +
+
+
+{% endblock %} diff --git a/readthedocs/templates/projects/project_edit_base.html b/readthedocs/templates/projects/project_edit_base.html index cc85e8883..ca4699010 100644 --- a/readthedocs/templates/projects/project_edit_base.html +++ b/readthedocs/templates/projects/project_edit_base.html @@ -23,6 +23,7 @@
  • {% trans "Translations" %}
  • {% trans "Subprojects" %}
  • {% trans "Integrations" %}
    • +
  • {% trans "Environment Variables" %}
  • {% trans "Notifications" %}
    • {% if USE_PROMOS %}
  • {% trans "Advertising" %}
    • From 9a203285a3c2c58286e64e8b83b21c727ca24a9c Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 4 Dec 2018 18:23:38 +0100 Subject: [PATCH 04/17] Form test for environment variables --- .../rtd_tests/tests/test_project_forms.py | 55 ++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_project_forms.py b/readthedocs/rtd_tests/tests/test_project_forms.py index a40752250..c3b097267 100644 --- a/readthedocs/rtd_tests/tests/test_project_forms.py +++ b/readthedocs/rtd_tests/tests/test_project_forms.py @@ -25,13 +25,14 @@ from readthedocs.projects.constants import ( ) from readthedocs.projects.exceptions import ProjectSpamError from readthedocs.projects.forms import ( + EnvironmentVariableForm, ProjectAdvancedForm, ProjectBasicsForm, ProjectExtraForm, TranslationForm, UpdateProjectForm, ) -from readthedocs.projects.models import Project +from readthedocs.projects.models import Project, EnvironmentVariable class TestProjectForms(TestCase): @@ -478,3 +479,55 @@ class TestTranslationForms(TestCase): instance=self.project_b_en ) self.assertTrue(form.is_valid()) + + +class TestProjectEnvironmentVariablesForm(TestCase): + + def setUp(self): + self.project = get(Project) + + def test_use_invalid_names(self): + data = { + 'name': 'READTHEDOCS__INVALID', + 'value': 'string here', + } + form = EnvironmentVariableForm(data, project=self.project) + self.assertFalse(form.is_valid()) + self.assertIn( + "Variable name can't start with READTHEDOCS", + form.errors['name'], + ) + data = { + 'name': '__INVALID', + 'value': 'string here', + } + form = EnvironmentVariableForm(data, project=self.project) + self.assertFalse(form.is_valid()) + self.assertIn( + "Variable name can't start with __ (double underscore)", + form.errors['name'], + ) + + get(EnvironmentVariable, name='EXISTENT_VAR', project=self.project) + data = { + 'name': 'EXISTENT_VAR', + 'value': 'string here', + } + form = EnvironmentVariableForm(data, project=self.project) + self.assertFalse(form.is_valid()) + self.assertIn( + 'There is already a variable with this name for this project', + form.errors['name'], + ) + + def test_create(self): + data = { + 'name': 'MYTOKEN', + 'value': 'string here', + } + form = EnvironmentVariableForm(data, project=self.project) + form.save() + + self.assertEqual(EnvironmentVariable.objects.count(), 1) + self.assertEqual(EnvironmentVariable.objects.first().name, 'MYTOKEN') + self.assertEqual(EnvironmentVariable.objects.first().value, 'string here') From 74274cfd28664bf4b22bc50006a0458fe6b98614 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 4 Dec 2018 19:08:56 +0100 Subject: [PATCH 05/17] Add `environmentvariable_pk` for testing URLs --- readthedocs/rtd_tests/tests/test_privacy_urls.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_privacy_urls.py b/readthedocs/rtd_tests/tests/test_privacy_urls.py index 277c566e2..1e064eee8 100644 --- a/readthedocs/rtd_tests/tests/test_privacy_urls.py +++ b/readthedocs/rtd_tests/tests/test_privacy_urls.py @@ -14,7 +14,7 @@ from taggit.models import Tag from readthedocs.builds.models import Build, BuildCommandResult from readthedocs.core.utils.tasks import TaskNoPermission from readthedocs.integrations.models import HttpExchange, Integration -from readthedocs.projects.models import Project, Domain +from readthedocs.projects.models import Project, Domain, EnvironmentVariable from readthedocs.oauth.models import RemoteRepository, RemoteOrganization from readthedocs.rtd_tests.utils import create_user @@ -150,6 +150,7 @@ class ProjectMixin(URLAccessMixin): status_code=200, ) self.domain = get(Domain, url='http://docs.foobar.com', project=self.pip) + self.environment_variable = get(EnvironmentVariable, project=self.pip) self.default_kwargs = { 'project_slug': self.pip.slug, 'subproject_slug': self.subproject.slug, @@ -162,6 +163,7 @@ class ProjectMixin(URLAccessMixin): 'domain_pk': self.domain.pk, 'integration_pk': self.integration.pk, 'exchange_pk': self.webhook_exchange.pk, + 'environmentvariable_pk': self.environment_variable.pk, } From 63b208d44cedaee74c436ace85425e566600ac65 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 4 Dec 2018 19:39:20 +0100 Subject: [PATCH 06/17] Fix test to delete an environment variable --- readthedocs/projects/views/private.py | 5 ++++- readthedocs/rtd_tests/tests/test_privacy_urls.py | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index d753cf9a9..91d7d0250 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -907,4 +907,7 @@ class EnvironmentVariableDetail(EnvironmentVariableMixin, DetailView): class EnvironmentVariableDelete(EnvironmentVariableMixin, DeleteView): - pass + + # This removes the delete confirmation + def get(self, request, *args, **kwargs): + return self.http_method_not_allowed(request, *args, **kwargs) diff --git a/readthedocs/rtd_tests/tests/test_privacy_urls.py b/readthedocs/rtd_tests/tests/test_privacy_urls.py index 1e064eee8..937574fba 100644 --- a/readthedocs/rtd_tests/tests/test_privacy_urls.py +++ b/readthedocs/rtd_tests/tests/test_privacy_urls.py @@ -243,11 +243,13 @@ class PrivateProjectAdminAccessTest(PrivateProjectMixin, TestCase): '/dashboard/pip/integrations/sync/': {'status_code': 405}, '/dashboard/pip/integrations/{integration_id}/sync/': {'status_code': 405}, '/dashboard/pip/integrations/{integration_id}/delete/': {'status_code': 405}, + '/dashboard/pip/environmentvariables/{environmentvariable_id}/delete/': {'status_code': 405}, } def get_url_path_ctx(self): return { 'integration_id': self.integration.id, + 'environmentvariable_id': self.environment_variable.id, } def login(self): @@ -277,6 +279,7 @@ class PrivateProjectUserAccessTest(PrivateProjectMixin, TestCase): '/dashboard/pip/integrations/sync/': {'status_code': 405}, '/dashboard/pip/integrations/{integration_id}/sync/': {'status_code': 405}, '/dashboard/pip/integrations/{integration_id}/delete/': {'status_code': 405}, + '/dashboard/pip/environmentvariables/{environmentvariable_id}/delete/': {'status_code': 405}, } # Filtered out by queryset on projects that we don't own. @@ -285,6 +288,7 @@ class PrivateProjectUserAccessTest(PrivateProjectMixin, TestCase): def get_url_path_ctx(self): return { 'integration_id': self.integration.id, + 'environmentvariable_id': self.environment_variable.id, } def login(self): From 26eba5a539da3dfb657db4a0dbbd7d63de6bceae Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 4 Dec 2018 20:02:14 +0100 Subject: [PATCH 07/17] Lint --- readthedocs/projects/forms.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 32026059b..cb1c3fa0f 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -40,6 +40,7 @@ from readthedocs.projects.models import ( ) from readthedocs.redirects.models import Redirect + class ProjectForm(forms.ModelForm): """ From 67d29a47c56baa360ff7342923508c82a5e5bdf7 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 10 Dec 2018 10:55:28 +0100 Subject: [PATCH 08/17] Escape values for environment variables before saving them --- readthedocs/projects/models.py | 9 +++++++++ .../templates/projects/environmentvariable_form.html | 6 ------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index cb1899417..bfa635e09 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -8,6 +8,7 @@ import fnmatch import logging import os from builtins import object # pylint: disable=redefined-builtin +from six.moves import shlex_quote from django.conf import settings from django.contrib.auth.models import User @@ -1064,6 +1065,7 @@ class Feature(models.Model): return dict(self.FEATURES).get(self.feature_id, self.feature_id) +@python_2_unicode_compatible class EnvironmentVariable(TimeStampedModel, models.Model): name = models.CharField( max_length=128, @@ -1078,3 +1080,10 @@ class EnvironmentVariable(TimeStampedModel, models.Model): on_delete=models.CASCADE, help_text=_('Project where this variable will be used'), ) + + def __str__(self): + return self.name + + def save(self, *args, **kwargs): # pylint: disable=arguments-differ + self.value = shlex_quote(self.value) + return super(EnvironmentVariable, self).save(*args, **kwargs) diff --git a/readthedocs/templates/projects/environmentvariable_form.html b/readthedocs/templates/projects/environmentvariable_form.html index c1587bc8f..47c928831 100644 --- a/readthedocs/templates/projects/environmentvariable_form.html +++ b/readthedocs/templates/projects/environmentvariable_form.html @@ -12,12 +12,6 @@ {% block project_edit_content_header %}{% trans "Environment Variables" %}{% endblock %} {% block project_edit_content %} -

    - {% blocktrans trimmed %} - Notice that the values are not escaped when your builds are executed. - Special characters (for bash) should be escaped accordingly. - {% endblocktrans %} -

    From 88d70b5800cf47931162d3c671826d0409e7c2ac Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 10 Dec 2018 10:55:51 +0100 Subject: [PATCH 09/17] Add copy for list page --- readthedocs/templates/projects/environmentvariable_list.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/readthedocs/templates/projects/environmentvariable_list.html b/readthedocs/templates/projects/environmentvariable_list.html index 50123b04f..235b3e6a4 100644 --- a/readthedocs/templates/projects/environmentvariable_list.html +++ b/readthedocs/templates/projects/environmentvariable_list.html @@ -12,6 +12,8 @@ {% block project_edit_content_header %}{% trans "Environment Variables" %}{% endblock %} {% block project_edit_content %} +

    Environment variables allow you to change the way that your build behaves. Take into account that these environment variables are available to all build steps.

    +
    • From 74216af556d1550b6cd1ec443e29d00f4abb0d36 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 10 Dec 2018 11:14:48 +0100 Subject: [PATCH 10/17] Validate that name does not contain spaces on it --- readthedocs/projects/forms.py | 4 ++++ readthedocs/rtd_tests/tests/test_project_forms.py | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index cb1c3fa0f..d7aa684ae 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -796,4 +796,8 @@ class EnvironmentVariableForm(forms.ModelForm): raise forms.ValidationError( _('There is already a variable with this name for this project'), ) + elif ' ' in name: + raise forms.ValidationError( + _("Variable name can't contain spaces"), + ) return name diff --git a/readthedocs/rtd_tests/tests/test_project_forms.py b/readthedocs/rtd_tests/tests/test_project_forms.py index c3b097267..3ae26b289 100644 --- a/readthedocs/rtd_tests/tests/test_project_forms.py +++ b/readthedocs/rtd_tests/tests/test_project_forms.py @@ -487,6 +487,17 @@ class TestProjectEnvironmentVariablesForm(TestCase): self.project = get(Project) def test_use_invalid_names(self): + data = { + 'name': 'VARIABLE WITH SPACES', + 'value': 'string here', + } + form = EnvironmentVariableForm(data, project=self.project) + self.assertFalse(form.is_valid()) + self.assertIn( + "Variable name can't contain spaces", + form.errors['name'], + ) + data = { 'name': 'READTHEDOCS__INVALID', 'value': 'string here', @@ -497,6 +508,7 @@ class TestProjectEnvironmentVariablesForm(TestCase): "Variable name can't start with READTHEDOCS", form.errors['name'], ) + data = { 'name': '__INVALID', 'value': 'string here', From b4274960bffaac8c1e1a33a6131bb580c600af69 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 10 Dec 2018 11:41:05 +0100 Subject: [PATCH 11/17] Test for escaped variables --- readthedocs/rtd_tests/tests/test_project_forms.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_project_forms.py b/readthedocs/rtd_tests/tests/test_project_forms.py index 3ae26b289..e2fd09e2b 100644 --- a/readthedocs/rtd_tests/tests/test_project_forms.py +++ b/readthedocs/rtd_tests/tests/test_project_forms.py @@ -542,4 +542,15 @@ class TestProjectEnvironmentVariablesForm(TestCase): self.assertEqual(EnvironmentVariable.objects.count(), 1) self.assertEqual(EnvironmentVariable.objects.first().name, 'MYTOKEN') - self.assertEqual(EnvironmentVariable.objects.first().value, 'string here') + self.assertEqual(EnvironmentVariable.objects.first().value, "'string here'") + + data = { + 'name': 'ESCAPED', + 'value': r'string escaped here: #$\1[]{}\|', + } + form = EnvironmentVariableForm(data, project=self.project) + form.save() + + self.assertEqual(EnvironmentVariable.objects.count(), 2) + self.assertEqual(EnvironmentVariable.objects.first().name, 'ESCAPED') + self.assertEqual(EnvironmentVariable.objects.first().value, r"'string escaped here: #$\1[]{}\|'") From 3381e850353c721705930e365a401a18ca6b0449 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 11 Dec 2018 16:26:08 +0100 Subject: [PATCH 12/17] Validate with a regex for valid chars --- readthedocs/projects/forms.py | 5 +++++ readthedocs/rtd_tests/tests/test_project_forms.py | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index d7aa684ae..1b32c9d13 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -9,6 +9,7 @@ from __future__ import ( unicode_literals, ) +import re from random import choice from builtins import object @@ -800,4 +801,8 @@ class EnvironmentVariableForm(forms.ModelForm): raise forms.ValidationError( _("Variable name can't contain spaces"), ) + elif not re.fullmatch('[a-zA-Z0-9_]+', name): + raise forms.ValidationError( + _('Only letters, numbers and underscore are allowed'), + ) return name diff --git a/readthedocs/rtd_tests/tests/test_project_forms.py b/readthedocs/rtd_tests/tests/test_project_forms.py index e2fd09e2b..c0436d25b 100644 --- a/readthedocs/rtd_tests/tests/test_project_forms.py +++ b/readthedocs/rtd_tests/tests/test_project_forms.py @@ -509,6 +509,17 @@ class TestProjectEnvironmentVariablesForm(TestCase): form.errors['name'], ) + data = { + 'name': 'INVALID_CHAR*', + 'value': 'string here', + } + form = EnvironmentVariableForm(data, project=self.project) + self.assertFalse(form.is_valid()) + self.assertIn( + 'Only letters, numbers and underscore are allowed', + form.errors['name'], + ) + data = { 'name': '__INVALID', 'value': 'string here', From bd0b9fa42f9714764fd669ac63fb1f5aa0f6a6ba Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 18 Dec 2018 10:53:01 +0100 Subject: [PATCH 13/17] Coherence in style when mentioning the Admin steps --- docs/guides/specifying-dependencies.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guides/specifying-dependencies.rst b/docs/guides/specifying-dependencies.rst index 0019598a6..bac7c5fef 100644 --- a/docs/guides/specifying-dependencies.rst +++ b/docs/guides/specifying-dependencies.rst @@ -35,7 +35,7 @@ Using the project admin dashboard Once the requirements file has been created; - Login to Read the Docs and go to the project admin dashboard. -- Go to ``Admin > Advanced Settings > Requirements file``. +- Go to **Admin > Advanced Settings > Requirements file**. - Specify the path of the requirements file you just created. The path should be relative to the root directory of the documentation project. Using a conda environment file From dec39ba70315eca338359f8381af4610a3e56274 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 18 Dec 2018 10:53:34 +0100 Subject: [PATCH 14/17] Tip communicating that environment variables can be added manually --- docs/builds.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/builds.rst b/docs/builds.rst index c5bdfc2f3..288b9f6cc 100644 --- a/docs/builds.rst +++ b/docs/builds.rst @@ -225,3 +225,8 @@ The *Sphinx* and *Mkdocs* builders set the following RTD-specific environment va +-------------------------+--------------------------------------------------+----------------------+ | ``READTHEDOCS_PROJECT`` | The RTD name of the project which is being built | ``myexampleproject`` | +-------------------------+--------------------------------------------------+----------------------+ + +.. tip:: + + In case extra environment variables are needed to the build process (like secrets, tokens, etc), + you can add them going to **Admin > Environment Variables** in your project. From e7a2ea9c935f246613945197aef36cd4ddff7461 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 3 Jan 2019 17:37:55 +0100 Subject: [PATCH 15/17] Use re.fullmatch for Py3 and custom fullmatch for Py2 --- readthedocs/projects/forms.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 1b32c9d13..d208286bb 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -9,7 +9,18 @@ from __future__ import ( unicode_literals, ) -import re +try: + # TODO: remove this when we deprecate Python2 + # re.fullmatch is >= Py3.4 only + from re import fullmatch +except ImportError: + # https://stackoverflow.com/questions/30212413/backport-python-3-4s-regular-expression-fullmatch-to-python-2 + import re + + def fullmatch(regex, string, flags=0): + """Emulate python-3.4 re.fullmatch().""" # noqa + return re.match("(?:" + regex + r")\Z", string, flags=flags) + from random import choice from builtins import object @@ -801,7 +812,7 @@ class EnvironmentVariableForm(forms.ModelForm): raise forms.ValidationError( _("Variable name can't contain spaces"), ) - elif not re.fullmatch('[a-zA-Z0-9_]+', name): + elif not fullmatch('[a-zA-Z0-9_]+', name): raise forms.ValidationError( _('Only letters, numbers and underscore are allowed'), ) From 663f383dcbe031765d8c7f0526c9835fdf7ef791 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 14 Jan 2019 12:14:20 +0100 Subject: [PATCH 16/17] Add message on env variable detail --- .../templates/projects/environmentvariable_detail.html | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/readthedocs/templates/projects/environmentvariable_detail.html b/readthedocs/templates/projects/environmentvariable_detail.html index 289eaaeba..920091b4d 100644 --- a/readthedocs/templates/projects/environmentvariable_detail.html +++ b/readthedocs/templates/projects/environmentvariable_detail.html @@ -16,6 +16,13 @@ {% endblock %} {% block project_edit_content %} + +

      + {% blocktrans trimmed %} + The value of the environment variable is not shown here for sercurity purposes. + {% endblocktrans %} +

      + {% csrf_token %} From ca5cf05fc826f08773e8ddaae160da9082d3efbb Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 14 Jan 2019 12:45:46 +0100 Subject: [PATCH 17/17] Add a document guide with more documentation --- docs/builds.rst | 2 +- docs/guides/environment-variables.rst | 37 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 docs/guides/environment-variables.rst diff --git a/docs/builds.rst b/docs/builds.rst index 288b9f6cc..67ae1e3aa 100644 --- a/docs/builds.rst +++ b/docs/builds.rst @@ -229,4 +229,4 @@ The *Sphinx* and *Mkdocs* builders set the following RTD-specific environment va .. tip:: In case extra environment variables are needed to the build process (like secrets, tokens, etc), - you can add them going to **Admin > Environment Variables** in your project. + you can add them going to **Admin > Environment Variables** in your project. See :doc:`guides/environment-variables`. diff --git a/docs/guides/environment-variables.rst b/docs/guides/environment-variables.rst new file mode 100644 index 000000000..91ca94cab --- /dev/null +++ b/docs/guides/environment-variables.rst @@ -0,0 +1,37 @@ +I Need Secrets (or Environment Variables) in my Build +===================================================== + +It may happen that your documentation depends on an authenticated service to be built properly. +In this case, you will require some secrets to access these services. + +Read the Docs provides a way to define environment variables for your project to be used in the build process. +All these variables will be exposed to all the commands executed when building your documentation. + +To define an environment variable, you need to + +#. Go to your project **Admin > Environment Variables** +#. Click on "Add Environment Variable" button +#. Input a ``Name`` and ``Value`` (your secret needed here) +#. Click "Save" button + +.. note:: + + Values will never be exposed to users, even to owners of the project. Once you create an environment variable you won't be able to see its value anymore because of security purposes. + +After adding an environment variable from your project's admin, you can access it from your build process using Python, for example: + +.. code-block:: python + + # conf.py + import os + import requests + + # Access to our custom environment variables + username = os.environ.get('USERNAME') + password = os.environ.get('PASSWORD') + + # Request a username/password protected URL + response = requests.get( + 'https://httpbin.org/basic-auth/username/password', + auth=(username, password), + )