diff --git a/framework/permissions.py b/framework/permissions.py index 7e8d85f9..77492314 100644 --- a/framework/permissions.py +++ b/framework/permissions.py @@ -24,10 +24,20 @@ from google.appengine.api import users from internals import models -def can_admin_site(unused_user): +def can_admin_site(user): """Return True if the current user is allowed to administer the site.""" - # TODO(jrobbins): replace this with user.is_admin. - return users.is_current_user_admin() + # A user is an admin if they are an admin of the GAE project. + # TODO(jrobbins): delete this statement after legacy admins moved to AppUser. + if users.is_current_user_admin(): + return True + + # A user is an admin if they have an AppUser entity that has is_admin set. + if user: + app_user = models.AppUser.get_app_user(user.email()) + if app_user is not None: + return app_user.is_admin + + return False def can_view_feature(unused_user, unused_feature): diff --git a/framework/permissions_test.py b/framework/permissions_test.py index 28eb878a..5b82fa6d 100644 --- a/framework/permissions_test.py +++ b/framework/permissions_test.py @@ -25,6 +25,7 @@ from google.appengine.api import users from framework import basehandlers from framework import permissions +from internals import models class MockHandler(basehandlers.BaseHandler): @@ -84,6 +85,30 @@ class PermissionFunctionTests(unittest.TestCase): permissions.can_admin_site, tuple(), normal=False, special=False, admin=True, anon=False) + def test_can_admin_site__appuser(self): + """A registered AppUser that has is_admin set can admin the site.""" + email = 'app-admin@example.com' + testing_config.sign_in(email, 111) + user = users.get_current_user() + + # Make sure there is no left over entity from past runs. + query = models.AppUser.all().filter('email =', email) + for old_app_user in query.fetch(None): + old_app_user.delete() + + self.assertFalse(permissions.can_admin_site(user)) + + app_user = models.AppUser(email=email) + app_user.put() + self.assertFalse(permissions.can_admin_site(user)) + + app_user.is_admin = True + app_user.put() + print('user is %r' % user) + print('get_app_user is %r' % models.AppUser.get_app_user(email)) + print('get_app_user.is_admin is %r' % models.AppUser.get_app_user(email).is_admin) + self.assertTrue(permissions.can_admin_site(user)) + def test_can_view_feature(self): self.check_function_results( permissions.can_view_feature, (None,), diff --git a/framework/ramcache.py b/framework/ramcache.py index 8e9aeb90..a5b9a875 100644 --- a/framework/ramcache.py +++ b/framework/ramcache.py @@ -133,7 +133,7 @@ def delete(key): """Emulate the memcache.delete() method using a RAM cache.""" if key in global_cache: del global_cache[key] - flush_all() # Note: this is wasteful but infrequent in our app. + flush_all() # Note: this is wasteful but infrequent in our app. def flush_all(): diff --git a/framework/ramcache_test.py b/framework/ramcache_test.py index 21c43196..a590fdad 100644 --- a/framework/ramcache_test.py +++ b/framework/ramcache_test.py @@ -85,10 +85,10 @@ class RAMCacheFunctionTests(unittest.TestCase): @mock.patch('framework.ramcache.SharedInvalidate.invalidate') def testDelete_NotFound(self, mock_invalidate): - """Deleting an item that is not in the cache is a no-op.""" + """Deleting an item that is not in the cache still invalidates.""" ramcache.delete(KEY_5) - mock_invalidate.assert_not_called() + mock_invalidate.assert_called() @mock.patch('framework.ramcache.SharedInvalidate.invalidate') def testDelete_Found(self, mock_invalidate): diff --git a/internals/models.py b/internals/models.py index 01ea3a27..93413bee 100644 --- a/internals/models.py +++ b/internals/models.py @@ -1219,10 +1219,39 @@ class AppUser(DictModel): #user = db.UserProperty(required=True, verbose_name='Google Account') email = db.EmailProperty(required=True) - #is_admin = db.BooleanProperty(default=False) + is_admin = db.BooleanProperty(default=False) created = db.DateTimeProperty(auto_now_add=True) updated = db.DateTimeProperty(auto_now=True) + def put(self, **kwargs): + """when we update an AppUser, also invalidate ramcache.""" + key = super(DictModel, self).put(**kwargs) + cache_key = 'user|%s' % self.email + ramcache.delete(cache_key) + + def delete(self, **kwargs): + """when we delete an AppUser, also invalidate ramcache.""" + key = super(DictModel, self).delete(**kwargs) + cache_key = 'user|%s' % self.email + ramcache.delete(cache_key) + + @classmethod + def get_app_user(cls, email): + """Return the AppUser for the specified user, or None.""" + cache_key = 'user|%s' % email + cached_app_user = ramcache.get(cache_key) + if cached_app_user: + return cached_app_user + + query = cls.all() + query.filter('email =', email) + found_app_user = query.get() + if found_app_user: + ramcache.set(cache_key, found_app_user) + return found_app_user + + return None + def list_with_component(l, component): return [x for x in l if x.id() == component.key().id()] diff --git a/pages/users.py b/pages/users.py index 22c1849f..410d056e 100644 --- a/pages/users.py +++ b/pages/users.py @@ -57,12 +57,13 @@ class CreateUserAPIHandler(basehandlers.FlaskHandler): @permissions.require_admin_site def process_post_data(self): - email = flask.request.form['email'] + email = self.form['email'] # Don't add a duplicate email address. user = models.AppUser.all(keys_only=True).filter('email = ', email).get() if not user: user = models.AppUser(email=db.Email(email)) + user.is_admin = 'is_admin' in self.form user.put() response_json = user.format_for_template() diff --git a/static/elements/chromedash-userlist.js b/static/elements/chromedash-userlist.js index 6d164479..ff3eac3b 100644 --- a/static/elements/chromedash-userlist.js +++ b/static/elements/chromedash-userlist.js @@ -1,5 +1,6 @@ import {LitElement, css, html} from 'lit-element'; import SHARED_STYLES from '../css/shared.css'; +import {nothing} from 'lit-html'; class ChromedashUserlist extends LitElement { @@ -20,6 +21,18 @@ class ChromedashUserlist extends LitElement { return [ SHARED_STYLES, css` + form { + padding: var(--content-padding); + background: var(--card-background); + border: var(--card-border); + box-shadow: var(--card-box-shadow); + margin-bottom: var(--content-padding); + max-width: 20em; + } + form > * + * { + margin-top: var(--content-padding-half); + } + ul { margin-top: 10px; } @@ -52,8 +65,12 @@ class ChromedashUserlist extends LitElement { if (formEl.checkValidity()) { const email = formEl.querySelector('input[name="email"]').value; + const isAdmin = formEl.querySelector('input[name="is_admin"]').checked; const formData = new FormData(); formData.append('email', email); + if (isAdmin) { + formData.append('is_admin', 'on'); + } formData.append('token', this.token); const resp = await fetch(this.actionPath, { @@ -98,14 +115,25 @@ class ChromedashUserlist extends LitElement { render() { return html`
- - +
+ +
+
+ +
+
+ +
+ diff --git a/templates/admin/users/new.html b/templates/admin/users/new.html index ab27da86..fc2a2d75 100644 --- a/templates/admin/users/new.html +++ b/templates/admin/users/new.html @@ -6,7 +6,7 @@ {% block subheader %}
-

Allowlist a user

+

Application users

{% endblock %}