Skip to content

Commit

Permalink
Change configurable emails to platform parameters to make release bui…
Browse files Browse the repository at this point in the history 
…ld independent of these (oppia#20369)

* Add new platform parameters

* Migrate usage and set default to feconf

* Fix mistakes

* Fix tests

* Fix more tests

* Fix linter

* More fixes

* More fixes

* More fixes

* Address comments

* Fix lint issues

* Fix backend tests

* Fix more tests

* Fix more tests

* More fixes

* Fix more tests

* Fix more tests

* Fix more tests

* Fix more tests

* Attempt fix

* Add note on increasing expected keys count

* Fix message

---------

Co-authored-by: Hardik Goyal <[email protected]>
Co-authored-by: Akhilesh Kr. <[email protected]>
  • Loading branch information
@ankita240796 @HardikGoyal2003 @Akhilesh-max
3 people authored Sep 3, 2024
1 parent cc865b1 commit ce05db8
Show file tree
Hide file tree
Showing 37 changed files with 1,916 additions and 794 deletions.
8 changes: 6 additions & 2 deletions core/controllers/acl_decorators.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
from core.domain import email_manager
from core.domain import feature_flag_services
from core.domain import feedback_services
from core.domain import platform_parameter_list
from core.domain import platform_parameter_services
from core.domain import question_services
from core.domain import rights_manager
from core.domain import role_services
Expand Down Expand Up @@ -1347,7 +1349,7 @@ def test_primary_admin(
self: _SelfBaseHandlerType, **kwargs: Any
) -> _GenericHandlerFunctionReturnType:
"""Checks if the user is logged in and is a primary admin e.g. user with
email address equal to feconf.SYSTEM_EMAIL_ADDRESS.
email address equal to SYSTEM_EMAIL_ADDRESS.
Args:
**kwargs: *. Keyword arguments.
Expand All @@ -1364,7 +1366,9 @@ def test_primary_admin(
raise self.NotLoggedInException

email = user_services.get_email_from_user_id(self.user_id)
if email != feconf.SYSTEM_EMAIL_ADDRESS:
if email != platform_parameter_services.get_platform_parameter_value(
platform_parameter_list.ParamName.SYSTEM_EMAIL_ADDRESS.value
):
raise self.UnauthorizedUserException(
'%s cannot delete any user.' % self.user_id)

Expand Down
38 changes: 30 additions & 8 deletions core/controllers/acl_decorators_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
from core.domain import exp_domain
from core.domain import exp_services
from core.domain import feedback_services
from core.domain import platform_parameter_list
from core.domain import platform_parameter_services
from core.domain import question_domain
from core.domain import question_services
from core.domain import rights_domain
Expand Down Expand Up @@ -1793,7 +1795,11 @@ def get(self) -> None:

def setUp(self) -> None:
super().setUp()
self.signup(feconf.SYSTEM_EMAIL_ADDRESS, self.CURRICULUM_ADMIN_USERNAME)
self.system_email_address = (
platform_parameter_services.get_platform_parameter_value(
platform_parameter_list.ParamName.SYSTEM_EMAIL_ADDRESS.value))
assert isinstance(self.system_email_address, str)
self.signup(self.system_email_address, self.CURRICULUM_ADMIN_USERNAME)
self.signup(self.user_email, self.username)

self.signup(
Expand Down Expand Up @@ -1830,7 +1836,8 @@ def test_guest_user_cannot_access_release_coordinator_page(self) -> None:
self.logout()

def test_super_admin_cannot_access_release_coordinator_page(self) -> None:
self.login(feconf.SYSTEM_EMAIL_ADDRESS)
assert isinstance(self.system_email_address, str)
self.login(self.system_email_address)

with self.swap(self, 'testapp', self.mock_testapp):
response = self.get_json(
Expand Down Expand Up @@ -2287,7 +2294,11 @@ def get(self) -> None:

def setUp(self) -> None:
super().setUp()
self.signup(feconf.SYSTEM_EMAIL_ADDRESS, self.CURRICULUM_ADMIN_USERNAME)
self.system_email_address = (
platform_parameter_services.get_platform_parameter_value(
platform_parameter_list.ParamName.SYSTEM_EMAIL_ADDRESS.value))
assert isinstance(self.system_email_address, str)
self.signup(self.system_email_address, self.CURRICULUM_ADMIN_USERNAME)
self.signup(self.user_email, self.username)

self.signup(
Expand Down Expand Up @@ -2322,7 +2333,8 @@ def test_guest_user_cannot_access_release_coordinator_page(self) -> None:
self.logout()

def test_super_admin_cannot_access_release_coordinator_page(self) -> None:
self.login(feconf.SYSTEM_EMAIL_ADDRESS)
assert isinstance(self.system_email_address, str)
self.login(self.system_email_address)

with self.swap(self, 'testapp', self.mock_testapp):
response = self.get_json('/run-anny-job', expected_status_int=401)
Expand Down Expand Up @@ -2419,7 +2431,11 @@ def get(self) -> None:

def setUp(self) -> None:
super().setUp()
self.signup(feconf.SYSTEM_EMAIL_ADDRESS, self.CURRICULUM_ADMIN_USERNAME)
self.system_email_address = (
platform_parameter_services.get_platform_parameter_value(
platform_parameter_list.ParamName.SYSTEM_EMAIL_ADDRESS.value))
assert isinstance(self.system_email_address, str)
self.signup(self.system_email_address, self.CURRICULUM_ADMIN_USERNAME)
self.signup(self.user_email, self.username)

self.signup(
Expand Down Expand Up @@ -2456,7 +2472,8 @@ def test_guest_user_cannot_access_release_coordinator_page(self) -> None:
self.logout()

def test_super_admin_cannot_access_release_coordinator_page(self) -> None:
self.login(feconf.SYSTEM_EMAIL_ADDRESS)
assert isinstance(self.system_email_address, str)
self.login(self.system_email_address)

with self.swap(self, 'testapp', self.mock_testapp):
response = self.get_json(
Expand Down Expand Up @@ -2617,7 +2634,11 @@ def get(self) -> None:

def setUp(self) -> None:
super().setUp()
self.signup(feconf.SYSTEM_EMAIL_ADDRESS, self.CURRICULUM_ADMIN_USERNAME)
self.system_email_address = (
platform_parameter_services.get_platform_parameter_value(
platform_parameter_list.ParamName.SYSTEM_EMAIL_ADDRESS.value))
assert isinstance(self.system_email_address, str)
self.signup(self.system_email_address, self.CURRICULUM_ADMIN_USERNAME)
self.signup(self.user_email, self.username)
self.mock_testapp = webtest.TestApp(webapp2.WSGIApplication(
[webapp2.Route('/mock/', self.MockHandler)],
Expand All @@ -2635,7 +2656,8 @@ def test_not_logged_user_cannot_delete_any_user(self) -> None:
self.get_json('/mock/', expected_status_int=401)

def test_primary_admin_can_delete_any_user(self) -> None:
self.login(feconf.SYSTEM_EMAIL_ADDRESS)
assert isinstance(self.system_email_address, str)
self.login(self.system_email_address)
with self.swap(self, 'testapp', self.mock_testapp):
response = self.get_json('/mock/')
self.assertEqual(response['success'], 1)
Expand Down
10 changes: 7 additions & 3 deletions core/controllers/admin.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2178,7 +2178,9 @@ def put(self) -> None:
NotFoundException. No such user exists.
"""
assert self.normalized_payload is not None
if self.email != feconf.ADMIN_EMAIL_ADDRESS:
if self.email != parameter_services.get_platform_parameter_value(
platform_parameter_list.ParamName.ADMIN_EMAIL_ADDRESS.value
):
raise self.UnauthorizedUserException(
'Only the default system admin can manage super admins')
username = self.normalized_payload['username']
Expand All @@ -2202,7 +2204,9 @@ def delete(self) -> None:
super admin account.
"""
assert self.normalized_request is not None
if self.email != feconf.ADMIN_EMAIL_ADDRESS:
admin_email_address = parameter_services.get_platform_parameter_value(
platform_parameter_list.ParamName.ADMIN_EMAIL_ADDRESS.value)
if self.email != admin_email_address:
raise self.UnauthorizedUserException(
'Only the default system admin can manage super admins')
username = self.normalized_request['username']
Expand All @@ -2211,7 +2215,7 @@ def delete(self) -> None:
if user_settings is None:
raise self.NotFoundException('No such user exists')

if user_settings.email == feconf.ADMIN_EMAIL_ADDRESS:
if user_settings.email == admin_email_address:
raise self.InvalidInputException(
'Cannot revoke privileges from the default super admin account')

Expand Down
Loading

0 comments on commit ce05db8

Please sign in to comment.