Skip to content

Commit

Permalink
Api: Makes sure invisible endpoint items are inaccessible
Browse files Browse the repository at this point in the history
TYPE: Bugfix
LINK: OGC-1992
  • Loading branch information
Daverball authored Dec 18, 2024
1 parent 7663a13 commit 9e31787
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 8 deletions.
5 changes: 4 additions & 1 deletion src/onegov/api/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ def __init__(
):
self.message = (
exception.message
if exception and hasattr(exception, 'message') else message
if exception and hasattr(exception, 'message') else
exception.title
if exception and hasattr(exception, 'title') else
message
)
self.status_code = (
exception.status_code
Expand Down
2 changes: 2 additions & 0 deletions src/onegov/api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
from morepath.request import Response


# TODO: Do we allow this to update request.identity, so we can elevate
# privileges for what a given API token is allowed to see?
def authenticate(request: 'CoreRequest') -> ApiKey:
if request.authorization is None:
raise HTTPUnauthorized()
Expand Down
13 changes: 12 additions & 1 deletion src/onegov/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from onegov.api.utils import authenticate, check_rate_limit
from onegov.core.security import Public
from onegov.form.fields import HoneyPotField
from webob.exc import HTTPMethodNotAllowed, HTTPUnauthorized
from webob.exc import HTTPMethodNotAllowed, HTTPNotFound, HTTPUnauthorized
from wtforms import HiddenField


Expand Down Expand Up @@ -156,6 +156,12 @@ def add_headers(response: 'Response') -> None:
assert endpoint is not None
links = self.links or {}
data = self.data or {}

# make sure we are actually supposed to be able to see this
# the API shouldn't include invisible items either (for now)
if (item := self.item) and not request.is_visible(item):
raise HTTPNotFound()

payload: dict[str, JSONObject] = {
'collection': {
'version': '1.0',
Expand Down Expand Up @@ -222,6 +228,11 @@ def edit_api_endpoint_item(
if api_key.read_only:
raise HTTPUnauthorized()

# make sure we are actually supposed to be able to see this
# the API shouldn't include invisible items either (for now)
if (item := self.item) and not request.is_visible(item):
raise HTTPNotFound()

def walk_errors(
errors: 'Sequence[str] | _FormErrors',
prefix: str | None
Expand Down
28 changes: 25 additions & 3 deletions tests/onegov/api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,45 @@
from onegov.api import ApiEndpoint
from onegov.core import Framework
from onegov.core.utils import Bunch
from onegov.form import Form
from pytest import fixture
from tests.shared.client import Client
from tests.shared.utils import create_app
from wtforms import StringField
from wtforms.validators import InputRequired


class App(Framework, ApiApp):
pass


class ItemForm(Form):
title = StringField(validators=[InputRequired()])


class Collection:

def __init__(self):
self.items = {
'1': Bunch(id=1, title='First item', a=1, b='2'),
'2': Bunch(id=2, title='Second item', a=5, b='6'),
'3': Bunch(id=3, title='Hidden item', a=2, b='3', hidden=True),
}
self.batch = [
Bunch(id=1, title='First item', a=1, b='2'),
Bunch(id=2, title='Second item', a=5, b='6')
item
for item in self.items.values()
if getattr(item, 'hidden', False) is False
]
self.next = None
self.previous = None

def by_id(self, id_):
return {str(x.id): x for x in self.batch}.get(str(id_))
return self.items.get(str(id_))


class Endpoint(ApiEndpoint):
endpoint = 'endpoint'
form_class = ItemForm
filter = []

def __init__(self, app, extra_parameters=None, page=None):
Expand All @@ -43,11 +57,19 @@ def item_data(self, item):
def item_links(self, item):
return {'b': item.b}

def apply_changes(self, item, form):
return


@App.setting(section='api', name='endpoints')
def api_endpoints():
return [Endpoint]

@App.permission_rule(model=Bunch, permission=object)
@App.permission_rule(model=Bunch, permission=object, identity=None)
def has_item_permission(app, identity, model, permission):
return getattr(model, 'hidden', False) is False


@fixture(scope='function')
def app(request):
Expand Down
3 changes: 2 additions & 1 deletion tests/onegov/api/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ def test_api_endpoint(app, endpoint_class):
# ... by_id
assert endpoint_class(app).by_id(1).id == 1
assert endpoint_class(app).by_id(2).id == 2
assert endpoint_class(app).by_id(3) is None
assert endpoint_class(app).by_id(3).id == 3
assert endpoint_class(app).by_id(4) is None

# .... item_data
assert endpoint_class(app).item_data(Bunch(title=1, a=2)) == {
Expand Down
101 changes: 99 additions & 2 deletions tests/onegov/api/test_views.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,26 @@
import transaction

from freezegun import freeze_time
from collection_json import Collection
from onegov.api.models import ApiKey
from onegov.user import UserCollection
from unittest.mock import patch
from uuid import uuid4


def test_view_api(client):

user = UserCollection(client.app.session()).add(
username='[email protected]', password='a', role='admin'
)
# create an access key with write access
uuid = uuid4()
key = ApiKey(
name='key', read_only=False, last_used=None, key=uuid, user=user
)
client.app.session().add(key)
transaction.commit()

# Collection
response = client.get('/api')
headers = response.headers
Expand Down Expand Up @@ -58,7 +75,12 @@ def test_view_api(client):
{'href': '6', 'rel': 'b'}
]
}
]
],
'template': {
'data': [
{'name': 'title', 'prompt': 'Title'},
],
}
}
}
assert len(Collection.from_json(response.body).links) == 2
Expand Down Expand Up @@ -87,11 +109,49 @@ def test_view_api(client):
{'href': '2', 'rel': 'b'}
]
},
]
],
'template': {
'data': [
{'name': 'title', 'prompt': 'Title'},
],
}
}
}
assert len(Collection.from_json(response.body).items) == 1

