Fixing how user model is extended

UPDATE.md

@@ -0,0 +1,16 @@
+This document details changes that aren't fully backwards compatible.
+This doc is here to inform of change of behavior as well as
+to explain actions that may be required to take while upgrading.
+
+# 0.19.0
+* We introduced `superset.security_manager.SupersetSecurityManager`,
+that derives `flask_appbuilder.security.sqla.manager.SecurityManager`.
+This derivation of FAB's SecurityManager was necessary in order to
+introduce new attributes to `SupersetUser` like the `image_url` surfaced
+in the profile page as well as in the new dashboard stats footer.
+
+Knowing that the authentication in FAB is implemented by deriving their
+`SecurityManager`, if you have your own auth setup in that way, you'll now
+have to derive `SupersetSecurityManager` instead.
+
+

docs/security.rst

@@ -4,6 +4,15 @@ Security in Superset is handled by Flask AppBuilder (FAB). FAB is a
 "Simple and rapid application development framework, built on top of Flask.".
 FAB provides authentication, user management, permissions and roles.
 
+FAB supports multiple types of authentications:
+in-database, LDAP, OpenID, OAuth, REMOTE_USER, or custom.
+
+To setup/customize authentication for Superset, follow
+`FAB's documentation<https://flask-appbuilder.readthedocs.io/en/latest/security.html#security>`_, but instead of deriving ``flask_appbuilder.security.sqla.manager.SecurityManager``,
+derive Superset's ``superset.security_manager.SupersetSecurityManager``.
+
+Note that Superset extends FAB's ``SecurityManger`` to extend
+the user model as specified in the FAB documentation.
 
 Provided Roles
 --------------

superset/__init__.py

@@ -19,6 +19,7 @@
 
 from superset.connectors.connector_registry import ConnectorRegistry
 from superset import utils, config  # noqa
+from superset.security_manager import SupersetSecurityManager
 
 
 APP_DIR = os.path.dirname(__file__)
@@ -77,25 +78,25 @@ def get_js_manifest():
 migrate = Migrate(app, db, directory=APP_DIR + "/migrations")
 
 # Logging configuration
-logging.basicConfig(format=app.config.get('LOG_FORMAT'))
-logging.getLogger().setLevel(app.config.get('LOG_LEVEL'))
-
-if app.config.get('ENABLE_TIME_ROTATE'):
-    logging.getLogger().setLevel(app.config.get('TIME_ROTATE_LOG_LEVEL'))
-    handler = TimedRotatingFileHandler(app.config.get('FILENAME'),
-                                       when=app.config.get('ROLLOVER'),
-                                       interval=app.config.get('INTERVAL'),
-                                       backupCount=app.config.get('BACKUP_COUNT'))
+logging.basicConfig(format=conf.get('LOG_FORMAT'))
+logging.getLogger().setLevel(conf.get('LOG_LEVEL'))
+
+if conf.get('ENABLE_TIME_ROTATE'):
+    logging.getLogger().setLevel(conf.get('TIME_ROTATE_LOG_LEVEL'))
+    handler = TimedRotatingFileHandler(conf.get('FILENAME'),
+                                       when=conf.get('ROLLOVER'),
+                                       interval=conf.get('INTERVAL'),
+                                       backupCount=conf.get('BACKUP_COUNT'))
     logging.getLogger().addHandler(handler)
 
-if app.config.get('ENABLE_CORS'):
+if conf.get('ENABLE_CORS'):
     from flask_cors import CORS
-    CORS(app, **app.config.get('CORS_OPTIONS'))
+    CORS(app, **conf.get('CORS_OPTIONS'))
 
-if app.config.get('ENABLE_PROXY_FIX'):
+if conf.get('ENABLE_PROXY_FIX'):
     app.wsgi_app = ProxyFix(app.wsgi_app)
 
-if app.config.get('ENABLE_CHUNK_ENCODING'):
+if conf.get('ENABLE_CHUNK_ENCODING'):
     class ChunkedEncodingFix(object):
 
         def __init__(self, app):
@@ -109,13 +110,13 @@ def __call__(self, environ, start_response):
             return self.app(environ, start_response)
     app.wsgi_app = ChunkedEncodingFix(app.wsgi_app)
 
-if app.config.get('UPLOAD_FOLDER'):
+if conf.get('UPLOAD_FOLDER'):
     try:
-        os.makedirs(app.config.get('UPLOAD_FOLDER'))
+        os.makedirs(conf.get('UPLOAD_FOLDER'))
     except OSError:
         pass
 
-for middleware in app.config.get('ADDITIONAL_MIDDLEWARE'):
+for middleware in conf.get('ADDITIONAL_MIDDLEWARE'):
     app.wsgi_app = middleware(app.wsgi_app)
 
 
@@ -124,20 +125,27 @@ class MyIndexView(IndexView):
     def index(self):
         return redirect('/superset/welcome')
 
+SecurityManager = (
+    conf.get("CUSTOM_SECURITY_MANAGER") or SupersetSecurityManager)
+if not issubclass(SecurityManager, SupersetSecurityManager):
+    raise Exception(
+        "Security manager needs to be a subclass of "
+        "superset.security.SupersetSecurityManager")
+
 appbuilder = AppBuilder(
     app, db.session,
     base_template='superset/base.html',
     indexview=MyIndexView,
-    security_manager_class=app.config.get("CUSTOM_SECURITY_MANAGER"))
+    security_manager_class=SecurityManager)
 
 sm = appbuilder.sm
 
 get_session = appbuilder.get_session
-results_backend = app.config.get("RESULTS_BACKEND")
+results_backend = conf.get("RESULTS_BACKEND")
 
 # Registering sources
-module_datasource_map = app.config.get("DEFAULT_MODULE_DS_MAP")
-module_datasource_map.update(app.config.get("ADDITIONAL_MODULE_DS_MAP"))
+module_datasource_map = conf.get("DEFAULT_MODULE_DS_MAP")
+module_datasource_map.update(conf.get("ADDITIONAL_MODULE_DS_MAP"))
 ConnectorRegistry.register_sources(module_datasource_map)
 
 from superset import views  # noqa

superset/assets/javascripts/dashboard/Dashboard.jsx

@@ -73,10 +73,12 @@ function initDashboardView(dashboard) {
     <GridLayout dashboard={dashboard} />,
     document.getElementById('grid-container'),
   );
