Skip to content

fix: Check user permission before exporting #6581

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Nov 27, 2019
Merged

fix: Check user permission before exporting #6581

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3dccf6a
Check proper permissions before exporting an event
prateekj117 Nov 12, 2019
56d0c94
Correct CI errors
prateekj117 Nov 12, 2019
ae10e8a
Simplify logic
prateekj117 Nov 12, 2019
9d3a0f7
Merge branch 'development' into event-export-permissions
prateekj117 Nov 18, 2019
9c7112e
Add a separate decorator for getting event id.
prateekj117 Nov 18, 2019
5f3ab66
Required changes.
prateekj117 Nov 21, 2019
dc326f9
Required changes.
prateekj117 Nov 26, 2019
81c6553
Required changes
prateekj117 Nov 26, 2019
b7b0676
Required changes.
prateekj117 Nov 26, 2019
ba24a32
Required changes
prateekj117 Nov 27, 2019
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
174 changes: 62 additions & 112 deletions app/api/exports.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@

from flask import send_file, make_response, jsonify, url_for, \
current_app, request, Blueprint
from flask_jwt_extended import jwt_required, current_user
from flask_jwt_extended import current_user

from app.api.helpers.export_helpers import export_event_json, create_export_job
from app.api.helpers.permissions import is_coorganizer, get_event_id
from app.api.helpers.utilities import TASK_RESULTS
from app.models import db
from app.models.event import Event

export_routes = Blueprint('exports', __name__, url_prefix='/v1')

Expand All @@ -19,9 +18,10 @@
}


@export_routes.route('/events/<string:event_identifier>/export/json', methods=['POST'])
@jwt_required
def export_event(event_identifier):
@export_routes.route('/events/<string:event_identifier>/export/json', methods=['POST'], endpoint='export_event')
@get_event_id
@is_coorganizer
def export_event(event_id):
from .helpers.tasks import export_event_task

settings = EXPORT_SETTING
Expand All @@ -30,11 +30,6 @@ def export_event(event_identifier):
settings['document'] = request.json.get('document', False)
settings['audio'] = request.json.get('audio', False)

if not event_identifier.isdigit():
event = db.session.query(Event).filter_by(identifier=event_identifier).first()
event_id = event.id
else:
event_id = event_identifier
# queue task
task = export_event_task.delay(
current_user.email, event_id, settings)
Expand All @@ -53,8 +48,9 @@ def export_event(event_identifier):
)


@export_routes.route('/events/<string:event_id>/exports/<path:path>')
@jwt_required
@export_routes.route('/events/<string:event_id>/exports/<path:path>', endpoint='export_download')
@get_event_id
@is_coorganizer
def export_download(event_id, path):
if not path.startswith('/'):
path = '/' + path
Expand All @@ -65,15 +61,10 @@ def export_download(event_id, path):
return response


@export_routes.route('/events/<string:event_identifier>/export/xcal', methods=['GET'])
@jwt_required
def export_event_xcal(event_identifier):

if not event_identifier.isdigit():
event = db.session.query(Event).filter_by(identifier=event_identifier).first()
event_id = str(event.id)
else:
event_id = event_identifier
@export_routes.route('/events/<string:event_identifier>/export/xcal', methods=['GET'], endpoint='export_event_xcal')
@get_event_id
@is_coorganizer
def export_event_xcal(event_id):

from .helpers.tasks import export_xcal_task

Expand All @@ -94,15 +85,10 @@ def event_export_task_base(event_id, settings):
return path


@export_routes.route('/events/<string:event_identifier>/export/ical', methods=['GET'])
@jwt_required
def export_event_ical(event_identifier):
if not event_identifier.isdigit():
event = db.session.query(Event).filter_by(identifier=event_identifier).first()
event_id = str(event.id)
else:
event_id = event_identifier

@export_routes.route('/events/<string:event_identifier>/export/ical', methods=['GET'], endpoint='export_event_ical')
@get_event_id
@is_coorganizer
def export_event_ical(event_id):
from .helpers.tasks import export_ical_task

task = export_ical_task.delay(event_id)
Expand All @@ -114,15 +100,11 @@ def export_event_ical(event_identifier):
)


@export_routes.route('/events/<string:event_identifier>/export/pentabarf', methods=['GET'])
@jwt_required
def export_event_pentabarf(event_identifier):
if not event_identifier.isdigit():
event = db.session.query(Event).filter_by(identifier=event_identifier).first()
event_id = str(event.id)
else:
event_id = event_identifier

