Skip to content

New Security Review API #4925

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

Draft
wants to merge 3 commits into
base: security-reviews-new-fields
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions api/security_review_api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Copyright 2025 Google Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License")
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import requests

from chromestatus_openapi.models import (
CreateLaunchIssueRequest, SuccessMessage)

from framework import permissions
from framework import basehandlers, origin_trials_client
from internals.core_models import FeatureEntry
from internals.review_models import Gate

class SecurityReviewAPI(basehandlers.APIHandler):

def do_post(self, **kwargs):
"""Endpoint to create a new security review in IssueTracker.

Returns:
The issue ID if the security review issue was successfully created.
"""
body = CreateLaunchIssueRequest.from_dict(
self.get_json_param_dict())
# Check that feature ID is provided.
if body.feature_id is None:
self.abort(400, msg='No feature specified.')
if not isinstance(body.feature_id, int):
self.abort(400, msg='Invalid feature ID.')
feature: FeatureEntry | None = FeatureEntry.get_by_id(body.feature_id)
if feature is None:
self.abort(404, msg=f'Feature {body.feature_id} not found')
if feature.launch_issue_id is not None:
self.abort(400, msg='Feature already has a security review issue.')

# Validate the user has edit permissions and redirect if needed.
redirect_resp = permissions.validate_feature_edit_permission(
self, body.feature_id)
if redirect_resp:
self.abort(403, msg='User does not have permission to edit this feature.')

# Check that gate_id is provided.
if body.gate_id is None:
self.abort(400, msg='No gate specified.')
if not isinstance(body.gate_id, int):
self.abort(400, msg='Invalid gate ID.')
gate: Gate | None = Gate.get_by_id(body.gate_id)
if gate is None:
self.abort(404, msg=f'Gate {body.gate_id} not found')

try:
issue_id, failed_reason = (
origin_trials_client.create_launch_issue(
body.feature_id, body.gate_id, feature.continuity_id))
except requests.exceptions.RequestException:
self.abort(500, 'Error obtaining origin trial data from API')
except KeyError:
self.abort(500, 'Malformed response from origin trials API')

if issue_id is not None:
feature.launch_issue_id = issue_id
feature.put()
if failed_reason:
self.abort(500, failed_reason)
return SuccessMessage(message=f'Security review issue {issue_id} created')
160 changes: 160 additions & 0 deletions api/security_review_api_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
# Copyright 2025 Google Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License")
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from unittest import mock

import flask
import werkzeug.exceptions # Flask HTTP stuff.

import testing_config # Must be imported before the module under test.
from api import security_review_api
from internals.core_models import FeatureEntry, Stage
from internals.review_models import Gate, Vote

test_app = flask.Flask(__name__)


class SecurityReviewAPITest(testing_config.CustomTestCase):

def setUp(self):
self.feature_1 = FeatureEntry(
feature_type=1, name='feature one', summary='sum', category=1,
owner_emails=['[email protected]'], continuity_id=123)
self.feature_1.put()
self.feature_1_id = self.feature_1.key.integer_id()
self.ot_stage_1 = Stage(
feature_id=self.feature_1_id, stage_type=150,
origin_trial_id='-1234567890')
self.ot_stage_1.put()

self.ot_gate_1 = Gate(feature_id=self.feature_1_id,
stage_id=self.ot_stage_1.key.integer_id(),
gate_type=3, state=Vote.APPROVED)
self.ot_gate_1.put()
self.request_path = (
f'/api/v0/{self.feature_1_id}/{self.ot_stage_1.key.integer_id()}'
'/create-security-review-issue')

self.handler = security_review_api.SecurityReviewAPI()

def test_do_post__feature_id_missing(self):
"""Give a 400 if the feature ID is not provided."""
testing_config.sign_in('[email protected]', 1234567890)
json_body = {'gate_id': self.ot_gate_1.key.integer_id()}
with test_app.test_request_context(
self.request_path, method='POST', json=json_body):
with self.assertRaises(werkzeug.exceptions.BadRequest):
self.handler.do_post()

