Add chromium-committers appengine app.

chromium-committers/auth_util.py

+# Copyright (c) 2013 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Utilities for generating and verifying user authentication."""
+
+__author__ = 'agable@google.com (Aaron Gable)'
+
+
+import functools
+import logging
+
+from google.appengine.api import users
+from google.appengine.ext import ndb
+
+import constants
+import model
+
+
+def CheckUserAuth(handler):
+  """Decorator for webapp2 request handler methods.
+
+  Only use on webapp2.RequestHandler methods (e.g. get, post, put).
+
+  Checks to see if the user is logged in, and if they are
+  * If they are an administrator of the app, or
+  * If their email appears in the list of allowed addresses
+
+  Sets request.authenticated to 'user' if successful. Otherwise, None.
+  """
+  @functools.wraps(handler)
+  def wrapper(self, *args, **kwargs):
+    """Does the real legwork and calls the wrapped handler."""
+    def abort_auth(log_msg):
+      """Helper method to be an exit hatch when authentication fails."""
+      logging.warning(log_msg)
+      self.request.authenticated = None
+      handler(self, *args, **kwargs)
+
+    def finish_auth(log_msg):
+      """Helper method to be an exit hatch when authentication succeeds."""
+      logging.info(log_msg)
+      self.request.authenticated = 'user'
+      handler(self, *args, **kwargs)

[iannucci, 2013/10/09 18:57:30]

Should these return the value of handler (and we should return the calls of
these functions)? Or is that never used?

[agable, 2013/10/09 19:12:31]

webapp2 doesn't require or even want handlers to return. It waits for the
handler method (get()) to exit and then just grabs the response object from the
ResponseHandler object. All that's important is that self.request and
self.response are maintained; no returns necessary.

+
+    if getattr(self.request, 'authenticated', None):
+      finish_auth('Already authenticated.')
+      return

[iannucci, 2013/10/09 18:57:30]

I would just make the else: on here set authenticated to None

[agable, 2013/10/09 19:12:31]

Not sure I agree with this. I think the current structure makes sense: If
already authenticated, finish. Otherwise, either finish or abort. And each of
those directly sets the authenticated field. Having it default to None here and
then having finish and abort be asymmetric seems harder to read to me
(less-linear control flow). But I'll change it if you insist.

+
+    user = users.get_current_user()
+    if not user:
+      abort_auth('No logged in user.')
+      return
+
+    if users.is_current_user_admin():
+      finish_auth('User is admin.')
+      return
+
+    email = user.email()
+    email_list = ndb.Key(model.EmailList, constants.LIST).get()
+    allowed_emails = email_list.emails if email_list else []
+
+    if email in allowed_emails:
+      finish_auth('User in allowed email list.')
+      return
+
+    if (email.endswith('@google.com') and
+        email.replace('@google.com', '@chromium.org') in allowed_emails):
+      finish_auth('User in allowed email list via google -> chromium map.')

[iannucci, 2013/10/09 18:57:30]

:(... this is teh hax... Can we at least have this as a separate filter
decorator or something? auth_utils.ApplyDumbEmailDomainHacks ? Unless we really
really want this behavior always for all users of this decorator...

[agable, 2013/10/09 19:12:31]

I believe this is the best way to do this. Later we will change how this works
by using the *real* google -> chromium map (when this app hosts it) but for now
this is the best. And I do think we will want at least behavior similar to this
forever and always, since I think it is just silly and inconvenient to only let
@chromium accounts see this list, or to add all @google accounts explicitly to
the list too.

+      return
+
+    abort_auth('User not in allowed email list.')
+
+  return wrapper
+
+
+def RequireAuth(handler):
+  """Decorator for webapp2 request handler methods.
+
+  Only use on webapp2.RequestHandler methods (e.g. get, post, put),
+  and only after using a 'Check____Auth' decorator.
+
+  Expects the handler's self.request.authenticated to be not-None.
+  If it doesn't exist or evaluates to False, 403s. Otherwise, passes
+  control to the wrapped handler.
+  """
+  @functools.wraps(handler)
+  def wrapper(self, *args, **kwargs):
+    """Does the real legwork and calls the wrapped handler."""
+    if not getattr(self.request, 'authenticated', None):

[iannucci, 2013/10/09 18:57:30]

either this should be checking is not None, or the docstring should be updated
to say it expects a non False-ish value. 

[agable, 2013/10/09 19:12:31]

SG, will be done in patchset 6 (after you finish reviewing patchset 5 so we
don't get even more out-of-sync).

+      self.abort(403)
+    else:
+      handler(self, *args, **kwargs)
+
+  return wrapper