Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions forms_pro/api/form.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,24 @@ def get_form(form_id: str) -> dict:
}


@frappe.whitelist(allow_guest=True)
def get_link_field_options(
doctype: str,
filters: dict | None = None,
page_length: int = 20,
) -> list[str]:
meta = frappe.get_meta(doctype)
title_field = meta.title_field or "name"

results = frappe.get_all(
doctype=doctype,
filters=filters or {},
page_length=page_length,
fields=["name as value", f"{title_field} as label"],
)
return results
Comment on lines +63 to +78
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

CRITICAL: Guest access without permission checks exposes sensitive data.

This endpoint allows unauthenticated users to query any doctype without permission validation. This is a serious security vulnerability that could expose:

  • Sensitive business data (customers, invoices, employees, etc.)
  • System metadata useful for reconnaissance attacks
  • Data that should be restricted by Frappe's permission system
🔒 Recommended security fixes

Option 1: Remove guest access and add permission checks (recommended)

-@frappe.whitelist(allow_guest=True)
+@frappe.whitelist()
 def get_link_field_options(
     doctype: str,
     filters: dict | None = None,
     page_length: int = 20,
 ) -> list[str]:
+    # Validate user has read permission for the doctype
+    if not frappe.has_permission(doctype, "read"):
+        frappe.throw(_("You do not have permission to access this doctype"))
+
     meta = frappe.get_meta(doctype)
     title_field = meta.title_field or "name"

Option 2: If guest access is required, implement a whitelist of allowed doctypes

 @frappe.whitelist(allow_guest=True)
 def get_link_field_options(
     doctype: str,
     filters: dict | None = None,
     page_length: int = 20,
 ) -> list[str]:
+    # Only allow specific doctypes for guest access
+    ALLOWED_DOCTYPES = ["Country", "State", "City"]  # Add safe doctypes here
+    if doctype not in ALLOWED_DOCTYPES:
+        frappe.throw(_("Access to this doctype is not allowed"))
+
     meta = frappe.get_meta(doctype)
     title_field = meta.title_field or "name"
🤖 Prompt for AI Agents
In @forms_pro/api/form.py around lines 63 - 78, get_link_field_options currently
exposes any doctype to unauthenticated callers because of
@frappe.whitelist(allow_guest=True); remove or disable allow_guest and enforce
permission checks inside get_link_field_options by verifying the caller has read
access with frappe.has_permission(doctype, "read", user) (or
frappe.has_permission for each record if needed) before calling frappe.get_all,
and if guest access must be kept implement a strict allowlist of permitted
doctypes and validate the requested doctype against that list; ensure the
function raises/returns an error when permission fails and do not return results
for unauthorized doctypes.

Comment on lines +64 to +78
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Incorrect return type annotation.

The function is annotated to return list[str] but actually returns list[dict] since frappe.get_all with the fields parameter returns dictionaries with "value" and "label" keys.

📝 Fix the return type
+from typing import TypedDict
+
+class LinkOption(TypedDict):
+    value: str
+    label: str
+
 @frappe.whitelist(allow_guest=True)
 def get_link_field_options(
     doctype: str,
     filters: dict | None = None,
     page_length: int = 20,
-) -> list[str]:
+) -> list[LinkOption]:

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In @forms_pro/api/form.py around lines 64 - 78, The return type annotation on
get_link_field_options is wrong: frappe.get_all with fields returns a list of
dicts (each with "value" and "label"), not list[str]; update the function
signature's return type from list[str] to list[dict[str, str]] (or list[dict])
and adjust any related typing imports or docstring if present to reflect that
the function returns a list of dictionaries with "value" and "label".



