Incubator-superset: CUSTOM_SECURITY_MANAGER not working in 0.34.0rc1

[ericandrewmeadows]

App routing is broken in the latest release with respect to Custom Security Manager. Please deprecate this release.

Expected results

When attempting to access the root url for Superset, this should redirect to the login page if the user is not logged in. This redirect is failing to work when AUTH_TYPE = AUTH_OID

Actual results

The app routes to /superset/welcome

Screenshots

If applicable, add screenshots to help explain your problem.

How to reproduce the bug

1. Use a custom security manager, akin to https://stackoverflow.com/questions/54010314/using-keycloakopenid-connect-with-apache-superset#54024394
2. Deploy
3. Navigate to bare url

Environment

(please complete the following information):

• superset version: 0.34.0rc1
• python version: 3.6
• node.js version: 10.0
• npm version: npm -v

Checklist

Make sure these boxes are checked before submitting your issue - thank you!

• [x] I have checked the superset logs for python stacktraces and included it here as text if there are any.
• [x] I have reproduced the issue with at least the latest released version of superset.
• [x] I have checked the issue tracker for the same issue and I haven't found one similar.

Additional context

This works perfectly fine on 0.33.0rc1, so I have had to downgrade. Because of this issue, where the routing is not correct, I believe this issue should be resolved and 0.34.0rc1 should be pulled.

[issue-label-bot[bot]]

Issue-Label Bot is automatically applying the label #bug to this issue, with a confidence of 0.96. Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

[mmutiso]

Hi @ericandrewmeadows ,

We had the same issue as explained in that StackOverflow question although we are using a different openId connect provider called IdentityServer4.

We, therefore, did a custom security manager that does the integration by extending the superset security manager. I would be happy to provide my approach if need be. It involved a little bit of code and understanding of the workings of openId connect(built on top of OAuth2)

[dpgaspar]

Hi @ericandrewmeadows,

Can you please provide:

• server logs for 0.33.0rc1 and 0.34.0rc1?
• Your custom SM

This could be a FAB related issue, feel free to open an issue there so we can narrow it down

[villebro]

App routing is broken in the latest release with respect to Custom Security Manager. Please deprecate this release.

IMO, the 0.34 release has a good deal of bug fixes/improvements that merit releasing 0.34 rather than 0.33. As 0.34 will (hopefully) become the first official ASF Superset release, there will for sure be many bugs that surface as people try it out. Rest assured committers (myself included) will give reported issues high priority to make sure that bugs are ironed out of the 0.34 branch (along with master) as quickly as possible.

[ericandrewmeadows]

Custom SM:

# https://stackoverflow.com/questions/54010314/ ...
#   using-keycloakopenid-connect-with-apache-superset#54024394
from flask import redirect, request
from flask_appbuilder.security.manager import AUTH_OID
from superset.security import SupersetSecurityManager
from flask_oidc import OpenIDConnect
from flask_appbuilder.security.views import AuthOIDView
from flask_login import login_user
from urllib.parse import quote
from flask_appbuilder.views import ModelView, SimpleFormView, expose
import logging
import os

# Testing Okta
from okta import UsersClient
from okta.framework.ApiClient import ApiClient

# DELETE
import json

DEFAULT_USER_ROLE = 'Gamma'
USER_ROLE_TRANSLATION_DICT = {
    'Charts Admin': 'Admin',
    'Charts Alpha Users': 'Alpha',
    'software': 'Software',
}


class AuthOIDCView(AuthOIDView):
    def get_given_and_family_name_from_name(self, info):
        name = info.get('name')
        first_space_location = name.find(' ')

        given_name = info.get('given_name', name[:first_space_location])
        family_name = info.get('family_name', name[first_space_location+1:])
        return (given_name, family_name)

    def get_user_info(self, okta_client, uid):
        response = ApiClient.get_path(okta_client, '/{0}'.format(uid))
        user_info = json.loads(response.text)
        return user_info

    def get_account_creation_info(self, user_info):
        profile = user_info['profile']
        username = profile.get('login')
        first_name = profile.get('firstName')
        last_name = profile.get('lastName')
        email = profile.get('email')
        return (username, first_name, last_name, email)

    def create_user(self, sm, user_info):
        (username, first_name, last_name, email) =\
            self.get_account_creation_info(user_info)
        role = sm.find_role(DEFAULT_USER_ROLE)
        user = sm.add_user(username, first_name, last_name, email, role)
        return user

    def get_group_info(self, okta_client, uid):
        response = ApiClient.get_path(okta_client, '/{0}/groups'.format(uid))
        group_info = json.loads(response.text)
        return group_info

    def get_group_membership(self, group_info):
        group_membership = [group["profile"]["name"] for group in group_info]
        return group_membership

    def get_or_create_roles(self, sm, group_membership):
        roles = [
            sm.add_role(USER_ROLE_TRANSLATION_DICT.get(group, group))
            for group in group_membership
        ]
        return roles

    def update_user_roles(self, sm, user, group_info):
        group_membership = self.get_group_membership(group_info)
        roles = self.get_or_create_roles(sm, group_membership)
        existing_roles = user.roles
        for role in roles:
            if role in existing_roles:
                continue
            user.roles.append(role)
            logging.info(f'Role added to {user.username}: {role.name}')
        return user

    @expose('/login/', methods=['GET', 'POST'])
    def login(self, flag=True):
        sm = self.appbuilder.sm
        oidc = sm.oid

        @self.appbuilder.sm.oid.require_login
        def handle_login():
            uid = oidc.user_getfield("sub")
            user = sm.auth_user_oid(uid)

            okta_client = UsersClient(
                os.environ['ORG_URL'],
                os.environ['OKTA_AUTH_TOKEN']
            )

            user_info = self.get_user_info(okta_client, uid)
            user_active = user_info['status'] == "ACTIVE"

            if not user_active:
                return
            if user is None:
                user = self.create_user(sm, user_info)
            logging.info(f"User:  {user}")

            group_info = self.get_group_info(okta_client, uid)
            user = self.update_user_roles(sm, user, group_info)
            sm.update_user(user)

            login_user(user, remember=False)
            return redirect(self.appbuilder.get_url_for_index)

        return handle_login()

    @expose('/logout/', methods=['GET', 'POST'])
    def logout(self):

        oidc = self.appbuilder.sm.oid

        oidc.logout()
        super(AuthOIDCView, self).logout()        
        redirect_url = request.url_root.strip('/') + self.appbuilder.get_url_for_login

        return redirect(oidc.client_secrets.get('issuer') + '/protocol/openid-connect/logout?redirect_uri=' + quote(redirect_url))

class OIDCSecurityManager(SupersetSecurityManager):
    def __init__(self, appbuilder):
        super(OIDCSecurityManager, self).__init__(appbuilder)
        if self.auth_type == AUTH_OID:
            self.oid = OpenIDConnect(self.appbuilder.get_app)
        self.authoidview = AuthOIDCView

[ericandrewmeadows]

Sorry - accidentally opened & closed.
In my superset_config.py, this is how I am implementing the OAuth:

from flask_appbuilder.security.manager import AUTH_OID
    from create_client_secret_json import create_client_secret_json
    from security import OIDCSecurityManager

    SECRETS_FILENAME = 'client_secret.json'
    client_secrets_dict = create_client_secret_json(SECRETS_FILENAME)
    os.environ['ORG_URL'] = client_secrets_dict['issuer']

    AUTH_TYPE = AUTH_OID
    OIDC_CLIENT_SECRETS = SECRETS_FILENAME
    OIDC_ID_TOKEN_COOKIE_SECURE = True
    OIDC_USER_INFO_ENABLED = True
    AUTH_USER_REGISTRATION = True
    AUTH_USER_REGISTRATION_ROLE = 'Gamma'
    OIDC_SCOPES = ["openid", "email", "profile", "groups"]
    OIDC_INTROSPECTION_AUTH_METHOD = os.environ['OID_AUTH_METHOD']
    CUSTOM_SECURITY_MANAGER = OIDCSecurityManager

The server logs actually do not give any issues, but with the custom SM, it is not ending up reaching handle_login(), and /login is not being properly routed to. When I end up accessing /login directly, I end up having the endpoint say I did not pass in a specified redirect_uri because the variable ENABLE_PROXY_FIX also appears to be broken because the port is coming through on the request. Intuitively, this is all from the Talisman library.

[ericandrewmeadows]

@villebro - my main item is that v0.33.0rc1 -> v0.34.0rc1 broke custom security, and is a large issue. I am also beginning to make commits and help this as well, I am just waiting on some people to comment on the features I am contributing on.

[kalimuthu123]

i need to create an authentication with jwt identity to access api inside it

[ericandrewmeadows]

Any updates here?

[dpgaspar]

Have you tried to disable Talisman, TALISMAN_ENABLED=False?

[ericandrewmeadows]

@dpgaspar - no go on that. There is something in the routes which is circumventing the security manager with v0.34.0rc (1, and 2).

Relevant settings from above, tested just now:

ENABLE_PROXY_FIX = True
TALISMAN_ENABLED = False

[ericandrewmeadows]

The symptom is very direct, now that I was able to trace it out even more: ENABLE_PROXY_FIX is the piece which is wrong. The port is bring appended to requests in the header, which is causing routing issues.

[ericandrewmeadows]

Testing a proposed fix for this and will put up a PR.