Skip to content

Commit

Permalink
Revert "Milestone 2 of ML pipeline migration (oppia#10060)" (oppia#10578
Browse files Browse the repository at this point in the history
)

This reverts commit 4c8d60e.
  • Loading branch information
Kevin Thomas authored Sep 7, 2020
1 parent 628f66a commit 5d0095c
Show file tree
Hide file tree
Showing 47 changed files with 871 additions and 1,645 deletions.
1 change: 0 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ jobs:
- persist_to_workspace:
root: /home/circleci/
paths:
- oppia/core/domain/proto/
- oppia/node_modules/
- oppia/third_party/
- oppia_tools/
Expand Down
2 changes: 0 additions & 2 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,9 @@
# Answer classification team.
/core/controllers/classifier*.py @prasanna08
/core/domain/classifier*.py @prasanna08
/core/domain/proto/ @prasanna08
/core/storage/classifier/ @prasanna08
/core/templates/domain/classifier/ @prasanna08
/extensions/classifiers/ @prasanna08
/prototool.yaml @prasanna08


# Audio and Translation team.
Expand Down
2 changes: 0 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
*.swo
*.bak
gae_runtime/*
# Ignore the auto-generated protobuf python code.
core/domain/proto/*.py
# We have added .git over here such that "git check-ignore" command should
# ignore the files and dirs inside the .git dir.
.git/*
Expand Down
6 changes: 0 additions & 6 deletions appengine_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,6 @@
raise Exception('Invalid path for oppia_tools library: %s' % PIL_PATH)
sys.path.insert(0, PIL_PATH)

PROTOBUF_PATH = os.path.join(OPPIA_TOOLS_PATH, 'protobuf-3.12.0')
if not os.path.isdir(PROTOBUF_PATH):
raise Exception('Invalid path for oppia_tools library: %s' % (
PROTOBUF_PATH))
sys.path.insert(0, PROTOBUF_PATH)

THIRD_PARTY_LIBS = [
os.path.join(
ROOT_PATH, 'third_party', 'backports.functools_lru_cache-1.6.1'),
Expand Down
40 changes: 0 additions & 40 deletions core/controllers/acl_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

from constants import constants
from core.controllers import base
from core.domain import classifier_services
from core.domain import classroom_services
from core.domain import feedback_services
from core.domain import question_services
Expand Down Expand Up @@ -2851,42 +2850,3 @@ def test_can_play_entity(self, entity_type, entity_id, **kwargs):
test_can_play_entity.__wrapped__ = True

return test_can_play_entity


def is_from_oppia_ml(handler):
"""Decorator to check whether the incoming request is from a valid Oppia-ML
VM instance.
Args:
handler: function. The function to be decorated.
Returns:
function. The newly decorated function that now can check if incoming
request is from a valid VM instance.
"""
def test_request_originates_from_valid_oppia_ml_instance(self, **kwargs):
"""Checks if the incoming request is from a valid Oppia-ML VM
instance.
Args:
**kwargs: *. Keyword arguments.
Returns:
*. The return value of the decorated function.
Raises:
UnauthorizedUserException. If incoming request is not from a valid
Oppia-ML VM instance.
"""
message, vm_id, signature = (
self.extract_request_message_vm_id_and_signature())
if vm_id == feconf.DEFAULT_VM_ID and not constants.DEV_MODE:
raise self.UnauthorizedUserException
if not classifier_services.verify_signature(message, vm_id, signature):
raise self.UnauthorizedUserException

return handler(self, **kwargs)

test_request_originates_from_valid_oppia_ml_instance.__wrapped__ = True

return test_request_originates_from_valid_oppia_ml_instance
91 changes: 0 additions & 91 deletions core/controllers/acl_decorators_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,8 @@
from __future__ import absolute_import # pylint: disable=import-only-modules
from __future__ import unicode_literals # pylint: disable=import-only-modules

import json

from constants import constants
from core.controllers import acl_decorators
from core.controllers import base
from core.domain import classifier_services
from core.domain import question_services
from core.domain import rights_domain
from core.domain import rights_manager
Expand All @@ -38,7 +34,6 @@
from core.domain import user_services
from core.tests import test_utils
import feconf
import python_utils

import webapp2
import webtest
Expand Down Expand Up @@ -3434,89 +3429,3 @@ def test_voice_artist_can_only_save_assigned_exploration(self):
self.get_json(
'/mock/%s' % self.published_exp_id_2, expected_status_int=401)
self.logout()


class OppiaMLAccessDecoratorTest(test_utils.GenericTestBase):
"""Tests for oppia_ml_access decorator."""

class MockHandler(base.OppiaMLVMHandler):
REQUIRE_PAYLOAD_CSRF_CHECK = False
GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_JSON

def extract_request_message_vm_id_and_signature(self):
"""Returns message, vm_id and signature retrived from incoming
request.
Returns:
tuple(str). Message at index 0, vm_id at index 1 and signature
at index 2.
"""
signature = self.payload.get('signature')
vm_id = self.payload.get('vm_id')
message = self.payload.get('message')
return message, vm_id, signature

@acl_decorators.is_from_oppia_ml
def post(self):
self.render_json({'job_id': 'new_job'})

def setUp(self):
super(OppiaMLAccessDecoratorTest, self).setUp()
self.mock_testapp = webtest.TestApp(webapp2.WSGIApplication(
[webapp2.Route('/ml/nextjobhandler', self.MockHandler)],
debug=feconf.DEBUG,
))

def test_unauthorized_vm_cannot_fetch_jobs(self):
payload = {}
payload['vm_id'] = 'fake_vm'
secret = 'fake_secret'
payload['message'] = json.dumps('malicious message')
payload['signature'] = classifier_services.generate_signature(
python_utils.convert_to_bytes(secret),
payload['message'], payload['vm_id'])

with self.swap(self, 'testapp', self.mock_testapp):
self.post_json(
'/ml/nextjobhandler', payload,
expected_status_int=401)

def test_default_vm_id_raises_exception_in_prod_mode(self):
payload = {}
payload['vm_id'] = feconf.DEFAULT_VM_ID
secret = feconf.DEFAULT_VM_SHARED_SECRET
payload['message'] = json.dumps('malicious message')
payload['signature'] = classifier_services.generate_signature(
python_utils.convert_to_bytes(secret),
payload['message'], payload['vm_id'])
with self.swap(self, 'testapp', self.mock_testapp):
with self.swap(constants, 'DEV_MODE', False):
self.post_json(
'/ml/nextjobhandler', payload, expected_status_int=401)

def test_that_invalid_signature_raises_exception(self):
payload = {}
payload['vm_id'] = feconf.DEFAULT_VM_ID
secret = feconf.DEFAULT_VM_SHARED_SECRET
payload['message'] = json.dumps('malicious message')
payload['signature'] = classifier_services.generate_signature(
python_utils.convert_to_bytes(secret),
'message', payload['vm_id'])

with self.swap(self, 'testapp', self.mock_testapp):
self.post_json(
'/ml/nextjobhandler', payload, expected_status_int=401)

def test_that_no_excpetion_is_raised_when_valid_vm_access(self):
payload = {}
payload['vm_id'] = feconf.DEFAULT_VM_ID
secret = feconf.DEFAULT_VM_SHARED_SECRET
payload['message'] = json.dumps('message')
payload['signature'] = classifier_services.generate_signature(
python_utils.convert_to_bytes(secret),
payload['message'], payload['vm_id'])

with self.swap(self, 'testapp', self.mock_testapp):
json_response = self.post_json('/ml/nextjobhandler', payload)

self.assertEqual(json_response['job_id'], 'new_job')
20 changes: 0 additions & 20 deletions core/controllers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -635,23 +635,3 @@ def get(self):
self.render_json({
'token': csrf_token,
})


class OppiaMLVMHandler(BaseHandler):
"""Base class for the handlers that communicate with Oppia-ML VM instances.
"""

def extract_request_message_vm_id_and_signature(self):
"""Returns the message, vm_id and signature from the incoming request.
Since incoming request can be either a protobuf serialized binary or
a JSON object, the derived classes must implement the necessary
logic to decode the incoming request and return a tuple of size 3
where message is at index 0, vm_id is at index 1 and signature is at
index 2.
Raises:
NotImplementedError. The derived child classes must implement the
necessary logic as described above.
"""
raise NotImplementedError
103 changes: 0 additions & 103 deletions core/controllers/base_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@
import types

from constants import constants
from core.controllers import acl_decorators
from core.controllers import base
from core.domain import classifier_services
from core.domain import exp_domain
from core.domain import exp_services
from core.domain import rights_domain
Expand Down Expand Up @@ -447,39 +445,6 @@ def mock_os_path_join_for_pillow(*args):
# 'unused-import' would appear if this line was not disabled.
import appengine_config # pylint: disable-all

def test_valid_protobuf_path(self):
# We need to re-import main here to make it look like a
# local variable so that we can again re-import main later.
import appengine_config
assert_raises_regexp_context_manager = self.assertRaisesRegexp(
Exception, 'Invalid path for oppia_tools library: invalid_path')

def mock_os_path_join_for_protobuf(*args):
"""Mocks path for 'Protobuf' with an invalid path. This is done by
substituting os.path.join to return an invalid path. This is
needed to test the scenario where the 'Pillow' path points
to a non-existent directory.
"""
path = ''
if args[1] == 'protobuf-3.12.0':
return 'invalid_path'
else:
path = '/'.join(args)
return path

protobuf_path_swap = self.swap(
os.path, 'join', mock_os_path_join_for_protobuf)
# We need to delete the existing module else the re-importing
# would just call the existing module.
del sys.modules['appengine_config']

with assert_raises_regexp_context_manager, protobuf_path_swap:
# This pragma is needed since we are re-importing under
# invalid conditions. The pylint error messages
# 'reimported', 'unused-variable', 'redefined-outer-name' and
# 'unused-import' would appear if this line was not disabled.
import appengine_config # pylint: disable-all

def test_valid_third_party_library_path(self):
# We need to re-import appengine_config here to make it look like a
# local variable so that we can again re-import appengine_config later.
Expand Down Expand Up @@ -1223,71 +1188,3 @@ def test_valid_token_is_returned(self):

self.assertTrue(base.CsrfTokenManager.is_csrf_token_valid(
None, csrf_token))


class OppiaMLVMHandlerTests(test_utils.GenericTestBase):
"""Unit tests for OppiaMLVMHandler class."""

class IncorrectMockVMHandler(base.OppiaMLVMHandler):
"""Derived VM Handler class with missing function implementation for
extract_request_message_vm_id_and_signature function.
"""

REQUIRE_PAYLOAD_CSRF_CHECK = False

@acl_decorators.is_from_oppia_ml
def post(self):
return self.render_json({})

class CorrectMockVMHandler(base.OppiaMLVMHandler):
"""Derived VM Handler class with
extract_request_message_vm_id_and_signature function implementation.
"""

REQUIRE_PAYLOAD_CSRF_CHECK = False

def extract_request_message_vm_id_and_signature(self):
"""Returns the message, vm_id and signature retrieved from the
incoming requests.
"""
signature = self.payload.get('signature')
vm_id = self.payload.get('vm_id')
message = self.payload.get('message')
return message, vm_id, signature

@acl_decorators.is_from_oppia_ml
def post(self):
self.render_json({'job_id': 'new_job'})

def setUp(self):
super(OppiaMLVMHandlerTests, self).setUp()
self.mock_testapp = webtest.TestApp(webapp2.WSGIApplication([
webapp2.Route('/incorrectmock', self.IncorrectMockVMHandler),
webapp2.Route('/correctmock', self.CorrectMockVMHandler)],
debug=feconf.DEBUG,
))

def test_that_incorrect_derived_class_raises_exception(self):
payload = {}
payload['vm_id'] = feconf.DEFAULT_VM_ID
secret = feconf.DEFAULT_VM_SHARED_SECRET
payload['message'] = json.dumps('message')
payload['signature'] = classifier_services.generate_signature(
python_utils.convert_to_bytes(secret),
payload['message'], payload['vm_id'])

with self.swap(self, 'testapp', self.mock_testapp):
self.post_json(
'/incorrectmock', payload, expected_status_int=500)

def test_that_correct_derived_class_does_not_raise_exception(self):
payload = {}
payload['vm_id'] = feconf.DEFAULT_VM_ID
secret = feconf.DEFAULT_VM_SHARED_SECRET
payload['message'] = json.dumps('message')
payload['signature'] = classifier_services.generate_signature(
python_utils.convert_to_bytes(secret),
payload['message'], payload['vm_id'])
with self.swap(self, 'testapp', self.mock_testapp):
self.post_json(
'/correctmock', payload, expected_status_int=200)
Loading

0 comments on commit 5d0095c

Please sign in to comment.