Skip to content

Commit c7bc2c5

Browse files
committed
remove wildcard handling from rls policies
1 parent 13913e0 commit c7bc2c5

7 files changed

Lines changed: 92 additions & 108 deletions

File tree

api/config.go

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,13 @@ import (
1010

1111
var DefaultConfig = Config{
1212
Postgrest: PostgrestConfig{
13-
Version: "v10.0.0",
14-
DBRole: "postgrest_api",
15-
AnonDBRole: "",
16-
Port: 3000,
17-
AdminPort: 3001,
18-
MaxRows: 2000,
13+
Version: "v10.0.0",
14+
DBRole: "postgrest_api",
15+
DBRoleBypass: "rls-bypasser",
16+
AnonDBRole: "",
17+
Port: 3000,
18+
AdminPort: 3001,
19+
MaxRows: 2000,
1920
},
2021
}
2122

@@ -123,15 +124,22 @@ func (c Config) GetUsername() string {
123124
}
124125

125126
type PostgrestConfig struct {
126-
Port int
127-
Disable bool
128-
LogLevel string
129-
URL string
130-
Version string
131-
JWTSecret string
132-
DBRole string
127+
Port int
128+
Disable bool
129+
LogLevel string
130+
URL string
131+
Version string
132+
JWTSecret string
133+
AdminPort int
134+
135+
// DBRole is the PostgREST role used for authenticated requests.
136+
DBRole string
137+
138+
// DBRoleBypass is the PostgREST role used to bypass RLS for admin requests.
139+
DBRoleBypass string
140+
141+
// AnonDBRole is the PostgREST role used for unauthenticated requests.
133142
AnonDBRole string
134-
AdminPort int
135143

136144
// A hard limit to the number of rows PostgREST will fetch from a view, table, or stored procedure.
137145
// Limits payload size for accidental or malicious requests.

db.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,12 @@ func setStatementTimeouts(ctx dutyContext.Context, config api.Config) {
259259
logger.Errorf(err.Error())
260260
}
261261

262+
if config.Postgrest.DBRoleBypass != "" {
263+
if err := ctx.DB().Raw(fmt.Sprintf(`ALTER ROLE %s SET statement_timeout = '%0fs'`, config.Postgrest.DBRoleBypass, postgrestTimeout.Seconds())).Error; err != nil {
264+
logger.Errorf(err.Error())
265+
}
266+
}
267+
262268
if config.Postgrest.AnonDBRole != "" {
263269
if err := ctx.DB().Raw(fmt.Sprintf(`ALTER ROLE %s SET statement_timeout = '%0fs'`, config.Postgrest.AnonDBRole, postgrestTimeout.Seconds())).Error; err != nil {
264270
logger.Errorf(err.Error())

migrate/migrate.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,19 @@ func grantPostgrestRolesToCurrentUser(pool *sql.DB, config api.Config) error {
270270
"ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT ALL PRIVILEGES ON TABLES TO %s"); err != nil {
271271
return err
272272
}
273+
274+
if config.Postgrest.DBRoleBypass != "" {
275+
if err := createRole(pool, config.Postgrest.DBRoleBypass, config,
276+
"GRANT USAGE, SELECT ON ALL SEQUENCES IN SCHEMA public TO %s",
277+
"GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO %s",
278+
"GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA public TO %s",
279+
"ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT ALL PRIVILEGES ON TABLES TO %s"); err != nil {
280+
return err
281+
}
282+
if _, err := pool.Exec(fmt.Sprintf("ALTER ROLE %s BYPASSRLS", config.Postgrest.DBRoleBypass)); err != nil {
283+
logger.GetLogger("migrate").Errorf("Failed to set BYPASSRLS for role %s: %v", config.Postgrest.DBRoleBypass, err)
284+
}
285+
}
273286
if err := createRole(pool, config.Postgrest.AnonDBRole, config,
274287
"GRANT SELECT ON ALL TABLES IN SCHEMA public TO %s",
275288
"ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT SELECT ON TABLES TO %s"); err != nil {

rls/payload.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,26 @@ func (t *Payload) Fingerprint() string {
6767

6868
// Injects the payload as local parameter
6969
func (t Payload) SetPostgresSessionRLS(db *gorm.DB) error {
70-
return t.setPostgresSessionRLS(db, true)
70+
return t.setPostgresSessionRLS(db, true, "postgrest_api")
7171
}
7272

7373
// Injects the payload as sessions parameter
7474
func (t Payload) SetGlobalPostgresSessionRLS(db *gorm.DB) error {
75-
return t.setPostgresSessionRLS(db, false)
75+
return t.setPostgresSessionRLS(db, false, "postgrest_api")
7676
}
7777

78-
func (t Payload) setPostgresSessionRLS(db *gorm.DB, local bool) error {
78+
func (t Payload) SetPostgresSessionRLSWithRole(db *gorm.DB, role string) error {
79+
return t.setPostgresSessionRLS(db, true, role)
80+
}
81+
82+
func (t Payload) SetGlobalPostgresSessionRLSWithRole(db *gorm.DB, role string) error {
83+
return t.setPostgresSessionRLS(db, false, role)
84+
}
85+
86+
func (t Payload) setPostgresSessionRLS(db *gorm.DB, local bool, role string) error {
87+
if role == "" {
88+
return fmt.Errorf("role is required")
89+
}
7990
rlsJSON, err := json.Marshal(t)
8091
if err != nil {
8192
return fmt.Errorf("failed to marshall to json: %w", err)
@@ -86,7 +97,7 @@ func (t Payload) setPostgresSessionRLS(db *gorm.DB, local bool) error {
8697
scope = "LOCAL"
8798
}
8899

89-
if err := db.Exec(fmt.Sprintf("SET %s ROLE postgrest_api", scope)).Error; err != nil {
100+
if err := db.Exec(fmt.Sprintf("SET %s ROLE %s", scope, role)).Error; err != nil {
90101
return fmt.Errorf("failed to set role: %w", err)
91102
}
92103

tests/rls_test.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,6 @@ func payloadWithScopes(scopeIDs ...*uuid.UUID) func() rls.Payload {
4343
}
4444
}
4545

46-
func payloadDisabled() func() rls.Payload {
47-
return func() rls.Payload {
48-
return rls.Payload{Disable: true}
49-
}
50-
}
51-
5246
func resetScopes(db *gorm.DB, tables ...string) {
5347
for _, table := range tables {
5448
Expect(db.Exec(fmt.Sprintf("UPDATE %s SET __scope = NULL", table)).Error).To(BeNil())
@@ -112,7 +106,6 @@ var _ = Describe("RLS scopes", Ordered, ContinueOnFailure, func() {
112106
Entry("no scopes", scopeCase{payload: payloadNoScopes(), expectedCount: lo.ToPtr(int64(0))}),
113107
Entry("aws scope", scopeCase{payload: payloadWithScopes(&awsScopeID), expectedCount: &awsConfigs}),
114108
Entry("combined scopes", scopeCase{payload: payloadWithScopes(&awsScopeID, &demoScopeID), expectedCount: &awsOrDemo}),
115-
Entry("rls disabled", scopeCase{payload: payloadDisabled(), expectedCount: &totalConfigs}),
116109
)
117110
})
118111
}

views/035_rls_utils.sql

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,3 @@
1-
-- isolated from 9998_rls_enable.sql because generated tables in the view use it.
2-
CREATE
3-
OR REPLACE FUNCTION is_rls_disabled() RETURNS BOOLEAN AS $$
4-
DECLARE
5-
jwt_claims TEXT;
6-
BEGIN
7-
jwt_claims := current_setting('request.jwt.claims', TRUE);
8-
RETURN (jwt_claims IS NULL
9-
OR jwt_claims = ''
10-
OR jwt_claims::jsonb ->> 'disable_rls' IS NOT NULL);
11-
END;
12-
$$ LANGUAGE plpgsql SECURITY INVOKER;
13-
141
-- rls_scope_access returns scope UUIDs from request.jwt.claims (empty when missing).
152
CREATE
163
OR REPLACE FUNCTION rls_scope_access() RETURNS UUID[] AS $$

views/9998_rls_enable.sql

Lines changed: 36 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,7 @@ DROP POLICY IF EXISTS config_items_auth ON config_items;
5757
CREATE POLICY config_items_auth ON config_items
5858
FOR ALL TO postgrest_api, postgrest_anon
5959
USING (
60-
CASE WHEN (SELECT is_rls_disabled()) THEN TRUE
61-
ELSE
62-
(config_items.__scope && rls_scope_access())
63-
END
60+
(config_items.__scope && rls_scope_access())
6461
);
6562

6663
-- Policy config_changes
@@ -69,14 +66,12 @@ DROP POLICY IF EXISTS config_changes_auth ON config_changes;
6966
CREATE POLICY config_changes_auth ON config_changes
7067
FOR ALL TO postgrest_api, postgrest_anon
7168
USING (
72-
CASE WHEN (SELECT is_rls_disabled()) THEN TRUE
73-
ELSE EXISTS (
69+
EXISTS (
7470
-- just leverage the RLS on config_items
7571
SELECT 1
7672
FROM config_items
7773
WHERE config_items.id = config_changes.config_id
7874
)
79-
END
8075
);
8176

8277
-- Policy config_analysis
@@ -85,14 +80,12 @@ DROP POLICY IF EXISTS config_analysis_auth ON config_analysis;
8580
CREATE POLICY config_analysis_auth ON config_analysis
8681
FOR ALL TO postgrest_api, postgrest_anon
8782
USING (
88-
CASE WHEN (SELECT is_rls_disabled()) THEN TRUE
89-
ELSE EXISTS (
83+
EXISTS (
9084
-- just leverage the RLS on config_items
9185
SELECT 1
9286
FROM config_items
9387
WHERE config_items.id = config_analysis.config_id
9488
)
95-
END
9689
);
9790

9891
-- Policy config_relationships
@@ -101,13 +94,9 @@ DROP POLICY IF EXISTS config_relationships_auth ON config_relationships;
10194
CREATE POLICY config_relationships_auth ON config_relationships
10295
FOR ALL TO postgrest_api, postgrest_anon
10396
USING (
104-
CASE WHEN (SELECT is_rls_disabled()) THEN TRUE
105-
ELSE (
106-
-- just leverage the RLS on config_items - user must have access to both items
107-
EXISTS (SELECT 1 FROM config_items WHERE config_items.id = config_relationships.config_id)
108-
AND EXISTS (SELECT 1 FROM config_items WHERE config_items.id = config_relationships.related_id)
109-
)
110-
END
97+
-- just leverage the RLS on config_items - user must have access to both items
98+
EXISTS (SELECT 1 FROM config_items WHERE config_items.id = config_relationships.config_id)
99+
AND EXISTS (SELECT 1 FROM config_items WHERE config_items.id = config_relationships.related_id)
111100
);
112101

113102
-- Policy config_component_relationships
@@ -116,14 +105,12 @@ DROP POLICY IF EXISTS config_component_relationships_auth ON config_component_re
116105
CREATE POLICY config_component_relationships_auth ON config_component_relationships
117106
FOR ALL TO postgrest_api, postgrest_anon
118107
USING (
119-
CASE WHEN (SELECT is_rls_disabled()) THEN TRUE
120-
ELSE EXISTS (
108+
EXISTS (
121109
-- just leverage the RLS on config_items
122110
SELECT 1
123111
FROM config_items
124112
WHERE config_items.id = config_component_relationships.config_id
125113
)
126-
END
127114
);
128115

129116
-- Policy components
@@ -132,10 +119,7 @@ DROP POLICY IF EXISTS components_auth ON components;
132119
CREATE POLICY components_auth ON components
133120
FOR ALL TO postgrest_api, postgrest_anon
134121
USING (
135-
CASE WHEN (SELECT is_rls_disabled()) THEN TRUE
136-
ELSE
137-
(components.__scope && rls_scope_access())
138-
END
122+
(components.__scope && rls_scope_access())
139123
);
140124

141125
-- Policy canaries
@@ -144,10 +128,7 @@ DROP POLICY IF EXISTS canaries_auth ON canaries;
144128
CREATE POLICY canaries_auth ON canaries
145129
FOR ALL TO postgrest_api, postgrest_anon
146130
USING (
147-
CASE WHEN (SELECT is_rls_disabled()) THEN TRUE
148-
ELSE
149-
(canaries.__scope && rls_scope_access())
150-
END
131+
(canaries.__scope && rls_scope_access())
151132
);
152133

153134
-- Policy playbooks
@@ -156,10 +137,7 @@ DROP POLICY IF EXISTS playbooks_auth ON playbooks;
156137
CREATE POLICY playbooks_auth ON playbooks
157138
FOR ALL TO postgrest_api, postgrest_anon
158139
USING (
159-
CASE WHEN (SELECT is_rls_disabled()) THEN TRUE
160-
ELSE
161-
(playbooks.__scope && rls_scope_access())
162-
END
140+
(playbooks.__scope && rls_scope_access())
163141
);
164142

165143
-- Policy playbook_runs
@@ -168,31 +146,27 @@ DROP POLICY IF EXISTS playbook_runs_auth ON playbook_runs;
168146
CREATE POLICY playbook_runs_auth ON playbook_runs
169147
FOR ALL TO postgrest_api, postgrest_anon
170148
USING (
171-
CASE WHEN (SELECT is_rls_disabled()) THEN TRUE
172-
ELSE (
173-
-- User must have access to the playbook
174-
EXISTS (
175-
SELECT 1
176-
FROM playbooks
177-
WHERE playbooks.id = playbook_runs.playbook_id
178-
)
179-
AND
180-
-- AND if run has a config_id, user must have access to that config
181-
(playbook_runs.config_id IS NULL OR EXISTS (
182-
SELECT 1
183-
FROM config_items
184-
WHERE config_items.id = playbook_runs.config_id
185-
))
186-
AND
187-
-- AND if run has a check_id, user must have access to that check (via its canary)
188-
(playbook_runs.check_id IS NULL OR EXISTS (
189-
SELECT 1
190-
FROM checks
191-
WHERE checks.id = playbook_runs.check_id
192-
))
193-
-- Note: component_id check omitted (phasing out topology soon)
149+
-- User must have access to the playbook
150+
EXISTS (
151+
SELECT 1
152+
FROM playbooks
153+
WHERE playbooks.id = playbook_runs.playbook_id
194154
)
195-
END
155+
AND
156+
-- AND if run has a config_id, user must have access to that config
157+
(playbook_runs.config_id IS NULL OR EXISTS (
158+
SELECT 1
159+
FROM config_items
160+
WHERE config_items.id = playbook_runs.config_id
161+
))
162+
AND
163+
-- AND if run has a check_id, user must have access to that check (via its canary)
164+
(playbook_runs.check_id IS NULL OR EXISTS (
165+
SELECT 1
166+
FROM checks
167+
WHERE checks.id = playbook_runs.check_id
168+
))
169+
-- Note: component_id check omitted (phasing out topology soon)
196170
);
197171

198172
-- Policy checks
@@ -201,14 +175,12 @@ DROP POLICY IF EXISTS checks_auth ON checks;
201175
CREATE POLICY checks_auth ON checks
202176
FOR ALL TO postgrest_api, postgrest_anon
203177
USING (
204-
CASE WHEN (SELECT is_rls_disabled()) THEN TRUE
205-
ELSE EXISTS (
178+
EXISTS (
206179
-- just leverage the RLS on canaries
207180
SELECT 1
208181
FROM canaries
209182
WHERE canaries.id = checks.canary_id
210183
)
211-
END
212184
);
213185

214186
-- Policy views
@@ -217,10 +189,7 @@ DROP POLICY IF EXISTS views_auth ON views;
217189
CREATE POLICY views_auth ON views
218190
FOR ALL TO postgrest_api, postgrest_anon
219191
USING (
220-
CASE WHEN (SELECT is_rls_disabled()) THEN TRUE
221-
ELSE
222-
(views.__scope && rls_scope_access())
223-
END
192+
(views.__scope && rls_scope_access())
224193
);
225194

226195
-- Policy view_panels (inherits from parent views table)
@@ -229,13 +198,10 @@ DROP POLICY IF EXISTS view_panels_auth ON view_panels;
229198
CREATE POLICY view_panels_auth ON view_panels
230199
FOR ALL TO postgrest_api, postgrest_anon
231200
USING (
232-
CASE WHEN (SELECT is_rls_disabled()) THEN TRUE
233-
ELSE
234-
EXISTS (
235-
SELECT 1 FROM views
236-
WHERE views.id = view_panels.view_id
237-
)
238-
END
201+
EXISTS (
202+
SELECT 1 FROM views
203+
WHERE views.id = view_panels.view_id
204+
)
239205
);
240206

241207
ALTER VIEW analysis_by_config SET (security_invoker = true);

0 commit comments

Comments
 (0)