def test_do_post__feature_not_found(self):
"""Give a 404 if the no such feature exists."""
testing_config.sign_in('[email protected]', 1234567890)
json_body = {'feature_id': 999, 'gate_id': self.ot_gate_1.key.integer_id()}
with test_app.test_request_context(
self.request_path, method='POST', json=json_body):
with self.assertRaises(werkzeug.exceptions.NotFound):
self.handler.do_post()

def test_do_post__gate_id_missing(self):
"""Give a 400 if the no such stage exists."""
testing_config.sign_in('[email protected]', 1234567890)
json_body = {'feature_id': self.feature_1_id}
with test_app.test_request_context(
self.request_path, method='POST', json=json_body):
with self.assertRaises(werkzeug.exceptions.BadRequest):
self.handler.do_post()

def test_do_post__gate_not_found(self):
testing_config.sign_in('[email protected]', 1234567890)
"""Give a 404 if the no such stage exists."""
json_body = {'feature_id': self.feature_1_id, 'gate_id': 999}
with test_app.test_request_context(
self.request_path, method='POST', json=json_body):
with self.assertRaises(werkzeug.exceptions.NotFound):
self.handler.do_post()

@mock.patch('framework.origin_trials_client.create_launch_issue')
def test_do_post__no_continuity_id(self, mock_create):
"""Request is still processed if no continuity ID exists."""
testing_config.sign_in('[email protected]', 1234567890)
mock_create.return_value = (456, None)

json_body = {
'feature_id': self.feature_1_id,
'gate_id': self.ot_gate_1.key.integer_id()}
with test_app.test_request_context(
self.request_path, method='POST', json=json_body):
self.handler.do_post()
mock_create.assert_called_once()
self.assertEqual(456, self.feature_1.launch_issue_id)

def test_do_post__anon(self):
"""Anon users cannot request origin trials."""
testing_config.sign_out()
json_body = {
'feature_id': self.feature_1_id,
'gate_id': self.ot_gate_1.key.integer_id()}
with test_app.test_request_context(
self.request_path, method='POST', json=json_body):
with self.assertRaises(werkzeug.exceptions.Forbidden):
self.handler.do_post()

@mock.patch('framework.origin_trials_client.create_launch_issue')
def test_do_post__valid(self, mock_create):
"""A valid request is processed successfully."""
testing_config.sign_in('[email protected]', 1234567890)
mock_create.return_value = (456, None)

json_body = {
'feature_id': self.feature_1_id,
'gate_id': self.ot_gate_1.key.integer_id()}
with test_app.test_request_context(
self.request_path, method='POST', json=json_body):
self.handler.do_post()
mock_create.assert_called_once()
self.assertEqual(123, self.feature_1.continuity_id)
self.assertEqual(456, self.feature_1.launch_issue_id)

@mock.patch('framework.origin_trials_client.create_launch_issue')
def test_do_post__failed(self, mock_create):
"""A RequestException from the OT API is handled correctly."""
testing_config.sign_in('[email protected]', 1234567890)
mock_create.side_effect = werkzeug.exceptions.InternalServerError

json_body = {
'feature_id': self.feature_1_id,
'gate_id': self.ot_gate_1.key.integer_id()}
with test_app.test_request_context(
self.request_path, method='POST', json=json_body):
with self.assertRaises(werkzeug.exceptions.InternalServerError):
self.handler.do_post()
mock_create.assert_called_once()
self.assertEqual(None, self.feature_1.launch_issue_id)

@mock.patch('framework.origin_trials_client.create_launch_issue')
def test_do_patch__failed_with_reason(self, mock_create):
"""A response from the OT API containing a failed_reason is handled."""
testing_config.sign_in('[email protected]', 1234567890)
mock_create.return_value = (456, 'Failed to create issue.')

