Skip to content

Commit

Permalink
Fix oppia#11462: Migrate to Firebase authentication (oppia#12172)
Browse files Browse the repository at this point in the history
* switch constants

* implement redirect flow

* implement login page

* fix dependencies

* add login to build

* fix tests

* e2e fixes

* e2e fixes

* own login-page

* fix tests

* only use return_url when redirecting to signup

* fix tests

* fix firebase tests

* simplify e2e login flow

* use https on emulator mode

* use EMULATOR_MODE

* prepare for admin population

* hit 100% codecov

* add admin-page option to manage firebase super admins

* add revoke endpoint

* fix lint

* respect measurementId

* respect databaseURL

* remove unused config

* add end point to initialize firebase super admin

* add untested destroy_firebase_accounts function

* fix lint

* update tests

* add PREVENT_NEW_SIGNUPS bool to support seeding firebase

* use delete_users() API

* pin to commit

* use user_ids_to_clear

* forgive missing auth models in wipeout

* remove sharding from populate job

* use new deployment constants

* auto refresh until all accounts are deleted

* fix typo

* move firebase forms to bottom of admin page

* introduce SeedFirebaseOneOffJob

* initialize_app in main.py

* add TestBase.swap_with_call_pattern

* add validator tests

* add utils.partion

* fix requirements

* hit 100% codecov

* fix scripts

* improve documentation

* fix update_timestamps error

* fix requirements checks

* fix broken test

* add mock import

* address comments

* fix dependencies

* try fixing e2e tests

* fix codecov

* fix tests

* fix unused variable

* fix broken test

* fix admin-tab to print exception message

* stop returning app

* use swap_with_call_counter

* stop ignoring emulator logs

* avoid using custom_claims directly

* make CallCounter's f arg required

* address comments

* fix lint

* use get

* address comments

* fix topicAndStoryViewer.js

* fix broken tests

* use old argument

* add documentation

* add firebase.json

* add codeowner

* fix e2e tests attempt

* add ignored errors

* fix toHaveSize error

* just visit logout url

* add lighthouse login logic

* add logout page

* add confirmation check

* remove unused tests

* add redirect_url support

* Revert "add logout page"; Revert "add confirmation check"; Revert "remove unused tests"; Revert "add redirect_url support".

* disable broken test
  • Loading branch information
brianrodri authored Mar 23, 2021
1 parent 0a19832 commit d81b16e
Show file tree
Hide file tree
Showing 70 changed files with 3,633 additions and 1,334 deletions.
7 changes: 7 additions & 0 deletions .firebase.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"emulators": {
"auth": {
"port": "9099"
}
}
}
2 changes: 2 additions & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@
/core/templates/combined-tests.spec.ts @srijanreddy98
/core/templates/pages/interaction-specs.constants.ajs.ts @jameesjohn @vojtechjelinek
/core/templates/pages/interaction-specs.constants.ts @nithusha21
/core/templates/pages/login-page/ @brianrodri
/core/templates/I18nFooter.ts @nithusha21
/core/templates/i18n-footer.directive.html @nithusha21
/core/templates/services/app.service*.ts @srijanreddy98
Expand All @@ -475,6 +476,7 @@
# TODO(#11811): Replace @BenHenning with @seanlip after 2021-02-07.
/redis.conf @BenHenning
/release_constants.json @DubeySandeep @nithusha21
/.firebase.json @brianrodri


