houqp commented on a change in pull request #10594:
URL: https://github.com/apache/airflow/pull/10594#discussion_r483298085
##########
File path: airflow/api_connexion/security.py
##########
@@ -16,25 +16,61 @@
# under the License.
from functools import wraps
-from typing import Callable, TypeVar, cast
+from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast
from flask import Response, current_app
-from airflow.api_connexion.exceptions import Unauthenticated
+from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated
T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
-def requires_authentication(function: T):
- """Decorator for functions that require authentication"""
+def check_authentication():
+ """Checks that the request has valid authorization information."""
+ response = current_app.api_auth.requires_authentication(Response)()
+ if response.status_code != 200:
+ # since this handler only checks authentication, not authorization,
+ # we should always return 401
+ raise Unauthenticated(headers=response.headers)
- @wraps(function)
- def decorated(*args, **kwargs):
- response = current_app.api_auth.requires_authentication(Response)()
- if response.status_code != 200:
- # since this handler only checks authentication, not authorization,
- # we should always return 401
- raise Unauthenticated(headers=response.headers)
- return function(*args, **kwargs)
- return cast(T, decorated)
+def check_authorization(permissions=None, dag_id=None):
Review comment:
nitpick, could you add type annotations to this function?
##########
File path: airflow/api_connexion/security.py
##########
@@ -16,25 +16,61 @@
# under the License.
from functools import wraps
-from typing import Callable, TypeVar, cast
+from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast
from flask import Response, current_app
-from airflow.api_connexion.exceptions import Unauthenticated
+from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated
T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
-def requires_authentication(function: T):
- """Decorator for functions that require authentication"""
+def check_authentication():
+ """Checks that the request has valid authorization information."""
+ response = current_app.api_auth.requires_authentication(Response)()
+ if response.status_code != 200:
+ # since this handler only checks authentication, not authorization,
+ # we should always return 401
+ raise Unauthenticated(headers=response.headers)
- @wraps(function)
- def decorated(*args, **kwargs):
- response = current_app.api_auth.requires_authentication(Response)()
- if response.status_code != 200:
- # since this handler only checks authentication, not authorization,
- # we should always return 401
- raise Unauthenticated(headers=response.headers)
- return function(*args, **kwargs)
- return cast(T, decorated)
+def check_authorization(permissions=None, dag_id=None):
+ """Checks that the logged in user has the specified permissions."""
+
+ if not permissions:
+ permissions = []
Review comment:
shouldn't we doing an early return here instead of creating an empty
permissions list?
##########
File path: airflow/www/security.py
##########
@@ -521,6 +524,17 @@ def sync_roles(self):
self.update_admin_perm_view()
self.clean_perms()
+ def sync_resource_permissions(self, permissions=None):
+ """
+ Populates resource-based permissions.
+ """
+ if not permissions:
+ permissions = []
Review comment:
same as my previous comment, seems like we can just do early return here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