@export_routes.route('/events/<string:event_identifier>/export/pentabarf', methods=['GET'],
endpoint='export_event_pentabarf')
@get_event_id
@is_coorganizer
def export_event_pentabarf(event_id):
from .helpers.tasks import export_pentabarf_task

task = export_pentabarf_task.delay(event_id)
Expand All @@ -134,15 +116,11 @@ def export_event_pentabarf(event_identifier):
)


@export_routes.route('/events/<string:event_identifier>/export/orders/csv', methods=['GET'])
@jwt_required
def export_orders_csv(event_identifier):
if not event_identifier.isdigit():
event = db.session.query(Event).filter_by(identifier=event_identifier).first()
event_id = str(event.id)
else:
event_id = event_identifier

@export_routes.route('/events/<string:event_identifier>/export/orders/csv', methods=['GET'],
endpoint='export_orders_csv')
@get_event_id
@is_coorganizer
def export_orders_csv(event_id):
from .helpers.tasks import export_order_csv_task

task = export_order_csv_task.delay(event_id)
Expand All @@ -154,15 +132,11 @@ def export_orders_csv(event_identifier):
)


@export_routes.route('/events/<string:event_identifier>/export/orders/pdf', methods=['GET'])
@jwt_required
def export_orders_pdf(event_identifier):
if not event_identifier.isdigit():
event = db.session.query(Event).filter_by(identifier=event_identifier).first()
event_id = str(event.id)
else:
event_id = event_identifier

@export_routes.route('/events/<string:event_identifier>/export/orders/pdf', methods=['GET'],
endpoint='export_orders_pdf')
@get_event_id
@is_coorganizer
def export_orders_pdf(event_id):
from .helpers.tasks import export_order_pdf_task

task = export_order_pdf_task.delay(event_id)
Expand All @@ -174,15 +148,11 @@ def export_orders_pdf(event_identifier):
)


@export_routes.route('/events/<string:event_identifier>/export/attendees/csv', methods=['GET'])
@jwt_required
def export_attendees_csv(event_identifier):
if not event_identifier.isdigit():
event = db.session.query(Event).filter_by(identifier=event_identifier).first()
event_id = str(event.id)
else:
event_id = event_identifier

@export_routes.route('/events/<string:event_identifier>/export/attendees/csv', methods=['GET'],
endpoint='export_attendees_csv')
@get_event_id
@is_coorganizer
def export_attendees_csv(event_id):
from .helpers.tasks import export_attendees_csv_task

task = export_attendees_csv_task.delay(event_id)
Expand All @@ -194,15 +164,11 @@ def export_attendees_csv(event_identifier):
)


@export_routes.route('/events/<string:event_identifier>/export/attendees/pdf', methods=['GET'])
@jwt_required
def export_attendees_pdf(event_identifier):
if not event_identifier.isdigit():
event = db.session.query(Event).filter_by(identifier=event_identifier).first()
event_id = str(event.id)
else:
event_id = event_identifier

@export_routes.route('/events/<string:event_identifier>/export/attendees/pdf', methods=['GET'],
endpoint='export_attendees_pdf')
@get_event_id
@is_coorganizer
def export_attendees_pdf(event_id):
from .helpers.tasks import export_attendees_pdf_task

task = export_attendees_pdf_task.delay(event_id)
Expand All @@ -214,15 +180,11 @@ def export_attendees_pdf(event_identifier):
)


@export_routes.route('/events/<string:event_identifier>/export/sessions/csv', methods=['GET'])
@jwt_required
def export_sessions_csv(event_identifier):
if not event_identifier.isdigit():
event = db.session.query(Event).filter_by(identifier=event_identifier).first()
event_id = str(event.id)
else:
event_id = event_identifier

@export_routes.route('/events/<string:event_identifier>/export/sessions/csv', methods=['GET'],
endpoint='export_sessions_csv')
@get_event_id
@is_coorganizer
def export_sessions_csv(event_id):
from .helpers.tasks import export_sessions_csv_task

task = export_sessions_csv_task.delay(event_id)
Expand All @@ -234,15 +196,11 @@ def export_sessions_csv(event_identifier):
)


@export_routes.route('/events/<string:event_identifier>/export/speakers/csv', methods=['GET'])
@jwt_required
def export_speakers_csv(event_identifier):
if not event_identifier.isdigit():
event = db.session.query(Event).filter_by(identifier=event_identifier).first()
event_id = str(event.id)
else:
event_id = event_identifier

@export_routes.route('/events/<string:event_identifier>/export/speakers/csv', methods=['GET'],
endpoint='export_speakers_csv')
@get_event_id
@is_coorganizer
def export_speakers_csv(event_id):
from .helpers.tasks import export_speakers_csv_task