# Edit Item
with freeze_time('2020-02-02 20:20'):
# Without a token that has write permissions we can't change anything
response = client.put(
'/api/endpoint/1',
params='title=Changed',
status=401
)
headers = response.headers
assert headers['Content-Type'] == 'application/vnd.collection+json'
assert response.json == {
'collection': {
'version': '1.0',
'href': 'http://localhost/api/endpoint/1',
'error': {'message': 'Unauthorized'}
}
}
assert Collection.from_json(response.body).version == '1.0'

# Hidden Item
with freeze_time('2020-02-02 20:20'):
response = client.get('/api/endpoint/3', status=404)
headers = response.headers
assert headers['Content-Type'] == 'application/vnd.collection+json'
assert response.json == {
'collection': {
'version': '1.0',
'href': 'http://localhost/api/endpoint/3',
'error': {'message': 'Not Found'}
}
}
assert Collection.from_json(response.body).version == '1.0'

# Rate Limit
client.app.rate_limit = (2, 900)
with freeze_time('2020-02-02 20:20'):
Expand Down Expand Up @@ -139,3 +199,40 @@ def test_view_api(client):
}
}
assert Collection.from_json(response.body).version == '1.0'


# Authorize
headers = {"Authorization": f"Bearer {uuid}"}
response = client.get('/api/authenticate', headers=headers)
assert response.status_code == 200
resp = response.body.decode('utf-8')
assert resp.startswith('{"token":')

token = resp.split('"')[3]
headers = {"Authorization": f"Bearer {token}"}

# Edit Item
response = client.put(
'/api/endpoint/1',
params='title=Changed',
headers=headers
)
assert response.status_code == 200

# Edit Hidden Item
response = client.put(
'/api/endpoint/3',
params='title=Changed',
headers=headers,
status=404
)
headers = response.headers
assert headers['Content-Type'] == 'application/vnd.collection+json'
assert response.json == {
'collection': {
'version': '1.0',
'href': 'http://localhost/api/endpoint/3',
'error': {'message': 'Not Found'}
}
}
assert Collection.from_json(response.body).version == '1.0'

0 comments on commit 9e31787

Please sign in to comment.