-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Allow all /api/v2/ CORS if the Domain is known #4880
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ | |
from django.db.models import Q, Count | ||
from django.dispatch import receiver | ||
from future.backports.urllib.parse import urlparse | ||
from rest_framework.permissions import SAFE_METHODS | ||
|
||
from readthedocs.oauth.models import RemoteOrganization | ||
from readthedocs.projects.models import Project, Domain | ||
|
@@ -49,6 +50,12 @@ def decide_if_cors(sender, request, **kwargs): # pylint: disable=unused-argumen | |
if request.path_info.startswith('/api/v2/sustainability'): | ||
return True | ||
|
||
# Don't do domain checking for APIv2 when the Domain is known | ||
if request.path_info.startswith('/api/v2/') and request.method in SAFE_METHODS: | ||
domain = Domain.objects.filter(domain__icontains=host) | ||
if domain.exists(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm -0 on allowing all API calls to all projects from all Domain's. We should still be mapping the domain -> project, so I think we can just add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you connect the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing we can do here is just allow safe methods for |
||
return True | ||
|
||
valid_url = False | ||
for url in WHITELIST_URLS: | ||
if request.path_info.startswith(url): | ||
|
@@ -69,7 +76,7 @@ def decide_if_cors(sender, request, **kwargs): # pylint: disable=unused-argumen | |
|
||
domain = Domain.objects.filter( | ||
Q(domain__icontains=host), | ||
Q(project=project) | Q(project__subprojects__child=project) | ||
Q(project=project) | Q(project__subprojects__child=project), | ||
) | ||
if domain.exists(): | ||
return True | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to allow to access the API no matter which domain the request comes from?