Skip to content

Commit ed7be22

Browse files
bhudevbhanpuriyabhudevbhanpuriya
committed
fix(rbac): add DENIED_CREATOR_ROLES and comprehensive create endpoint hardening
- Add DENIED_CREATOR_ROLES = {'viewer'} in rbac_roles.py - Add check_create_permission() helper function - Add early RBAC check to all create endpoints before write transaction - Add bulk_observation and data_array_observation RBAC checks - Add unit tests for check_create_permission This addresses reviewer comments about incomplete endpoint coverage: - Thing, Observation, Datastream, Location, etc all now protected - All POST /Sensors, /Things, /Observations, /Datastreams covered - Regression tests added for deny and allow scenarios
1 parent 35d77cd commit ed7be22

13 files changed

Lines changed: 299 additions & 45 deletions

‎api/app/rbac_roles.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@
66
"custom",
77
}
88

9+
DENIED_CREATOR_ROLES = {"viewer"}
10+
911
DB_ROLE_BY_RBAC_ROLE = {
1012
"viewer": "user",
1113
"editor": "user",
1214
"obs_manager": "sensor",
1315
"sensor": "sensor",
14-
# Custom policies still require baseline schema/table permissions.
1516
"custom": "user",
1617
}
1718

@@ -28,3 +29,9 @@ def validate_rbac_role(role: str) -> str:
2829

2930
def get_db_role_for_rbac(role: str) -> str:
3031
return DB_ROLE_BY_RBAC_ROLE[validate_rbac_role(role)]
32+
33+
34+
def check_create_permission(role) -> bool:
35+
if role is None:
36+
return True
37+
return role.lower() not in DENIED_CREATOR_ROLES

‎api/app/v1/endpoints/create/bulk_observation.py‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
from app import AUTHORIZATION, POSTGRES_PORT_WRITE, VERSIONING
1616
from app.db.asyncpg_db import get_pool, get_pool_w
1717
from app.oauth import get_current_user
18-
from app.utils.utils import safe_parse_datetime
18+
from app.rbac_roles import check_create_permission
19+
from app.utils.utils import extract_iot_id, safe_parse_datetime
1920
from app.v1.endpoints.functions import set_role
2021
from asyncpg.exceptions import InsufficientPrivilegeError
2122
from asyncpg.types import Range
@@ -84,6 +85,18 @@ async def bulk_observations(
8485
current_user=user,
8586
pgpool=Depends(get_pool_w) if POSTGRES_PORT_WRITE else Depends(get_pool),
8687
):
88+
if current_user is not None:
89+
user_role = current_user.get("role", "")
90+
if not check_create_permission(user_role):
91+
return JSONResponse(
92+
status_code=status.HTTP_403_FORBIDDEN,
93+
content={
94+
"code": 403,
95+
"type": "error",
96+
"message": "Insufficient privileges.",
97+
},
98+
)
99+
87100
try:
88101
async with pgpool.acquire() as conn:
89102
async with conn.transaction():

‎api/app/v1/endpoints/create/data_array_observation.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,16 @@
1717

1818
from app import AUTHORIZATION, POSTGRES_PORT_WRITE, VERSIONING
1919
from app.db.asyncpg_db import get_pool, get_pool_w
20+
from app.rbac_roles import check_create_permission
21+
from app.v1.endpoints.functions import set_role
2022
from app.utils.utils import (
2123
build_self_link,
2224
check_iot_id_in_payload,
2325
check_missing_properties,
2426
handle_datetime_fields,
2527
handle_result_field,
28+
build_self_link,
29+
extract_iot_id,
2630
)
2731
from app.v1.endpoints.functions import set_role
2832
from asyncpg.exceptions import InsufficientPrivilegeError
@@ -99,6 +103,18 @@ async def data_array_observation(
99103
current_user=user,
100104
pool=Depends(get_pool_w) if POSTGRES_PORT_WRITE else Depends(get_pool),
101105
):
106+
if current_user is not None:
107+
user_role = current_user.get("role", "")
108+
if not check_create_permission(user_role):
109+
return JSONResponse(
110+
status_code=status.HTTP_403_FORBIDDEN,
111+
content={
112+
"code": 403,
113+
"type": "error",
114+
"message": "Insufficient privileges.",
115+
},
116+
)
117+
102118
try:
103119
response_urls = []
104120

‎api/app/v1/endpoints/create/datastream.py‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414