task = export_speakers_csv_task.delay(event_id)
Expand All @@ -254,15 +212,11 @@ def export_speakers_csv(event_identifier):
)


@export_routes.route('/events/<string:event_identifier>/export/sessions/pdf', methods=['GET'])
@jwt_required
def export_sessions_pdf(event_identifier):
if not event_identifier.isdigit():
event = db.session.query(Event).filter_by(identifier=event_identifier).first()
event_id = str(event.id)
else:
event_id = event_identifier

@export_routes.route('/events/<string:event_identifier>/export/sessions/pdf', methods=['GET'],
endpoint='export_sessions_pdf')
@get_event_id
@is_coorganizer
def export_sessions_pdf(event_id):
from .helpers.tasks import export_sessions_pdf_task

task = export_sessions_pdf_task.delay(event_id)
Expand All @@ -274,15 +228,11 @@ def export_sessions_pdf(event_identifier):
)


@export_routes.route('/events/<string:event_identifier>/export/speakers/pdf', methods=['GET'])
@jwt_required
def export_speakers_pdf(event_identifier):
if not event_identifier.isdigit():
event = db.session.query(Event).filter_by(identifier=event_identifier).first()
event_id = str(event.id)
else:
event_id = event_identifier

@export_routes.route('/events/<string:event_identifier>/export/speakers/pdf', methods=['GET'],
endpoint='export_speakers_pdf')
@get_event_id
@is_coorganizer
def export_speakers_pdf(event_id):
from .helpers.tasks import export_speakers_pdf_task

task = export_speakers_pdf_task.delay(event_id)
Expand Down
34 changes: 30 additions & 4 deletions app/api/helpers/permissions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
from functools import wraps
from flask import current_app as app
from flask_jwt_extended import verify_jwt_in_request, current_user

from app.api.helpers.db import save_to_db
from app.api.helpers.errors import ForbiddenError
from flask import request
from datetime import datetime
from app.models import db
from app.models.event import Event


def second_order_decorator(inner_dec):
Expand Down Expand Up @@ -145,6 +146,30 @@ def decorated_function(*args, **kwargs):
return decorated_function


@second_order_decorator(jwt_required)
def to_event_id(func):
"""
Get event id from event identifier.
:param f:
:return:
"""

@wraps(f)
Copy link

Choose a reason for hiding this comment

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

undefined name 'f'

def decorated_function(*args, **kwargs):

if 'event_identifier' in kwargs:
if not kwargs['event_identifier'].isdigit():
event = db.session.query(Event).filter_by(identifier=kwargs['event_identifier']).first()
kwargs['event_id'] = event.id
else:
kwargs['event_id'] = kwargs['event_identifier']

return f(*args, **kwargs)
Copy link

Choose a reason for hiding this comment

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

undefined name 'f'


return decorated_function



@second_order_decorator(jwt_required)
def is_coorganizer(f):
"""
Expand All @@ -157,10 +182,11 @@ def is_coorganizer(f):
def decorated_function(*args, **kwargs):
user = current_user

if user.is_staff:
return f(*args, **kwargs)
if 'event_id' in kwargs and user.has_event_access(kwargs['event_id']):
if user.is_staff or ('event_id' in kwargs and user.has_event_access(kwargs['event_id'])):
if 'event_identifier' in kwargs:
kwargs.pop('event_identifier', None)
Copy link
Member

Choose a reason for hiding this comment

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

Why's this needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because then we will also have to pass an additional parameter of event_identifier in the function after using this decorator which is not used anywhere.

Copy link
Member

Choose a reason for hiding this comment

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

But this can break a function where they actually are passing event_identifier

Copy link
Member Author

Choose a reason for hiding this comment

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

@iamareebjamal Already considered this point. Check the implementation of the older is_coorganizer function, it always used to check using event_id only.

Copy link
Member

Choose a reason for hiding this comment

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

That's not what I'm talking about. Consider a function where event_identifier is being received as a parameter and this decorator is applied there

Copy link
Member Author

Choose a reason for hiding this comment

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

@iamareebjamal Considered this as well. It isn't the case anywhere right now.

Copy link
Member

Choose a reason for hiding this comment

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

Right now isn't good enough, try to create a method with this decorator and see if it fails

Copy link
Member Author

Choose a reason for hiding this comment

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

@iamareebjamal It will fail in case if the function wants event_identifier.

return f(*args, **kwargs)

return ForbiddenError({'source': ''}, 'Co-organizer access is required.').respond()

return decorated_function
Expand Down