From 35e3609e122b92171c4b1c9e6677d20c3f86167a Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Sat, 13 Feb 2021 01:20:41 +0100 Subject: [PATCH 01/26] Create a login endpoint with JWT authentication --- airflow/api_connexion/openapi/v1.yaml | 27 +++++++ airflow/api_connexion/schemas/role_schema.py | 53 ++++++++++++++ airflow/api_connexion/security.py | 9 ++- airflow/api_connexion/webserver_auth.py | 77 ++++++++++++++++++++ tests/api_connexion/test_webserver_auth.py | 75 +++++++++++++++++++ 5 files changed, 238 insertions(+), 3 deletions(-) create mode 100644 airflow/api_connexion/schemas/role_schema.py create mode 100644 airflow/api_connexion/webserver_auth.py create mode 100644 tests/api_connexion/test_webserver_auth.py diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 7885e8d912c5a..6e293d245a803 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1490,6 +1490,32 @@ paths: '404': $ref: '#/components/responses/NotFound' + /login: + post: + summary: User login + description: | + Verify user and return a user object and JWT token as well + x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint + operationId: login + tags: [User] + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/Login' + responses: + '200': + description: Success. + content: + application/json: + schema: + $ref: '#/components/schemas/UserLogin' + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthenticated' + components: # Reusable schemas (data models) schemas: @@ -3296,6 +3322,7 @@ tags: - name: Role - name: Permission - name: User + - name: Auth externalDocs: url: https://airflow.apache.org/docs/apache-airflow/stable/ diff --git a/airflow/api_connexion/schemas/role_schema.py b/airflow/api_connexion/schemas/role_schema.py new file mode 100644 index 0000000000000..77f83600f084f --- /dev/null +++ b/airflow/api_connexion/schemas/role_schema.py @@ -0,0 +1,53 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import List, NamedTuple + +from flask_appbuilder.security.sqla.models import Role +from marshmallow import Schema, fields +from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field + + +class RoleCollectionItemSchema(SQLAlchemySchema): + """Role item shema""" + + class Meta: + """Meta""" + + model = Role + + id = auto_field(dump_only=True) + name = auto_field() + permissions = auto_field() + + +class RoleCollection(NamedTuple): + """List of roles""" + + roles: List[Role] + total_entries: int + + +class RoleCollectionSchema(Schema): + """List of roles""" + + roles = fields.List(fields.Nested(RoleCollectionItemSchema)) + total_entries = fields.Int() + + +role_collection_item_schema = RoleCollectionSchema() +role_collection_schema = RoleCollectionSchema() diff --git a/airflow/api_connexion/security.py b/airflow/api_connexion/security.py index 4faa9ed8f3457..0d97880098bb4 100644 --- a/airflow/api_connexion/security.py +++ b/airflow/api_connexion/security.py @@ -14,13 +14,13 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. - from functools import wraps from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast from flask import Response, current_app from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated +from airflow.api_connexion.webserver_auth import requires_authentication T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name @@ -28,10 +28,13 @@ def check_authentication() -> None: """Checks that the request has valid authorization information.""" response = current_app.api_auth.requires_authentication(Response)() - if response.status_code != 200: + jwt_response = requires_authentication(Response)() + if response.status_code != 200 and jwt_response.status_code != 200: # since this handler only checks authentication, not authorization, # we should always return 401 - raise Unauthenticated(headers=response.headers) + if response.status_code != 200: + raise Unauthenticated(headers=response.headers) + raise Unauthenticated(headers=jwt_response.headers) def requires_access(permissions: Optional[Sequence[Tuple[str, str]]] = None) -> Callable[[T], T]: diff --git a/airflow/api_connexion/webserver_auth.py b/airflow/api_connexion/webserver_auth.py new file mode 100644 index 0000000000000..e96fe56b8d211 --- /dev/null +++ b/airflow/api_connexion/webserver_auth.py @@ -0,0 +1,77 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from datetime import datetime, timedelta +from functools import wraps +from typing import Callable, TypeVar, cast + +import jwt +from flask import Response, current_app, request + +from airflow.configuration import conf + +SECRET = conf.get("webserver", "secret_key") +T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name + + +def encode_auth_token(user_id): + """Generate authentication token""" + expire_time = conf.getint("webserver", "session_lifetime_minutes") + payload = { + 'exp': datetime.utcnow() + timedelta(minutes=expire_time), + 'iat': datetime.utcnow(), + 'sub': user_id, + } + return jwt.encode(payload, SECRET, algorithm='HS256') + + +def decode_auth_token(auth_token): + """Decode authentication token""" + try: + payload = jwt.decode(auth_token, SECRET, algorithms=['HS256']) + return payload['sub'] + except (jwt.ExpiredSignatureError, jwt.InvalidTokenError): + return + + +def auth_current_user(): + """Checks the authentication and return the current user""" + auth = request.headers.get("Authorization", None) + token = None + if auth: + try: + token = auth.split(" ")[1] + except IndexError: + return + if token: + user = decode_auth_token(token) + if not isinstance(user, int): + user = None + return user + + +def requires_authentication(function: T): + """Decorator for functions that require authentication""" + + @wraps(function) + def decorated(*args, **kwargs): + if auth_current_user() is not None: + return function(*args, **kwargs) + else: + return Response("Unauthorized", 401, {"WWW-Authenticate": "Bearer"}) + + return cast(T, decorated) diff --git a/tests/api_connexion/test_webserver_auth.py b/tests/api_connexion/test_webserver_auth.py new file mode 100644 index 0000000000000..888664249efbe --- /dev/null +++ b/tests/api_connexion/test_webserver_auth.py @@ -0,0 +1,75 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import unittest + +from flask_login import current_user + +from airflow.www.app import create_app +from tests.test_utils.config import conf_vars + + +class TestWebserverAuth(unittest.TestCase): + def setUp(self) -> None: + with conf_vars( + {("webserver", "session_lifetime_minutes"): "1", ("webserver", "secret_key"): "secret"} + ): + self.app = create_app(testing=True) + + self.appbuilder = self.app.appbuilder # pylint: disable=no-member + role_admin = self.appbuilder.sm.find_role("Admin") + self.tester = self.appbuilder.sm.find_user(username="test") + if not self.tester: + self.appbuilder.sm.add_user( + username="test", + first_name="test", + last_name="test", + email="test@fab.org", + role=role_admin, + password="test", + ) + + def test_successful_login(self): + payload = {"username": "test", "password": "test"} + with self.app.test_client() as test_client: + response = test_client.post("api/v1/login", json=payload) + assert isinstance(response.json["token"], str) + assert response.json['email'] == "test@fab.org" + + def test_can_view_other_endpoints(self): + payload = {"username": "test", "password": "test"} + with self.app.test_client() as test_client: + response = test_client.post("api/v1/login", json=payload) + token = response.json["token"] + + response2 = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + token}) + assert current_user.email == "test@fab.org" + + assert response2.status_code == 200 + assert response2.json == { + "pools": [ + { + "name": "default_pool", + "slots": 128, + "occupied_slots": 0, + "running_slots": 0, + "queued_slots": 0, + "open_slots": 128, + }, + ], + "total_entries": 1, + } From 9a730dab3a46a739f994116e6aa8f474daae2483 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Sat, 13 Feb 2021 01:36:20 +0100 Subject: [PATCH 02/26] fixup! Create a login endpoint with JWT authentication --- tests/api_connexion/test_webserver_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/api_connexion/test_webserver_auth.py b/tests/api_connexion/test_webserver_auth.py index 888664249efbe..ceb6a86ddc078 100644 --- a/tests/api_connexion/test_webserver_auth.py +++ b/tests/api_connexion/test_webserver_auth.py @@ -48,7 +48,7 @@ def test_successful_login(self): with self.app.test_client() as test_client: response = test_client.post("api/v1/login", json=payload) assert isinstance(response.json["token"], str) - assert response.json['email'] == "test@fab.org" + assert response.json["user"]['email'] == "test@fab.org" def test_can_view_other_endpoints(self): payload = {"username": "test", "password": "test"} From 382dcd4d668441ff0cbb3739fdd89976be58696c Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Sun, 14 Feb 2021 12:24:54 +0100 Subject: [PATCH 03/26] fixup! fixup! Create a login endpoint with JWT authentication --- airflow/api_connexion/schemas/role_schema.py | 45 ++++++++++++++++++-- airflow/api_connexion/webserver_auth.py | 12 ++++-- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/airflow/api_connexion/schemas/role_schema.py b/airflow/api_connexion/schemas/role_schema.py index 77f83600f084f..06e3d9e78cb5f 100644 --- a/airflow/api_connexion/schemas/role_schema.py +++ b/airflow/api_connexion/schemas/role_schema.py @@ -17,11 +17,48 @@ from typing import List, NamedTuple -from flask_appbuilder.security.sqla.models import Role +from flask_appbuilder.security.sqla.models import Permission, PermissionView, Role, ViewMenu from marshmallow import Schema, fields from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field +class PermissionSchema(SQLAlchemySchema): + """Permission Schema""" + + class Meta: + """Meta""" + + model = Permission + + id = auto_field() + name = auto_field() + + +class ViewMenuSchema(SQLAlchemySchema): + """ViewMenu Schema""" + + class Meta: + """Meta""" + + model = ViewMenu + + id = auto_field() + name = auto_field() + + +class PermissionViewSchema(SQLAlchemySchema): + """Permission View Schema""" + + class Meta: + """Meta""" + + model = PermissionView + + id = auto_field() + permission = fields.Nested(PermissionSchema) + view_menu = fields.Nested(ViewMenuSchema) + + class RoleCollectionItemSchema(SQLAlchemySchema): """Role item shema""" @@ -30,9 +67,9 @@ class Meta: model = Role - id = auto_field(dump_only=True) + id = auto_field() name = auto_field() - permissions = auto_field() + permissions = fields.List(fields.Nested(PermissionViewSchema)) class RoleCollection(NamedTuple): @@ -49,5 +86,5 @@ class RoleCollectionSchema(Schema): total_entries = fields.Int() -role_collection_item_schema = RoleCollectionSchema() +role_collection_item_schema = RoleCollectionItemSchema() role_collection_schema = RoleCollectionSchema() diff --git a/airflow/api_connexion/webserver_auth.py b/airflow/api_connexion/webserver_auth.py index e96fe56b8d211..6974c9a5005d3 100644 --- a/airflow/api_connexion/webserver_auth.py +++ b/airflow/api_connexion/webserver_auth.py @@ -20,8 +20,9 @@ from typing import Callable, TypeVar, cast import jwt -from flask import Response, current_app, request +from flask import Response, request +from airflow.api_connexion.exceptions import BadRequest, PermissionDenied from airflow.configuration import conf SECRET = conf.get("webserver", "secret_key") @@ -44,8 +45,10 @@ def decode_auth_token(auth_token): try: payload = jwt.decode(auth_token, SECRET, algorithms=['HS256']) return payload['sub'] - except (jwt.ExpiredSignatureError, jwt.InvalidTokenError): - return + except jwt.ExpiredSignatureError: + raise BadRequest(detail="The signature is expired, please login again") + except jwt.InvalidTokenError: + raise BadRequest(detail="The token provided is invalid") def auth_current_user(): @@ -56,12 +59,13 @@ def auth_current_user(): try: token = auth.split(" ")[1] except IndexError: - return + raise BadRequest(detail="The authorization header is malformed") if token: user = decode_auth_token(token) if not isinstance(user, int): user = None return user + raise PermissionDenied(detail="You do not have permission to view this resource, please login") def requires_authentication(function: T): From 2652af1f686b1cf04418288d1cd3dfd2636ddabd Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Sun, 14 Feb 2021 13:23:01 +0100 Subject: [PATCH 04/26] separate models into different schemas --- .../schemas/permission_schema.py | 63 +++++++++++++++++++ airflow/api_connexion/schemas/role_schema.py | 39 +----------- .../api_connexion/schemas/view_menu_schema.py | 48 ++++++++++++++ 3 files changed, 113 insertions(+), 37 deletions(-) create mode 100644 airflow/api_connexion/schemas/permission_schema.py create mode 100644 airflow/api_connexion/schemas/view_menu_schema.py diff --git a/airflow/api_connexion/schemas/permission_schema.py b/airflow/api_connexion/schemas/permission_schema.py new file mode 100644 index 0000000000000..d6186f75451a0 --- /dev/null +++ b/airflow/api_connexion/schemas/permission_schema.py @@ -0,0 +1,63 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import List, NamedTuple + +from flask_appbuilder.security.sqla.models import Permission, PermissionView +from marshmallow import Schema, fields +from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field + +from airflow.api_connexion.schemas.view_menu_schema import ViewMenuCollectionItemSchema + + +class PermissionCollectionItemSchema(SQLAlchemySchema): + """Permission Schema""" + + class Meta: + """Meta""" + + model = Permission + + id = auto_field() + name = auto_field() + + +class PermissionCollection(NamedTuple): + """Permission Collection""" + + permissions: List[Permission] + total_entries: int + + +class PermissionCollectionSchema(Schema): + """Permissions list schema""" + + permissions = fields.List(fields.Nested(PermissionCollectionItemSchema)) + total_entries = fields.Int() + + +class PermissionViewSchema(SQLAlchemySchema): + """Permission View Schema""" + + class Meta: + """Meta""" + + model = PermissionView + + id = auto_field() + permission = fields.Nested(PermissionCollectionItemSchema) + view_menu = fields.Nested(ViewMenuCollectionItemSchema) diff --git a/airflow/api_connexion/schemas/role_schema.py b/airflow/api_connexion/schemas/role_schema.py index 06e3d9e78cb5f..3f43f159c9d93 100644 --- a/airflow/api_connexion/schemas/role_schema.py +++ b/airflow/api_connexion/schemas/role_schema.py @@ -17,46 +17,11 @@ from typing import List, NamedTuple -from flask_appbuilder.security.sqla.models import Permission, PermissionView, Role, ViewMenu +from flask_appbuilder.security.sqla.models import Role from marshmallow import Schema, fields from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field - -class PermissionSchema(SQLAlchemySchema): - """Permission Schema""" - - class Meta: - """Meta""" - - model = Permission - - id = auto_field() - name = auto_field() - - -class ViewMenuSchema(SQLAlchemySchema): - """ViewMenu Schema""" - - class Meta: - """Meta""" - - model = ViewMenu - - id = auto_field() - name = auto_field() - - -class PermissionViewSchema(SQLAlchemySchema): - """Permission View Schema""" - - class Meta: - """Meta""" - - model = PermissionView - - id = auto_field() - permission = fields.Nested(PermissionSchema) - view_menu = fields.Nested(ViewMenuSchema) +from airflow.api_connexion.schemas.permission_schema import PermissionViewSchema class RoleCollectionItemSchema(SQLAlchemySchema): diff --git a/airflow/api_connexion/schemas/view_menu_schema.py b/airflow/api_connexion/schemas/view_menu_schema.py new file mode 100644 index 0000000000000..bc1b18f18d1a0 --- /dev/null +++ b/airflow/api_connexion/schemas/view_menu_schema.py @@ -0,0 +1,48 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import List, NamedTuple + +from flask_appbuilder.security.sqla.models import ViewMenu +from marshmallow import Schema, fields +from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field + + +class ViewMenuCollectionItemSchema(SQLAlchemySchema): + """ViewMenu Schema""" + + class Meta: + """Meta""" + + model = ViewMenu + + id = auto_field() + name = auto_field() + + +class ViewMenuCollection(NamedTuple): + """ViewMenu list""" + + view_menus: List[ViewMenu] + total_entries: int + + +class ViewMenuCollectionSchema(Schema): + """ViewMenu list schema""" + + view_menus = fields.List(fields.Nested(ViewMenuCollectionItemSchema)) + total_entries = fields.Int() From a1eac3a294b231d0226d278142fb11fd0195be3e Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 15 Feb 2021 15:13:04 +0100 Subject: [PATCH 05/26] add tests for schema and endpoint --- airflow/api_connexion/webserver_auth.py | 12 +++++------- tests/api_connexion/test_webserver_auth.py | 8 ++------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/airflow/api_connexion/webserver_auth.py b/airflow/api_connexion/webserver_auth.py index 6974c9a5005d3..dcc7834cafb46 100644 --- a/airflow/api_connexion/webserver_auth.py +++ b/airflow/api_connexion/webserver_auth.py @@ -22,7 +22,6 @@ import jwt from flask import Response, request -from airflow.api_connexion.exceptions import BadRequest, PermissionDenied from airflow.configuration import conf SECRET = conf.get("webserver", "secret_key") @@ -45,10 +44,9 @@ def decode_auth_token(auth_token): try: payload = jwt.decode(auth_token, SECRET, algorithms=['HS256']) return payload['sub'] - except jwt.ExpiredSignatureError: - raise BadRequest(detail="The signature is expired, please login again") - except jwt.InvalidTokenError: - raise BadRequest(detail="The token provided is invalid") + except (jwt.ExpiredSignatureError, jwt.InvalidTokenError): + # Do not raise exception here due to auth_backend auth + return def auth_current_user(): @@ -59,13 +57,13 @@ def auth_current_user(): try: token = auth.split(" ")[1] except IndexError: - raise BadRequest(detail="The authorization header is malformed") + # Do not raise exception here due to auth_backend auth + return if token: user = decode_auth_token(token) if not isinstance(user, int): user = None return user - raise PermissionDenied(detail="You do not have permission to view this resource, please login") def requires_authentication(function: T): diff --git a/tests/api_connexion/test_webserver_auth.py b/tests/api_connexion/test_webserver_auth.py index ceb6a86ddc078..35baaca70178a 100644 --- a/tests/api_connexion/test_webserver_auth.py +++ b/tests/api_connexion/test_webserver_auth.py @@ -25,9 +25,7 @@ class TestWebserverAuth(unittest.TestCase): def setUp(self) -> None: - with conf_vars( - {("webserver", "session_lifetime_minutes"): "1", ("webserver", "secret_key"): "secret"} - ): + with conf_vars({("webserver", "session_lifetime_minutes"): "1"}): self.app = create_app(testing=True) self.appbuilder = self.app.appbuilder # pylint: disable=no-member @@ -54,11 +52,9 @@ def test_can_view_other_endpoints(self): payload = {"username": "test", "password": "test"} with self.app.test_client() as test_client: response = test_client.post("api/v1/login", json=payload) + assert current_user.email == "test@fab.org" token = response.json["token"] - response2 = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + token}) - assert current_user.email == "test@fab.org" - assert response2.status_code == 200 assert response2.json == { "pools": [ From bd11dcca3ec810dbe62393aedade0c30cb6088ce Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 16 Feb 2021 02:39:57 +0100 Subject: [PATCH 06/26] fix api spec and apply more review suggestions --- airflow/api_connexion/openapi/v1.yaml | 12 +++- .../schemas/permission_schema.py | 59 ++++++++++++++++++- airflow/api_connexion/schemas/role_schema.py | 55 ----------------- .../api_connexion/schemas/view_menu_schema.py | 48 --------------- 4 files changed, 68 insertions(+), 106 deletions(-) delete mode 100644 airflow/api_connexion/schemas/role_schema.py delete mode 100644 airflow/api_connexion/schemas/view_menu_schema.py diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 6e293d245a803..4685c35535190 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1503,7 +1503,17 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Login' + type: object + properties: + username: + description: The username of the user + type: string + nullable: false + + password: + description: The password of the user + type: string + nullable: false responses: '200': description: Success. diff --git a/airflow/api_connexion/schemas/permission_schema.py b/airflow/api_connexion/schemas/permission_schema.py index d6186f75451a0..21daf16d5ec90 100644 --- a/airflow/api_connexion/schemas/permission_schema.py +++ b/airflow/api_connexion/schemas/permission_schema.py @@ -17,11 +17,35 @@ from typing import List, NamedTuple -from flask_appbuilder.security.sqla.models import Permission, PermissionView +from flask_appbuilder.security.sqla.models import Permission, PermissionView, Role, ViewMenu from marshmallow import Schema, fields from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field -from airflow.api_connexion.schemas.view_menu_schema import ViewMenuCollectionItemSchema + +class ViewMenuCollectionItemSchema(SQLAlchemySchema): + """ViewMenu Schema""" + + class Meta: + """Meta""" + + model = ViewMenu + + id = auto_field() + name = auto_field() + + +class ViewMenuCollection(NamedTuple): + """ViewMenu list""" + + view_menus: List[ViewMenu] + total_entries: int + + +class ViewMenuCollectionSchema(Schema): + """ViewMenu list schema""" + + view_menus = fields.List(fields.Nested(ViewMenuCollectionItemSchema)) + total_entries = fields.Int() class PermissionCollectionItemSchema(SQLAlchemySchema): @@ -61,3 +85,34 @@ class Meta: id = auto_field() permission = fields.Nested(PermissionCollectionItemSchema) view_menu = fields.Nested(ViewMenuCollectionItemSchema) + + +class RoleCollectionItemSchema(SQLAlchemySchema): + """Role item shema""" + + class Meta: + """Meta""" + + model = Role + + id = auto_field() + name = auto_field() + permissions = fields.List(fields.Nested(PermissionViewSchema)) + + +class RoleCollection(NamedTuple): + """List of roles""" + + roles: List[Role] + total_entries: int + + +class RoleCollectionSchema(Schema): + """List of roles""" + + roles = fields.List(fields.Nested(RoleCollectionItemSchema)) + total_entries = fields.Int() + + +role_collection_item_schema = RoleCollectionItemSchema() +role_collection_schema = RoleCollectionSchema() diff --git a/airflow/api_connexion/schemas/role_schema.py b/airflow/api_connexion/schemas/role_schema.py deleted file mode 100644 index 3f43f159c9d93..0000000000000 --- a/airflow/api_connexion/schemas/role_schema.py +++ /dev/null @@ -1,55 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -from typing import List, NamedTuple - -from flask_appbuilder.security.sqla.models import Role -from marshmallow import Schema, fields -from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field - -from airflow.api_connexion.schemas.permission_schema import PermissionViewSchema - - -class RoleCollectionItemSchema(SQLAlchemySchema): - """Role item shema""" - - class Meta: - """Meta""" - - model = Role - - id = auto_field() - name = auto_field() - permissions = fields.List(fields.Nested(PermissionViewSchema)) - - -class RoleCollection(NamedTuple): - """List of roles""" - - roles: List[Role] - total_entries: int - - -class RoleCollectionSchema(Schema): - """List of roles""" - - roles = fields.List(fields.Nested(RoleCollectionItemSchema)) - total_entries = fields.Int() - - -role_collection_item_schema = RoleCollectionItemSchema() -role_collection_schema = RoleCollectionSchema() diff --git a/airflow/api_connexion/schemas/view_menu_schema.py b/airflow/api_connexion/schemas/view_menu_schema.py deleted file mode 100644 index bc1b18f18d1a0..0000000000000 --- a/airflow/api_connexion/schemas/view_menu_schema.py +++ /dev/null @@ -1,48 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -from typing import List, NamedTuple - -from flask_appbuilder.security.sqla.models import ViewMenu -from marshmallow import Schema, fields -from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field - - -class ViewMenuCollectionItemSchema(SQLAlchemySchema): - """ViewMenu Schema""" - - class Meta: - """Meta""" - - model = ViewMenu - - id = auto_field() - name = auto_field() - - -class ViewMenuCollection(NamedTuple): - """ViewMenu list""" - - view_menus: List[ViewMenu] - total_entries: int - - -class ViewMenuCollectionSchema(Schema): - """ViewMenu list schema""" - - view_menus = fields.List(fields.Nested(ViewMenuCollectionItemSchema)) - total_entries = fields.Int() From 19284fc536c33e2210058660515609b28df05bac Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 16 Feb 2021 15:53:46 +0100 Subject: [PATCH 07/26] remove username and password schema in api spec and use headers as in review --- airflow/api_connexion/openapi/v1.yaml | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 4685c35535190..a1af437792966 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1498,22 +1498,7 @@ paths: x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint operationId: login tags: [User] - requestBody: - required: true - content: - application/json: - schema: - type: object - properties: - username: - description: The username of the user - type: string - nullable: false - - password: - description: The password of the user - type: string - nullable: false + responses: '200': description: Success. From e37316d7724bdacec0076743eee1a63456abe9e2 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 16 Feb 2021 23:56:18 +0100 Subject: [PATCH 08/26] add more tests and improve code --- tests/api_connexion/test_webserver_auth.py | 29 +++++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/tests/api_connexion/test_webserver_auth.py b/tests/api_connexion/test_webserver_auth.py index 35baaca70178a..540c8f27fa6f4 100644 --- a/tests/api_connexion/test_webserver_auth.py +++ b/tests/api_connexion/test_webserver_auth.py @@ -16,12 +16,19 @@ # under the License. import unittest +from base64 import b64encode +from datetime import datetime, timedelta +import jwt from flask_login import current_user +from airflow.configuration import conf from airflow.www.app import create_app from tests.test_utils.config import conf_vars +# We need to test with real secret for the none algorithm +SECRET = conf.get("webserver", "secret_key") + class TestWebserverAuth(unittest.TestCase): def setUp(self) -> None: @@ -42,16 +49,16 @@ def setUp(self) -> None: ) def test_successful_login(self): - payload = {"username": "test", "password": "test"} + token = "Basic " + b64encode(b"test:test").decode() with self.app.test_client() as test_client: - response = test_client.post("api/v1/login", json=payload) + response = test_client.post("api/v1/login", headers={"Authorization": token}) assert isinstance(response.json["token"], str) assert response.json["user"]['email'] == "test@fab.org" def test_can_view_other_endpoints(self): - payload = {"username": "test", "password": "test"} + token = "Basic " + b64encode(b"test:test").decode() with self.app.test_client() as test_client: - response = test_client.post("api/v1/login", json=payload) + response = test_client.post("api/v1/login", headers={"Authorization": token}) assert current_user.email == "test@fab.org" token = response.json["token"] response2 = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + token}) @@ -69,3 +76,17 @@ def test_can_view_other_endpoints(self): ], "total_entries": 1, } + + def test_raises_for_the_none_algorithm(self): + token = "Basic " + b64encode(b"test:test").decode() + payload = { + 'exp': datetime.utcnow() + timedelta(minutes=10), + 'iat': datetime.utcnow(), + 'sub': self.tester.id, + } + forgedtoken = jwt.encode(payload, key=None, algorithm=None).decode() + with self.app.test_client() as test_client: + test_client.post("api/v1/login", headers={"Authorization": token}) + assert current_user.email == "test@fab.org" + response = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + forgedtoken}) + assert response.status_code == 401 From 71d31d0eab1198bef13b9ef3b3e22a385e902177 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Thu, 18 Feb 2021 17:21:15 +0100 Subject: [PATCH 09/26] Don't handle auth in login endpoint and make login endpoint handle remote_user --- airflow/api_connexion/webserver_auth.py | 28 +++++++++++---- tests/api_connexion/test_webserver_auth.py | 42 +++++++++++++++++++--- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/airflow/api_connexion/webserver_auth.py b/airflow/api_connexion/webserver_auth.py index dcc7834cafb46..1412ab0d81b17 100644 --- a/airflow/api_connexion/webserver_auth.py +++ b/airflow/api_connexion/webserver_auth.py @@ -20,7 +20,9 @@ from typing import Callable, TypeVar, cast import jwt -from flask import Response, request +from flask import Response, current_app, request +from flask_appbuilder.const import AUTH_LDAP +from flask_login import login_user from airflow.configuration import conf @@ -46,24 +48,38 @@ def decode_auth_token(auth_token): return payload['sub'] except (jwt.ExpiredSignatureError, jwt.InvalidTokenError): # Do not raise exception here due to auth_backend auth - return + return None def auth_current_user(): """Checks the authentication and return the current user""" - auth = request.headers.get("Authorization", None) + auth_header = request.headers.get("Authorization", None) + ab_security_manager = current_app.appbuilder.sm token = None - if auth: + user = None + if auth_header: try: - token = auth.split(" ")[1] + token = auth_header.split(" ")[1] except IndexError: # Do not raise exception here due to auth_backend auth - return + return None + if auth_header and auth_header.startswith("Basic"): + auth = request.authorization + if auth is None or not (auth.username and auth.password): + return None + if ab_security_manager.auth_type == AUTH_LDAP: + user = ab_security_manager.auth_user_ldap(auth.username, auth.password) + if user is None: + user = ab_security_manager.auth_user_db(auth.username, auth.password) + if user is not None: + login_user(user, remember=False) + return user if token: user = decode_auth_token(token) if not isinstance(user, int): user = None return user + return None def requires_authentication(function: T): diff --git a/tests/api_connexion/test_webserver_auth.py b/tests/api_connexion/test_webserver_auth.py index 540c8f27fa6f4..07971f7b3b000 100644 --- a/tests/api_connexion/test_webserver_auth.py +++ b/tests/api_connexion/test_webserver_auth.py @@ -21,14 +21,12 @@ import jwt from flask_login import current_user +from parameterized import parameterized -from airflow.configuration import conf from airflow.www.app import create_app +from tests.test_utils.api_connexion_utils import assert_401 from tests.test_utils.config import conf_vars -# We need to test with real secret for the none algorithm -SECRET = conf.get("webserver", "secret_key") - class TestWebserverAuth(unittest.TestCase): def setUp(self) -> None: @@ -90,3 +88,39 @@ def test_raises_for_the_none_algorithm(self): assert current_user.email == "test@fab.org" response = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + forgedtoken}) assert response.status_code == 401 + + @parameterized.expand( + [ + ("basic",), + ("basic ",), + ("bearer",), + ("test:test",), + (b64encode(b"test:test").decode(),), + ("bearer ",), + ("basic: ",), + ("basic 123",), + ] + ) + def test_malformed_headers(self, token): + with self.app.test_client() as test_client: + response = test_client.get("/api/v1/pools", headers={"Authorization": token}) + assert response.status_code == 401 + assert response.headers["Content-Type"] == "application/problem+json" + assert response.headers["WWW-Authenticate"] == "Basic" + assert_401(response) + + @parameterized.expand( + [ + ("basic " + b64encode(b"test").decode(),), + ("basic " + b64encode(b"test:").decode(),), + ("basic " + b64encode(b"test:123").decode(),), + ("basic " + b64encode(b"test test").decode(),), + ] + ) + def test_invalid_auth_header(self, token): + with self.app.test_client() as test_client: + response = test_client.get("/api/v1/pools", headers={"Authorization": token}) + assert response.status_code == 401 + assert response.headers["Content-Type"] == "application/problem+json" + assert response.headers["WWW-Authenticate"] == "Basic" + assert_401(response) From a28dacfba036d21d64d7aed6c1487b1ef45b7466 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Thu, 25 Feb 2021 15:51:20 +0100 Subject: [PATCH 10/26] Update permission names and make login auth more robust --- .../schemas/permission_schema.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/airflow/api_connexion/schemas/permission_schema.py b/airflow/api_connexion/schemas/permission_schema.py index 21daf16d5ec90..273bd98d72426 100644 --- a/airflow/api_connexion/schemas/permission_schema.py +++ b/airflow/api_connexion/schemas/permission_schema.py @@ -22,8 +22,8 @@ from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field -class ViewMenuCollectionItemSchema(SQLAlchemySchema): - """ViewMenu Schema""" +class ResourceCollectionItemSchema(SQLAlchemySchema): + """Permission Resources Schema""" class Meta: """Meta""" @@ -34,17 +34,17 @@ class Meta: name = auto_field() -class ViewMenuCollection(NamedTuple): - """ViewMenu list""" +class ResourceCollection(NamedTuple): + """Permission Resources list""" - view_menus: List[ViewMenu] + resources: List[ViewMenu] total_entries: int -class ViewMenuCollectionSchema(Schema): +class ResourceCollectionSchema(Schema): """ViewMenu list schema""" - view_menus = fields.List(fields.Nested(ViewMenuCollectionItemSchema)) + resources = fields.List(fields.Nested(ResourceCollectionItemSchema)) total_entries = fields.Int() @@ -63,14 +63,14 @@ class Meta: class PermissionCollection(NamedTuple): """Permission Collection""" - permissions: List[Permission] + actions: List[Permission] total_entries: int class PermissionCollectionSchema(Schema): """Permissions list schema""" - permissions = fields.List(fields.Nested(PermissionCollectionItemSchema)) + actions = fields.List(fields.Nested(PermissionCollectionItemSchema)) total_entries = fields.Int() @@ -83,8 +83,8 @@ class Meta: model = PermissionView id = auto_field() - permission = fields.Nested(PermissionCollectionItemSchema) - view_menu = fields.Nested(ViewMenuCollectionItemSchema) + permission = fields.Nested(PermissionCollectionItemSchema, data_key="action") + view_menu = fields.Nested(ResourceCollectionItemSchema, data_key="resource") class RoleCollectionItemSchema(SQLAlchemySchema): From 89f005662059bce750e5d975cd25b8878a082de2 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 26 Feb 2021 09:12:14 +0100 Subject: [PATCH 11/26] Use remote user auth backend in test --- tests/api_connexion/test_webserver_auth.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/api_connexion/test_webserver_auth.py b/tests/api_connexion/test_webserver_auth.py index 07971f7b3b000..9cde45ff0e344 100644 --- a/tests/api_connexion/test_webserver_auth.py +++ b/tests/api_connexion/test_webserver_auth.py @@ -26,11 +26,12 @@ from airflow.www.app import create_app from tests.test_utils.api_connexion_utils import assert_401 from tests.test_utils.config import conf_vars +from tests.test_utils.db import clear_db_pools class TestWebserverAuth(unittest.TestCase): def setUp(self) -> None: - with conf_vars({("webserver", "session_lifetime_minutes"): "1"}): + with conf_vars({("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend"}): self.app = create_app(testing=True) self.appbuilder = self.app.appbuilder # pylint: disable=no-member @@ -45,6 +46,10 @@ def setUp(self) -> None: role=role_admin, password="test", ) + clear_db_pools() + + def tearDown(self) -> None: + clear_db_pools() def test_successful_login(self): token = "Basic " + b64encode(b"test:test").decode() @@ -106,7 +111,6 @@ def test_malformed_headers(self, token): response = test_client.get("/api/v1/pools", headers={"Authorization": token}) assert response.status_code == 401 assert response.headers["Content-Type"] == "application/problem+json" - assert response.headers["WWW-Authenticate"] == "Basic" assert_401(response) @parameterized.expand( @@ -122,5 +126,4 @@ def test_invalid_auth_header(self, token): response = test_client.get("/api/v1/pools", headers={"Authorization": token}) assert response.status_code == 401 assert response.headers["Content-Type"] == "application/problem+json" - assert response.headers["WWW-Authenticate"] == "Basic" assert_401(response) From fd0d320e4457a495e1e5bc26da17595bcee9338e Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 3 Mar 2021 04:09:27 +0100 Subject: [PATCH 12/26] Use flask_jwt_extended, minimize PR and store JWT in httpOnly cookies --- .../api_connexion/endpoints/auth_endpoint.py | 67 ++++++++++ airflow/api_connexion/openapi/v1.yaml | 54 +++++++- .../schemas/permission_schema.py | 118 ------------------ airflow/api_connexion/security.py | 13 +- airflow/api_connexion/webserver_auth.py | 64 +++------- airflow/www/app.py | 8 +- airflow/www/extensions/init_security.py | 15 +++ openapitools.json | 7 ++ ...user_endpoint.py => test_auth_endpoint.py} | 0 tests/api_connexion/test_webserver_auth.py | 23 ++-- 10 files changed, 185 insertions(+), 184 deletions(-) create mode 100644 airflow/api_connexion/endpoints/auth_endpoint.py delete mode 100644 airflow/api_connexion/schemas/permission_schema.py create mode 100644 openapitools.json rename tests/api_connexion/endpoints/{test_user_endpoint.py => test_auth_endpoint.py} (100%) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py new file mode 100644 index 0000000000000..cb95f501ec957 --- /dev/null +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -0,0 +1,67 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from flask import current_app, jsonify +from flask_jwt_extended import ( + create_access_token, + create_refresh_token, + get_jwt_identity, + jwt_refresh_token_required, + set_access_cookies, + set_refresh_cookies, + unset_jwt_cookies, +) + +from airflow.api_connexion import security + + +def login(): + """User login""" + ab_security_manager = current_app.appbuilder.sm + user = ab_security_manager.current_user + if user: + access_token = create_access_token(user.id) + refresh_token = create_refresh_token(user.id) + resp = jsonify({"logged_in": True}) + set_access_cookies(resp, access_token) + set_refresh_cookies(resp, refresh_token) + return resp + security.check_authentication() + user = current_app.appbuilder.sm.current_user + access_token = create_access_token(user.id) + refresh_token = create_refresh_token(user.id) + resp = jsonify({"logged_in": True}) + set_access_cookies(resp, access_token) + set_refresh_cookies(resp, refresh_token) + return resp + + +@jwt_refresh_token_required +def token_refresh(): + """Refresh token""" + current_user = get_jwt_identity() + access_token = create_access_token(identity=current_user) + resp = jsonify({"fresh": True}) + set_access_cookies(resp, access_token) + return resp + + +def logout(): + """Sign out""" + resp = jsonify({'logged_out': True}) + unset_jwt_cookies(resp) + return resp, 200 diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index a1af437792966..053a60e3e624d 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1492,12 +1492,12 @@ paths: /login: post: - summary: User login + summary: Auth login description: | - Verify user and return a user object and JWT token as well - x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint + Verify user and set jwt token in cookies + x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint operationId: login - tags: [User] + tags: [Auth] responses: '200': @@ -1505,12 +1505,56 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/UserLogin' + properties: + logged_in: + type: boolean + nullable: false + description: Indicate user is logged in '400': $ref: '#/components/responses/BadRequest' '401': $ref: '#/components/responses/Unauthenticated' + /logout: + post: + summary: Auth logout + description: Logout user by unsetting cookies + x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint + operationId: logout + tags: [Auth] + + responses: + '200': + description: Success + content: + application/json: + schema: + properties: + logged_out: + type: boolean + description: Indicate user is logged out + '400': + $ref: '#/components/responses/BadRequest' + + /refresh: + post: + summary: Refresh token + description: Create fresh token for user + x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint + operationId: token_refresh + tags: [Auth] + + responses: + '200': + description: Success + content: + application/json: + schema: + properties: + fresh: + type: boolean + description: Indicate token is fresh + components: # Reusable schemas (data models) schemas: diff --git a/airflow/api_connexion/schemas/permission_schema.py b/airflow/api_connexion/schemas/permission_schema.py deleted file mode 100644 index 273bd98d72426..0000000000000 --- a/airflow/api_connexion/schemas/permission_schema.py +++ /dev/null @@ -1,118 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -from typing import List, NamedTuple - -from flask_appbuilder.security.sqla.models import Permission, PermissionView, Role, ViewMenu -from marshmallow import Schema, fields -from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field - - -class ResourceCollectionItemSchema(SQLAlchemySchema): - """Permission Resources Schema""" - - class Meta: - """Meta""" - - model = ViewMenu - - id = auto_field() - name = auto_field() - - -class ResourceCollection(NamedTuple): - """Permission Resources list""" - - resources: List[ViewMenu] - total_entries: int - - -class ResourceCollectionSchema(Schema): - """ViewMenu list schema""" - - resources = fields.List(fields.Nested(ResourceCollectionItemSchema)) - total_entries = fields.Int() - - -class PermissionCollectionItemSchema(SQLAlchemySchema): - """Permission Schema""" - - class Meta: - """Meta""" - - model = Permission - - id = auto_field() - name = auto_field() - - -class PermissionCollection(NamedTuple): - """Permission Collection""" - - actions: List[Permission] - total_entries: int - - -class PermissionCollectionSchema(Schema): - """Permissions list schema""" - - actions = fields.List(fields.Nested(PermissionCollectionItemSchema)) - total_entries = fields.Int() - - -class PermissionViewSchema(SQLAlchemySchema): - """Permission View Schema""" - - class Meta: - """Meta""" - - model = PermissionView - - id = auto_field() - permission = fields.Nested(PermissionCollectionItemSchema, data_key="action") - view_menu = fields.Nested(ResourceCollectionItemSchema, data_key="resource") - - -class RoleCollectionItemSchema(SQLAlchemySchema): - """Role item shema""" - - class Meta: - """Meta""" - - model = Role - - id = auto_field() - name = auto_field() - permissions = fields.List(fields.Nested(PermissionViewSchema)) - - -class RoleCollection(NamedTuple): - """List of roles""" - - roles: List[Role] - total_entries: int - - -class RoleCollectionSchema(Schema): - """List of roles""" - - roles = fields.List(fields.Nested(RoleCollectionItemSchema)) - total_entries = fields.Int() - - -role_collection_item_schema = RoleCollectionItemSchema() -role_collection_schema = RoleCollectionSchema() diff --git a/airflow/api_connexion/security.py b/airflow/api_connexion/security.py index 0d97880098bb4..9079a662c9437 100644 --- a/airflow/api_connexion/security.py +++ b/airflow/api_connexion/security.py @@ -28,13 +28,16 @@ def check_authentication() -> None: """Checks that the request has valid authorization information.""" response = current_app.api_auth.requires_authentication(Response)() + if response.status_code == 200: + return jwt_response = requires_authentication(Response)() - if response.status_code != 200 and jwt_response.status_code != 200: - # since this handler only checks authentication, not authorization, - # we should always return 401 - if response.status_code != 200: - raise Unauthenticated(headers=response.headers) + if jwt_response.status_code == 200: + return + elif jwt_response.status_code != 200: raise Unauthenticated(headers=jwt_response.headers) + # since this handler only checks authentication, not authorization, + # we should always return 401 + raise Unauthenticated(headers=response.headers) def requires_access(permissions: Optional[Sequence[Tuple[str, str]]] = None) -> Callable[[T], T]: diff --git a/airflow/api_connexion/webserver_auth.py b/airflow/api_connexion/webserver_auth.py index 1412ab0d81b17..0ea099d68cc6f 100644 --- a/airflow/api_connexion/webserver_auth.py +++ b/airflow/api_connexion/webserver_auth.py @@ -22,6 +22,7 @@ import jwt from flask import Response, current_app, request from flask_appbuilder.const import AUTH_LDAP +from flask_jwt_extended import verify_jwt_in_request from flask_login import login_user from airflow.configuration import conf @@ -30,55 +31,30 @@ T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name -def encode_auth_token(user_id): - """Generate authentication token""" - expire_time = conf.getint("webserver", "session_lifetime_minutes") - payload = { - 'exp': datetime.utcnow() + timedelta(minutes=expire_time), - 'iat': datetime.utcnow(), - 'sub': user_id, - } - return jwt.encode(payload, SECRET, algorithm='HS256') - - -def decode_auth_token(auth_token): - """Decode authentication token""" - try: - payload = jwt.decode(auth_token, SECRET, algorithms=['HS256']) - return payload['sub'] - except (jwt.ExpiredSignatureError, jwt.InvalidTokenError): - # Do not raise exception here due to auth_backend auth - return None - - def auth_current_user(): """Checks the authentication and return the current user""" auth_header = request.headers.get("Authorization", None) - ab_security_manager = current_app.appbuilder.sm - token = None - user = None if auth_header: - try: - token = auth_header.split(" ")[1] - except IndexError: - # Do not raise exception here due to auth_backend auth - return None - if auth_header and auth_header.startswith("Basic"): - auth = request.authorization - if auth is None or not (auth.username and auth.password): - return None - if ab_security_manager.auth_type == AUTH_LDAP: - user = ab_security_manager.auth_user_ldap(auth.username, auth.password) - if user is None: - user = ab_security_manager.auth_user_db(auth.username, auth.password) - if user is not None: - login_user(user, remember=False) - return user - if token: - user = decode_auth_token(token) - if not isinstance(user, int): + if auth_header.startswith("Basic"): user = None - return user + auth = request.authorization + if auth is None or not (auth.username and auth.password): + return None + ab_security_manager = current_app.appbuilder.sm + if ab_security_manager.auth_type == AUTH_LDAP: + user = ab_security_manager.auth_user_ldap(auth.username, auth.password) + if user is None: + user = ab_security_manager.auth_user_db(auth.username, auth.password) + if user is not None: + login_user(user, remember=False) + return user + if auth_header.startswith('Bearer'): + try: + verify_jwt_in_request() + return 1 + except Exception: + pass + return None return None diff --git a/airflow/www/app.py b/airflow/www/app.py index 3302f36d82811..ef4dde32f2d09 100644 --- a/airflow/www/app.py +++ b/airflow/www/app.py @@ -34,7 +34,11 @@ from airflow.www.extensions.init_dagbag import init_dagbag from airflow.www.extensions.init_jinja_globals import init_jinja_globals from airflow.www.extensions.init_manifest_files import configure_manifest_files -from airflow.www.extensions.init_security import init_api_experimental_auth, init_xframe_protection +from airflow.www.extensions.init_security import ( + init_api_experimental_auth, + init_jwt_auth, + init_xframe_protection, +) from airflow.www.extensions.init_session import init_permanent_session from airflow.www.extensions.init_views import ( init_api_connexion, @@ -112,6 +116,8 @@ def create_app(config=None, testing=False): init_api_experimental_auth(flask_app) + init_jwt_auth(flask_app) + Cache(app=flask_app, config={'CACHE_TYPE': 'filesystem', 'CACHE_DIR': '/tmp'}) init_flash_views(flask_app) diff --git a/airflow/www/extensions/init_security.py b/airflow/www/extensions/init_security.py index 544deebeb3af4..eeee2ca7ca670 100644 --- a/airflow/www/extensions/init_security.py +++ b/airflow/www/extensions/init_security.py @@ -15,8 +15,11 @@ # specific language governing permissions and limitations # under the License. import logging +from datetime import timedelta from importlib import import_module +from flask_jwt_extended import JWTManager + from airflow.configuration import conf from airflow.exceptions import AirflowConfigException, AirflowException @@ -55,3 +58,15 @@ def init_api_experimental_auth(app): except ImportError as err: log.critical("Cannot import %s for API authentication due to: %s", auth_backend, err) raise AirflowException(err) + + +def init_jwt_auth(app): + """Initialize flask jwt extended""" "" + app.config['JWT_TOKEN_LOCATION'] = ['cookies'] + app.config['JWT_TOKEN_EXPIRES'] = timedelta(minutes=30) + app.config['JWT_REFRESH_TOKEN_EXPIRES'] = timedelta(days=1) + app.config['JWT_ACCESS_COOKIE_PATH'] = '/api/' + app.config['JWT_REFRESH_COOKIE_PATH'] = '/token/refresh' + app.config['JWT_CSRF_CHECK_FORM'] = True + app.config['JWT_COOKIE_CSRF_PROTECT'] = True + JWTManager(app) diff --git a/openapitools.json b/openapitools.json new file mode 100644 index 0000000000000..8ef4cb7217f5b --- /dev/null +++ b/openapitools.json @@ -0,0 +1,7 @@ +{ + "$schema": "node_modules/@openapitools/openapi-generator-cli/config.schema.json", + "spaces": 2, + "generator-cli": { + "version": "5.0.1" + } +} diff --git a/tests/api_connexion/endpoints/test_user_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py similarity index 100% rename from tests/api_connexion/endpoints/test_user_endpoint.py rename to tests/api_connexion/endpoints/test_auth_endpoint.py diff --git a/tests/api_connexion/test_webserver_auth.py b/tests/api_connexion/test_webserver_auth.py index 9cde45ff0e344..23a2388194a8c 100644 --- a/tests/api_connexion/test_webserver_auth.py +++ b/tests/api_connexion/test_webserver_auth.py @@ -54,17 +54,21 @@ def tearDown(self) -> None: def test_successful_login(self): token = "Basic " + b64encode(b"test:test").decode() with self.app.test_client() as test_client: - response = test_client.post("api/v1/login", headers={"Authorization": token}) - assert isinstance(response.json["token"], str) - assert response.json["user"]['email'] == "test@fab.org" + test_client.post("api/v1/login", headers={"Authorization": token}) + cookie = next( + (cookie for cookie in test_client.cookie_jar if cookie.name == "access_token_cookie"), None + ) + assert isinstance(cookie.value, str) def test_can_view_other_endpoints(self): token = "Basic " + b64encode(b"test:test").decode() with self.app.test_client() as test_client: - response = test_client.post("api/v1/login", headers={"Authorization": token}) + test_client.post("api/v1/login", headers={"Authorization": token}) assert current_user.email == "test@fab.org" - token = response.json["token"] - response2 = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + token}) + cookie = next( + (cookie for cookie in test_client.cookie_jar if cookie.name == "access_token_cookie"), None + ) + response2 = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + cookie.value}) assert response2.status_code == 200 assert response2.json == { "pools": [ @@ -81,7 +85,6 @@ def test_can_view_other_endpoints(self): } def test_raises_for_the_none_algorithm(self): - token = "Basic " + b64encode(b"test:test").decode() payload = { 'exp': datetime.utcnow() + timedelta(minutes=10), 'iat': datetime.utcnow(), @@ -89,8 +92,6 @@ def test_raises_for_the_none_algorithm(self): } forgedtoken = jwt.encode(payload, key=None, algorithm=None).decode() with self.app.test_client() as test_client: - test_client.post("api/v1/login", headers={"Authorization": token}) - assert current_user.email == "test@fab.org" response = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + forgedtoken}) assert response.status_code == 401 @@ -115,9 +116,9 @@ def test_malformed_headers(self, token): @parameterized.expand( [ - ("basic " + b64encode(b"test").decode(),), + ("bearer " + b64encode(b"test").decode(),), ("basic " + b64encode(b"test:").decode(),), - ("basic " + b64encode(b"test:123").decode(),), + ("bearer " + b64encode(b"test:123").decode(),), ("basic " + b64encode(b"test test").decode(),), ] ) From a0af6767c485031aa79f61bc750dc8d0f0c4a54f Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 3 Mar 2021 04:43:35 +0100 Subject: [PATCH 13/26] fixup! Use flask_jwt_extended, minimize PR and store JWT in httpOnly cookies --- openapitools.json | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 openapitools.json diff --git a/openapitools.json b/openapitools.json deleted file mode 100644 index 8ef4cb7217f5b..0000000000000 --- a/openapitools.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "$schema": "node_modules/@openapitools/openapi-generator-cli/config.schema.json", - "spaces": 2, - "generator-cli": { - "version": "5.0.1" - } -} From 7ffaa5ec1cd22c43f2c6748865d30fbc8079a384 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Thu, 4 Mar 2021 05:03:25 +0100 Subject: [PATCH 14/26] Use auto refreshing of tokens and add models back in openapi spec with suggestions from code review --- .../api_connexion/endpoints/auth_endpoint.py | 29 ++------ airflow/api_connexion/openapi/v1.yaml | 39 +++------- .../schemas/permission_schema.py | 71 +++++++++++++++++++ airflow/api_connexion/webserver_auth.py | 14 ++-- airflow/config_templates/config.yml | 15 ++++ airflow/config_templates/default_airflow.cfg | 8 +++ airflow/www/extensions/init_security.py | 40 +++++++++-- tests/api_connexion/test_webserver_auth.py | 59 ++++++++------- 8 files changed, 174 insertions(+), 101 deletions(-) create mode 100644 airflow/api_connexion/schemas/permission_schema.py diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index cb95f501ec957..71d0926d35503 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -16,17 +16,10 @@ # under the License. from flask import current_app, jsonify -from flask_jwt_extended import ( - create_access_token, - create_refresh_token, - get_jwt_identity, - jwt_refresh_token_required, - set_access_cookies, - set_refresh_cookies, - unset_jwt_cookies, -) +from flask_jwt_extended import create_access_token, set_access_cookies, unset_jwt_cookies from airflow.api_connexion import security +from airflow.api_connexion.schemas.user_schema import user_collection_item_schema def login(): @@ -35,27 +28,13 @@ def login(): user = ab_security_manager.current_user if user: access_token = create_access_token(user.id) - refresh_token = create_refresh_token(user.id) - resp = jsonify({"logged_in": True}) + resp = jsonify(user_collection_item_schema.dump(user)) set_access_cookies(resp, access_token) - set_refresh_cookies(resp, refresh_token) return resp security.check_authentication() user = current_app.appbuilder.sm.current_user access_token = create_access_token(user.id) - refresh_token = create_refresh_token(user.id) - resp = jsonify({"logged_in": True}) - set_access_cookies(resp, access_token) - set_refresh_cookies(resp, refresh_token) - return resp - - -@jwt_refresh_token_required -def token_refresh(): - """Refresh token""" - current_user = get_jwt_identity() - access_token = create_access_token(identity=current_user) - resp = jsonify({"fresh": True}) + resp = jsonify(user_collection_item_schema.dump(user)) set_access_cookies(resp, access_token) return resp diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 053a60e3e624d..d82bdf4f125e6 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1492,12 +1492,12 @@ paths: /login: post: - summary: Auth login + summary: Webserver login description: | Verify user and set jwt token in cookies x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint operationId: login - tags: [Auth] + tags: [WebserverAuth] responses: '200': @@ -1505,11 +1505,7 @@ paths: content: application/json: schema: - properties: - logged_in: - type: boolean - nullable: false - description: Indicate user is logged in + $ref: '#/components/schemas/User' '400': $ref: '#/components/responses/BadRequest' '401': @@ -1517,11 +1513,11 @@ paths: /logout: post: - summary: Auth logout + summary: Webserver logout description: Logout user by unsetting cookies x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint operationId: logout - tags: [Auth] + tags: [WebserverAuth] responses: '200': @@ -1536,25 +1532,6 @@ paths: '400': $ref: '#/components/responses/BadRequest' - /refresh: - post: - summary: Refresh token - description: Create fresh token for user - x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint - operationId: token_refresh - tags: [Auth] - - responses: - '200': - description: Success - content: - application/json: - schema: - properties: - fresh: - type: boolean - description: Indicate token is fresh - components: # Reusable schemas (data models) schemas: @@ -3340,6 +3317,10 @@ components: Kerberos: type: http scheme: negotiate + WebserverAuth: + type: apiKey + in: cookie + name: access_token_cookie # The API will provide support for plugins to support various authorization mechanisms. # Detailed information will be available in the plugin specification. @@ -3361,7 +3342,7 @@ tags: - name: Role - name: Permission - name: User - - name: Auth + - name: WebserverAuth externalDocs: url: https://airflow.apache.org/docs/apache-airflow/stable/ diff --git a/airflow/api_connexion/schemas/permission_schema.py b/airflow/api_connexion/schemas/permission_schema.py new file mode 100644 index 0000000000000..b6d6ca3cdf3ac --- /dev/null +++ b/airflow/api_connexion/schemas/permission_schema.py @@ -0,0 +1,71 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import List, NamedTuple + +from flask_appbuilder.security.sqla.models import Permission, PermissionView, Role, ViewMenu +from marshmallow import Schema, fields +from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field + + +class ResourceSchema(SQLAlchemySchema): + """Permission Resources Schema""" + + class Meta: + """Meta""" + + model = ViewMenu + + name = auto_field() + + +class ActionSchema(SQLAlchemySchema): + """Permission Schema""" + + class Meta: + """Meta""" + + model = Permission + + name = auto_field() + + +class PermissionViewSchema(SQLAlchemySchema): + """Permission View Schema""" + + class Meta: + """Meta""" + + model = PermissionView + + permission = fields.Nested(ActionSchema, data_key="action") + view_menu = fields.Nested(ResourceSchema, data_key="resource") + + +class RoleSchema(SQLAlchemySchema): + """Role item shema""" + + class Meta: + """Meta""" + + model = Role + + name = auto_field() + permissions = fields.List(fields.Nested(PermissionViewSchema)) + + +role_schema = RoleSchema() diff --git a/airflow/api_connexion/webserver_auth.py b/airflow/api_connexion/webserver_auth.py index 0ea099d68cc6f..cc68eda07d870 100644 --- a/airflow/api_connexion/webserver_auth.py +++ b/airflow/api_connexion/webserver_auth.py @@ -15,11 +15,9 @@ # specific language governing permissions and limitations # under the License. -from datetime import datetime, timedelta from functools import wraps from typing import Callable, TypeVar, cast -import jwt from flask import Response, current_app, request from flask_appbuilder.const import AUTH_LDAP from flask_jwt_extended import verify_jwt_in_request @@ -48,13 +46,11 @@ def auth_current_user(): if user is not None: login_user(user, remember=False) return user - if auth_header.startswith('Bearer'): - try: - verify_jwt_in_request() - return 1 - except Exception: - pass - return None + try: + verify_jwt_in_request() + return 1 + except Exception: + pass return None diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml index 16f6f107f25f7..1b616e1edc648 100644 --- a/airflow/config_templates/config.yml +++ b/airflow/config_templates/config.yml @@ -728,6 +728,21 @@ type: string example: ~ default: "airflow.api.auth.backend.deny_all" + - name: jwt_access_token_expires + description: How long an access token should live before it expires(in minutes). + version_added: 2.0.2 + type: integer + example: ~ + default: "60" + - name: jwt_cookie_secure + description: | + If the secure flag should be set on your JWT cookies. This will only + allow the cookies to be sent over https. Defaults to False, + but in production this should likely be set to True. + version_added: 2.0.2 + type: boolean + example: ~ + default: "False" - name: maximum_page_limit description: | Used to set the maximum page limit for API requests diff --git a/airflow/config_templates/default_airflow.cfg b/airflow/config_templates/default_airflow.cfg index bdbc045b14562..b317b91fe4a36 100644 --- a/airflow/config_templates/default_airflow.cfg +++ b/airflow/config_templates/default_airflow.cfg @@ -392,6 +392,14 @@ enable_experimental_api = False # ("airflow.api.auth.backend.default" allows all requests for historic reasons) auth_backend = airflow.api.auth.backend.deny_all +# How long an access token should live before it expires(in minutes). +jwt_access_token_expires = 60 + +# If the secure flag should be set on your JWT cookies. This will only +# allow the cookies to be sent over https. Defaults to False, +# but in production this should likely be set to True. +jwt_cookie_secure = False + # Used to set the maximum page limit for API requests maximum_page_limit = 100 diff --git a/airflow/www/extensions/init_security.py b/airflow/www/extensions/init_security.py index eeee2ca7ca670..48a0a5cb45570 100644 --- a/airflow/www/extensions/init_security.py +++ b/airflow/www/extensions/init_security.py @@ -15,13 +15,20 @@ # specific language governing permissions and limitations # under the License. import logging -from datetime import timedelta +from datetime import datetime, timedelta from importlib import import_module -from flask_jwt_extended import JWTManager +from flask_jwt_extended import ( + JWTManager, + create_access_token, + get_jwt_identity, + get_raw_jwt, + set_access_cookies, +) from airflow.configuration import conf from airflow.exceptions import AirflowConfigException, AirflowException +from airflow.utils import timezone log = logging.getLogger(__name__) @@ -62,11 +69,30 @@ def init_api_experimental_auth(app): def init_jwt_auth(app): """Initialize flask jwt extended""" "" + exp_mins = conf.getint("api", "jwt_access_token_expires", fallback=60) app.config['JWT_TOKEN_LOCATION'] = ['cookies'] - app.config['JWT_TOKEN_EXPIRES'] = timedelta(minutes=30) - app.config['JWT_REFRESH_TOKEN_EXPIRES'] = timedelta(days=1) - app.config['JWT_ACCESS_COOKIE_PATH'] = '/api/' - app.config['JWT_REFRESH_COOKIE_PATH'] = '/token/refresh' - app.config['JWT_CSRF_CHECK_FORM'] = True + app.config['JWT_ACCESS_TOKEN_EXPIRES'] = timedelta(minutes=exp_mins) + app.config['JWT_CSRF_IN_COOKIES'] = True app.config['JWT_COOKIE_CSRF_PROTECT'] = True + app.config['JWT_COOKIE_SECURE'] = conf.get("api", "jwt_cookie_secure", fallback=True) JWTManager(app) + + # In flask_jwt_extension 4.0, this is the recommended way to refresh token + # https://flask-jwt-extended.readthedocs.io/en/stable/refreshing_tokens/#implicit-refreshing-with-cookies + # It's warned not to store refresh token in cookies unlike in 3.0 where they suggested storing refresh + # token in cookies + def refresh_token(response): + try: + exp_timestamp = get_raw_jwt()["exp"] + now = datetime.now(timezone.utc) + half_exp_time = exp_mins / 2 + target_timestamp = datetime.timestamp(now + timedelta(minutes=half_exp_time)) + if target_timestamp > exp_timestamp: + access_token = create_access_token(identity=get_jwt_identity()) + set_access_cookies(response, access_token) + return response + except (RuntimeError, KeyError): + # Case where there is not a valid JWT. Just return the original respone + return response + + app.after_request(refresh_token) diff --git a/tests/api_connexion/test_webserver_auth.py b/tests/api_connexion/test_webserver_auth.py index 23a2388194a8c..eeb86ad4c731d 100644 --- a/tests/api_connexion/test_webserver_auth.py +++ b/tests/api_connexion/test_webserver_auth.py @@ -20,36 +20,36 @@ from datetime import datetime, timedelta import jwt -from flask_login import current_user +from flask_jwt_extended import create_access_token from parameterized import parameterized +from airflow.security import permissions from airflow.www.app import create_app -from tests.test_utils.api_connexion_utils import assert_401 +from tests.test_utils.api_connexion_utils import assert_401, create_user, delete_user from tests.test_utils.config import conf_vars -from tests.test_utils.db import clear_db_pools class TestWebserverAuth(unittest.TestCase): - def setUp(self) -> None: + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() with conf_vars({("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend"}): - self.app = create_app(testing=True) + cls.app = create_app(testing=True) + cls.appbuilder = cls.app.appbuilder # pylint: disable=no-member + create_user( + cls.app, # type: ignore + username="test", + role_name="Admin", + permissions=[(permissions.ACTION_CAN_READ, permissions.RESOURCE_POOL)], + ) + cls.tester = cls.appbuilder.sm.find_user(username="test") - self.appbuilder = self.app.appbuilder # pylint: disable=no-member - role_admin = self.appbuilder.sm.find_role("Admin") - self.tester = self.appbuilder.sm.find_user(username="test") - if not self.tester: - self.appbuilder.sm.add_user( - username="test", - first_name="test", - last_name="test", - email="test@fab.org", - role=role_admin, - password="test", - ) - clear_db_pools() + @classmethod + def tearDownClass(cls) -> None: + delete_user(cls.app, username="test") # type: ignore - def tearDown(self) -> None: - clear_db_pools() + def setUp(self) -> None: + self.client = self.app.test_client() def test_successful_login(self): token = "Basic " + b64encode(b"test:test").decode() @@ -61,16 +61,12 @@ def test_successful_login(self): assert isinstance(cookie.value, str) def test_can_view_other_endpoints(self): - token = "Basic " + b64encode(b"test:test").decode() - with self.app.test_client() as test_client: - test_client.post("api/v1/login", headers={"Authorization": token}) - assert current_user.email == "test@fab.org" - cookie = next( - (cookie for cookie in test_client.cookie_jar if cookie.name == "access_token_cookie"), None - ) - response2 = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + cookie.value}) - assert response2.status_code == 200 - assert response2.json == { + with self.app.app_context(): + token = create_access_token(self.tester.id) + self.client.set_cookie("localhost", 'access_token_cookie', token) + response = self.client.get("/api/v1/pools") + assert response.status_code == 200 + assert response.json == { "pools": [ { "name": "default_pool", @@ -91,8 +87,9 @@ def test_raises_for_the_none_algorithm(self): 'sub': self.tester.id, } forgedtoken = jwt.encode(payload, key=None, algorithm=None).decode() + self.client.set_cookie("localhost", 'access_token_cookie', forgedtoken) with self.app.test_client() as test_client: - response = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + forgedtoken}) + response = test_client.get("/api/v1/pools") assert response.status_code == 401 @parameterized.expand( From 4e1a9f2066242ee2cdc253caa0b2ea6f8fbdd6d5 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Thu, 4 Mar 2021 15:45:42 +0100 Subject: [PATCH 15/26] improve code and test --- airflow/api_connexion/security.py | 4 ++-- airflow/api_connexion/webserver_auth.py | 2 +- tests/api_connexion/test_webserver_auth.py | 18 ++++++++++++++++-- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/airflow/api_connexion/security.py b/airflow/api_connexion/security.py index 9079a662c9437..7b5f88891f6ff 100644 --- a/airflow/api_connexion/security.py +++ b/airflow/api_connexion/security.py @@ -33,10 +33,10 @@ def check_authentication() -> None: jwt_response = requires_authentication(Response)() if jwt_response.status_code == 200: return - elif jwt_response.status_code != 200: - raise Unauthenticated(headers=jwt_response.headers) # since this handler only checks authentication, not authorization, # we should always return 401 + if jwt_response.status_code != 200: + raise Unauthenticated(headers=jwt_response.headers) raise Unauthenticated(headers=response.headers) diff --git a/airflow/api_connexion/webserver_auth.py b/airflow/api_connexion/webserver_auth.py index cc68eda07d870..2087481680e80 100644 --- a/airflow/api_connexion/webserver_auth.py +++ b/airflow/api_connexion/webserver_auth.py @@ -30,7 +30,7 @@ def auth_current_user(): - """Checks the authentication and return the current user""" + """Checks the authentication and return a value""" auth_header = request.headers.get("Authorization", None) if auth_header: if auth_header.startswith("Basic"): diff --git a/tests/api_connexion/test_webserver_auth.py b/tests/api_connexion/test_webserver_auth.py index eeb86ad4c731d..ea44c1a1246a9 100644 --- a/tests/api_connexion/test_webserver_auth.py +++ b/tests/api_connexion/test_webserver_auth.py @@ -51,7 +51,9 @@ def tearDownClass(cls) -> None: def setUp(self) -> None: self.client = self.app.test_client() - def test_successful_login(self): + def test_basic_auth_can_get_token(self): + # For most users that would use username and password especially in local + # deployment, we check that on a successful login, a token is generated token = "Basic " + b64encode(b"test:test").decode() with self.app.test_client() as test_client: test_client.post("api/v1/login", headers={"Authorization": token}) @@ -60,7 +62,7 @@ def test_successful_login(self): ) assert isinstance(cookie.value, str) - def test_can_view_other_endpoints(self): + def test_token_in_cookie_can_view_endpoints(self): with self.app.app_context(): token = create_access_token(self.tester.id) self.client.set_cookie("localhost", 'access_token_cookie', token) @@ -125,3 +127,15 @@ def test_invalid_auth_header(self, token): assert response.status_code == 401 assert response.headers["Content-Type"] == "application/problem+json" assert_401(response) + + @parameterized.expand( + [("access_token_cookies"), ("access_token_cook"), ("access_token"), ("csrf_access_token")] + ) + def test_invalid_cookie_name_fails(self, name): + with self.app.app_context(): + token = create_access_token(self.tester.id) + self.client.set_cookie("localhost", name, token) + response = self.client.get("/api/v1/pools") + assert response.status_code == 401 + assert response.headers["Content-Type"] == "application/problem+json" + assert_401(response) From e6a68b31f1502376b1d376193833fd16b2fa2249 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Thu, 4 Mar 2021 18:23:30 +0100 Subject: [PATCH 16/26] Add configuration to disable DB login for REST API --- .../schemas/permission_schema.py | 4 +- airflow/api_connexion/webserver_auth.py | 3 +- airflow/config_templates/config.yml | 6 + airflow/config_templates/default_airflow.cfg | 3 + .../endpoints/test_auth_endpoint.py | 265 +++++------------- 5 files changed, 80 insertions(+), 201 deletions(-) diff --git a/airflow/api_connexion/schemas/permission_schema.py b/airflow/api_connexion/schemas/permission_schema.py index b6d6ca3cdf3ac..e853573102ea3 100644 --- a/airflow/api_connexion/schemas/permission_schema.py +++ b/airflow/api_connexion/schemas/permission_schema.py @@ -15,10 +15,8 @@ # specific language governing permissions and limitations # under the License. -from typing import List, NamedTuple - from flask_appbuilder.security.sqla.models import Permission, PermissionView, Role, ViewMenu -from marshmallow import Schema, fields +from marshmallow import fields from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field diff --git a/airflow/api_connexion/webserver_auth.py b/airflow/api_connexion/webserver_auth.py index 2087481680e80..ab69ced15ef89 100644 --- a/airflow/api_connexion/webserver_auth.py +++ b/airflow/api_connexion/webserver_auth.py @@ -31,8 +31,9 @@ def auth_current_user(): """Checks the authentication and return a value""" + db_login_disabled = conf.getboolean("api", "disable_db_login") auth_header = request.headers.get("Authorization", None) - if auth_header: + if auth_header and not db_login_disabled: if auth_header.startswith("Basic"): user = None auth = request.authorization diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml index 1b616e1edc648..86be046422170 100644 --- a/airflow/config_templates/config.yml +++ b/airflow/config_templates/config.yml @@ -743,6 +743,12 @@ type: boolean example: ~ default: "False" + - name: disable_db_login + description: Disable use of username and password to login to the REST API + version_added: 2.0.2 + type: boolean + example: ~ + default: "False" - name: maximum_page_limit description: | Used to set the maximum page limit for API requests diff --git a/airflow/config_templates/default_airflow.cfg b/airflow/config_templates/default_airflow.cfg index b317b91fe4a36..b01c7215bd224 100644 --- a/airflow/config_templates/default_airflow.cfg +++ b/airflow/config_templates/default_airflow.cfg @@ -400,6 +400,9 @@ jwt_access_token_expires = 60 # but in production this should likely be set to True. jwt_cookie_secure = False +# Disable use of username and password to login to the REST API +disable_db_login = False + # Used to set the maximum page limit for API requests maximum_page_limit = 100 diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index b52c54cfdd112..6fc1cf454fe44 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -14,212 +14,83 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -import pytest -from flask_appbuilder.security.sqla.models import User -from parameterized import parameterized -from airflow.api_connexion.exceptions import EXCEPTIONS_LINK_MAP -from airflow.security import permissions -from airflow.utils import timezone -from tests.test_utils.api_connexion_utils import assert_401, create_user, delete_user -from tests.test_utils.config import conf_vars - -DEFAULT_TIME = "2020-06-11T18:00:00+00:00" +import unittest +from base64 import b64encode +from airflow.www import app +from tests.test_utils.api_connexion_utils import create_user, delete_user +from tests.test_utils.config import conf_vars -@pytest.fixture(scope="module") -def configured_app(minimal_app_for_api): - app = minimal_app_for_api - create_user( - app, # type: ignore - username="test", - role_name="Test", - permissions=[ - (permissions.ACTION_CAN_LIST, permissions.RESOURCE_USER_DB_MODELVIEW), - (permissions.ACTION_CAN_SHOW, permissions.RESOURCE_USER_DB_MODELVIEW), - ], - ) - create_user(app, username="test_no_permissions", role_name="TestNoPermissions") # type: ignore +TEST_USERNAME = "test" - yield app - delete_user(app, username="test") # type: ignore - delete_user(app, username="test_no_permissions") # type: ignore +class TestLoginEndpoint(unittest.TestCase): + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + with conf_vars( + { + ("webserver", "session_lifetime_minutes"): "1", + ("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend", + } + ): + cls.app = app.create_app(testing=True) # type:ignore + create_user(cls.app, username="test", role_name="Test") # type: ignore + @classmethod + def tearDownClass(cls) -> None: + delete_user(cls.app, username="test") # type: ignore -class TestUserEndpoint: - @pytest.fixture(autouse=True) - def setup_attrs(self, configured_app) -> None: - self.app = configured_app + def setUp(self) -> None: self.client = self.app.test_client() # type:ignore - self.session = self.app.appbuilder.get_session - def teardown_method(self) -> None: - # Delete users that have our custom default time - users = self.session.query(User).filter(User.changed_on == timezone.parse(DEFAULT_TIME)).all() - for user in users: - self.session.delete(user) - self.session.commit() - - def _create_users(self, count, roles=None): - # create users with defined created_on and changed_on date - # for easy testing - if roles is None: - roles = [] - return [ - User( - first_name=f'test{i}', - last_name=f'test{i}', - username=f'TEST_USER{i}', - email=f'mytest@test{i}.org', - roles=roles or [], - created_on=timezone.parse(DEFAULT_TIME), - changed_on=timezone.parse(DEFAULT_TIME), + def test_user_can_login_with_basic_auth(self): + token = "Basic " + b64encode(b"test:test").decode() + self.client.post("api/v1/login", headers={"Authorization": token}) + cookie = next( + (cookie for cookie in self.client.cookie_jar if cookie.name == "access_token_cookie"), None + ) + assert cookie is not None + assert isinstance(cookie.value, str) + + def test_no_authorization_header_raises(self): + response = self.client.post("api/v1/login") + assert response.status_code == 401 + + def test_malformed_authorization_header_fails(self): + token = "Basic " + b64encode(b"test").decode() + response = self.client.post("api/v1/login", headers={"Authorization": token}) + assert response.status_code == 401 + + def test_wrong_password_or_username_fails(self): + token = "Basic " + b64encode(b"test:tes").decode() + response = self.client.post("api/v1/login", headers={"Authorization": token}) + assert response.status_code == 401 + + def test_remote_auth_user_can_get_token(self): + self.client.post("api/v1/login", environ_overrides={'REMOTE_USER': "test"}) + cookie = next( + (cookie for cookie in self.client.cookie_jar if cookie.name == "access_token_cookie"), None + ) + assert cookie is not None + assert isinstance(cookie.value, str) + + @conf_vars({("api", "disable_db_login"): "True"}) + def test_user_cannot_login_through_db_when_db_login_is_disabled(self): + token = "Basic " + b64encode(b"test:test").decode() + with self.app.test_client() as test_client: + test_client.post("api/v1/login", headers={"Authorization": token}) + cookie = next( + (cookie for cookie in test_client.cookie_jar if cookie.name == "access_token_cookie"), None ) - for i in range(1, count + 1) - ] + assert cookie is None - -class TestGetUser(TestUserEndpoint): - def test_should_respond_200(self): - users = self._create_users(1) - self.session.add_all(users) - self.session.commit() - response = self.client.get("/api/v1/users/TEST_USER1", environ_overrides={'REMOTE_USER': "test"}) - assert response.status_code == 200 - assert response.json == { - 'active': None, - 'changed_on': DEFAULT_TIME, - 'created_on': DEFAULT_TIME, - 'email': 'mytest@test1.org', - 'fail_login_count': None, - 'first_name': 'test1', - 'last_login': None, - 'last_name': 'test1', - 'login_count': None, - 'roles': [], - 'user_id': users[0].id, - 'username': 'TEST_USER1', - } - - def test_should_respond_404(self): - response = self.client.get("/api/v1/users/invalid-user", environ_overrides={'REMOTE_USER': "test"}) - assert response.status_code == 404 - assert { - 'detail': "The User with username `invalid-user` was not found", - 'status': 404, - 'title': 'User not found', - 'type': EXCEPTIONS_LINK_MAP[404], - } == response.json - - def test_should_raises_401_unauthenticated(self): - response = self.client.get("/api/v1/users/TEST_USER1") - assert_401(response) - - def test_should_raise_403_forbidden(self): - response = self.client.get( - "/api/v1/users/TEST_USER1", environ_overrides={'REMOTE_USER': "test_no_permissions"} + @conf_vars({("api", "disable_db_login"): "True"}) + def test_cookie_is_set_for_remote_user_when_db_login_is_disabled(self): + self.client.post("api/v1/login", environ_overrides={'REMOTE_USER': "test"}) + cookie = next( + (cookie for cookie in self.client.cookie_jar if cookie.name == "access_token_cookie"), None ) - assert response.status_code == 403 - - -class TestGetUsers(TestUserEndpoint): - def test_should_response_200(self): - response = self.client.get("/api/v1/users", environ_overrides={'REMOTE_USER': "test"}) - assert response.status_code == 200 - assert response.json["total_entries"] == 2 - usernames = [user["username"] for user in response.json["users"] if user] - assert usernames == ['test', 'test_no_permissions'] - - def test_should_raises_401_unauthenticated(self): - response = self.client.get("/api/v1/users") - assert_401(response) - - def test_should_raise_403_forbidden(self): - response = self.client.get("/api/v1/users", environ_overrides={'REMOTE_USER': "test_no_permissions"}) - assert response.status_code == 403 - - -class TestGetUsersPagination(TestUserEndpoint): - @parameterized.expand( - [ - ("/api/v1/users?limit=1", ["test"]), - ("/api/v1/users?limit=2", ["test", "test_no_permissions"]), - ( - "/api/v1/users?offset=5", - [ - "TEST_USER4", - "TEST_USER5", - "TEST_USER6", - "TEST_USER7", - "TEST_USER8", - "TEST_USER9", - "TEST_USER10", - ], - ), - ( - "/api/v1/users?offset=0", - [ - "test", - "test_no_permissions", - "TEST_USER1", - "TEST_USER2", - "TEST_USER3", - "TEST_USER4", - "TEST_USER5", - "TEST_USER6", - "TEST_USER7", - "TEST_USER8", - "TEST_USER9", - "TEST_USER10", - ], - ), - ("/api/v1/users?limit=1&offset=5", ["TEST_USER4"]), - ("/api/v1/users?limit=1&offset=1", ["test_no_permissions"]), - ( - "/api/v1/users?limit=2&offset=2", - ["TEST_USER1", "TEST_USER2"], - ), - ] - ) - def test_handle_limit_offset(self, url, expected_usernames): - users = self._create_users(10) - self.session.add_all(users) - self.session.commit() - response = self.client.get(url, environ_overrides={'REMOTE_USER': "test"}) - assert response.status_code == 200 - assert response.json["total_entries"] == 12 - usernames = [user["username"] for user in response.json["users"] if user] - assert usernames == expected_usernames - - def test_should_respect_page_size_limit_default(self): - users = self._create_users(200) - self.session.add_all(users) - self.session.commit() - - response = self.client.get("/api/v1/users", environ_overrides={'REMOTE_USER': "test"}) - assert response.status_code == 200 - # Explicitly add the 2 users on setUp - assert response.json["total_entries"] == 200 + len(['test', 'test_no_permissions']) - assert len(response.json["users"]) == 100 - - def test_limit_of_zero_should_return_default(self): - users = self._create_users(200) - self.session.add_all(users) - self.session.commit() - - response = self.client.get("/api/v1/users?limit=0", environ_overrides={'REMOTE_USER': "test"}) - assert response.status_code == 200 - # Explicit add the 2 users on setUp - assert response.json["total_entries"] == 200 + len(['test', 'test_no_permissions']) - assert len(response.json["users"]) == 100 - - @conf_vars({("api", "maximum_page_limit"): "150"}) - def test_should_return_conf_max_if_req_max_above_conf(self): - users = self._create_users(200) - self.session.add_all(users) - self.session.commit() - - response = self.client.get("/api/v1/users?limit=180", environ_overrides={'REMOTE_USER': "test"}) - assert response.status_code == 200 - assert len(response.json['users']) == 150 + assert cookie is not None + assert isinstance(cookie.value, str) From 586b5d672541b4366ad769b02e606658f40d1f95 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 5 Mar 2021 06:42:39 +0100 Subject: [PATCH 17/26] fixup! Add configuration to disable DB login for REST API --- airflow/api_connexion/webserver_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow/api_connexion/webserver_auth.py b/airflow/api_connexion/webserver_auth.py index ab69ced15ef89..d55e7ff924e4e 100644 --- a/airflow/api_connexion/webserver_auth.py +++ b/airflow/api_connexion/webserver_auth.py @@ -50,7 +50,7 @@ def auth_current_user(): try: verify_jwt_in_request() return 1 - except Exception: + except Exception: # pylint: disable=too-broad-except pass return None From c01130580736ae8f9f1cd7782f9e00331bb25d4f Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 5 Mar 2021 14:04:22 +0100 Subject: [PATCH 18/26] log exception and fix pylint --- airflow/api_connexion/webserver_auth.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/airflow/api_connexion/webserver_auth.py b/airflow/api_connexion/webserver_auth.py index d55e7ff924e4e..28ce4562a2be6 100644 --- a/airflow/api_connexion/webserver_auth.py +++ b/airflow/api_connexion/webserver_auth.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. +import logging from functools import wraps from typing import Callable, TypeVar, cast @@ -28,6 +29,8 @@ SECRET = conf.get("webserver", "secret_key") T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name +log = logging.getLogger(__name__) + def auth_current_user(): """Checks the authentication and return a value""" @@ -50,8 +53,8 @@ def auth_current_user(): try: verify_jwt_in_request() return 1 - except Exception: # pylint: disable=too-broad-except - pass + except Exception as err: # pylint: disable=broad-except + log.debug("Can't verify jwt: %s", str(err)) return None From d50811f6036bea90b4d0177803c7ec2d32eccdea Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 5 Mar 2021 15:13:04 +0100 Subject: [PATCH 19/26] apply suggestion from code review --- airflow/api_connexion/security.py | 7 ++----- airflow/api_connexion/webserver_auth.py | 20 +++----------------- 2 files changed, 5 insertions(+), 22 deletions(-) diff --git a/airflow/api_connexion/security.py b/airflow/api_connexion/security.py index 7b5f88891f6ff..95e1626881a05 100644 --- a/airflow/api_connexion/security.py +++ b/airflow/api_connexion/security.py @@ -20,7 +20,7 @@ from flask import Response, current_app from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated -from airflow.api_connexion.webserver_auth import requires_authentication +from airflow.api_connexion.webserver_auth import auth_current_user T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name @@ -30,13 +30,10 @@ def check_authentication() -> None: response = current_app.api_auth.requires_authentication(Response)() if response.status_code == 200: return - jwt_response = requires_authentication(Response)() - if jwt_response.status_code == 200: + if auth_current_user(): return # since this handler only checks authentication, not authorization, # we should always return 401 - if jwt_response.status_code != 200: - raise Unauthenticated(headers=jwt_response.headers) raise Unauthenticated(headers=response.headers) diff --git a/airflow/api_connexion/webserver_auth.py b/airflow/api_connexion/webserver_auth.py index 28ce4562a2be6..783efcd1a9760 100644 --- a/airflow/api_connexion/webserver_auth.py +++ b/airflow/api_connexion/webserver_auth.py @@ -16,10 +16,9 @@ # under the License. import logging -from functools import wraps -from typing import Callable, TypeVar, cast +from typing import Callable, TypeVar -from flask import Response, current_app, request +from flask import current_app, request from flask_appbuilder.const import AUTH_LDAP from flask_jwt_extended import verify_jwt_in_request from flask_login import login_user @@ -52,20 +51,7 @@ def auth_current_user(): return user try: verify_jwt_in_request() - return 1 + return True except Exception as err: # pylint: disable=broad-except log.debug("Can't verify jwt: %s", str(err)) return None - - -def requires_authentication(function: T): - """Decorator for functions that require authentication""" - - @wraps(function) - def decorated(*args, **kwargs): - if auth_current_user() is not None: - return function(*args, **kwargs) - else: - return Response("Unauthorized", 401, {"WWW-Authenticate": "Bearer"}) - - return cast(T, decorated) From d3f72258596bcba6dfb7371e1c1fade5c1aaafc1 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 8 Mar 2021 15:58:02 +0100 Subject: [PATCH 20/26] Remove role and Permission models as they are not needed at this time --- .../schemas/permission_schema.py | 69 ------------------- 1 file changed, 69 deletions(-) delete mode 100644 airflow/api_connexion/schemas/permission_schema.py diff --git a/airflow/api_connexion/schemas/permission_schema.py b/airflow/api_connexion/schemas/permission_schema.py deleted file mode 100644 index e853573102ea3..0000000000000 --- a/airflow/api_connexion/schemas/permission_schema.py +++ /dev/null @@ -1,69 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -from flask_appbuilder.security.sqla.models import Permission, PermissionView, Role, ViewMenu -from marshmallow import fields -from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field - - -class ResourceSchema(SQLAlchemySchema): - """Permission Resources Schema""" - - class Meta: - """Meta""" - - model = ViewMenu - - name = auto_field() - - -class ActionSchema(SQLAlchemySchema): - """Permission Schema""" - - class Meta: - """Meta""" - - model = Permission - - name = auto_field() - - -class PermissionViewSchema(SQLAlchemySchema): - """Permission View Schema""" - - class Meta: - """Meta""" - - model = PermissionView - - permission = fields.Nested(ActionSchema, data_key="action") - view_menu = fields.Nested(ResourceSchema, data_key="resource") - - -class RoleSchema(SQLAlchemySchema): - """Role item shema""" - - class Meta: - """Meta""" - - model = Role - - name = auto_field() - permissions = fields.List(fields.Nested(PermissionViewSchema)) - - -role_schema = RoleSchema() From 08947444d10dc13a1cf6850325973eac69739498 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 8 Mar 2021 16:39:33 +0100 Subject: [PATCH 21/26] fixup! Remove role and Permission models as they are not needed at this time --- docs/spelling_wordlist.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index bd32afdd30370..63dfdbca9905b 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -910,6 +910,7 @@ json jthomas jupyter jupytercmd +jwt kaxil kaxilnaik keepalive From b25f2aad7f7427b0d826b52ba9f29c4b81a05e06 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 9 Mar 2021 06:47:12 +0100 Subject: [PATCH 22/26] Clear db pools during test setup --- tests/api_connexion/test_webserver_auth.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/api_connexion/test_webserver_auth.py b/tests/api_connexion/test_webserver_auth.py index ea44c1a1246a9..9d0a20f836089 100644 --- a/tests/api_connexion/test_webserver_auth.py +++ b/tests/api_connexion/test_webserver_auth.py @@ -27,6 +27,7 @@ from airflow.www.app import create_app from tests.test_utils.api_connexion_utils import assert_401, create_user, delete_user from tests.test_utils.config import conf_vars +from tests.test_utils.db import clear_db_pools class TestWebserverAuth(unittest.TestCase): @@ -50,6 +51,7 @@ def tearDownClass(cls) -> None: def setUp(self) -> None: self.client = self.app.test_client() + clear_db_pools() def test_basic_auth_can_get_token(self): # For most users that would use username and password especially in local From 4ae0322464a61e8208b4c59e9106251c29ccec15 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 17 Mar 2021 17:39:00 +0100 Subject: [PATCH 23/26] Resolve conflict --- airflow/api_connexion/openapi/v1.yaml | 134 +++++++++++++------------- 1 file changed, 67 insertions(+), 67 deletions(-) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index d82bdf4f125e6..02679f8323e8c 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1623,6 +1623,73 @@ components: $ref: '#/components/schemas/UserCollectionItem' - $ref: '#/components/schemas/CollectionInfo' + Role: + description: Role item + type: object + properties: + name: + type: string + description: The name of the role + actions: + type: array + items: + $ref: '#/components/schemas/ActionResource' + + RoleCollection: + description: Role Collections + type: object + allOf: + - type: object + properties: + roles: + type: array + items: + $ref: '#/components/schemas/Role' + - $ref: '#/components/schemas/CollectionInfo' + + Action: + description: Action Item + type: object + properties: + name: + type: string + description: The name of the permission "action" + nullable: false + + ActionCollection: + description: Action Collection + type: object + allOf: + - type: object + properties: + actions: + type: array + items: + $ref: '#/components/schemas/Action' + - $ref: '#/components/schemas/CollectionInfo' + + Resource: + description: A "resource" on which permissions are granted. + type: object + properties: + name: + type: string + description: The name of the resource + nullable: false + + ActionResource: + description: The Action-Resource item + type: object + properties: + action: + type: object + $ref: '#/components/schemas/Action' + description: The permission action + resource: + type: object + $ref: '#/components/schemas/Resource' + description: The permission resource + ConnectionCollectionItem: description: > Connection collection item. @@ -2406,73 +2473,6 @@ components: $ref: '#/components/schemas/PluginCollectionItem' - $ref: '#/components/schemas/CollectionInfo' - Role: - description: Role item - type: object - properties: - name: - type: string - description: The name of the role - actions: - type: array - items: - $ref: '#/components/schemas/ActionResource' - - RoleCollection: - description: Role Collections - type: object - allOf: - - type: object - properties: - roles: - type: array - items: - $ref: '#/components/schemas/Role' - - $ref: '#/components/schemas/CollectionInfo' - - Action: - description: Action Item - type: object - properties: - name: - type: string - description: The name of the permission "action" - nullable: false - - ActionCollection: - description: Action Collection - type: object - allOf: - - type: object - properties: - actions: - type: array - items: - $ref: '#/components/schemas/Action' - - $ref: '#/components/schemas/CollectionInfo' - - Resource: - description: A "resource" on which permissions are granted. - type: object - properties: - name: - type: string - description: The name of the resource - nullable: false - - ActionResource: - description: The Action-Resource item - type: object - properties: - action: - type: object - $ref: '#/components/schemas/Action' - description: The permission action - resource: - type: object - $ref: '#/components/schemas/Resource' - description: The permission resource - # Configuration ConfigOption: type: object From 17fdee5fae655643536c2e8611264df9c14ffa15 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 17 Mar 2021 22:39:36 +0100 Subject: [PATCH 24/26] Move auth code to security manager and fix tests --- .../api_connexion/endpoints/auth_endpoint.py | 15 +---- airflow/www/security.py | 10 ++++ .../endpoints/test_auth_endpoint.py | 57 ++++++++++++------- tests/api_connexion/test_webserver_auth.py | 2 +- 4 files changed, 51 insertions(+), 33 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 71d0926d35503..64088cee863f8 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -16,25 +16,16 @@ # under the License. from flask import current_app, jsonify -from flask_jwt_extended import create_access_token, set_access_cookies, unset_jwt_cookies +from flask_jwt_extended import set_access_cookies, unset_jwt_cookies -from airflow.api_connexion import security from airflow.api_connexion.schemas.user_schema import user_collection_item_schema def login(): """User login""" ab_security_manager = current_app.appbuilder.sm - user = ab_security_manager.current_user - if user: - access_token = create_access_token(user.id) - resp = jsonify(user_collection_item_schema.dump(user)) - set_access_cookies(resp, access_token) - return resp - security.check_authentication() - user = current_app.appbuilder.sm.current_user - access_token = create_access_token(user.id) - resp = jsonify(user_collection_item_schema.dump(user)) + access_token = ab_security_manager.create_access_token() + resp = jsonify(user_collection_item_schema.dump(ab_security_manager.current_user)) set_access_cookies(resp, access_token) return resp diff --git a/airflow/www/security.py b/airflow/www/security.py index 499465700602e..9f72d3ef6850b 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -23,10 +23,12 @@ from flask_appbuilder.security.sqla import models as sqla_models from flask_appbuilder.security.sqla.manager import SecurityManager from flask_appbuilder.security.sqla.models import PermissionView, Role, User +from flask_jwt_extended import create_access_token from sqlalchemy import or_ from sqlalchemy.orm import joinedload from airflow import models +from airflow.api_connexion import security as webserver_security from airflow.exceptions import AirflowException from airflow.models import DagModel from airflow.security import permissions @@ -659,3 +661,11 @@ def check_authorization( return False return True + + def create_access_token(self): + """Creates access token for api auth use""" + user = self.current_user + if user: + return create_access_token(user.id) + webserver_security.check_authentication() + return create_access_token(self.current_user.id) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 6fc1cf454fe44..09554fee47aeb 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -15,36 +15,38 @@ # specific language governing permissions and limitations # under the License. -import unittest from base64 import b64encode -from airflow.www import app +import pytest + from tests.test_utils.api_connexion_utils import create_user, delete_user from tests.test_utils.config import conf_vars TEST_USERNAME = "test" -class TestLoginEndpoint(unittest.TestCase): - @classmethod - def setUpClass(cls) -> None: - super().setUpClass() - with conf_vars( - { - ("webserver", "session_lifetime_minutes"): "1", - ("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend", - } - ): - cls.app = app.create_app(testing=True) # type:ignore - create_user(cls.app, username="test", role_name="Test") # type: ignore - - @classmethod - def tearDownClass(cls) -> None: - delete_user(cls.app, username="test") # type: ignore - - def setUp(self) -> None: +@pytest.fixture(scope="module") +def configured_app(minimal_app_for_api): + app = minimal_app_for_api + create_user( + app, # type: ignore + username="test", + role_name="Test", + ) + + yield app + + delete_user(app, username="test") # type: ignore + + +class TestBase: + @pytest.fixture(autouse=True) + def setup_attrs(self, configured_app) -> None: + self.app = configured_app self.client = self.app.test_client() # type:ignore + +class TestLoginEndpoint(TestBase): def test_user_can_login_with_basic_auth(self): token = "Basic " + b64encode(b"test:test").decode() self.client.post("api/v1/login", headers={"Authorization": token}) @@ -94,3 +96,18 @@ def test_cookie_is_set_for_remote_user_when_db_login_is_disabled(self): ) assert cookie is not None assert isinstance(cookie.value, str) + + +class TestLogout(TestBase): + def test_logout_unsets_cookie(self): + self.client.post("api/v1/login", environ_overrides={'REMOTE_USER': "test"}) + cookie = next( + (cookie for cookie in self.client.cookie_jar if cookie.name == "access_token_cookie"), None + ) + assert cookie is not None + assert isinstance(cookie.value, str) + self.client.post("api/v1/logout", environ_overrides={'REMOTE_USER': "test"}) + cookie = next( + (cookie for cookie in self.client.cookie_jar if cookie.name == "access_token_cookie"), None + ) + assert cookie is None diff --git a/tests/api_connexion/test_webserver_auth.py b/tests/api_connexion/test_webserver_auth.py index 9d0a20f836089..ce6fa5ed83d19 100644 --- a/tests/api_connexion/test_webserver_auth.py +++ b/tests/api_connexion/test_webserver_auth.py @@ -40,7 +40,7 @@ def setUpClass(cls) -> None: create_user( cls.app, # type: ignore username="test", - role_name="Admin", + role_name="test", permissions=[(permissions.ACTION_CAN_READ, permissions.RESOURCE_POOL)], ) cls.tester = cls.appbuilder.sm.find_user(username="test") From ef4fdb2055423458bcdec6a04b027727025514ea Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 19 Mar 2021 19:01:01 +0100 Subject: [PATCH 25/26] add back mistakenly deleted test_user_endpoint --- .../endpoints/test_user_endpoint.py | 225 ++++++++++++++++++ 1 file changed, 225 insertions(+) create mode 100644 tests/api_connexion/endpoints/test_user_endpoint.py diff --git a/tests/api_connexion/endpoints/test_user_endpoint.py b/tests/api_connexion/endpoints/test_user_endpoint.py new file mode 100644 index 0000000000000..b52c54cfdd112 --- /dev/null +++ b/tests/api_connexion/endpoints/test_user_endpoint.py @@ -0,0 +1,225 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import pytest +from flask_appbuilder.security.sqla.models import User +from parameterized import parameterized + +from airflow.api_connexion.exceptions import EXCEPTIONS_LINK_MAP +from airflow.security import permissions +from airflow.utils import timezone +from tests.test_utils.api_connexion_utils import assert_401, create_user, delete_user +from tests.test_utils.config import conf_vars + +DEFAULT_TIME = "2020-06-11T18:00:00+00:00" + + +@pytest.fixture(scope="module") +def configured_app(minimal_app_for_api): + app = minimal_app_for_api + create_user( + app, # type: ignore + username="test", + role_name="Test", + permissions=[ + (permissions.ACTION_CAN_LIST, permissions.RESOURCE_USER_DB_MODELVIEW), + (permissions.ACTION_CAN_SHOW, permissions.RESOURCE_USER_DB_MODELVIEW), + ], + ) + create_user(app, username="test_no_permissions", role_name="TestNoPermissions") # type: ignore + + yield app + + delete_user(app, username="test") # type: ignore + delete_user(app, username="test_no_permissions") # type: ignore + + +class TestUserEndpoint: + @pytest.fixture(autouse=True) + def setup_attrs(self, configured_app) -> None: + self.app = configured_app + self.client = self.app.test_client() # type:ignore + self.session = self.app.appbuilder.get_session + + def teardown_method(self) -> None: + # Delete users that have our custom default time + users = self.session.query(User).filter(User.changed_on == timezone.parse(DEFAULT_TIME)).all() + for user in users: + self.session.delete(user) + self.session.commit() + + def _create_users(self, count, roles=None): + # create users with defined created_on and changed_on date + # for easy testing + if roles is None: + roles = [] + return [ + User( + first_name=f'test{i}', + last_name=f'test{i}', + username=f'TEST_USER{i}', + email=f'mytest@test{i}.org', + roles=roles or [], + created_on=timezone.parse(DEFAULT_TIME), + changed_on=timezone.parse(DEFAULT_TIME), + ) + for i in range(1, count + 1) + ] + + +class TestGetUser(TestUserEndpoint): + def test_should_respond_200(self): + users = self._create_users(1) + self.session.add_all(users) + self.session.commit() + response = self.client.get("/api/v1/users/TEST_USER1", environ_overrides={'REMOTE_USER': "test"}) + assert response.status_code == 200 + assert response.json == { + 'active': None, + 'changed_on': DEFAULT_TIME, + 'created_on': DEFAULT_TIME, + 'email': 'mytest@test1.org', + 'fail_login_count': None, + 'first_name': 'test1', + 'last_login': None, + 'last_name': 'test1', + 'login_count': None, + 'roles': [], + 'user_id': users[0].id, + 'username': 'TEST_USER1', + } + + def test_should_respond_404(self): + response = self.client.get("/api/v1/users/invalid-user", environ_overrides={'REMOTE_USER': "test"}) + assert response.status_code == 404 + assert { + 'detail': "The User with username `invalid-user` was not found", + 'status': 404, + 'title': 'User not found', + 'type': EXCEPTIONS_LINK_MAP[404], + } == response.json + + def test_should_raises_401_unauthenticated(self): + response = self.client.get("/api/v1/users/TEST_USER1") + assert_401(response) + + def test_should_raise_403_forbidden(self): + response = self.client.get( + "/api/v1/users/TEST_USER1", environ_overrides={'REMOTE_USER': "test_no_permissions"} + ) + assert response.status_code == 403 + + +class TestGetUsers(TestUserEndpoint): + def test_should_response_200(self): + response = self.client.get("/api/v1/users", environ_overrides={'REMOTE_USER': "test"}) + assert response.status_code == 200 + assert response.json["total_entries"] == 2 + usernames = [user["username"] for user in response.json["users"] if user] + assert usernames == ['test', 'test_no_permissions'] + + def test_should_raises_401_unauthenticated(self): + response = self.client.get("/api/v1/users") + assert_401(response) + + def test_should_raise_403_forbidden(self): + response = self.client.get("/api/v1/users", environ_overrides={'REMOTE_USER': "test_no_permissions"}) + assert response.status_code == 403 + + +class TestGetUsersPagination(TestUserEndpoint): + @parameterized.expand( + [ + ("/api/v1/users?limit=1", ["test"]), + ("/api/v1/users?limit=2", ["test", "test_no_permissions"]), + ( + "/api/v1/users?offset=5", + [ + "TEST_USER4", + "TEST_USER5", + "TEST_USER6", + "TEST_USER7", + "TEST_USER8", + "TEST_USER9", + "TEST_USER10", + ], + ), + ( + "/api/v1/users?offset=0", + [ + "test", + "test_no_permissions", + "TEST_USER1", + "TEST_USER2", + "TEST_USER3", + "TEST_USER4", + "TEST_USER5", + "TEST_USER6", + "TEST_USER7", + "TEST_USER8", + "TEST_USER9", + "TEST_USER10", + ], + ), + ("/api/v1/users?limit=1&offset=5", ["TEST_USER4"]), + ("/api/v1/users?limit=1&offset=1", ["test_no_permissions"]), + ( + "/api/v1/users?limit=2&offset=2", + ["TEST_USER1", "TEST_USER2"], + ), + ] + ) + def test_handle_limit_offset(self, url, expected_usernames): + users = self._create_users(10) + self.session.add_all(users) + self.session.commit() + response = self.client.get(url, environ_overrides={'REMOTE_USER': "test"}) + assert response.status_code == 200 + assert response.json["total_entries"] == 12 + usernames = [user["username"] for user in response.json["users"] if user] + assert usernames == expected_usernames + + def test_should_respect_page_size_limit_default(self): + users = self._create_users(200) + self.session.add_all(users) + self.session.commit() + + response = self.client.get("/api/v1/users", environ_overrides={'REMOTE_USER': "test"}) + assert response.status_code == 200 + # Explicitly add the 2 users on setUp + assert response.json["total_entries"] == 200 + len(['test', 'test_no_permissions']) + assert len(response.json["users"]) == 100 + + def test_limit_of_zero_should_return_default(self): + users = self._create_users(200) + self.session.add_all(users) + self.session.commit() + + response = self.client.get("/api/v1/users?limit=0", environ_overrides={'REMOTE_USER': "test"}) + assert response.status_code == 200 + # Explicit add the 2 users on setUp + assert response.json["total_entries"] == 200 + len(['test', 'test_no_permissions']) + assert len(response.json["users"]) == 100 + + @conf_vars({("api", "maximum_page_limit"): "150"}) + def test_should_return_conf_max_if_req_max_above_conf(self): + users = self._create_users(200) + self.session.add_all(users) + self.session.commit() + + response = self.client.get("/api/v1/users?limit=180", environ_overrides={'REMOTE_USER': "test"}) + assert response.status_code == 200 + assert len(response.json['users']) == 150 From b3dc520edec486f9b1fc2a56c62e3cd7d649b537 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 22 Mar 2021 09:37:56 +0100 Subject: [PATCH 26/26] Do not move role/permissions in openapi in this PR --- airflow/api_connexion/openapi/v1.yaml | 134 +++++++++++++------------- 1 file changed, 67 insertions(+), 67 deletions(-) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 02679f8323e8c..d82bdf4f125e6 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1623,73 +1623,6 @@ components: $ref: '#/components/schemas/UserCollectionItem' - $ref: '#/components/schemas/CollectionInfo' - Role: - description: Role item - type: object - properties: - name: - type: string - description: The name of the role - actions: - type: array - items: - $ref: '#/components/schemas/ActionResource' - - RoleCollection: - description: Role Collections - type: object - allOf: - - type: object - properties: - roles: - type: array - items: - $ref: '#/components/schemas/Role' - - $ref: '#/components/schemas/CollectionInfo' - - Action: - description: Action Item - type: object - properties: - name: - type: string - description: The name of the permission "action" - nullable: false - - ActionCollection: - description: Action Collection - type: object - allOf: - - type: object - properties: - actions: - type: array - items: - $ref: '#/components/schemas/Action' - - $ref: '#/components/schemas/CollectionInfo' - - Resource: - description: A "resource" on which permissions are granted. - type: object - properties: - name: - type: string - description: The name of the resource - nullable: false - - ActionResource: - description: The Action-Resource item - type: object - properties: - action: - type: object - $ref: '#/components/schemas/Action' - description: The permission action - resource: - type: object - $ref: '#/components/schemas/Resource' - description: The permission resource - ConnectionCollectionItem: description: > Connection collection item. @@ -2473,6 +2406,73 @@ components: $ref: '#/components/schemas/PluginCollectionItem' - $ref: '#/components/schemas/CollectionInfo' + Role: + description: Role item + type: object + properties: + name: + type: string + description: The name of the role + actions: + type: array + items: + $ref: '#/components/schemas/ActionResource' + + RoleCollection: + description: Role Collections + type: object + allOf: + - type: object + properties: + roles: + type: array + items: + $ref: '#/components/schemas/Role' + - $ref: '#/components/schemas/CollectionInfo' + + Action: + description: Action Item + type: object + properties: + name: + type: string + description: The name of the permission "action" + nullable: false + + ActionCollection: + description: Action Collection + type: object + allOf: + - type: object + properties: + actions: + type: array + items: + $ref: '#/components/schemas/Action' + - $ref: '#/components/schemas/CollectionInfo' + + Resource: + description: A "resource" on which permissions are granted. + type: object + properties: + name: + type: string + description: The name of the resource + nullable: false + + ActionResource: + description: The Action-Resource item + type: object + properties: + action: + type: object + $ref: '#/components/schemas/Action' + description: The permission action + resource: + type: object + $ref: '#/components/schemas/Resource' + description: The permission resource + # Configuration ConfigOption: type: object