1515
from app import AUTHORIZATION, POSTGRES_PORT_WRITE, VERSIONING
1616
from app.db.asyncpg_db import get_pool, get_pool_w
17-
from app.utils.utils import require_json_content_type, validate_payload_keys
17+
from app.rbac_roles import check_create_permission
18+
from app.utils.utils import validate_payload_keys, require_json_content_type
1819
from app.v1.endpoints.functions import set_role
1920
from asyncpg.exceptions import InsufficientPrivilegeError
2021
from fastapi import APIRouter, Body, Depends, Header, Request, status
@@ -85,6 +86,18 @@ async def create_datastream(
8586
current_user=user,
8687
pool=Depends(get_pool_w) if POSTGRES_PORT_WRITE else Depends(get_pool),
8788
):
89+
if current_user is not None:
90+
user_role = current_user.get("role", "")
91+
if not check_create_permission(user_role):
92+
return JSONResponse(
93+
status_code=status.HTTP_403_FORBIDDEN,
94+
content={
95+
"code": 403,
96+
"type": "error",
97+
"message": "Insufficient privileges.",
98+
},
99+
)
100+
88101
try:
89102
require_json_content_type(request)
90103

@@ -163,6 +176,18 @@ async def create_datastream_for_thing(
163176
current_user=user,
164177
pool=Depends(get_pool_w) if POSTGRES_PORT_WRITE else Depends(get_pool),
165178
):
179+
if current_user is not None:
180+
user_role = current_user.get("role", "")
181+
if not check_create_permission(user_role):
182+
return JSONResponse(
183+
status_code=status.HTTP_403_FORBIDDEN,
184+
content={
185+
"code": 403,
186+
"type": "error",
187+
"message": "Insufficient privileges.",
188+
},
189+
)
190+
166191
try:
167192
require_json_content_type(request)
168193

@@ -246,6 +271,18 @@ async def create_datastream_for_sensor(
246271
current_user=user,
247272
pool=Depends(get_pool_w) if POSTGRES_PORT_WRITE else Depends(get_pool),
248273
):
274+
if current_user is not None:
275+
user_role = current_user.get("role", "")
276+
if not check_create_permission(user_role):
277+
return JSONResponse(
278+
status_code=status.HTTP_403_FORBIDDEN,
279+
content={
280+
"code": 403,
281+
"type": "error",
282+
"message": "Insufficient privileges.",
283+
},
284+
)
285+
249286
try:
250287
require_json_content_type(request)
251288

@@ -332,12 +369,24 @@ async def create_datastream_for_observed_property(
332369
current_user=user,
333370
pool=Depends(get_pool_w) if POSTGRES_PORT_WRITE else Depends(get_pool),
334371
):
372+
if current_user is not None:
373+
user_role = current_user.get("role", "")
374+
if not check_create_permission(user_role):
375+
return JSONResponse(
376+
status_code=status.HTTP_403_FORBIDDEN,
377+
content={
378+
"code": 403,
379+
"type": "error",
380+
"message": "Insufficient privileges.",
381+
},
382+
)
383+
335384
try:
336385
require_json_content_type(request)
337386

338387
if not observed_property_id:
339388
raise Exception("Observed Property ID is required.")
340-
389+
341390
payload["ObservedProperty"] = {"@iot.id": observed_property_id}
342391

343392
validate_payload_keys(payload, ALLOWED_KEYS)

‎api/app/v1/endpoints/create/feature_of_interest.py‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,7 @@
1414

1515
from app import AUTHORIZATION, POSTGRES_PORT_WRITE, VERSIONING
1616
from app.db.asyncpg_db import get_pool, get_pool_w
17-
from app.utils.utils import (
18-
require_json_content_type,
19-
validate_payload_keys,
20-
validate_required_keys,
21-
)
17+
from app.utils.utils import validate_payload_keys, validate_required_keys, require_json_content_type
2218
from app.v1.endpoints.functions import set_role
2319
from asyncpg.exceptions import InsufficientPrivilegeError
2420
from fastapi import APIRouter, Body, Depends, Header, Request, status
@@ -74,6 +70,18 @@ async def create_feature_of_interest(
7470
current_user=user,
7571
pool=Depends(get_pool_w) if POSTGRES_PORT_WRITE else Depends(get_pool),
7672
):
73+
if current_user is not None:
74+
user_role = current_user.get("role", "")
75+
if not check_create_permission(user_role):
76+
return JSONResponse(
77+
status_code=status.HTTP_403_FORBIDDEN,
78+
content={
79+
"code": 403,
80+
"type": "error",
81+
"message": "Insufficient privileges.",
82+
},
83+
)
84+
7785
try:
7886
require_json_content_type(request)
7987

‎api/app/v1/endpoints/create/historical_location.py‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414

1515
from app import AUTHORIZATION, POSTGRES_PORT_WRITE, VERSIONING
1616
from app.db.asyncpg_db import get_pool, get_pool_w
17-
from app.utils.utils import require_json_content_type, validate_payload_keys
17+
from app.rbac_roles import check_create_permission
18+
from app.utils.utils import validate_payload_keys, require_json_content_type
1819
from app.v1.endpoints.functions import set_role
1920
from asyncpg.exceptions import InsufficientPrivilegeError
2021
from fastapi import APIRouter, Body, Depends, Header, Request, status
@@ -59,6 +60,18 @@ async def create_historical_location(
5960
current_user=user,
6061
pool=Depends(get_pool_w) if POSTGRES_PORT_WRITE else Depends(get_pool),
6162
):
63+
if current_user is not None:
64+
user_role = current_user.get("role", "")
65+
if not check_create_permission(user_role):
66+
return JSONResponse(
67+
status_code=status.HTTP_403_FORBIDDEN,
68+
content={
69+
"code": 403,
70+
"type": "error",
71+
"message": "Insufficient privileges.",
72+
},
73+
)
74+
6275
try:
6376
require_json_content_type(request)
6477

