From 3744a3cf12c22a9728b0a68d114aa0b31897abe1 Mon Sep 17 00:00:00 2001 From: Vincent <97131062+vincbeck@users.noreply.github.com> Date: Fri, 9 Feb 2024 11:16:35 -0500 Subject: [PATCH] D401 support in fab provider --- .../fab/auth_manager/decorators/auth.py | 2 +- .../fab/auth_manager/fab_auth_manager.py | 2 +- .../auth_manager/security_manager/override.py | 52 +++++++++---------- pyproject.toml | 3 -- 4 files changed, 28 insertions(+), 31 deletions(-) diff --git a/airflow/providers/fab/auth_manager/decorators/auth.py b/airflow/providers/fab/auth_manager/decorators/auth.py index 95f97c8e795904..7089be08fc56df 100644 --- a/airflow/providers/fab/auth_manager/decorators/auth.py +++ b/airflow/providers/fab/auth_manager/decorators/auth.py @@ -67,7 +67,7 @@ def decorated(*args, **kwargs): def _has_access_fab(permissions: Sequence[tuple[str, str]] | None = None) -> Callable[[T], T]: """ - Factory for decorator that checks current user's permissions against required permissions. + Check current user's permissions against required permissions. This decorator is only kept for backward compatible reasons. The decorator ``airflow.www.auth.has_access``, which redirects to this decorator, is widely used in user plugins. diff --git a/airflow/providers/fab/auth_manager/fab_auth_manager.py b/airflow/providers/fab/auth_manager/fab_auth_manager.py index dfa53ef78b5b08..696709ae6cad8e 100644 --- a/airflow/providers/fab/auth_manager/fab_auth_manager.py +++ b/airflow/providers/fab/auth_manager/fab_auth_manager.py @@ -446,7 +446,7 @@ def _get_fab_resource_types(dag_access_entity: DagAccessEntity) -> tuple[str, .. def _resource_name_for_dag(self, dag_id: str) -> str: """ - Returns the FAB resource name for a DAG id. + Return the FAB resource name for a DAG id. :param dag_id: the DAG id diff --git a/airflow/providers/fab/auth_manager/security_manager/override.py b/airflow/providers/fab/auth_manager/security_manager/override.py index 9fe89f8a69edbf..6f5c0f72c66985 100644 --- a/airflow/providers/fab/auth_manager/security_manager/override.py +++ b/airflow/providers/fab/auth_manager/security_manager/override.py @@ -531,7 +531,7 @@ def auth_rate_limit(self) -> str: @property def auth_role_public(self): - """Gets the public role.""" + """Get the public role.""" return self.appbuilder.app.config["AUTH_ROLE_PUBLIC"] @property @@ -571,7 +571,7 @@ def auth_ldap_tls_demand(self): @property def auth_ldap_server(self): - """Gets the LDAP server object.""" + """Get the LDAP server object.""" return self.appbuilder.get_app.config["AUTH_LDAP_SERVER"] @property @@ -650,7 +650,7 @@ def api_login_allow_multiple_providers(self): @property def auth_username_ci(self): - """Gets the auth username for CI.""" + """Get the auth username for CI.""" return self.appbuilder.get_app.config.get("AUTH_USERNAME_CI", True) @property @@ -685,7 +685,7 @@ def auth_roles_sync_at_login(self) -> bool: @property def auth_role_admin(self): - """Gets the admin role.""" + """Get the admin role.""" return self.appbuilder.get_app.config["AUTH_ROLE_ADMIN"] @property @@ -697,7 +697,7 @@ def oauth_whitelists(self): return self.oauth_allow_list def create_builtin_roles(self): - """Returns FAB builtin roles.""" + """Return FAB builtin roles.""" return self.appbuilder.app.config.get("FAB_ROLES", {}) @property @@ -1445,7 +1445,7 @@ def add_user( password="", hashed_password="", ): - """Generic function to create user.""" + """Create a user.""" try: user = self.user_model() user.first_name = first_name @@ -1504,7 +1504,7 @@ def add_register_user(self, username, first_name, last_name, email, password="", return None def find_user(self, username=None, email=None): - """Finds user by username or email.""" + """Find user by username or email.""" if username: try: if self.auth_username_ci: @@ -1549,7 +1549,7 @@ def update_user(self, user: User) -> bool: def del_register_user(self, register_user): """ - Deletes registration object from database. + Delete registration object from database. :param register_user: RegisterUser object to delete """ @@ -1598,7 +1598,7 @@ def update_user_auth_stat(self, user, success=True): def get_action(self, name: str) -> Action: """ - Gets an existing action record. + Get an existing action record. :param name: name """ @@ -1606,7 +1606,7 @@ def get_action(self, name: str) -> Action: def create_action(self, name): """ - Adds an action to the backend, model action. + Add an action to the backend, model action. :param name: name of the action: 'can_add','can_edit' etc... @@ -1626,7 +1626,7 @@ def create_action(self, name): def delete_action(self, name: str) -> bool: """ - Deletes a permission action. + Delete a permission action. :param name: Name of action to delete (e.g. can_read). """ @@ -1659,7 +1659,7 @@ def delete_action(self, name: str) -> bool: def get_resource(self, name: str) -> Resource: """ - Returns a resource record by name, if it exists. + Return a resource record by name, if it exists. :param name: Name of resource """ @@ -1685,12 +1685,12 @@ def create_resource(self, name) -> Resource: return resource def get_all_resources(self) -> list[Resource]: - """Gets all existing resource records.""" + """Get all existing resource records.""" return self.get_session.query(self.resource_model).all() def delete_resource(self, name: str) -> bool: """ - Deletes a Resource from the backend. + Delete a Resource from the backend. :param name: name of the resource @@ -1728,7 +1728,7 @@ def get_permission( resource_name: str, ) -> Permission | None: """ - Gets a permission made with the given action->resource pair, if the permission already exists. + Get a permission made with the given action->resource pair, if the permission already exists. :param action_name: Name of action :param resource_name: Name of resource @@ -1753,7 +1753,7 @@ def get_resource_permissions(self, resource: Resource) -> Permission: def create_permission(self, action_name, resource_name) -> Permission | None: """ - Adds a permission on a resource to the backend. + Add a permission on a resource to the backend. :param action_name: name of the action to add: 'can_add','can_edit' etc... @@ -1781,7 +1781,7 @@ def create_permission(self, action_name, resource_name) -> Permission | None: def delete_permission(self, action_name: str, resource_name: str) -> None: """ - Deletes the permission linking an action->resource pair. + Delete the permission linking an action->resource pair. Doesn't delete the underlying action or resource. @@ -1846,7 +1846,7 @@ def remove_permission_from_role(self, role: Role, permission: Permission) -> Non self.get_session.rollback() def get_oid_identity_url(self, provider_name: str) -> str | None: - """Returns the OIDC identity provider URL.""" + """Return the OIDC identity provider URL.""" for provider in self.openid_providers: if provider.get("name") == provider_name: return provider.get("url") @@ -2091,7 +2091,7 @@ def oauth_user_info_getter( func: Callable[[AirflowSecurityManagerV2, str, dict[str, Any] | None], dict[str, Any]], ): """ - Decorator function to be the OAuth user info getter for all the providers. + Get OAuth user info for all the providers. Receives provider and response return a dict with the information returned from the provider. The returned user info dict should have its keys with the same name as the User Model. @@ -2210,7 +2210,7 @@ def get_oauth_user_info(self, provider: str, resp: dict[str, Any]) -> dict[str, @staticmethod def oauth_token_getter(): - """Authentication (OAuth) token getter function.""" + """Get authentication (OAuth) token.""" token = session.get("oauth") log.debug("Token Get: %s", token) return token @@ -2220,7 +2220,7 @@ def check_authorization( perms: Sequence[tuple[str, str]] | None = None, dag_id: str | None = None, ) -> bool: - """Checks that the logged in user has the specified permissions.""" + """Check the logged-in user has the specified permissions.""" if not perms: return True @@ -2254,7 +2254,7 @@ def set_oauth_session(self, provider, oauth_response): def get_oauth_token_key_name(self, provider): """ - Returns the token_key name for the oauth provider. + Return the token_key name for the oauth provider. If none is configured defaults to oauth_token this is configured using OAUTH_PROVIDERS and token_key key. @@ -2275,7 +2275,7 @@ def get_oauth_token_secret_name(self, provider): def auth_user_oauth(self, userinfo): """ - Method for authenticating user with OAuth. + Authenticate user with OAuth. :userinfo: dict with user information (keys are the same as User model columns) @@ -2608,7 +2608,7 @@ def _get_user_permission_resources( return result def _has_access_builtin_roles(self, role, action_name: str, resource_name: str) -> bool: - """Checks permission on builtin role.""" + """Check permission on builtin role.""" perms = self.builtin_roles.get(role.name, []) for _resource_name, _action_name in perms: if re2.match(_resource_name, resource_name) and re2.match(_action_name, action_name): @@ -2647,7 +2647,7 @@ def _get_all_non_dag_permissions(self) -> dict[tuple[str, str], Permission]: """ Get permissions except those that are for specific DAGs. - Returns a dict with a key of (action_name, resource_name) and value of permission + Return a dict with a key of (action_name, resource_name) and value of permission with all permissions except those that are for specific DAGs. """ return { @@ -2689,7 +2689,7 @@ def _get_root_dag_id(self, dag_id: str) -> str: @staticmethod def _cli_safe_flash(text: str, level: str) -> None: - """Shows a flash in a web context or prints a message if not.""" + """Show a flash in a web context or prints a message if not.""" if has_request_context(): flash(Markup(text), level) else: diff --git a/pyproject.toml b/pyproject.toml index 1856aac2a4f34c..cad94c51fec142 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1373,9 +1373,6 @@ combine-as-imports = true "airflow/providers/common/io/xcom/backend.py" = ["D401"] "airflow/providers/databricks/hooks/databricks.py" = ["D401"] "airflow/providers/databricks/operators/databricks.py" = ["D401"] -"airflow/providers/fab/auth_manager/decorators/auth.py" = ["D401"] -"airflow/providers/fab/auth_manager/fab_auth_manager.py" = ["D401"] -"airflow/providers/fab/auth_manager/security_manager/override.py" = ["D401"] "airflow/providers/google/cloud/hooks/automl.py" = ["D401"] "airflow/providers/google/cloud/hooks/bigquery.py" = ["D401"] "airflow/providers/google/cloud/hooks/bigquery_dts.py" = ["D401"]