apache / superset

Apache Superset is a Data Visualization and Data Exploration Platform
https://superset.apache.org/
Apache License 2.0
62.47k stars 13.74k forks source link

Role is not mandatory for a user when using OAuth login #29523

Open MalavikaN1 opened 3 months ago

MalavikaN1 commented 3 months ago

Bug description

Role is mandatory when default superset login is used. But when using OAuth Login, the role becomes optional for a user. This in turn leads to 500 Internal Server Error when a user that has no role logins to superset.

How to reproduce the bug

  1. Configure superset to use any other OAuth login.
  2. Go to List Users and change the role of any user to None and then save it.
  3. Login using that user whose role has been removed.
  4. Notice 500 Internal Server Error.

Screenshots/recordings

Superset version

3.1.3

Python version

3.9

Node version

16

Browser

Chrome

Additional context

![Uploading Screenshot 2024-07-09 143701.png…]()

Checklist

dosubot[bot] commented 3 months ago

To resolve the 500 Internal Server Error that occurs when a user without a role logs in using OAuth in Superset, you need to ensure that the user is assigned a role upon login. This can be done by modifying the OAuth configuration to assign a default role to users who do not have one.

Here is an example of how you can configure this in Superset:

  1. Update the OAuth configuration: Ensure that the OAuth provider is configured to map roles correctly. You can set a default role for users who do not have one.

  2. Modify the AUTH_USER_REGISTRATION_ROLE setting: This setting in Superset allows you to specify a default role for new users. You can set this in your superset_config.py file.

# superset_config.py

# Enable user registration
AUTH_USER_REGISTRATION = True

# The default role to assign to new users
AUTH_USER_REGISTRATION_ROLE = "Gamma"  # or any other role you want to assign

By setting AUTH_USER_REGISTRATION_ROLE, you ensure that any new user who logs in via OAuth and does not have a role will be assigned the specified default role, preventing the 500 Internal Server Error.

Make sure to restart your Superset instance after making these changes to apply the new configuration.

For more detailed configuration options, refer to the Superset documentation and ensure your superset_config.py is correctly set up as described in the Configuring Superset guide [1][2].

To continue talking to Dosu, mention @dosu.

MalavikaN1 commented 3 months ago

@dosu This is only applicable if I can create a new user through superset in the OAuth. In our case, we do not want to create a new user and the option to create a new user is shouldn't be there. We just want the role to be a mandatory field and for existing users, at least one role should be present.

dosubot[bot] commented 3 months ago

To configure Superset to make the role a mandatory field for users logging in via OAuth and ensure that existing users have at least one role assigned without creating new users, you can use the AUTH_USER_REGISTRATION_ROLE and AUTH_ROLES_SYNC_AT_LOGIN settings in your superset_config.py file.

Here is how you can configure it:

  1. Set a Default Role for New Users: Ensure that new users who register via OAuth are assigned a default role.
  2. Sync Roles at Login: Ensure that roles are synced every time a user logs in.

Add the following configuration to your superset_config.py:

# Ensure that users have at least one role assigned upon registration
AUTH_USER_REGISTRATION = True
AUTH_USER_REGISTRATION_ROLE = "Gamma"  # Default role for new users

# Sync roles at each login to ensure users have the correct roles
AUTH_ROLES_SYNC_AT_LOGIN = True

# Map OAuth groups to Superset roles if needed
AUTH_ROLES_MAPPING = {
    "oauth_group_name": ["SupersetRole1", "SupersetRole2"],
    # Add more mappings as needed
}

This configuration ensures that:

By setting AUTH_USER_REGISTRATION_ROLE, you ensure that new users have at least one role. By enabling AUTH_ROLES_SYNC_AT_LOGIN, you ensure that existing users' roles are updated at each login, preventing the creation of new users without roles [1][2].

To continue talking to Dosu, mention @dosu.

MalavikaN1 commented 3 months ago

@dosu But does this ensure that role becomes a mandatory field and on editing a user profile the role cannot be left empty?

dosubot[bot] commented 3 months ago

To ensure that the role becomes a mandatory field in Superset and cannot be left empty when editing a user profile, you need to add validation logic to check if the roles field is populated. This can be done by modifying the validate function to include a check for the roles field.

Here is the updated validate function:

const validate = () => {
  if (
    currentRule?.name &&
    currentRule?.clause &&
    currentRule.tables?.length &&
    currentRule.roles?.length
  ) {
    setDisableSave(false);
  } else {
    setDisableSave(true);
  }
};

This ensures that the disableSave state is set to true if the roles field is empty, making the save button disabled until the roles field is populated [1].

Additionally, you can customize the user model to enforce that the role field is mandatory by modifying the UserModelView to include a validation that ensures the role field is not empty. This can be done by overriding the UserModelView and adding a custom form with the required validation.

Here is an example:

from flask_appbuilder import ModelView
from flask_appbuilder.forms import DynamicForm
from wtforms import validators, SelectMultipleField
from superset import app, db
from superset.security.manager import SupersetSecurityManager

class CustomUserForm(DynamicForm):
    roles = SelectMultipleField(
        'Roles',
        validators=[validators.DataRequired(message="Role field is mandatory")],
        coerce=int
    )

class CustomUserModelView(ModelView):
    datamodel = db.Model
    add_form = CustomUserForm
    edit_form = CustomUserForm

