Skip to content

Commit e6b81c4

Browse files
authored
Some problems with RLS editor (supabase#45662)
## Context Addresses some issues found with the RLS editor from recent changes - Creating a "SELECT" or "INSERT" policy via templates wasn't working (might have been [this PR](supabase#45560)) that introduced the bug) - SELECT -> SQL error as we were incorrectly adding a with check statement in the query - INSERT -> UI issue, there's a bit of complexity as we're using 1 code editor for `using` and `check` statements - Badge color for "UPDATE" based templates is off <img width="446" height="114" alt="image" src="https://github.com/user-attachments/assets/66fd0c1a-c20c-406d-983e-2c02680bb235" /> - Renaming a policy, the initial alter query statement shouldn't be using the new name <img width="596" height="288" alt="image" src="https://github.com/user-attachments/assets/0b6822d5-e5f5-440e-8942-8e19bd7bf4c3" /> ## To test - [ ] Verify that you can create a policy for all templates in the Auth policies page + Realtime policies page (as long as no SQL error - some templates are using tables as examples that might not exist in the DB) - [ ] Likewise, verify that you can manually create + update policies as well <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Style** * Updated UPDATE template badge styling to a darker blue color scheme with reduced opacity. * **Bug Fixes** * Fixed policy name display logic to correctly show the selected policy's existing name during renaming operations. * Improved SQL fragment loading and check expression handling for INSERT command policy templates. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 5d155df commit e6b81c4

3 files changed

Lines changed: 27 additions & 12 deletions

File tree

apps/studio/components/interfaces/Auth/Policies/PolicyEditorPanel/LockedQuerySection.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@ import { Lock } from 'lucide-react'
44
interface LockedCreateQuerySection {
55
schema: string
66
selectedPolicy?: PostgresPolicy
7+
isRenamingPolicy: boolean
78
formFields: { name: string; table: string; behavior: string; command: string; roles: string }
89
}
910

1011
export const LockedCreateQuerySection = ({
1112
schema,
13+
isRenamingPolicy,
1214
selectedPolicy,
1315
formFields,
1416
}: LockedCreateQuerySection) => {
@@ -31,7 +33,7 @@ export const LockedCreateQuerySection = ({
3133
<p className="px-6 font-mono text-sm text-foreground-light select-none">1</p>
3234
<p className="font-mono tracking-tighter">
3335
<span className="text-[#569cd6]">{isEditing ? 'alter' : 'create'}</span> policy "
34-
{name.length === 0 ? 'policy_name' : name}"
36+
{isRenamingPolicy ? selectedPolicy?.name : name.length === 0 ? 'policy_name' : name}"
3537
</p>
3638
</div>
3739
<div className="flex items-start" style={{ fontSize: '14px' }}>

apps/studio/components/interfaces/Auth/Policies/PolicyEditorPanel/PolicyTemplates.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ export const PolicyTemplates = ({
105105
className={cn(
106106
'rounded-sm! font-mono',
107107
template.command === 'UPDATE'
108-
? 'bg-blue-400 text-blue-900 border border-blue-800'
108+
? 'bg-blue-900/50 text-blue-200 border border-blue-800'
109109
: ''
110110
)}
111111
variant={

apps/studio/components/interfaces/Auth/Policies/PolicyEditorPanel/index.tsx

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ const defaultValues = {
8080

8181
/**
8282
* Using memo for this component because everything rerenders on window focus because of outside fetches
83+
* Note: For INSERT command, editor one holds the check expression (not using)
84+
Whereas for others: editor one = using, editor two = optional check
8385
*/
8486
export const PolicyEditorPanel = memo(function ({
8587
visible,
@@ -184,8 +186,7 @@ export const PolicyEditorPanel = memo(function ({
184186
const onSubmit = (data: z.infer<typeof FormSchema>) => {
185187
const { name, table, behavior, command, roles } = data
186188

187-
// For INSERT: editor one holds the check expression (not using)
188-
// For others: editor one = using, editor two = optional check
189+
// [Joshen] Refer to top note as to why we're using the "USING" section for INSERT
189190
const usingExpr = command !== 'insert' ? using : undefined
190191
const checkExpr = command === 'insert' ? using : check
191192

@@ -280,14 +281,16 @@ export const PolicyEditorPanel = memo(function ({
280281
command: command.toLowerCase(),
281282
roles: roles.length === 1 && roles[0] === 'public' ? '' : roles.join(', '),
282283
})
283-
if (selectedPolicy.definition) setUsing(safeSql` ${selectedPolicy.definition}`)
284-
if (selectedPolicy.check && selectedPolicy.command === 'INSERT')
285-
setUsing(safeSql` ${selectedPolicy.check}`)
286-
if (selectedPolicy.check && selectedPolicy.command !== 'INSERT')
287-
setCheck(safeSql` ${selectedPolicy.check}`)
288-
if (selectedPolicy.check && selectedPolicy.command !== 'INSERT') {
289-
setShowCheckBlock(true)
284+
285+
// [Joshen] Refer to top note as to why we're using the "USING" section for INSERT
286+
if (selectedPolicy.definition) {
287+
setUsing(safeSql` ${selectedPolicy.definition}`)
288+
}
289+
if (selectedPolicy.check) {
290+
if (selectedPolicy.command === 'INSERT') setUsing(safeSql` ${selectedPolicy.check}`)
291+
else setCheck(safeSql` ${selectedPolicy.check}`)
290292
}
293+
291294
setRolesFragment(
292295
roles.length === 1 && roles[0] === 'public'
293296
? safeSql`public`
@@ -356,6 +359,7 @@ export const PolicyEditorPanel = memo(function ({
356359
<LockedCreateQuerySection
357360
schema={schema}
358361
selectedPolicy={selectedPolicy}
362+
isRenamingPolicy={isRenamingPolicy}
359363
formFields={{ name, table, behavior, command, roles }}
360364
/>
361365

@@ -578,7 +582,16 @@ export const PolicyEditorPanel = memo(function ({
578582
form.setValue('roles', value.roles.join(', ') ?? '')
579583

580584
setUsing(safeSql` ${value.definition}`)
581-
setCheck(safeSql` ${value.check}`)
585+
586+
// [Joshen] Refer to top note as to why we're using the "USING" section for INSERT
587+
if (value.check) {
588+
if (value.command === 'INSERT') {
589+
setUsing(safeSql` ${value.check}`)
590+
} else {
591+
setCheck(safeSql` ${value.check}`)
592+
}
593+
}
594+
582595
setRolesFragment(
583596
value.roles.length === 0 ||
584597
(value.roles.length === 1 && value.roles[0] === 'public')

0 commit comments

Comments
 (0)