feat: support default group permission

[grudloffev]

Summary:

This pull request addresses issue #83. The proposed solution introduces a new environment variable, DEFAULT_MLFLOW_GROUP_PERMISSION, that defines the default group permission assigned to assets created by members of a group. This enhancement facilitates scenarios where elevated access is granted within a group (i.e. READ or MANAGE) while restricting the permissions for users from other groups by using the fallback with lower access (i.e. NO_PERMISSIONS or READ).

Currently, upon creation of assets, no group permission is assigned, and therefore the fallback defined by the DEFAULT_MLFLOW_PERMISSION environment variable is effectively used. To achieve the aforementioned use cases, the group permissions should be assigned manually over the assets, either by the end user or through a separate service.

Details

• New Feature: Introduced the DEFAULT_MLFLOW_GROUP_PERMISSION environment variable.
• Purpose: Default group permission set on experiment and model creation.
• Example configuration:

export DEFAULT_MLFLOW_PERMISSION="NO_PERMISSIONS"
export DEFAULT_MLFLOW_GROUP_PERMISSION="READ"

• Impact: This setting enables use cases where group members have shared access, while ensuring that different groups have limited access.

Implementation:

• Previous behaviour is maintained when environment variable is unset.
• New environment variable is added to the config (mlflow-oidc-auth/config.py).
• New get_user_groups util function (mlflow-oidc-auth/utils.py).
• In callback, user groups are added to the session variable (mlflow-oidc-auth/views/authentication.py).
• Experiment and Model creation after hooks are modified so that the the default group permission is assigned over the user groups (mlflow-oidc-auth/hooks/after_request.py).
• Model deletion after hook is modified to remove the group permissions (mlflow-oidc-auth/hooks/after_request.py).
• The after hook _set_managed_*_permission is renamed to _set_initial_*_permission to better reflect the new functionality.
• Added DEFAULT_MLFLOW_PERMISSION and DEFAULT_MLFLOW_GROUP_PERMISSION descriptions in readme and PyPI docs.
• Tests for all covered scenarios of get_user_groups (mlflow-oidc-auth/tests/test_utils.py).

Benefits:

• Enhanced Flexibility: Users can now configure default permissions at the group level, providing more granular control over access management.
• Improved Security: By isolating access between groups, organizations can better manage data security and privacy.
• Ease of Use: The environment variable approach simplifies configuration.

TODO:

• Working feature implementation
• Rebase once feat: group permission fix #87 (or an equivalent PR) is added to main.
• Add documentation
• Add tests

Additional Notes:

• The current implementation and it's compatibility may need to be revisited after the discussion at Permissions based on groups not always calculated correctly in case of multiple group memberships #80 is finished.

[grudloffev]

Hi @kharkevich, I implemented this feature for one use case and just wanted to share in case it is useful here. Please let me now if you think it aligns with the vision of the repo and I will be happy to continue working on it.

[grudloffev]

Hi @kharkevich, I see the latest release made some modifications that need to be addressed. Please let me know if you think this might be merged as I would be happy to work on a rebase

[grudloffev]

Removed use of session to store and retrieve user groups, aligning with master. Guessing there is a tradeoff here between efficiency (storing in session) and having the most up to date info.

I can check afterwards if adding the get_user_groups from utils.py to handle_user_and_group_management in auth.py makes sense.

[Copilot]

Pull Request Overview

This PR introduces default group permission support for experiments and models by adding the DEFAULT_MLFLOW_GROUP_PERMISSION environment variable and updating the corresponding hooks and tests. Key changes include:

• Adding new utility functions (get_user_groups and filter_groups) to retrieve and filter user groups.
• Refactoring after-request hooks to assign group permissions during experiment/model creation and deletion.
• Updating configuration files, documentation, and tests to support the new default group permission setting.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
mlflow_oidc_auth/utils.py	Added functions get_user_groups and filter_groups to enable group permission handling.
mlflow_oidc_auth/tests/test_utils.py	Updated tests to cover group permission detection via plugin, bearer token, and store retrieval.
mlflow_oidc_auth/hooks/after_request.py	Refactored and renamed hooks to assign initial group permissions and adjust deletion handling.
mlflow_oidc_auth/config.py	Added new environment variable DEFAULT_MLFLOW_GROUP_PERMISSION.
docs/configuration/index.md, README.md	Updated configuration documentation to include the new default group permission.

Comments suppressed due to low confidence (3)

mlflow_oidc_auth/hooks/after_request.py:40

• Consider explicitly comparing 'config.DEFAULT_MLFLOW_GROUP_PERMISSION' against None rather than relying solely on truthiness. This ensures that an empty string, if intended as a valid permission, is handled correctly.

if permission := config.DEFAULT_MLFLOW_GROUP_PERMISSION:

mlflow_oidc_auth/utils.py:346

• Ensure that 'config.OIDC_ADMIN_GROUP_NAME' is always defined and not None. If there is a possibility it might be unset, consider providing a default value or adding validation to avoid unexpected behavior during group filtering.

available_groups = config.OIDC_GROUP_NAME + [config.OIDC_ADMIN_GROUP_NAME]

mlflow_oidc_auth/tests/test_utils.py:243

• Consider verifying that the test_request_context properly sets request.authorization, as setting the header alone may not automatically populate it depending on Flask's configuration. You might need to simulate or inject the authorization object explicitly for more robust testing.

with self.app.test_request_context(headers={'Authorization': 'Bearer test_token'}):

[grudloffev]

just tested the latest rebase and it is working fine as before in my test environment. I just had to do another rebase for the latest fix