-  render(
-    <Footer dashboard={dashboard} />,
-    document.getElementById('dash-footer'),
-  );
+  if (dashboard.ENABLE_DASHBOARD_STATS) {
+    render(
+      <Footer dashboard={dashboard} />,
+      document.getElementById('dash-footer'),
+    );
+  }
 
   // Displaying widget controls on hover
   $('.react-grid-item').hover(

superset/assets/javascripts/dashboard/components/Footer.jsx

@@ -85,7 +85,10 @@ class Footer extends React.PureComponent {
                 <i className="fa fa-wrench" />
               </TooltipWrapper>
             </Navbar.Text>
-            <span className="profile-images pull-left">
+            <span
+              className="profile-images pull-left"
+              style={{ marginRight: 50 }}
+            >
               {dashboard.owners.map(user => this.renderUserIcon(user))}
             </span>
           </span>

superset/config.py

@@ -313,6 +313,10 @@ class CeleryConfig(object):
 # into a proxied one
 TRACKING_URL_TRANSFORMER = lambda x: x
 
+# Makes it possible to disable the dashboard footer where usage stats are
+# exposed
+ENABLE_DASHBOARD_STATS = True
+
 try:
     if CONFIG_PATH_ENV_VAR in os.environ:
         # Explicitly import config module that is not in pythonpath; useful

superset/migrations/versions/cd8aba67c8b6_image_url.py

@@ -5,20 +5,28 @@
 Create Date: 2017-04-18 16:21:26.561856
 
 """
+from alembic import op
+import sqlalchemy as sa
 
 # revision identifiers, used by Alembic.
 revision = 'cd8aba67c8b6'
 down_revision = 'e8976cf2d39e'
 
-from alembic import op
-import sqlalchemy as sa
-
 
 def upgrade():
-    op.add_column('ab_user', sa.Column('image_url', sa.String(length=1000), nullable=True))
-    op.add_column('ab_user', sa.Column('slack_username', sa.String(length=500), nullable=True))
+    try:
+        with op.batch_alter_table('ab_user', schema=None) as batch_op:
+            batch_op.add_column(
+                'ab_user',
+                sa.Column('image_url', sa.String(length=1000), nullable=True))
+            batch_op.add_column(
+                'ab_user',
+                sa.Column('slack_username', sa.String(length=500), nullable=True))
+    except Exception:
+        pass
 
 
 def downgrade():
-    op.drop_column('ab_user', 'slack_username')
-    op.drop_column('ab_user', 'image_url')
+    with op.batch_alter_table('ab_user', schema=None) as batch_op:
+        batch_op.drop_column('ab_user', 'slack_username')
+        batch_op.drop_column('ab_user', 'image_url')

superset/migrations/versions/e58b53fc8605_.py

@@ -0,0 +1,22 @@
+"""empty message
+
+Revision ID: e58b53fc8605
+Revises: ('cd8aba67c8b6', 'a65458420354')
+Create Date: 2017-06-17 10:17:40.227962
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'e58b53fc8605'
+down_revision = ('cd8aba67c8b6', 'a65458420354')
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    pass
+
+
+def downgrade():
+    pass

superset/models/core.py

@@ -859,56 +859,3 @@ def user_roles(self):
                 href = "{} Role".format(r.name)
             action_list = action_list + '<li>' + href + '</li>'
         return '<ul>' + action_list + '</ul>'
-
-
-class SupersetUser(ab_models.User):
-    """Deriving FAB's USER to enrich it"""
-    image_url = Column(String(1000))
-    slack_username = Column(String(500))
-
-    @property
-    def full_name(self):
-        return self.first_name + ' ' + self.last_name
-
-    @property
-    def data(self):
-        """Returns a json-serializable representation of the self"""
-        return {
-            'id': self.id,
-            'first_name': self.first_name,
-            'last_name': self.last_name,
-            'slack_username': self.slack_username,
-            'email': self.email,
-            'image_url': self.image_url,
-            'username': self.username,
-            'is_active': self.is_active(),
-            'created_on': self.created_on.isoformat(),
-        }
-
-    @property
-    def data_extended(self):
-        """Superset of data property with roles and perms"""
-        d = self.data
-        roles = {}
-        permissions = defaultdict(set)
-        for role in self.roles:
-            perms = set()
-            for perm in role.permissions:
-                perms.add(
-                    (perm.permission.name, perm.view_menu.name)
-                )
-                if perm.permission.name in (
-                        'datasource_access', 'database_access'):
-                    permissions[perm.permission.name].add(perm.view_menu.name)
-            roles[role.name] = [
-                [perm.permission.name, perm.view_menu.name]
-                for perm in role.permissions
-            ]
-        d.update({
-            'roles': roles,
-            'permissions': permissions,
-        })
-        return d
-
-# hoo hoo hoo hee hee hee
-#ab_models.User = SupersetUser

superset/models/helpers.py

@@ -166,6 +166,7 @@ def set_perm(mapper, connection, target):  # noqa
     # add to view menu if not already exists
     merge_perm(sm, 'datasource_access', target.get_perm(), connection)
 
+
 def user_data(user):
     """Returns a json-serializable representation of the user"""
     return {

superset/security.py

@@ -5,6 +5,7 @@
 from __future__ import unicode_literals
 
 import logging
+
 from flask_appbuilder.security.sqla import models as ab_models
 
 from superset import conf, db, sm

superset/security_manager.py

@@ -0,0 +1,73 @@
+from collections import defaultdict
+
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from flask_appbuilder.security import views as fab_views
+from flask_appbuilder.security.sqla import models as ab_models
+from flask_appbuilder.security.sqla.manager import SecurityManager
+
+from sqlalchemy import Column, String
+
+
+class SupersetUser(ab_models.User):
+    """Deriving FAB's USER to enrich it"""
+    image_url = Column(String(1000))
+    slack_username = Column(String(500))
+
+    @property
+    def full_name(self):
+        return self.first_name + ' ' + self.last_name
+
+    @property
+    def data(self):
+        """Returns a json-serializable representation of the self"""
+        return {
+            'id': self.id,
+            'first_name': self.first_name,
+            'last_name': self.last_name,
+            'slack_username': self.slack_username,
+            'email': self.email,
+            'image_url': self.image_url,
+            'username': self.username,
+            'is_active': self.is_active(),
+            'created_on': self.created_on.isoformat(),
+        }
+
+    @property
+    def data_extended(self):
+        """Superset of data property with roles and perms"""
+        d = self.data
+        roles = {}
+        permissions = defaultdict(set)
+        for role in self.roles:
+            perms = set()
+            for perm in role.permissions:
+                perms.add(
+                    (perm.permission.name, perm.view_menu.name)
+                )
+                if perm.permission.name in (
+                        'datasource_access', 'database_access'):
+                    permissions[perm.permission.name].add(perm.view_menu.name)
+            roles[role.name] = [
+                [perm.permission.name, perm.view_menu.name]
+                for perm in role.permissions
+            ]
+        d.update({
+            'roles': roles,
+            'permissions': permissions,
+        })
+        return d
+
+
+class SupersetUserModelView(fab_views.UserDBModelView):
+    route_base = '/userext'
+    datamodel = SQLAInterface(SupersetUser)
+    edit_columns = fab_views.UserDBModelView.edit_columns + [
+        'image_url', 'slack_username']
+    add_columns = fab_views.UserDBModelView.add_columns + [
+        'image_url', 'slack_username']
+fab_views.UserDBModelView = SupersetUserModelView
+
+
+class SupersetSecurityManager(SecurityManager):
+    user_model = SupersetUser
+    userdbmodelview = SupersetUserModelView