# Miscellaneous.
Expand Down
2 changes: 1 addition & 1 deletion .isort.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
force_single_line=true
force_sort_within_sections=true
ignore_whitespace=true
known_third_party=apache_beam,backports.functools_lru_cache,browsermobproxy,cloudstorage,contextlib2,elasticsearch,firebase_admin,google.api_core,google.appengine,google.cloud,google.protobuf,jinja2,mapreduce,mutagen,pipeline,pkg_resources,psutil,pylatexenc,pylint,requests,requests_mock,selenium,six,skulpt,webapp2,webapp2_extras,webtest,yaml
known_third_party=apache_beam,backports.functools_lru_cache,browsermobproxy,cloudstorage,contextlib2,elasticsearch,firebase_admin,google.api_core,google.appengine,google.cloud,google.protobuf,jinja2,mapreduce,mock,mutagen,pipeline,pkg_resources,psutil,pylatexenc,pylint,requests,requests_mock,selenium,six,skulpt,webapp2,webapp2_extras,webtest,yaml
line_length=80
sections=FUTURE,STDLIB,FIRSTPARTY,THIRDPARTY,LOCALFOLDER
15 changes: 15 additions & 0 deletions app_dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,17 @@ handlers:
X-Xss-Protection: "1; mode=block"
secure: always
expiration: "0"
- url: /login
static_files: webpack_bundles/login-page.mainpage.html
upload: webpack_bundles/login-page.mainpage.html
http_headers:
Pragma: no-cache
Strict-Transport-Security: "max-age=31536000; includeSubDomains"
X-Content-Type-Options: "nosniff"
X-Frame-Options: "DENY"
X-Xss-Protection: "1; mode=block"
secure: always
expiration: "0"
- url: /creator-guidelines
static_files: webpack_bundles/playbook.mainpage.html
upload: webpack_bundles/playbook.mainpage.html
Expand Down Expand Up @@ -273,6 +284,10 @@ env_variables:
# redirects to these instructions to disable URL fetch:
# https://cloud.google.com/appengine/docs/standard/python/sockets#making_httplib_use_sockets
GAE_USE_SOCKETS_HTTPLIB : "anyvalue"
# FIREBASE_AUTH_EMULATOR_HOST is needed to allow the Firebase SDK to connect
# with the Firebase emulator. THIS MUST NOT BE DEPLOYED TO PRODUCTION. We
# protect against this in the build script.
FIREBASE_AUTH_EMULATOR_HOST: "localhost:9099"

skip_files:
# .pyc and .pyo files
Expand Down
13 changes: 7 additions & 6 deletions assets/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5439,21 +5439,22 @@ export default {
"ANALYTICS_ID": "",
"SITE_NAME_FOR_ANALYTICS": "",

"FIREBASE_AUTH_ENABLED": false,
"FIREBASE_AUTH_ENABLED": true,

// TODO(#11462): Delete this after Firebase authentication has been deployed.
"ENABLE_LOGIN_PAGE": true,

// Data required for Firebase authentication.
//
// NOTE TO RELEASE COORDINATORS: Please change these to the production values,
// and change useEmulator to be false, before deploying to production.
"FIREBASE_CONFIG_API_KEY": "fake-api-key",
"FIREBASE_CONFIG_APP_ID": "",
"FIREBASE_CONFIG_AUTH_DOMAIN": "",
"FIREBASE_CONFIG_DATABASE_URL": "",
"FIREBASE_CONFIG_GOOGLE_CLIENT_ID": "",
"FIREBASE_CONFIG_MESSAGING_SENDER_ID": "",
"FIREBASE_CONFIG_PROJECT_ID": "dev-project-id",
"FIREBASE_CONFIG_STORAGE_BUCKET": "",
"FIREBASE_EMULATOR_ENABLED": true,
"FIREBASE_CONFIG_MESSAGING_SENDER_ID": "",
"FIREBASE_CONFIG_APP_ID": "",
"FIREBASE_CONFIG_GOOGLE_CLIENT_ID": "",

"ALLOW_YAML_FILE_UPLOAD": false,

Expand Down
51 changes: 51 additions & 0 deletions core/controllers/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from core import jobs_registry
from core.controllers import acl_decorators
from core.controllers import base
from core.domain import auth_services
from core.domain import caching_services
from core.domain import collection_services
from core.domain import config_domain
Expand Down Expand Up @@ -729,6 +730,56 @@ def post(self):
self.render_json({})


class AdminGrantSuperAdminPrivilegesHandler(base.BaseHandler):
"""Handler for granting a user super admin privileges."""

GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_JSON

@acl_decorators.can_access_admin_page
def get(self):
if self.email != feconf.ADMIN_EMAIL_ADDRESS:
raise self.UnauthorizedUserException(
'Only the default system admin can manage super admins')

username = self.request.get('username', None)
if username is None:
raise self.InvalidInputException('Missing username param')

user_id = user_services.get_user_id_from_username(username)
if user_id is None:
raise self.InvalidInputException('No such user exists')

auth_services.grant_super_admin_privileges(user_id)
self.render_json(self.values)


class AdminRevokeSuperAdminPrivilegesHandler(base.BaseHandler):
"""Handler for revoking a user's super admin privileges."""

GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_JSON

@acl_decorators.can_access_admin_page
def get(self):
if self.email != feconf.ADMIN_EMAIL_ADDRESS:
raise self.UnauthorizedUserException(
'Only the default system admin can manage super admins')

username = self.request.get('username', None)
if username is None:
raise self.InvalidInputException('Missing username param')

user_settings = user_services.get_user_settings_from_username(username)
if user_settings is None:
raise self.InvalidInputException('No such user exists')

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

auth_services.revoke_super_admin_privileges(user_settings.user_id)
self.render_json(self.values)


class AdminJobOutputHandler(base.BaseHandler):
"""Retrieves job output to show on the admin page."""

Expand Down
111 changes: 111 additions & 0 deletions core/controllers/admin_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
from core.domain import user_services
from core.domain import wipeout_service
from core.platform import models
from core.platform.auth import firebase_auth_services
from core.tests import test_utils
import feconf
import utils
Expand Down Expand Up @@ -86,6 +87,7 @@ class AdminIntegrationTest(test_utils.GenericTestBase):
def setUp(self):
"""Complete the signup process for self.ADMIN_EMAIL."""
super(AdminIntegrationTest, self).setUp()
self.signup(feconf.ADMIN_EMAIL_ADDRESS, 'testsuper')
self.signup(self.ADMIN_EMAIL, self.ADMIN_USERNAME)
self.signup(self.EDITOR_EMAIL, self.EDITOR_USERNAME)
self.admin_id = self.get_user_id_from_email(self.ADMIN_EMAIL)
Expand Down Expand Up @@ -1033,6 +1035,115 @@ def test_update_flag_rules_with_unexpected_exception_returns_500(self):
feature.name)
self.logout()

