diff --git a/frontend/src/scenes/surveys/SurveyAppearance.tsx b/frontend/src/scenes/surveys/SurveyAppearance.tsx index 8b78e2aa90d93..ed3fc50036ce0 100644 --- a/frontend/src/scenes/surveys/SurveyAppearance.tsx +++ b/frontend/src/scenes/surveys/SurveyAppearance.tsx @@ -269,7 +269,7 @@ export function BaseAppearance({ )}
-
{question}
+
{/* Using dangerouslySetInnerHTML is safe here, because it's taking the user's input and showing it to the same user. They can try passing in arbitrary scripts, but it would show up only for them, so it's like trying to XSS yourself, where you already have all the data. Furthermore, sanitization should catch all obvious attempts */} @@ -638,8 +638,14 @@ export function SurveyThankYou({ appearance }: { appearance: SurveyAppearanceTyp {cancel}
-

{appearance?.thankYouMessageHeader || 'Thank you!'}

-
{appearance?.thankYouMessageDescription || ''}
+

+
diff --git a/frontend/src/scenes/surveys/SurveyEdit.tsx b/frontend/src/scenes/surveys/SurveyEdit.tsx index 63f20619f7a2e..1db795f0b0d36 100644 --- a/frontend/src/scenes/surveys/SurveyEdit.tsx +++ b/frontend/src/scenes/surveys/SurveyEdit.tsx @@ -158,93 +158,16 @@ export default function SurveyEdit(): JSX.Element { label="Description (optional)" > {({ value, onChange }) => ( - <> - - setWritingHTMLDescription( - key === 'html' - ) - } - tabs={[ - { - key: 'text', - label: ( - - Text - - ), - content: ( - - onChange(v) - } - /> - ), - }, - { - key: 'html', - label: ( - - HTML - - ), - content: ( -
- - onChange( - v ?? '' - ) - } - height={150} - options={{ - minimap: { - enabled: - false, - }, - wordWrap: 'on', - scrollBeyondLastLine: - false, - automaticLayout: - true, - fixedOverflowWidgets: - true, - lineNumbers: - 'off', - glyphMargin: - false, - folding: false, - }} - /> -
- ), - }, - ]} - /> - {question.description && - question.description - ?.toLowerCase() - .includes(' - Scripts won't run in the survey - popup and we'll remove these on - save. Use the API question mode - to run your own scripts in - surveys. - - )} - + )} - @@ -967,3 +895,73 @@ export default function SurveyEdit(): JSX.Element {
) } + +export function HTMLEditor({ + value, + onChange, + writingHTMLDescription, + setWritingHTMLDescription, + textPlaceholder, +}: { + value?: string + onChange: (value: any) => void + writingHTMLDescription: boolean + setWritingHTMLDescription: (writingHTML: boolean) => void + textPlaceholder?: string +}): JSX.Element { + return ( + <> + setWritingHTMLDescription(key === 'html')} + tabs={[ + { + key: 'text', + label: Text, + content: ( + onChange(v)} + placeholder={textPlaceholder} + /> + ), + }, + { + key: 'html', + label: HTML, + content: ( +
+ onChange(v ?? '')} + height={150} + options={{ + minimap: { + enabled: false, + }, + wordWrap: 'on', + scrollBeyondLastLine: false, + automaticLayout: true, + fixedOverflowWidgets: true, + lineNumbers: 'off', + glyphMargin: false, + folding: false, + }} + /> +
+ ), + }, + ]} + /> + {value && value?.toLowerCase().includes(' + Scripts won't run in the survey popup and we'll remove these on save. Use the API question mode to + run your own scripts in surveys. + + )} + + ) +} diff --git a/frontend/src/scenes/surveys/surveyLogic.tsx b/frontend/src/scenes/surveys/surveyLogic.tsx index ea19f8b378bb9..c4dcb237b3dd6 100644 --- a/frontend/src/scenes/surveys/surveyLogic.tsx +++ b/frontend/src/scenes/surveys/surveyLogic.tsx @@ -787,6 +787,10 @@ function sanitizeQuestions(surveyPayload: Partial): Partial { if (!surveyPayload.questions) { return surveyPayload } + + const sanitizedThankYouHeader = sanitize(surveyPayload.appearance?.thankYouMessageHeader || '') + const sanitizedThankYouDescription = sanitize(surveyPayload.appearance?.thankYouMessageDescription || '') + return { ...surveyPayload, questions: surveyPayload.questions?.map((rawQuestion) => { @@ -796,5 +800,10 @@ function sanitizeQuestions(surveyPayload: Partial): Partial { question: sanitize(rawQuestion.question || ''), } }), + appearance: { + ...surveyPayload.appearance, + ...(sanitizedThankYouHeader && { thankYouMessageHeader: sanitizedThankYouHeader }), + ...(sanitizedThankYouDescription && { thankYouMessageDescription: sanitizedThankYouDescription }), + }, } } diff --git a/posthog/api/survey.py b/posthog/api/survey.py index 7d672aadf5938..81523782a86a2 100644 --- a/posthog/api/survey.py +++ b/posthog/api/survey.py @@ -89,6 +89,23 @@ class Meta: ] read_only_fields = ["id", "linked_flag", "targeting_flag", "created_at"] + def validate_appearance(self, value): + if value is None: + return value + + if not isinstance(value, dict): + raise serializers.ValidationError("Appearance must be an object") + + thank_you_message = value.get("thankYouMessageHeader") + if thank_you_message and nh3.is_html(thank_you_message): + value["thankYouMessageHeader"] = nh3.clean(thank_you_message) + + thank_you_description = value.get("thankYouMessageDescription") + if thank_you_description and nh3.is_html(thank_you_description): + value["thankYouMessageDescription"] = nh3.clean(thank_you_description) + + return value + def validate_questions(self, value): if value is None: return value diff --git a/posthog/api/test/test_survey.py b/posthog/api/test/test_survey.py index 54404d66b145b..3d64d3e1da046 100644 --- a/posthog/api/test/test_survey.py +++ b/posthog/api/test/test_survey.py @@ -685,6 +685,10 @@ def test_create_basic_survey_question_validation(self): "question": "What do you think of the new notebooks feature?", }, ], + "appearance": { + "thankYouMessageHeader": "Thanks for your feedback!", + "thankYouMessageDescription": "We'll use it to make notebooks better.", + }, }, format="json", ) @@ -702,6 +706,10 @@ def test_create_basic_survey_question_validation(self): "question": "What do you think of the new notebooks feature?", }, ] + assert response_data["appearance"] == { + "thankYouMessageHeader": "Thanks for your feedback!", + "thankYouMessageDescription": "We'll use it to make notebooks better.", + } assert response_data["created_by"]["id"] == self.user.id def test_update_basic_survey_question_validation(self): @@ -728,6 +736,9 @@ def test_update_basic_survey_question_validation(self): "question": "What do you think of the new notebooks feature?", }, ], + "appearance": { + "thankYouMessageDescription": "We'll use it to make notebooks better.", + }, }, format="json", ) @@ -745,6 +756,9 @@ def test_update_basic_survey_question_validation(self): "question": "What do you think of the new notebooks feature?", }, ] + assert response_data["appearance"] == { + "thankYouMessageDescription": "We'll use it to make notebooks better.", + } assert response_data["created_by"]["id"] == self.user.id def test_cleaning_empty_questions(self): @@ -755,6 +769,10 @@ def test_cleaning_empty_questions(self): "description": "Get feedback on the new notebooks feature", "type": "popover", "questions": [], + "appearance": { + "thankYouMessageHeader": " ", + "thankYouMessageDescription": "", + }, }, format="json", ) @@ -763,6 +781,25 @@ def test_cleaning_empty_questions(self): assert Survey.objects.filter(id=response_data["id"]).exists() assert response_data["name"] == "Notebooks beta release survey" assert response_data["questions"] == [] + assert response_data["appearance"] == { + "thankYouMessageHeader": " ", + "thankYouMessageDescription": "", + } + + def test_validate_thank_you_with_invalid_type(self): + response = self.client.post( + f"/api/projects/{self.team.id}/surveys/", + data={ + "name": "Notebooks beta release survey", + "description": "Get feedback on the new notebooks feature", + "type": "popover", + "appearance": "invalid", + }, + format="json", + ) + response_data = response.json() + assert response.status_code == status.HTTP_400_BAD_REQUEST, response_data + assert response_data["detail"] == "Appearance must be an object" def test_validate_question_with_missing_text(self): response = self.client.post(