class CustomSecurityManager(SupersetSecurityManager):
    usermodelview = CustomUserModelView

appbuilder = app.appbuilder
appbuilder.security_manager_class = CustomSecurityManager
appbuilder.init_app(app, db.session)

In this example:

This setup ensures that the role field is mandatory when creating or editing a user in Superset [2][3].

To continue talking to Dosu, mention @dosu.

MalavikaN1 commented 3 months ago

@dosu Using native login, it is already a mandatory field, why is it when using Oauth this is not a mandatory field. Also, we are not maintaining codebase instead using docker image. So, editing the code suggested as above is not possible

dosubot[bot] commented 3 months ago

To ensure that the role field is mandatory for users logging in via OAuth in Superset without modifying the codebase, you can configure the AUTH_USER_REGISTRATION_ROLE and AUTH_ROLES_MAPPING in your superset_config.py file. This will ensure that users are assigned a default role upon registration and map OAuth groups to Superset roles.

Here is how you can configure it:

  1. Set the default user registration role:

    AUTH_USER_REGISTRATION_ROLE = "Public"  # or any other default role you prefer
  2. Map OAuth groups to Superset roles to ensure users are assigned roles based on their OAuth group:

    AUTH_ROLES_MAPPING = {
        "oauth_group_name": ["SupersetRole1", "SupersetRole2"],
        "another_oauth_group": ["AnotherSupersetRole"]
    }
  3. Ensure roles are synced at login:

    AUTH_ROLES_SYNC_AT_LOGIN = True

By setting these configurations, you ensure that every user logging in via OAuth is assigned a role, making the role field effectively mandatory.

Here is the complete configuration snippet for superset_config.py:

# Default user self registration role
AUTH_USER_REGISTRATION_ROLE = "Public"

# Mapping OAUTH groups to Superset roles
AUTH_ROLES_MAPPING = {
    "oauth_group_name": ["SupersetRole1", "SupersetRole2"],
    "another_oauth_group": ["AnotherSupersetRole"]
}

# Sync roles at login
AUTH_ROLES_SYNC_AT_LOGIN = True

This configuration ensures that users logging in via OAuth are assigned roles based on their OAuth group, making the role field mandatory without modifying the codebase [1][2].

To continue talking to Dosu, mention @dosu.

MalavikaN1 commented 3 months ago

@dosu this is not working

dosubot[bot] commented 3 months ago

To ensure that the role field is mandatory for users logging in via OAuth in Superset without modifying the codebase, you can configure the AUTH_USER_REGISTRATION_ROLE and AUTH_ROLES_SYNC_AT_LOGIN settings in your superset_config.py file. This will enforce role assignment during user registration and synchronization at login.

Here is how you can configure it:

# Enable user self-registration
AUTH_USER_REGISTRATION = True

# Set the default user self-registration role
AUTH_USER_REGISTRATION_ROLE = "Public"

# Ensure roles are synced at every login
AUTH_ROLES_SYNC_AT_LOGIN = True

# Map OAuth groups to Superset roles
AUTH_ROLES_MAPPING = {
    "superset_users": ["Gamma", "Alpha"],
    "superset_admins": ["Admin"],
}

This configuration ensures that every user who logs in via OAuth will have a role assigned, and their roles will be synchronized at each login. Additionally, you can refer to the main configuration file for more details on authentication and role settings [1][2].

To continue talking to Dosu, mention @dosu.

MalavikaN1 commented 3 months ago

@dosu I can still see that no role is required for a user and it is giving 500 internal server exception

dosubot[bot] commented 3 months ago

To ensure that the role field is mandatory for users logging in via OAuth and prevent the 500 Internal Server Error when a user without a role logs in, you can customize the oauth_user_info method in your CustomSsoSecurityManager to check for the presence of a role and handle the case where a role is missing.

Here is an example of how you can modify the CustomSsoSecurityManager to enforce this requirement:

import logging
from superset.security import SupersetSecurityManager
from flask_appbuilder.security.sqla.models import Role

class CustomSsoSecurityManager(SupersetSecurityManager):
    def oauth_user_info(self, provider, response=None):
        logging.debug("Oauth2 provider: {0}.".format(provider))
        if provider == 'your_oauth_provider':
            me = self.appbuilder.sm.oauth_remotes[provider].get('userDetails').data
            logging.debug("user_data: {0}".format(me))

            # Ensure the user has a role
            if 'role' not in me or not me['role']:
                raise Exception("User does not have a role assigned")

            # Map the role from the OAuth response to a Superset role
            role_name = me['role']
            role = self.find_role(role_name)
            if not role:
                raise Exception(f"Role {role_name} does not exist in Superset")

            return {
                'name': me['name'],
                'email': me['email'],
                'id': me['user_name'],
                'username': me['user_name'],
                'first_name': '',
                'last_name': '',
                'role': role_name
            }

# In your superset_config.py
from custom_sso_security_manager import CustomSsoSecurityManager
CUSTOM_SECURITY_MANAGER = CustomSsoSecurityManager

In this example, the oauth_user_info method checks if the role field is present in the OAuth response. If the role is missing, it raises an exception. Additionally, it maps the role from the OAuth response to a Superset role and ensures that the role exists in Superset. If the role does not exist, it raises an exception. This ensures that users without a role cannot log in, preventing the 500 Internal Server Error [1].

To continue talking to Dosu, mention @dosu.