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

Jump to bottom

Add X-Frame-Options Middleware class. #1766

Merged
merged 2 commits into from
Jan 15, 2020
Merged

Add X-Frame-Options Middleware class. #1766

merged 2 commits into from
Jan 15, 2020

Conversation

WinnyTroy
Copy link
Contributor

@WinnyTroy WinnyTroy commented Jan 13, 2020 •
edited by ivermac
Loading

Set the x-frame-options header. Guide from django's documentation here
Fixes #1767

@WinnyTroy
Add X-Frame-Options Middleware class.
f274937 
Set default header to 'SAMEORIGIN'
ivermac
ivermac previously approved these changes Jan 14, 2020
View reviewed changes
Copy link
Contributor

@ivermac ivermac left a comment

Choose a reason for hiding this comment

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

Looks good to me! What do you think @ukanga

moshthepitt
moshthepitt reviewed Jan 14, 2020
View reviewed changes
onadata/settings/common.py Outdated
)

X_FRAME_OPTIONS = 'SAMEORIGIN'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we should make the default "DENY" as recommended in Django 3.0 - see https://docs.djangoproject.com/en/3.0/ref/clickjacking/

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair point

@WNjihia WNjihia added the QA+ PR passed QA testing label Jan 15, 2020
@WNjihia
Copy link

WNjihia commented Jan 15, 2020

Tested this and it checks out. /accounts/login doesn't load on the iframe
cc: @ivermac @WinnyTroy

@ivermac ivermac self-requested a review January 15, 2020 08:27
ivermac
ivermac approved these changes Jan 15, 2020
View reviewed changes
Copy link
Contributor

@ivermac ivermac left a comment

Choose a reason for hiding this comment

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

lgtm!

@DavisRayM DavisRayM merged commit e7ae49c into master Jan 15, 2020
@DavisRayM DavisRayM deleted the add_xframe_options_header branch January 15, 2020 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moshthepitt moshthepitt moshthepitt left review comments

@ivermac ivermac ivermac approved these changes

@ukanga ukanga Awaiting requested review from ukanga

Assignees
No one assigned
Labels
QA+ PR passed QA testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add X-Frame-Options Middleware class
5 participants
@WinnyTroy @WNjihia @moshthepitt @ivermac @DavisRayM