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.

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

Create SecureHeadersMiddleware.py #507

Merged
merged 10 commits into from
Dec 22, 2018
Merged

Conversation

vaibhavmule
Copy link
Contributor

Closes #502

@vaibhavmule vaibhavmule requested a review from a team December 19, 2018 07:10
@coveralls
Copy link

coveralls commented Dec 19, 2018

Coverage Status

Coverage increased (+0.02%) to 89.555% when pulling 0838bb3 on vaibhavmule-secure-headers into f56944a on 2.1.

@josephmancuso
Copy link
Member

josephmancuso commented Dec 20, 2018

Also need to add tests. Since you made a branch from this repo I can add them tonight if I get around to it. Test really would be just passing in a request object, running the after method and then asserting some of the headers were set correctly.

@vaibhavmule
Copy link
Contributor Author

@josephmancuso I wasn't sure about branch that's why I choose 2.1

def after(self):
"""Run This Middleware After The Route Executes.
"""
from config import middleware
Copy link
Member

Choose a reason for hiding this comment

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

@vaibhavmule this is important to note. Since we are adding it in this file, we can't import it or we will get a circular import. so we pass it in this after method instead to lazy load it. Not doing this was causing the middleware import to fail

@josephmancuso josephmancuso changed the base branch from 2.1 to develop December 22, 2018 20:31
@josephmancuso josephmancuso merged commit 3dad354 into develop Dec 22, 2018
@josephmancuso josephmancuso deleted the vaibhavmule-secure-headers branch December 23, 2018 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants