Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CUSTOM_SECURITY_MANAGER not working in 0.34.0rc1 #8062

Closed
3 tasks done
ericandrewmeadows opened this issue Aug 19, 2019 · 13 comments · Fixed by #8117
Closed
3 tasks done

CUSTOM_SECURITY_MANAGER not working in 0.34.0rc1 #8062

ericandrewmeadows opened this issue Aug 19, 2019 · 13 comments · Fixed by #8117
Labels
!deprecated-label:bug Deprecated label - Use #bug instead

Comments

@ericandrewmeadows
Copy link

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!

  • I have checked the superset logs for python stacktraces and included it here as text if there are any.
  • I have reproduced the issue with at least the latest released version of superset.
  • 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
Copy link

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

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

@issue-label-bot issue-label-bot bot added the !deprecated-label:bug Deprecated label - Use #bug instead label Aug 19, 2019
@mmutiso
Copy link
Contributor

mmutiso commented Aug 19, 2019

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
Copy link
Member

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
Copy link
Member

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
Copy link
Author

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
Copy link
Author

ericandrewmeadows commented Aug 19, 2019

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
Copy link
Author

ericandrewmeadows commented Aug 19, 2019

@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
Copy link

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

@ericandrewmeadows
Copy link
Author

Any updates here?

@dpgaspar
Copy link
Member

Have you tried to disable Talisman, TALISMAN_ENABLED=False?

@ericandrewmeadows
Copy link
Author

@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
Copy link
Author

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
!deprecated-label:bug Deprecated label - Use #bug instead
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants