Changeset View
Changeset View
Standalone View
Standalone View
swh/web/api/views/graph.py
| Show First 20 Lines • Show All 131 Lines • ▼ Show 20 Lines | |||||
| ) -> Union[Response, StreamingHttpResponse]: | ) -> Union[Response, StreamingHttpResponse]: | ||||
| if request.get_host() != SWH_WEB_INTERNAL_SERVER_NAME: | if request.get_host() != SWH_WEB_INTERNAL_SERVER_NAME: | ||||
| if not bool(request.user and request.user.is_authenticated): | if not bool(request.user and request.user.is_authenticated): | ||||
| return Response("Authentication credentials were not provided.", status=401) | return Response("Authentication credentials were not provided.", status=401) | ||||
| if not request.user.has_perm(API_GRAPH_PERM): | if not request.user.has_perm(API_GRAPH_PERM): | ||||
| return Response( | return Response( | ||||
| "You do not have permission to perform this action.", status=403 | "You do not have permission to perform this action.", status=403 | ||||
| ) | ) | ||||
| graph_query_url = get_config()["graph"]["server_url"] | |||||
| graph_config = get_config()["graph"] | |||||
| graph_query = unquote(graph_query) | graph_query = unquote(graph_query) | ||||
| graph_query_url = graph_config["server_url"] | |||||
| graph_query_url += graph_query | graph_query_url += graph_query | ||||
| parsed_url = urlparse(graph_query_url) | parsed_url = urlparse(graph_query_url) | ||||
| query_dict = QueryDict(parsed_url.query, mutable=True) | query_dict = QueryDict(parsed_url.query, mutable=True) | ||||
| query_dict.update(request.GET) | query_dict.update(request.GET) | ||||
| # clamp max_edges query parameter according to authentication | |||||
| if request.user.is_staff: | |||||
| max_edges = graph_config["max_edges"]["staff"] | |||||
| elif request.user.is_authenticated: | |||||
| max_edges = graph_config["max_edges"]["user"] | |||||
| else: | |||||
| max_edges = graph_config["max_edges"]["anonymous"] | |||||
| query_dict["max_edges"] = min( | |||||
| max_edges, int(query_dict.get("max_edges", max_edges + 1)) | |||||
seirl: I think this is potentially dangerous if an endpoint gets passed with a query parameter.
If… | |||||
Done Inline ActionsThis should not happen as the Web API URL will be parsed by django so graph_query will not contain any ? character and the query parameters will be available in request.GET. Nevertheless, we can have such issue if the ? got quoted to %3F in the Web API URL. I will handle that case in another diff. I also found out that I did not handle really well the error responses coming from the graph API, I will also handle that in a new diff. anlambert: This should not happen as the Web API URL will be parsed by django so `graph_query` will not… | |||||
| ) | |||||
| if query_dict: | if query_dict: | ||||
| graph_query_url = ( | graph_query_url = ( | ||||
| graph_query_url.split("?")[0] + "?" + query_dict.urlencode(safe="/;:") | graph_query_url.split("?")[0] + "?" + query_dict.urlencode(safe="/;:") | ||||
| ) | ) | ||||
| response = requests.get(graph_query_url, stream=True) | response = requests.get(graph_query_url, stream=True) | ||||
| if response.status_code != 200: | if response.status_code != 200: | ||||
| Show All 25 Lines | |||||
I think this is potentially dangerous if an endpoint gets passed with a query parameter.
If someone passes something like graph_query == "/graph/visit/nodes?prod=true" to the function, the resulting URL will look like https://graph.internal.softwareheritage.org/graph/visit/nodes?prod=true?max_edges=42 and the max_edges parameter will get silently ignored.
Instead, we could use something from urllib like this: https://stackoverflow.com/a/2506477 to ensure that the argument is properly added to the query (with a & if there is already a ?, or with a ? otherwise).