@frappe.whitelist()
def get_form_shared_with(form_id: str) -> list[frappe.Any]:
"""
Expand Down
4 changes: 2 additions & 2 deletions forms_pro/forms_pro/doctype/form_field/form_field.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"fieldtype": "Select",
"in_list_view": 1,
"label": "Fieldtype",
"options": "Data\nNumber\nEmail\nDate\nDate Time\nDate Range\nTime Picker\nPassword\nSelect\nSwitch\nTextarea\nText Editor",
"options": "Data\nNumber\nEmail\nDate\nDate Time\nDate Range\nTime Picker\nPassword\nSelect\nSwitch\nTextarea\nText Editor\nLink",
"reqd": 1
},
{
Expand Down Expand Up @@ -68,7 +68,7 @@
"index_web_pages_for_search": 1,
"istable": 1,
"links": [],
"modified": "2025-10-06 03:28:25.437379",
"modified": "2026-01-09 13:23:39.055114",
"modified_by": "Administrator",
"module": "Forms Pro",
"name": "Form Field",
Expand Down
1 change: 1 addition & 0 deletions forms_pro/forms_pro/doctype/form_field/form_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class FormField(Document):
"Switch",
"Textarea",
"Text Editor",
"Link",
]
label: DF.Data
options: DF.SmallText | None
Expand Down
52 changes: 49 additions & 3 deletions frontend/src/components/builder/FieldRenderer.vue
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
<script setup lang="ts">
import { computed } from "vue";
import { computed, ref, watch, onMounted } from "vue";
import { Asterisk } from "lucide-vue-next";
import RenderField from "../RenderField.vue";
import { createResource } from "frappe-ui";
import { useSubmissionForm } from "@/stores/submissionForm";

const submissionFormStore = useSubmissionForm();

const props = defineProps({
field: {
Expand Down Expand Up @@ -47,7 +51,15 @@ const getClasses = computed(() => {
}
});

const getOptions = () => {
type SelectOption = {
label: string;
value: string;
};

// Reactive ref to store options
const selectOptions = ref<string[] | SelectOption[] | null>(null);

const getOptions = async () => {
if (!fieldData.value.options) {
return "";
}
Expand All @@ -56,8 +68,42 @@ const getOptions = () => {
return fieldData.value.options.split("\n");
}

if (fieldData.value.fieldtype === "Link") {
const _options = createResource({
url: "forms_pro.api.form.get_link_field_options",
makeParams: () => {
return {
doctype: fieldData.value.options,
filters: {},
page_length: 999,
};
},
});
await _options.fetch();
return _options.data;
}

return fieldData.value.options;
};

// Load options when component mounts or field data changes
const loadOptions = async () => {
selectOptions.value = await getOptions();
};

// Watch for changes to field type or options
watch(
() => [fieldData.value.fieldtype, fieldData.value.options],
() => {
loadOptions();
},
{ immediate: true }
);

// Also load on mount
onMounted(() => {
loadOptions();
});
Comment on lines +95 to +106
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Duplicate option loading on mount.

The loadOptions function is called twice when the component mounts: once by the watcher with immediate: true and once by onMounted. This results in unnecessary duplicate API calls for Link fields.

⚡ Remove duplicate call
 // Watch for changes to field type or options
 watch(
     () => [fieldData.value.fieldtype, fieldData.value.options],
     () => {
         loadOptions();
     },
     { immediate: true }
 );
-
-// Also load on mount
-onMounted(() => {
-    loadOptions();
-});

The watcher with immediate: true already handles the initial load, so onMounted is redundant.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
watch(
() => [fieldData.value.fieldtype, fieldData.value.options],
() => {
loadOptions();
},
{ immediate: true }
);
// Also load on mount
onMounted(() => {
loadOptions();
});
watch(
() => [fieldData.value.fieldtype, fieldData.value.options],
() => {
loadOptions();
},
{ immediate: true }
);
🤖 Prompt for AI Agents
In @frontend/src/components/builder/FieldRenderer.vue around lines 95 - 106, The
component is calling loadOptions twice on mount (watcher with immediate: true
and onMounted), causing duplicate API calls; remove the redundant onMounted(()
=> { loadOptions(); }) block so the watcher (watch(() =>
[fieldData.value.fieldtype, fieldData.value.options], () => { loadOptions(); },
{ immediate: true })) uniquely handles initial and subsequent loads for
loadOptions tied to fieldData changes.

</script>
<template>
<div :class="getClasses" v-if="fieldData.fieldtype == 'Switch'">
Expand Down Expand Up @@ -151,7 +197,7 @@ const getOptions = () => {
:field="fieldData"
:class="{ 'pointer-events-none': inEditMode }"
:disabled="disabled"
:options="getOptions()"
:options="selectOptions"
/>
<small class="text-gray-500">
{{ fieldData.description }}
Expand Down
1 change: 1 addition & 0 deletions frontend/src/utils/form_fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export const CheckboxField: FormFieldType = {

export const formFields: FormFields[] = [
{ name: "Data", ...DataField },
{ name: "Link", ...SelectField },
{ name: "Number", ...NumberField },
{ name: "Email", ...EmailField },
{ name: "Date", ...DateField },
Expand Down