def test_grant_super_admin_privileges(self):
self.login(feconf.ADMIN_EMAIL_ADDRESS, is_super_admin=True)

grant_super_admin_privileges_stub = self.swap_with_call_counter(
firebase_auth_services, 'grant_super_admin_privileges')

with grant_super_admin_privileges_stub as call_counter:
response = self.get_json(
'/admingrantsuperadminhandler',
params={'username': self.ADMIN_USERNAME},
expected_status_int=200)

self.assertEqual(call_counter.times_called, 1)
self.assertNotIn('error', response)

def test_grant_super_admin_privileges_requires_system_default_admin(self):
self.login(self.ADMIN_EMAIL, is_super_admin=True)

grant_super_admin_privileges_stub = self.swap_with_call_counter(
firebase_auth_services, 'grant_super_admin_privileges')

with grant_super_admin_privileges_stub as call_counter:
response = self.get_json(
'/admingrantsuperadminhandler',
params={'username': self.ADMIN_USERNAME},
expected_status_int=401)

self.assertEqual(call_counter.times_called, 0)
self.assertEqual(
response['error'],
'Only the default system admin can manage super admins')

def test_grant_super_admin_privileges_fails_without_username(self):
self.login(feconf.ADMIN_EMAIL_ADDRESS, is_super_admin=True)

response = self.get_json(
'/admingrantsuperadminhandler', params={}, expected_status_int=400)

self.assertEqual(response['error'], 'Missing username param')

def test_grant_super_admin_privileges_fails_with_invalid_username(self):
self.login(feconf.ADMIN_EMAIL_ADDRESS, is_super_admin=True)

response = self.get_json(
'/admingrantsuperadminhandler', params={'username': 'fakeusername'},
expected_status_int=400)

self.assertEqual(response['error'], 'No such user exists')

def test_revoke_super_admin_privileges(self):
self.login(feconf.ADMIN_EMAIL_ADDRESS, is_super_admin=True)

revoke_super_admin_privileges_stub = self.swap_with_call_counter(
firebase_auth_services, 'revoke_super_admin_privileges')

with revoke_super_admin_privileges_stub as call_counter:
response = self.get_json(
'/adminrevokesuperadminhandler',
params={'username': self.ADMIN_USERNAME},
expected_status_int=200)