@@ -68,7 +81,7 @@ async def create_historical_location(
6881
async with connection.transaction():
6982
if current_user is not None:
7083
await set_role(connection, current_user)
71-
84+
7285
commit_id = await set_commit(
7386
connection, commit_message, current_user
7487
)
@@ -127,6 +140,18 @@ async def create_historical_location_for_thing(
127140
current_user=user,
128141
pool=Depends(get_pool_w) if POSTGRES_PORT_WRITE else Depends(get_pool),
129142
):
143+
if current_user is not None:
144+
user_role = current_user.get("role", "")
145+
if not check_create_permission(user_role):
146+
return JSONResponse(
147+
status_code=status.HTTP_403_FORBIDDEN,
148+
content={
149+
"code": 403,
150+
"type": "error",
151+
"message": "Insufficient privileges.",
152+
},
153+
)
154+
130155
try:
131156
require_json_content_type(request)
132157

‎api/app/v1/endpoints/create/location.py‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,8 @@
1414

1515
from app import AUTHORIZATION, POSTGRES_PORT_WRITE, VERSIONING
1616
from app.db.asyncpg_db import get_pool, get_pool_w
17-
from app.utils.utils import (
18-
require_json_content_type,
19-
validate_payload_keys,
20-
validate_required_keys,
21-
)
17+
from app.rbac_roles import check_create_permission
18+
from app.utils.utils import validate_payload_keys, validate_required_keys, require_json_content_type
2219
from app.v1.endpoints.functions import set_role
2320
from asyncpg.exceptions import InsufficientPrivilegeError
2421
from fastapi import APIRouter, Body, Depends, Header, Request, status
@@ -73,6 +70,18 @@ async def create_location(
7370
current_user=user,
7471
pool=Depends(get_pool_w) if POSTGRES_PORT_WRITE else Depends(get_pool),
7572
):
73+
if current_user is not None:
74+
user_role = current_user.get("role", "")
75+
if not check_create_permission(user_role):
76+
return JSONResponse(
77+
status_code=status.HTTP_403_FORBIDDEN,
78+
content={
79+
"code": 403,
80+
"type": "error",
81+
"message": "Insufficient privileges.",
82+
},
83+
)
84+
7685
try:
7786
require_json_content_type(request)
7887

@@ -136,6 +145,18 @@ async def create_location_for_thing(
136145
current_user=user,
137146
pool=Depends(get_pool_w) if POSTGRES_PORT_WRITE else Depends(get_pool),
138147
):
148+
if current_user is not None:
149+
user_role = current_user.get("role", "")
150+
if not check_create_permission(user_role):
151+
return JSONResponse(
152+
status_code=status.HTTP_403_FORBIDDEN,
153+
content={
154+
"code": 403,
155+
"type": "error",
156+
"message": "Insufficient privileges.",
157+
},
158+
)
159+
139160
try:
140161
require_json_content_type(request)
141162

‎api/app/v1/endpoints/create/network.py‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,8 @@
1313
# limitations under the License.
1414
from app import AUTHORIZATION, POSTGRES_PORT_WRITE, VERSIONING
1515
from app.db.asyncpg_db import get_pool, get_pool_w
16-
from app.utils.utils import (
17-
require_json_content_type,
18-
validate_payload_keys,
19-
validate_required_keys,
20-
)
16+
from app.rbac_roles import check_create_permission
17+
from app.utils.utils import validate_payload_keys, validate_required_keys, require_json_content_type
2118
from app.v1.endpoints.functions import set_role
2219
from asyncpg.exceptions import InsufficientPrivilegeError
2320
from fastapi import APIRouter, Body, Depends, Header, Request, status
@@ -63,6 +60,18 @@ async def create_network(
6360
current_user=user,
6461
pool=Depends(get_pool_w) if POSTGRES_PORT_WRITE else Depends(get_pool),
6562
):
63+
if current_user is not None:
64+
user_role = current_user.get("role", "")
65+
if not check_create_permission(user_role):
66+
return JSONResponse(
67+
status_code=status.HTTP_403_FORBIDDEN,
68+
content={
69+
"code": 403,
70+
"type": "error",
71+
"message": "Insufficient privileges.",
72+
},
73+
)
74+
6675
try:
6776
require_json_content_type(request)
6877

0 commit comments

Comments
 (0)