diff --git a/api/comments_api.py b/api/comments_api.py index 8c38204ba041..0b6ba774fccd 100644 --- a/api/comments_api.py +++ b/api/comments_api.py @@ -91,7 +91,11 @@ def do_post(self, **kwargs) -> dict[str, str]: user = self.get_current_user(required=True) comment_request = CommentsRequest.from_dict(self.request.json) + if not comment_request: + self.abort(400, msg='Invalid request') comment_content = comment_request.comment + if not comment_content: + self.abort(400, msg='Comment content is required') post_to_thread_type = comment_request.post_to_thread_type if comment_content: @@ -128,6 +132,8 @@ def do_post(self, **kwargs) -> dict[str, str]: def do_patch(self, **kwargs) -> dict[str, str]: patch_request = PatchCommentRequest.from_dict(self.request.json) + if not patch_request: + self.abort(400, msg='Invalid request') comment: Activity = Activity.get_by_id(patch_request.comment_id) user = self.get_current_user(required=True) diff --git a/api/component_users.py b/api/component_users.py index 5ff4c0b44181..14058edfcd91 100644 --- a/api/component_users.py +++ b/api/component_users.py @@ -50,6 +50,8 @@ def do_get(self, **kwargs): @permissions.require_admin_site def do_put(self, **kwargs) -> tuple[dict, int]: component_users_request = ComponentUsersRequest.from_dict(self.request.get_json(force=True)) + if not component_users_request: + self.abort(400, 'Invalid request') self.__update_subscribers_list(True, user_id=kwargs.get('user_id', None), blink_component_id=kwargs.get('component_id', None), primary=component_users_request.owner) @@ -59,6 +61,8 @@ def do_put(self, **kwargs) -> tuple[dict, int]: def do_delete(self, **kwargs) -> tuple[dict, int]: params = self.request.get_json(force=True) component_users_request = ComponentUsersRequest.from_dict(params) + if not component_users_request: + self.abort(400, 'Invalid request') self.__update_subscribers_list(False, user_id=kwargs.get('user_id', None), blink_component_id=kwargs.get('component_id', None), primary=component_users_request.owner) diff --git a/api/login_api.py b/api/login_api.py index f1cafe34c30b..40534699fcd5 100644 --- a/api/login_api.py +++ b/api/login_api.py @@ -30,6 +30,7 @@ def do_get(self, **kwargs): self.abort(405, valid_methods=['POST']) def do_post(self, **kwargs): + token = None try: request = SignInRequest.from_dict(self.request.json) token = request.credential @@ -37,7 +38,7 @@ def do_post(self, **kwargs): raise werkzeug.exceptions.BadRequest(description="Missing required field 'credential'") message = "Unable to Authenticate. Please sign in again." except ValueError: - message = "Invalid Request" + raise werkzeug.exceptions.BadRequest(description="Invalid Request") # Properly raise error try: idinfo = google.oauth2.id_token.verify_oauth2_token( diff --git a/api/permissions_api_test.py b/api/permissions_api_test.py index f77b6ef67f62..e069023aed0f 100644 --- a/api/permissions_api_test.py +++ b/api/permissions_api_test.py @@ -36,7 +36,7 @@ def test_get__anon(self): testing_config.sign_out() with test_app.test_request_context(self.request_path): actual = self.handler.do_get() - self.assertEqual({'user': None}, actual) + self.assertEqual({}, actual) def test_get__non_googler(self): """Non-googlers have no permissions by default""" diff --git a/api/review_latency_api.py b/api/review_latency_api.py index 516ee479fc2b..52360f3e6de3 100644 --- a/api/review_latency_api.py +++ b/api/review_latency_api.py @@ -15,12 +15,12 @@ import collections import logging from datetime import datetime, timedelta -from typing import Iterable, Any -from google.cloud import ndb # type: ignore +from typing import Any, Iterable from chromestatus_openapi.models.feature_link import FeatureLink from chromestatus_openapi.models.gate_latency import GateLatency from chromestatus_openapi.models.review_latency import ReviewLatency +from google.cloud import ndb # type: ignore from framework import basehandlers from internals import slo @@ -28,7 +28,6 @@ from internals.core_models import FeatureEntry from internals.review_models import Gate - DEFAULT_RECENT_DAYS = 90 # This means that the feature team has not yet requested this review. NOT_STARTED_LATENCY = -1 @@ -132,9 +131,10 @@ def convert_to_result_format( result: list[dict[str, Any]] = [] for fe in sorted_features: latencies = latencies_by_fid[fe.key.integer_id()] - review_latency = ReviewLatency( - FeatureLink(fe.key.integer_id(), fe.name), - [GateLatency(gate_type, days) + review_latency = ReviewLatency.model_construct(feature= + FeatureLink.model_construct(id=fe.key.integer_id(), name=fe.name), + gate_reviews= + [GateLatency.model_construct(gate_type=gate_type, latency_days=days) for (gate_type, days) in latencies] ) result.append(review_latency.to_dict()) diff --git a/api/reviews_api.py b/api/reviews_api.py index d6571d4a94bf..e032dd57119c 100644 --- a/api/reviews_api.py +++ b/api/reviews_api.py @@ -57,7 +57,11 @@ def do_get(self, **kwargs) -> dict[str, list[dict[str, Any]]]: # Note: We assume that anyone may view approvals. votes = Vote.get_votes(feature_id=feature_id, gate_id=gate_id) dicts = [converters.vote_value_to_json_dict(v) for v in votes] - return GetVotesResponse.from_dict({'votes': dicts}).to_dict() + response = GetVotesResponse.from_dict({'votes': dicts}) + if not response: + logging.error(f"unable to convert response from get_votes to GetVotesResponse dicts={dicts}") + return self.abort(500) + return response.to_dict() def do_post(self, **kwargs) -> dict[str, str]: """Set a user's vote value for the specified feature and gate.""" @@ -146,16 +150,24 @@ def do_get(self, **kwargs) -> dict[str, Any]: approvers = approval_defs.get_approvers(g['gate_type']) g['possible_assignee_emails'] = approvers - return GetGateResponse.from_dict({ + response = GetGateResponse.from_dict({ 'gates': dicts, - }).to_dict() + }) + if not response: + logging.error(f"unable to convert response from get_votes to GetGateResponse dicts={dicts}") + return self.abort(500) + return response.to_dict() def do_post(self, **kwargs) -> dict[str, str]: """Set the assignees for a gate.""" user, fe, gate, feature_id, gate_id = get_user_feature_and_gate( self, kwargs) request = PostGateRequest.from_dict(self.request.json) + if not request: + self.abort(400, msg='Invalid request') assignees = request.assignees + if not assignees: + self.abort(400, msg='No assignees provided') self.require_permissions(user, fe, gate) self.validate_assignees(assignees, fe, gate) diff --git a/api/spec_mentors_api.py b/api/spec_mentors_api.py index ba584bf448ec..0cdee45004f4 100644 --- a/api/spec_mentors_api.py +++ b/api/spec_mentors_api.py @@ -58,6 +58,6 @@ def do_get(self, **kwargs): []).append(FeatureLink(id=feature.key.integer_id(), name=feature.name)) return [ - SpecMentor(email, features).to_dict() + SpecMentor.model_construct(email=email, mentored_features=features).to_dict() for email, features in sorted(mentors.items()) ] diff --git a/client-src/elements/chromedash-admin-blink-page_test.ts b/client-src/elements/chromedash-admin-blink-page_test.ts index 7da841a8d6ee..7b82da4f5126 100644 --- a/client-src/elements/chromedash-admin-blink-page_test.ts +++ b/client-src/elements/chromedash-admin-blink-page_test.ts @@ -36,7 +36,7 @@ describe('chromedash-admin-blink-page', () => { const response: ComponentsUsersResponse = { users: [{id: 0, name: 'user0', email: 'user0@example.com'}], components: [ - {id: '0', name: 'component0', subscriber_ids: [0], owner_ids: [0]}, + {id: 0, name: 'component0', subscriber_ids: [0], owner_ids: [0]}, ], }; csOpenApiClientStub = sinon.createStubInstance(DefaultApi); diff --git a/framework/basehandlers.py b/framework/basehandlers.py index 88a914be35b8..bf7a3f574006 100644 --- a/framework/basehandlers.py +++ b/framework/basehandlers.py @@ -49,7 +49,6 @@ from flask import session from flask import render_template from flask_cors import CORS -from gen.py.chromestatus_openapi.chromestatus_openapi.models.base_model import Model # Our API responses are prefixed with this ro prevent attacks that # exploit