Skip to content
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

Update API export to allow input session id #178

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Update API export to allow input session id #178

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion src/pretalx/agenda/views/schedule.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ def get(self, request, *args, **kwargs):

exporter.schedule = self.schedule
if "-my" in exporter.identifier and self.request.user.id is None:
return HttpResponseRedirect(self.request.event.urls.login)
if request.GET.get('talks'):
Copy link
Member

@hongquan hongquan Aug 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Here is the original code:

if "-my" in exporter.identifier and self.request.user.id is None:
    return HttpResponseRedirect(self.request.event.urls.login)
favs_talks = SubmissionFavourite.objects.filter(user=self.request.user.id)

The author wanted to prevent SubmissionFavourite.objects.filter() from receiving None, so there is the check self.request.user.id is None, to force user logging-in.

But you insert your code in this if flow control and nullify the guard. After your code, return HttpResponseRedirect doesn't run and None strips to SubmissionFavourite.objects.filter.

Copy link
Contributor Author

@lcduong lcduong Aug 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @hongquan, this view is using for both talk and video component:

  1. From talk front end API call, 'talks' won't be put as request param, so if User not login, they will be forced to login page.
  2. From video API call, 'talks' is input as request params and from video, we not expecting User will be redirect to Talk component to login.

Idea for this changes is reuse the current API for Video schedule export, not impact flow at talk component

Copy link
Member

Choose a reason for hiding this comment

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

@lcduong Could you add your explanation as comment above that line?

exporter.talk_ids = request.GET.get('talks').split(',')
else:
return HttpResponseRedirect(self.request.event.urls.login)
Comment on lines +117 to +120
Copy link

Choose a reason for hiding this comment

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

🚨 issue (security): Security and logic flow concerns in authentication bypass

The new condition allows unauthenticated access to functionality previously requiring login. This could expose sensitive information or operations. Consider maintaining authentication and implementing a secure method for sharing specific talks. Additionally, this change increases the complexity of the logic flow, potentially making the code harder to maintain.

favs_talks = SubmissionFavourite.objects.filter(user=self.request.user.id)
if favs_talks.exists():
exporter.talk_ids = favs_talks[0].talk_list
Expand Down
Loading