self.assertEqual(call_counter.times_called, 1)
self.assertNotIn('error', response)

def test_revoke_super_admin_privileges_requires_system_default_admin(self):
self.login(self.ADMIN_EMAIL, is_super_admin=True)

revoke_super_admin_privileges_stub = self.swap_with_call_counter(
firebase_auth_services, 'revoke_super_admin_privileges')

with revoke_super_admin_privileges_stub as call_counter:
response = self.get_json(
'/adminrevokesuperadminhandler',
params={'username': self.ADMIN_USERNAME},
expected_status_int=401)

self.assertEqual(call_counter.times_called, 0)
self.assertEqual(
response['error'],
'Only the default system admin can manage super admins')

def test_revoke_super_admin_privileges_fails_without_username(self):
self.login(feconf.ADMIN_EMAIL_ADDRESS, is_super_admin=True)

response = self.get_json(
'/adminrevokesuperadminhandler', params={}, expected_status_int=400)

self.assertEqual(response['error'], 'Missing username param')

def test_revoke_super_admin_privileges_fails_with_invalid_username(self):
self.login(feconf.ADMIN_EMAIL_ADDRESS, is_super_admin=True)

response = self.get_json(
'/adminrevokesuperadminhandler',
params={'username': 'fakeusername'}, expected_status_int=400)

self.assertEqual(response['error'], 'No such user exists')

def test_revoke_super_admin_privileges_fails_for_default_admin(self):
self.login(feconf.ADMIN_EMAIL_ADDRESS, is_super_admin=True)

response = self.get_json(
'/adminrevokesuperadminhandler', params={'username': 'testsuper'},
expected_status_int=400)

self.assertEqual(
response['error'],
'Cannot revoke privileges from the default super admin account')


class GenerateDummyExplorationsTest(test_utils.GenericTestBase):
"""Test the conditions for generation of dummy explorations."""
Expand Down
31 changes: 30 additions & 1 deletion core/controllers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,38 @@ def load_template(filename):


class SessionBeginHandler(webapp2.RequestHandler):
"""Class which handles the creation of a new authentication session."""
"""Handler for creating new authentication sessions."""

def get(self):
"""Establishes a new auth session."""
auth_services.establish_auth_session(self.request, self.response)


class SessionEndHandler(webapp2.RequestHandler):
"""Handler for destroying existing authentication sessions."""

def get(self):
"""Destroys an existing auth session."""
auth_services.destroy_auth_session(self.response)


class SeedFirebaseHandler(webapp2.RequestHandler):
"""Handler for preparing Firebase and Oppia to run SeedFirebaseOneOffJob.
TODO(#11462): Delete this handler once the Firebase migration logic is
rollback-safe and all backup data is using post-migration data.
"""

def get(self):
"""Prepares Firebase and Oppia to run SeedFirebaseOneOffJob."""
try:
auth_services.seed_firebase()
except Exception:
logging.exception('Failed to prepare for SeedFirebaseOneOffJob')
finally:
self.redirect('/')


class LogoutPage(webapp2.RequestHandler):
"""Class which handles the logout URL."""

Expand Down Expand Up @@ -164,6 +189,7 @@ def __init__(self, request, response): # pylint: disable=super-init-not-called

self.user_id = None
self.username = None
self.email = None
self.partially_logged_in = False
self.user_is_scheduled_for_deletion = False

Expand All @@ -176,6 +202,8 @@ def __init__(self, request, response): # pylint: disable=super-init-not-called
# the not-fully registered user.
email = auth_claims.email
if 'signup?' in self.request.uri:
if not feconf.ENABLE_USER_CREATION:
raise Exception('New sign-ups are temporarily disabled')
user_settings = (
user_services.create_new_user(auth_id, email))
else:
Expand All @@ -185,6 +213,7 @@ def __init__(self, request, response): # pylint: disable=super-init-not-called
auth_services.destroy_auth_session(self.response)
return

self.email = user_settings.email
self.values['user_email'] = user_settings.email
self.user_id = user_settings.user_id

Expand Down
Loading

0 comments on commit d81b16e

Please sign in to comment.