json_body = {
'feature_id': self.feature_1_id,
'gate_id': self.ot_gate_1.key.integer_id()}
with test_app.test_request_context(
self.request_path, method='POST', json=json_body):
with self.assertRaises(werkzeug.exceptions.InternalServerError):
self.handler.do_post()
mock_create.assert_called_once()
# launch_issue_id should be set if one is returned, regardless of failure.
self.assertEqual(456, self.feature_1.launch_issue_id)
40 changes: 40 additions & 0 deletions framework/origin_trials_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,3 +371,43 @@ def extend_origin_trial(trial_id: str, end_milestone: int, intent_url: str):
except requests.exceptions.RequestException as e:
logging.exception('Failed to get response from origin trials API.')
raise e


def create_launch_issue(
feature_id: int, gate_id: int, continuity_id: int | None=None):
"""Send a request to create a new security review in IssueTracker.

Raises:
requests.exceptions.RequestException: If the request fails to connect or
the HTTP status code is not successful.
"""
if settings.DEV_MODE:
logging.info('Creation request will not be sent to origin trials API '
'in local environment.')
return
key = secrets.get_ot_api_key()
# Return if no API key is found.
if key is None:
return

access_token = _get_ot_access_token()
url = (f'{settings.OT_API_URL}/v1/security-review-issues:create')
headers = {'Authorization': f'Bearer {access_token}'}
json = {
'feature_id': feature_id,
'gate_id': gate_id,
}

if continuity_id:
json['continuity_id'] = continuity_id

try:
response = requests.post(
url, headers=headers, params={'key': key}, json=json)
logging.info(response.text)
response.raise_for_status()
except requests.exceptions.RequestException as e:
logging.exception('Failed to get response from origin trials API.')
raise e
response_json = response.json()
return response_json.get('issue_id'), response_json.get('failed_reason')
37 changes: 36 additions & 1 deletion framework/origin_trials_client_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ def test_activate_origin_trial__no_api_key(
self, mock_requests_post, mock_api_key_get):
"""If no API key is available, do not send activation request."""
mock_api_key_get.return_value = None
origin_trials_client.create_origin_trial(self.ot_stage)
origin_trials_client.activate_origin_trial(self.ot_stage)

mock_api_key_get.assert_called_once()
# POST request should not be executed with no API key.
Expand Down Expand Up @@ -360,3 +360,38 @@ def test_activate_origin_trial__with_api_key(
params={'key': 'api_key_value'},
json={'trial_id': '-1234567890'}
)

@mock.patch('framework.secrets.get_ot_api_key')
@mock.patch('requests.post')
def test_create_launch_issue__no_api_key(
self, mock_requests_post, mock_api_key_get):
"""If no API key is available, do not send request."""
mock_api_key_get.return_value = None
origin_trials_client.create_launch_issue(123, 456)

mock_api_key_get.assert_called_once()
# POST request should not be executed with no API key.
mock_requests_post.assert_not_called()

@mock.patch('framework.secrets.get_ot_api_key')
@mock.patch('framework.origin_trials_client._get_ot_access_token')
@mock.patch('requests.post')
def test_create_launch_issue__with_api_key(
self, mock_requests_post, mock_get_ot_access_token, mock_api_key_get):
"""If an API key is available, request should be sent."""
mock_requests_post.return_value = mock.MagicMock(
status_code=200, json=lambda : {
'issue_id': 789})
mock_get_ot_access_token.return_value = 'access_token'
mock_api_key_get.return_value = 'api_key_value'

origin_trials_client.create_launch_issue(123, 456, 789)

mock_api_key_get.assert_called_once()
mock_get_ot_access_token.assert_called_once()
mock_requests_post.assert_called_once_with(
f'{settings.OT_API_URL}/v1/security-review-issues:create',
headers={'Authorization': 'Bearer access_token'},
params={'key': 'api_key_value'},
json={'feature_id': 123, 'gate_id': 456, 'continuity_id': 789}
)
1 change: 1 addition & 0 deletions gen/js/chromestatus-openapi/.openapi-generator/FILES
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ src/models/ComponentsUser.ts
src/models/ComponentsUsersResponse.ts
src/models/CounterEntry.ts
src/models/CreateAccountRequest.ts
src/models/CreateLaunchIssueRequest.ts
src/models/CreateOriginTrialRequest.ts
src/models/DeleteAccount200Response.ts
src/models/DismissCueRequest.ts
Expand Down
Loading