From b243f1631508763bbb62890a508184e0585c44fa Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Thu, 25 Mar 2021 02:44:14 +0100 Subject: [PATCH 01/53] Initialize api auth flow --- .../api_connexion/endpoints/auth_endpoint.py | 112 ++++++++++++++++++ airflow/api_connexion/openapi/v1.yaml | 106 +++++++++++++++++ airflow/api_connexion/schemas/auth_schema.py | 60 ++++++++++ airflow/api_connexion/security.py | 11 +- 4 files changed, 287 insertions(+), 2 deletions(-) create mode 100644 airflow/api_connexion/endpoints/auth_endpoint.py create mode 100644 airflow/api_connexion/schemas/auth_schema.py diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py new file mode 100644 index 0000000000000..43ffb144edf7a --- /dev/null +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -0,0 +1,112 @@ +# 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 jwt +from flask import current_app, g, request, session +from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER +from flask_jwt_extended import create_access_token, create_refresh_token +from flask_login import login_user +from jsonschema import ValidationError + +from airflow.api_connexion.exceptions import BadRequest, NotFound, Unauthenticated +from airflow.api_connexion.schemas.auth_schema import info_schema, jwt_token_schema, login_form_schema +from airflow.configuration import conf + + +def get_info(): + """Get information about site including auth methods""" + security_manager = current_app.appbuilder.sm + oauth_providers = None + openid_providers = None + auth_type = security_manager.auth_type + type_mapping = { + AUTH_DB: "auth_db", + AUTH_LDAP: "auth_ldap", + AUTH_OID: "auth_oid", + AUTH_OAUTH: "auth_oauth", + AUTH_REMOTE_USER: "auth_remote_user", + } + + if auth_type == AUTH_OAUTH: + oauth_providers = security_manager.oauth_providers + if auth_type == AUTH_OID: + openid_providers = security_manager.openid_providers + return info_schema.dump( + { + "auth_type": type_mapping[auth_type], + "oauth_providers": oauth_providers, + "openid_providers": openid_providers, + } + ) + + +def auth_dblogin(): + """Handle DB login""" + security_manager = current_app.appbuilder.sm + auth_type = security_manager.auth_type + if g.user is not None and g.user.is_authenticated: + raise Unauthenticated(detail="Client already authenticated") # For security + if auth_type != AUTH_DB or auth_type != AUTH_LDAP: + raise BadRequest(detail="Authentication type do not match") + body = request.json + try: + data = login_form_schema.load(body) + except ValidationError as err: + raise BadRequest(detail=str(err)) + user = security_manager.auth_user_db(data['username'], data['password']) + if not user: + user = security_manager.auth_user_ldap(data['username'], data['password']) + if not user: + raise NotFound(detail="Invalid login") + login_user(user, remember=False) + token = create_access_token(identity=user.id, fresh=True) + refresh_token = create_refresh_token(user.id) + return jwt_token_schema.dump(dict(token=token, refresh_token=refresh_token)) + + +def refresh(): + """Refresh token""" + + +def auth_oauthlogin(provider, register=None): + """Handle Oauth login""" + api_base_path = "/api/v1/" + base_url = conf.get("webserver", "base_url") + api_base_path + appbuilder = current_app.appbuilder + if g.user is not None and g.user.is_authenticated: + raise Unauthenticated(detail="Client already authenticated") + state = jwt.encode( + request.args.to_dict(flat=False), + appbuilder.app.config["SECRET_KEY"], + algorithm="HS256", + ) + redirect_uri = base_url + f"authorized?provider={provider}" + try: + if register: + session["register"] = True + if provider == "twitter": + return appbuilder.sm.oauth_remotes[provider].authorize_redirect( + redirect_uri=redirect_uri + f"&state={state}" + ) + else: + return appbuilder.sm.oauth_remotes[provider].authorize_redirect( + redirect_uri=redirect_uri, + state=state.decode("ascii") if isinstance(state, bytes) else state, + ) + except Exception: + + return diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 2fd58ea326955..3b9bad310e139 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1572,6 +1572,58 @@ paths: '404': $ref: '#/components/responses/NotFound' + /auth_info: + get: + summary: Get authentication information + x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint + operationId: get_info + tags: [Auth] + responses: + '200': + description: Success. + content: + application/json: + schema: + $ref: '#/components/schemas/AuthInfo' + + /auth_dblogin: + post: + summary: Login user + x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint + operationId: auth_dblogin + tags: [Auth] + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/AuthDBLoginForm' + responses: + '200': + description: Success. + content: + application/json: + schema: + $ref: '#/components/schemas/JwtToken' + + /auth_oauth/{provider}: + parameters: + - $ref: '#/components/parameters/OauthProvider' + get: + summary: Login user using oauth + x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint + operationId: auth_oauthlogin + tags: [Auth] + parameters: + - $ref: '#/components/parameters/ShouldRegister' + responses: + '200': + description: Success. + content: + application/json: + schema: + $ref: '#/components/schemas/JwtToken' + components: # Reusable schemas (data models) schemas: @@ -2508,6 +2560,45 @@ components: type: object $ref: '#/components/schemas/Resource' description: The permission resource + # Auth + AuthInfo: + type: object + description: Information about the authentication method in use + properties: + auth_type: + type: string + description: Authentication type in use + oauth_providers: + type: array + items: + type: object + nullable: true + openid_providers: + type: array + items: + type: object + nullable: true + + AuthDBLoginForm: + type: object + properties: + username: + description: The username for authentication + example: admin + type: string + password: + description: The password for authentication + example: complex-password + type: string + + JwtToken: + description: Response object after successful authentication + type: object + properties: + token: + type: string + refresh_token: + type: string # Configuration ConfigOption: @@ -3267,6 +3358,21 @@ components: Prefix a field name with `-` to reverse the sort order. # Other parameters + OauthProvider: + in: path + name: provider + schema: + type: string + required: true + description: The name of the provider to authenticate + + ShouldRegister: + in: query + name: register + schema: + type: string + required: false + description: Whether to register user FileToken: in: path diff --git a/airflow/api_connexion/schemas/auth_schema.py b/airflow/api_connexion/schemas/auth_schema.py new file mode 100644 index 0000000000000..33d6015be4ad4 --- /dev/null +++ b/airflow/api_connexion/schemas/auth_schema.py @@ -0,0 +1,60 @@ +# 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 marshmallow import fields +from marshmallow.schema import Schema + + +class OAUTHSchema(Schema): + """A little information needed for UI""" + + name = fields.String() + icon = fields.String() + + +class OPENIDSchema(Schema): + """A little information needed for the UI""" + + name = fields.String() + url = fields.String() + + +class InfoSchema(Schema): + """Authentication Info Schema""" + + auth_type = fields.String() + oauth_providers = fields.List(fields.Nested(OAUTHSchema)) + openid_providers = fields.List(fields.Nested(OPENIDSchema)) + + +class LoginForm(Schema): + """Use to load credentials""" + + username = fields.String(required=True) + password = fields.String(required=True) + + +class JwtTokenSchema(Schema): + """Response object after successful authentication""" + + token = fields.String() + refresh_token = fields.String() + + +info_schema = InfoSchema() +login_form_schema = LoginForm() +jwt_token_schema = JwtTokenSchema() diff --git a/airflow/api_connexion/security.py b/airflow/api_connexion/security.py index 4faa9ed8f3457..57df760933fa7 100644 --- a/airflow/api_connexion/security.py +++ b/airflow/api_connexion/security.py @@ -19,6 +19,7 @@ from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast from flask import Response, current_app +from flask_jwt_extended import verify_jwt_in_request from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated @@ -28,10 +29,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: + if response.status_code == 200: + return + try: + verify_jwt_in_request() + return + except Exception: # pylint: disable=broad-except + pass # since this handler only checks authentication, not authorization, # we should always return 401 - raise Unauthenticated(headers=response.headers) + raise Unauthenticated(headers=response.headers) def requires_access(permissions: Optional[Sequence[Tuple[str, str]]] = None) -> Callable[[T], T]: From f46b1e02b70e2106acb8db7833745355ee319679 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Thu, 25 Mar 2021 16:14:30 +0100 Subject: [PATCH 02/53] add oauth endpoint --- .../api_connexion/endpoints/auth_endpoint.py | 57 +++++++++++++--- airflow/api_connexion/openapi/v1.yaml | 66 +++++++++++++++++-- 2 files changed, 110 insertions(+), 13 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 43ffb144edf7a..6f373d8a06306 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -15,6 +15,8 @@ # specific language governing permissions and limitations # under the License. +import logging + import jwt from flask import current_app, g, request, session from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER @@ -26,6 +28,8 @@ from airflow.api_connexion.schemas.auth_schema import info_schema, jwt_token_schema, login_form_schema from airflow.configuration import conf +log = logging.getLogger(__name__) + def get_info(): """Get information about site including auth methods""" @@ -85,28 +89,63 @@ def refresh(): def auth_oauthlogin(provider, register=None): """Handle Oauth login""" api_base_path = "/api/v1/" - base_url = conf.get("webserver", "base_url") + api_base_path + appbuilder = current_app.appbuilder + base_url = conf.get("webserver", "base_url") + api_base_path + redirect_uri = base_url + "oauth-authorized/" + provider if g.user is not None and g.user.is_authenticated: - raise Unauthenticated(detail="Client already authenticated") + pass # raise Unauthenticated(detail="Client already authenticated") + if appbuilder.sm.auth_type != AUTH_OAUTH: + raise BadRequest(detail="Authentication type do not match") state = jwt.encode( request.args.to_dict(flat=False), appbuilder.app.config["SECRET_KEY"], algorithm="HS256", ) - redirect_uri = base_url + f"authorized?provider={provider}" try: + auth_provider = appbuilder.sm.oauth_remotes[provider] if register: session["register"] = True if provider == "twitter": - return appbuilder.sm.oauth_remotes[provider].authorize_redirect( - redirect_uri=redirect_uri + f"&state={state}" - ) + return auth_provider.authorize_redirect(redirect_uri=redirect_uri + f"&state={state}") else: - return appbuilder.sm.oauth_remotes[provider].authorize_redirect( + return auth_provider.authorize_redirect( redirect_uri=redirect_uri, state=state.decode("ascii") if isinstance(state, bytes) else state, ) - except Exception: + except Exception: # pylint: disable=broad-except + raise NotFound(detail="Invalid login") + - return +def authorize_oauth(provider): + """Callback to authorize Oauth""" + appbuilder = current_app.appbuilder + resp = appbuilder.sm.oauth_remotes[provider].authorize_access_token() + if resp is None: + raise BadRequest(detail="You denied the request to sign in") + log.debug("OAUTH Authorized resp: %s", resp) + # Verify state + try: + jwt.decode( + request.args["state"], + appbuilder.app.config["SECRET_KEY"], + algorithms=["HS256"], + ) + except jwt.InvalidTokenError: + raise BadRequest(detail="State signature is not valid!") + # Retrieves specific user info from the provider + try: + userinfo = appbuilder.sm.oauth_user_info(provider, resp) + except Exception as e: # pylint: disable=broad-except + log.error("Error returning OAuth user info: %s", e) + user = None + else: + log.debug("User info retrieved from %s: %s", provider, userinfo) + user = appbuilder.sm.auth_user_oauth(userinfo) + + if user is None: + return NotFound(detail="Invalid login") + login_user(user) + token = create_access_token(user.id, fresh=True) + refresh_token = create_refresh_token(user.id) + return dict(token=token, refresh_token=refresh_token) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 3b9bad310e139..5b3c24432c943 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1607,8 +1607,6 @@ paths: $ref: '#/components/schemas/JwtToken' /auth_oauth/{provider}: - parameters: - - $ref: '#/components/parameters/OauthProvider' get: summary: Login user using oauth x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint @@ -1616,6 +1614,31 @@ paths: tags: [Auth] parameters: - $ref: '#/components/parameters/ShouldRegister' + - $ref: '#/components/parameters/OauthProvider' + responses: + '200': + description: Success. + content: + application/json: + schema: + type: object + properties: + auth_url: + type: string + /oauth-authorized/{provider}: + parameters: + - $ref: '#/components/parameters/OauthProvider' + get: + summary: Oauth authorization redirect + x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint + operationId: authorize_oauth + tags: [Auth] + parameters: + - $ref: '#/components/parameters/AuthScope' + - $ref: '#/components/parameters/AuthState' + - $ref: '#/components/parameters/AuthPrompt' + - $ref: '#/components/parameters/AuthUser' + - $ref: '#/components/parameters/AuthCode' responses: '200': description: Success. @@ -3357,7 +3380,7 @@ components: The name of the field to order the results by. Prefix a field name with `-` to reverse the sort order. - # Other parameters + # Auth parameters OauthProvider: in: path name: provider @@ -3365,7 +3388,41 @@ components: type: string required: true description: The name of the provider to authenticate - + AuthUser: + in: query + name: authuser + schema: + type: string + required: true + description: The authuser from provider + AuthScope: + in: query + name: scope + schema: + type: string + required: true + description: The auth scope from provider + AuthPrompt: + in: query + name: prompt + schema: + type: string + required: true + description: The auth prompt from provider + AuthState: + in: query + name: state + schema: + type: string + required: true + description: The auth state from provider + AuthCode: + in: query + name: code + schema: + type: string + required: true + description: The auth code from provider ShouldRegister: in: query name: register @@ -3374,6 +3431,7 @@ components: required: false description: Whether to register user + # Other parameters FileToken: in: path name: file_token From c96319dfdd6bdd0fa826859a1bde6afffb8f8617 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 26 Mar 2021 02:03:46 +0100 Subject: [PATCH 03/53] Add more auth flow and token refreshing --- .../api_connexion/endpoints/auth_endpoint.py | 76 +++++++----- airflow/api_connexion/openapi/v1.yaml | 109 ++++++++++-------- airflow/api_connexion/schemas/auth_schema.py | 11 +- airflow/www/app.py | 8 +- airflow/www/extensions/init_security.py | 37 ++++++ airflow/www/security.py | 12 +- .../endpoints/test_auth_endpoint.py | 16 +++ 7 files changed, 190 insertions(+), 79 deletions(-) create mode 100644 tests/api_connexion/endpoints/test_auth_endpoint.py diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 6f373d8a06306..e39e213fbba99 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -20,19 +20,21 @@ import jwt from flask import current_app, g, request, session from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER -from flask_jwt_extended import create_access_token, create_refresh_token from flask_login import login_user from jsonschema import ValidationError from airflow.api_connexion.exceptions import BadRequest, NotFound, Unauthenticated -from airflow.api_connexion.schemas.auth_schema import info_schema, jwt_token_schema, login_form_schema -from airflow.configuration import conf +from airflow.api_connexion.schemas.auth_schema import info_schema, login_form_schema log = logging.getLogger(__name__) -def get_info(): - """Get information about site including auth methods""" +def refresh(): + """Refresh token""" + + +def get_auth_info(): + """Get site authentication info""" security_manager = current_app.appbuilder.sm oauth_providers = None openid_providers = None @@ -64,7 +66,7 @@ def auth_dblogin(): auth_type = security_manager.auth_type if g.user is not None and g.user.is_authenticated: raise Unauthenticated(detail="Client already authenticated") # For security - if auth_type != AUTH_DB or auth_type != AUTH_LDAP: + if auth_type not in (AUTH_DB, AUTH_LDAP): raise BadRequest(detail="Authentication type do not match") body = request.json try: @@ -77,22 +79,12 @@ def auth_dblogin(): if not user: raise NotFound(detail="Invalid login") login_user(user, remember=False) - token = create_access_token(identity=user.id, fresh=True) - refresh_token = create_refresh_token(user.id) - return jwt_token_schema.dump(dict(token=token, refresh_token=refresh_token)) - - -def refresh(): - """Refresh token""" + return security_manager.create_access_token_and_dump_user() -def auth_oauthlogin(provider, register=None): +def auth_oauthlogin(provider, register=None, redirect_uri=None): """Handle Oauth login""" - api_base_path = "/api/v1/" - appbuilder = current_app.appbuilder - base_url = conf.get("webserver", "base_url") + api_base_path - redirect_uri = base_url + "oauth-authorized/" + provider if g.user is not None and g.user.is_authenticated: pass # raise Unauthenticated(detail="Client already authenticated") if appbuilder.sm.auth_type != AUTH_OAUTH: @@ -107,18 +99,24 @@ def auth_oauthlogin(provider, register=None): if register: session["register"] = True if provider == "twitter": - return auth_provider.authorize_redirect(redirect_uri=redirect_uri + f"&state={state}") + redirect_uri = redirect_uri + f"&state={state}" + auth_data = auth_provider.create_authorization_url(redirect_uri=redirect_uri) + auth_provider.save_authorize_data(request, redirect_uri=redirect_uri, **auth_data) + return dict(auth_url=auth_data['url']) else: - return auth_provider.authorize_redirect( + state = state.decode("ascii") if isinstance(state, bytes) else state + auth_data = auth_provider.create_authorization_url( redirect_uri=redirect_uri, - state=state.decode("ascii") if isinstance(state, bytes) else state, + state=state, ) - except Exception: # pylint: disable=broad-except - raise NotFound(detail="Invalid login") + auth_provider.save_authorize_data(request, redirect_uri=redirect_uri, **auth_data) + return dict(auth_url=auth_data['url']) + except Exception as err: # pylint: disable=broad-except + raise NotFound(detail=str(err)) -def authorize_oauth(provider): - """Callback to authorize Oauth""" +def authorize_oauth(provider, state): + """Callback to authorize Oauth.""" appbuilder = current_app.appbuilder resp = appbuilder.sm.oauth_remotes[provider].authorize_access_token() if resp is None: @@ -127,7 +125,7 @@ def authorize_oauth(provider): # Verify state try: jwt.decode( - request.args["state"], + state, appbuilder.app.config["SECRET_KEY"], algorithms=["HS256"], ) @@ -135,6 +133,7 @@ def authorize_oauth(provider): raise BadRequest(detail="State signature is not valid!") # Retrieves specific user info from the provider try: + appbuilder.sm.set_oauth_session(provider, resp) userinfo = appbuilder.sm.oauth_user_info(provider, resp) except Exception as e: # pylint: disable=broad-except log.error("Error returning OAuth user info: %s", e) @@ -144,8 +143,25 @@ def authorize_oauth(provider): user = appbuilder.sm.auth_user_oauth(userinfo) if user is None: - return NotFound(detail="Invalid login") + raise NotFound(detail="Invalid login") login_user(user) - token = create_access_token(user.id, fresh=True) - refresh_token = create_refresh_token(user.id) - return dict(token=token, refresh_token=refresh_token) + return appbuilder.sm.create_access_token_and_dump_user() + + +def auth_remoteuser(): + """Handle remote user auth""" + appbuilder = current_app.appbuilder + username = request.environ.get("REMOTE_USER") + if g.user is not None and g.user.is_authenticated: + pass # raise Unauthenticated(detail="Client already authenticated") + if appbuilder.sm.auth_type != AUTH_REMOTE_USER: + raise BadRequest(detail="Authentication type do not match") + if username: + user = appbuilder.sm.auth_user_remote_user(username) + if user is None: + raise NotFound(detail="Invalid login") + else: + login_user(user) + else: + raise NotFound(detail="Invalid login") + return appbuilder.sm.create_access_token_and_dump_user() diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 5b3c24432c943..02928f6e1dbb6 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1572,12 +1572,12 @@ paths: '404': $ref: '#/components/responses/NotFound' - /auth_info: + /auth-info: get: - summary: Get authentication information + summary: Get site authentication information x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint - operationId: get_info - tags: [Auth] + operationId: get_auth_info + tags: [Authentication] responses: '200': description: Success. @@ -1586,12 +1586,12 @@ paths: schema: $ref: '#/components/schemas/AuthInfo' - /auth_dblogin: + /auth-dblogin: post: summary: Login user x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint operationId: auth_dblogin - tags: [Auth] + tags: [Authentication] requestBody: required: true content: @@ -1604,48 +1604,72 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/JwtToken' + $ref: '#/components/schemas/User' + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthenticated' + - /auth_oauth/{provider}: + /auth-oauth/{provider}: get: - summary: Login user using oauth + summary: OAUTH user login x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint operationId: auth_oauthlogin - tags: [Auth] + tags: [Authentication] parameters: - $ref: '#/components/parameters/ShouldRegister' - $ref: '#/components/parameters/OauthProvider' + - $ref: '#/components/parameters/RedirectUri' responses: '200': description: Success. content: application/json: schema: - type: object - properties: - auth_url: - type: string + $ref: '#/components/schemas/AuthorizationUrl' + /oauth-authorized/{provider}: parameters: - $ref: '#/components/parameters/OauthProvider' get: - summary: Oauth authorization redirect + summary: Oauth authentication x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint operationId: authorize_oauth - tags: [Auth] + tags: [Authentication] parameters: - - $ref: '#/components/parameters/AuthScope' - $ref: '#/components/parameters/AuthState' - - $ref: '#/components/parameters/AuthPrompt' - - $ref: '#/components/parameters/AuthUser' - - $ref: '#/components/parameters/AuthCode' responses: '200': description: Success. content: application/json: schema: - $ref: '#/components/schemas/JwtToken' + $ref: '#/components/schemas/User' + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthenticated' + + + /auth-remoteuser: + get: + summary: Authenticate using remote user auth + x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint + operationId: auth_remoteuser + tags: [Authentication] + responses: + '200': + description: Success. + content: + application/json: + schema: + $ref: '#/components/schemas/User' + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthenticated' + components: # Reusable schemas (data models) @@ -2623,6 +2647,13 @@ components: refresh_token: type: string + AuthorizationUrl: + description: Oauth authorization URl + type: object + properties: + auth_url: + type: string + # Configuration ConfigOption: type: object @@ -3388,27 +3419,7 @@ components: type: string required: true description: The name of the provider to authenticate - AuthUser: - in: query - name: authuser - schema: - type: string - required: true - description: The authuser from provider - AuthScope: - in: query - name: scope - schema: - type: string - required: true - description: The auth scope from provider - AuthPrompt: - in: query - name: prompt - schema: - type: string - required: true - description: The auth prompt from provider + AuthState: in: query name: state @@ -3416,13 +3427,16 @@ components: type: string required: true description: The auth state from provider - AuthCode: + + RedirectUri: in: query - name: code + name: redirect_uri schema: type: string + format: url required: true - description: The auth code from provider + description: The oauth redirect uri from the frontend + ShouldRegister: in: query name: register @@ -3540,6 +3554,10 @@ components: Kerberos: type: http scheme: negotiate + JwtCookieAuth: + 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. @@ -3561,6 +3579,7 @@ tags: - name: Role - name: Permission - name: User + - name: Authentication externalDocs: url: https://airflow.apache.org/docs/apache-airflow/stable/ diff --git a/airflow/api_connexion/schemas/auth_schema.py b/airflow/api_connexion/schemas/auth_schema.py index 33d6015be4ad4..6a215a875a831 100644 --- a/airflow/api_connexion/schemas/auth_schema.py +++ b/airflow/api_connexion/schemas/auth_schema.py @@ -42,12 +42,20 @@ class InfoSchema(Schema): class LoginForm(Schema): - """Use to load credentials""" + """Used to load credentials""" username = fields.String(required=True) password = fields.String(required=True) +class LoginOpenIDForm(Schema): + """Used to load credentials""" + + username = fields.String() + openid = fields.String(required=True) + remember_me = fields.Boolean(default=False) + + class JwtTokenSchema(Schema): """Response object after successful authentication""" @@ -57,4 +65,5 @@ class JwtTokenSchema(Schema): info_schema = InfoSchema() login_form_schema = LoginForm() +login_openid_form = LoginOpenIDForm() jwt_token_schema = JwtTokenSchema() diff --git a/airflow/www/app.py b/airflow/www/app.py index 632e1d2727c6d..d3a8e5eace2fd 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_airflow_session_interface, init_permanent_session from airflow.www.extensions.init_views import ( init_api_connexion, @@ -129,7 +133,7 @@ def create_app(config=None, testing=False): init_error_handlers(flask_app) init_api_connexion(flask_app) init_api_experimental(flask_app) - + init_jwt_auth(flask_app) sync_appbuilder_roles(flask_app) init_jinja_globals(flask_app) diff --git a/airflow/www/extensions/init_security.py b/airflow/www/extensions/init_security.py index 544deebeb3af4..584d476f99074 100644 --- a/airflow/www/extensions/init_security.py +++ b/airflow/www/extensions/init_security.py @@ -15,10 +15,20 @@ # specific language governing permissions and limitations # under the License. import logging +from datetime import datetime, timedelta from importlib import import_module +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__) @@ -55,3 +65,30 @@ 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 Jwt manager""" + app.config['JWT_TOKEN_LOCATION'] = ['cookies'] + app.config.setdefault('JWT_ACCESS_TOKEN_EXPIRES', timedelta(minutes=5)) + app.config['JWT_CSRF_IN_COOKIES'] = True + app.config['JWT_COOKIE_CSRF_PROTECT'] = True + app.config.setdefault('JWT_COOKIE_SECURE', False) + jwt_manager = JWTManager() + jwt_manager.init_app(app) + jwt_manager.user_loader_callback_loader(app.appbuilder.sm.load_user_jwt) + + def refresh_expiring_jwts(response): + try: + exp_timestamp = get_raw_jwt()["exp"] + now = datetime.now(timezone.utc) + target_timestamp = datetime.timestamp(now + timedelta(minutes=3)) + 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_expiring_jwts) diff --git a/airflow/www/security.py b/airflow/www/security.py index d1bf9bc5565db..4c70f1a5ce4a7 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -20,13 +20,15 @@ import warnings from typing import Dict, Optional, Sequence, Set, Tuple -from flask import current_app, g +from flask import current_app, g, jsonify 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, set_access_cookies from sqlalchemy import or_ from sqlalchemy.orm import joinedload +from airflow.api_connexion.schemas.user_schema import user_collection_item_schema from airflow.exceptions import AirflowException from airflow.models import DagBag, DagModel from airflow.security import permissions @@ -730,6 +732,14 @@ def check_authorization( return True + def create_access_token_and_dump_user(self): + """Creates access token, set token in session and return user""" + user = self.current_user + token = create_access_token(user.id) + resp = jsonify(user_collection_item_schema.dump(user)) + set_access_cookies(resp, token) + return resp + class ApplessAirflowSecurityManager(AirflowSecurityManager): """Security Manager that doesn't need the whole flask app""" diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -0,0 +1,16 @@ +# 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 ed06c4a2a714870ded65dc1b300240511c94a1c7 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 26 Mar 2021 10:46:19 +0100 Subject: [PATCH 04/53] Add tests for dblogin and ldaplogin --- .../api_connexion/endpoints/auth_endpoint.py | 8 +- airflow/www/extensions/init_security.py | 6 +- airflow/www/security.py | 2 +- .../endpoints/test_auth_endpoint.py | 127 ++++++++++++++++++ 4 files changed, 135 insertions(+), 8 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index e39e213fbba99..b34e8d9fddea8 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -21,7 +21,7 @@ from flask import current_app, g, request, session from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER from flask_login import login_user -from jsonschema import ValidationError +from marshmallow import ValidationError from airflow.api_connexion.exceptions import BadRequest, NotFound, Unauthenticated from airflow.api_connexion.schemas.auth_schema import info_schema, login_form_schema @@ -72,7 +72,7 @@ def auth_dblogin(): try: data = login_form_schema.load(body) except ValidationError as err: - raise BadRequest(detail=str(err)) + raise BadRequest(detail=str(err.messages)) user = security_manager.auth_user_db(data['username'], data['password']) if not user: user = security_manager.auth_user_ldap(data['username'], data['password']) @@ -86,7 +86,7 @@ def auth_oauthlogin(provider, register=None, redirect_uri=None): """Handle Oauth login""" appbuilder = current_app.appbuilder if g.user is not None and g.user.is_authenticated: - pass # raise Unauthenticated(detail="Client already authenticated") + raise Unauthenticated(detail="Client already authenticated") if appbuilder.sm.auth_type != AUTH_OAUTH: raise BadRequest(detail="Authentication type do not match") state = jwt.encode( @@ -153,7 +153,7 @@ def auth_remoteuser(): appbuilder = current_app.appbuilder username = request.environ.get("REMOTE_USER") if g.user is not None and g.user.is_authenticated: - pass # raise Unauthenticated(detail="Client already authenticated") + raise Unauthenticated(detail="Client already authenticated") if appbuilder.sm.auth_type != AUTH_REMOTE_USER: raise BadRequest(detail="Authentication type do not match") if username: diff --git a/airflow/www/extensions/init_security.py b/airflow/www/extensions/init_security.py index 584d476f99074..a1b40b0b71efb 100644 --- a/airflow/www/extensions/init_security.py +++ b/airflow/www/extensions/init_security.py @@ -70,7 +70,7 @@ def init_api_experimental_auth(app): def init_jwt_auth(app): """Initialize Jwt manager""" app.config['JWT_TOKEN_LOCATION'] = ['cookies'] - app.config.setdefault('JWT_ACCESS_TOKEN_EXPIRES', timedelta(minutes=5)) + app.config.setdefault('JWT_ACCESS_TOKEN_EXPIRES', timedelta(minutes=5)) # Change this app.config['JWT_CSRF_IN_COOKIES'] = True app.config['JWT_COOKIE_CSRF_PROTECT'] = True app.config.setdefault('JWT_COOKIE_SECURE', False) @@ -82,13 +82,13 @@ def refresh_expiring_jwts(response): try: exp_timestamp = get_raw_jwt()["exp"] now = datetime.now(timezone.utc) - target_timestamp = datetime.timestamp(now + timedelta(minutes=3)) + target_timestamp = datetime.timestamp(now + timedelta(minutes=3)) # Change this when ready 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 + # Case where there is not a valid JWT. Just return the original response return response app.after_request(refresh_expiring_jwts) diff --git a/airflow/www/security.py b/airflow/www/security.py index 4c70f1a5ce4a7..111bde36f603e 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -733,7 +733,7 @@ def check_authorization( return True def create_access_token_and_dump_user(self): - """Creates access token, set token in session and return user""" + """Creates access token, set token in session and return user data""" user = self.current_user token = create_access_token(user.id) resp = jsonify(user_collection_item_schema.dump(user)) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 13a83393a9124..e3dad9918c428 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -14,3 +14,130 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import pytest +from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH + +from tests.test_utils.api_connexion_utils import delete_user +from tests.test_utils.fab_utils import create_user + + +@pytest.fixture(scope="module") +def configured_app(minimal_app_for_api): + app = minimal_app_for_api + create_user(app, username="test", role_name="Test") # type: ignore + + yield app + + delete_user(app, username="test") # type: ignore + + +class TestLoginEndpoint: + @pytest.fixture(autouse=True) + def setup_attrs(self, configured_app) -> None: + self.app = configured_app + self.client = self.app.test_client() # type:ignore + + +class TestDBLoginEndpoint(TestLoginEndpoint): + def test_user_can_login(self): + self.app.config['AUTH_TYPE'] = AUTH_DB + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.json['username'] == '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) + + def test_logged_in_user_cant_relogin(self): + self.app.config['AUTH_TYPE'] = AUTH_DB + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.json['username'] == 'test' + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.status_code == 401 + assert response.json['detail'] == "Client already authenticated" + + def test_incorrect_username_raises(self): + self.app.config['AUTH_TYPE'] = AUTH_DB + payload = {"username": "tests", "password": "test"} + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.status_code == 404 + assert response.json['detail'] == 'Invalid login' + + def test_post_body_conforms(self): + self.app.config['AUTH_TYPE'] = AUTH_DB + payload = {"username": "tests"} + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.status_code == 400 + assert response.json['detail'] == "{'password': ['Missing data for required field.']}" + + def test_auth_type_must_be_db(self): + self.app.config['AUTH_TYPE'] = AUTH_OAUTH + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.status_code == 400 + assert response.json['detail'] == 'Authentication type do not match' + + +class TestLDAPLoginEndpoint(TestLoginEndpoint): + def test_user_can_login(self): + self.app.config['AUTH_TYPE'] = AUTH_LDAP + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.json['username'] == '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) + + def test_logged_in_user_cant_relogin(self): + self.app.config['AUTH_TYPE'] = AUTH_LDAP + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.json['username'] == 'test' + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.status_code == 401 + assert response.json['detail'] == "Client already authenticated" + + def test_incorrect_username_raises(self): + self.app.config['AUTH_TYPE'] = AUTH_LDAP + payload = {"username": "tests", "password": "test"} + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.status_code == 404 + assert response.json['detail'] == 'Invalid login' + + def test_post_body_conforms(self): + self.app.config['AUTH_TYPE'] = AUTH_LDAP + payload = {"username": "tests"} + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.status_code == 400 + assert response.json['detail'] == "{'password': ['Missing data for required field.']}" + + def test_auth_type_must_be_db(self): + self.app.config['AUTH_TYPE'] = AUTH_OAUTH + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.status_code == 400 + assert response.json['detail'] == 'Authentication type do not match' + + +class TestOauthAuthorizationURLEndpoint(TestLoginEndpoint): + def test_can_generate_authorization_url(self): + pass + + def test_already_logged_in_user_cant_get_auth_url(self): + pass + + def test_incorrect_auth_type_raises(self): + pass + + +class TestAuthorizeOauth(TestLoginEndpoint): + def test_user_refused_sign_in_request(self): + pass + + def test_wrong_state_signature_raises(self): + pass From bf4f01fdbb6da123c6da0b12654835bc29f70d3f Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 26 Mar 2021 16:44:39 +0100 Subject: [PATCH 05/53] test oauth login flow --- .../api_connexion/endpoints/auth_endpoint.py | 8 ++-- airflow/api_connexion/openapi/v1.yaml | 8 ++-- .../endpoints/test_auth_endpoint.py | 42 +++++++++++++------ 3 files changed, 38 insertions(+), 20 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index b34e8d9fddea8..52ee3fbd388da 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -82,7 +82,7 @@ def auth_dblogin(): return security_manager.create_access_token_and_dump_user() -def auth_oauthlogin(provider, register=None, redirect_uri=None): +def auth_oauthlogin(provider, register=None, redirect_url=None): """Handle Oauth login""" appbuilder = current_app.appbuilder if g.user is not None and g.user.is_authenticated: @@ -99,17 +99,17 @@ def auth_oauthlogin(provider, register=None, redirect_uri=None): if register: session["register"] = True if provider == "twitter": - redirect_uri = redirect_uri + f"&state={state}" + redirect_uri = redirect_url + f"&state={state}" auth_data = auth_provider.create_authorization_url(redirect_uri=redirect_uri) auth_provider.save_authorize_data(request, redirect_uri=redirect_uri, **auth_data) return dict(auth_url=auth_data['url']) else: state = state.decode("ascii") if isinstance(state, bytes) else state auth_data = auth_provider.create_authorization_url( - redirect_uri=redirect_uri, + redirect_uri=redirect_url, state=state, ) - auth_provider.save_authorize_data(request, redirect_uri=redirect_uri, **auth_data) + auth_provider.save_authorize_data(request, redirect_uri=redirect_url, **auth_data) return dict(auth_url=auth_data['url']) except Exception as err: # pylint: disable=broad-except raise NotFound(detail=str(err)) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 02928f6e1dbb6..9d81b7585959c 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1620,7 +1620,7 @@ paths: parameters: - $ref: '#/components/parameters/ShouldRegister' - $ref: '#/components/parameters/OauthProvider' - - $ref: '#/components/parameters/RedirectUri' + - $ref: '#/components/parameters/RedirectUrL' responses: '200': description: Success. @@ -3428,14 +3428,14 @@ components: required: true description: The auth state from provider - RedirectUri: + RedirectUrL: in: query - name: redirect_uri + name: redirect_url schema: type: string format: url required: true - description: The oauth redirect uri from the frontend + description: The oauth redirect url from the frontend ShouldRegister: in: query diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index e3dad9918c428..fc566ec6f33be 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -14,6 +14,8 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from unittest import mock + import pytest from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH @@ -37,10 +39,13 @@ def setup_attrs(self, configured_app) -> None: self.app = configured_app self.client = self.app.test_client() # type:ignore + def auth_type(self, auth): + self.app.config['AUTH_TYPE'] = auth + class TestDBLoginEndpoint(TestLoginEndpoint): def test_user_can_login(self): - self.app.config['AUTH_TYPE'] = AUTH_DB + self.auth_type(AUTH_DB) payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.json['username'] == 'test' @@ -51,7 +56,7 @@ def test_user_can_login(self): assert isinstance(cookie.value, str) def test_logged_in_user_cant_relogin(self): - self.app.config['AUTH_TYPE'] = AUTH_DB + self.auth_type(AUTH_DB) payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.json['username'] == 'test' @@ -60,21 +65,21 @@ def test_logged_in_user_cant_relogin(self): assert response.json['detail'] == "Client already authenticated" def test_incorrect_username_raises(self): - self.app.config['AUTH_TYPE'] = AUTH_DB + self.auth_type(AUTH_DB) payload = {"username": "tests", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.status_code == 404 assert response.json['detail'] == 'Invalid login' def test_post_body_conforms(self): - self.app.config['AUTH_TYPE'] = AUTH_DB + self.auth_type(AUTH_DB) payload = {"username": "tests"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.status_code == 400 assert response.json['detail'] == "{'password': ['Missing data for required field.']}" def test_auth_type_must_be_db(self): - self.app.config['AUTH_TYPE'] = AUTH_OAUTH + self.auth_type(AUTH_OAUTH) payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.status_code == 400 @@ -83,7 +88,7 @@ def test_auth_type_must_be_db(self): class TestLDAPLoginEndpoint(TestLoginEndpoint): def test_user_can_login(self): - self.app.config['AUTH_TYPE'] = AUTH_LDAP + self.auth_type(AUTH_LDAP) payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.json['username'] == 'test' @@ -94,7 +99,7 @@ def test_user_can_login(self): assert isinstance(cookie.value, str) def test_logged_in_user_cant_relogin(self): - self.app.config['AUTH_TYPE'] = AUTH_LDAP + self.auth_type(AUTH_LDAP) payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.json['username'] == 'test' @@ -103,21 +108,21 @@ def test_logged_in_user_cant_relogin(self): assert response.json['detail'] == "Client already authenticated" def test_incorrect_username_raises(self): - self.app.config['AUTH_TYPE'] = AUTH_LDAP + self.auth_type(AUTH_LDAP) payload = {"username": "tests", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.status_code == 404 assert response.json['detail'] == 'Invalid login' def test_post_body_conforms(self): - self.app.config['AUTH_TYPE'] = AUTH_LDAP + self.auth_type(AUTH_LDAP) payload = {"username": "tests"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.status_code == 400 assert response.json['detail'] == "{'password': ['Missing data for required field.']}" def test_auth_type_must_be_db(self): - self.app.config['AUTH_TYPE'] = AUTH_OAUTH + self.auth_type(AUTH_OAUTH) payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.status_code == 400 @@ -125,8 +130,21 @@ def test_auth_type_must_be_db(self): class TestOauthAuthorizationURLEndpoint(TestLoginEndpoint): - def test_can_generate_authorization_url(self): - pass + @mock.patch("airflow.api_connexion.endpoints.auth_endpoint.jwt") + def test_can_generate_authorization_url(self, mock_jwt): + mock_state = mock.MagicMock() + self.app.appbuilder.sm.oauth_remotes = {"google": mock.MagicMock()} + self.app.appbuilder.sm.oauth_remotes['google'] = mock.MagicMock() + self.app.appbuilder.sm.oauth_remotes['google'].create_authorization_url.return_value = { + "state": "state", + "url": "authurl", + } + redirect_url = "http://localhost:8080" + mock_jwt.encode.return_value = mock_state + resp = self.client.get('api/v1/auth-oauth/google?register=True' f'&redirect_url={redirect_url}') + assert self.app.appbuilder.sm.oauth_remotes['google'].create_authorization_url.assert_called_with( + redirect_uri=redirect_url, state=mock_state + ) def test_already_logged_in_user_cant_get_auth_url(self): pass From 0d09ec8d071803ba95cbe0aa4438086991359c49 Mon Sep 17 00:00:00 2001 From: Kaxil Naik Date: Fri, 26 Mar 2021 17:13:52 +0000 Subject: [PATCH 06/53] Fix test & minor fix in OpenApi yaml --- airflow/api_connexion/openapi/v1.yaml | 4 ++-- .../endpoints/test_auth_endpoint.py | 24 +++++++++---------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 9d81b7585959c..f8831b3d329c6 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1620,7 +1620,7 @@ paths: parameters: - $ref: '#/components/parameters/ShouldRegister' - $ref: '#/components/parameters/OauthProvider' - - $ref: '#/components/parameters/RedirectUrL' + - $ref: '#/components/parameters/RedirectUrl' responses: '200': description: Success. @@ -3428,7 +3428,7 @@ components: required: true description: The auth state from provider - RedirectUrL: + RedirectUrl: in: query name: redirect_url schema: diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index fc566ec6f33be..618e4a2bd32b3 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -130,21 +130,19 @@ def test_auth_type_must_be_db(self): class TestOauthAuthorizationURLEndpoint(TestLoginEndpoint): - @mock.patch("airflow.api_connexion.endpoints.auth_endpoint.jwt") - def test_can_generate_authorization_url(self, mock_jwt): - mock_state = mock.MagicMock() - self.app.appbuilder.sm.oauth_remotes = {"google": mock.MagicMock()} - self.app.appbuilder.sm.oauth_remotes['google'] = mock.MagicMock() - self.app.appbuilder.sm.oauth_remotes['google'].create_authorization_url.return_value = { - "state": "state", - "url": "authurl", + @mock.patch("airflow.api_connexion.endpoints.auth_endpoint.jwt.encode") + def test_can_generate_authorization_url(self, mock_jwt_encode): + self.auth_type(AUTH_OAUTH) + mock_jwt_encode.return_value = "state" + mock_google_auth_provider = mock.Mock() + self.app.appbuilder.sm.oauth_remotes = { + "google": mock_google_auth_provider, } + mock_auth = mock.MagicMock(return_value={"state": "state", "url": "authurl"}) + mock_google_auth_provider.create_authorization_url = mock_auth redirect_url = "http://localhost:8080" - mock_jwt.encode.return_value = mock_state - resp = self.client.get('api/v1/auth-oauth/google?register=True' f'&redirect_url={redirect_url}') - assert self.app.appbuilder.sm.oauth_remotes['google'].create_authorization_url.assert_called_with( - redirect_uri=redirect_url, state=mock_state - ) + self.client.get(f'api/v1/auth-oauth/google?register=True&redirect_url={redirect_url}') + mock_auth.assert_called_once_with(redirect_uri=redirect_url, state="state") def test_already_logged_in_user_cant_get_auth_url(self): pass From 746de47e4171dc36c1a68695aafb12c04019c023 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 26 Mar 2021 22:09:53 +0100 Subject: [PATCH 07/53] add test for oauth authorize --- .../endpoints/test_auth_endpoint.py | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 618e4a2bd32b3..44ebf7934ee7b 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -144,16 +144,39 @@ def test_can_generate_authorization_url(self, mock_jwt_encode): self.client.get(f'api/v1/auth-oauth/google?register=True&redirect_url={redirect_url}') mock_auth.assert_called_once_with(redirect_uri=redirect_url, state="state") - def test_already_logged_in_user_cant_get_auth_url(self): - pass + @mock.patch("airflow.api_connexion.endpoints.auth_endpoint.jwt.encode") + def test_can_generate_authorization_url_for_twitter(self, mock_jwt_encode): + self.auth_type(AUTH_OAUTH) + mock_jwt_encode.return_value = "state" + mock_twitter_auth_provider = mock.Mock() + self.app.appbuilder.sm.oauth_remotes = { + "twitter": mock_twitter_auth_provider, + } + mock_auth = mock.MagicMock(return_value={"state": "state", "url": "authurl"}) + mock_twitter_auth_provider.create_authorization_url = mock_auth + redirect_url = "http://localhost:8080" + self.client.get(f'api/v1/auth-oauth/twitter?register=True&redirect_url={redirect_url}') + mock_auth.assert_called_once_with(redirect_uri=redirect_url + "&state=state") def test_incorrect_auth_type_raises(self): - pass + self.auth_type(AUTH_DB) + redirect_url = "http://localhost:8080" + resp = self.client.get(f'api/v1/auth-oauth/google?register=True&redirect_url={redirect_url}') + assert resp.status_code == 400 + assert resp.json['detail'] == "Authentication type do not match" class TestAuthorizeOauth(TestLoginEndpoint): def test_user_refused_sign_in_request(self): - pass + self.auth_type(AUTH_OAUTH) + mock_twitter_auth_provider = mock.Mock() + self.app.appbuilder.sm.oauth_remotes = { + "twitter": mock_twitter_auth_provider, + } + mock_twitter_auth_provider.authorize_access_token.return_value = None + response = self.client.get('api/v1/oauth-authorized/twitter?state=state') + assert response.status_code == 400 + assert response.json['detail'] == "You denied the request to sign in" def test_wrong_state_signature_raises(self): pass From 165c618d70903bb78abaafcb2e4f64b61952ec94 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Sun, 28 Mar 2021 10:34:23 +0100 Subject: [PATCH 08/53] fix ldap sign in --- airflow/api_connexion/endpoints/auth_endpoint.py | 5 +++-- setup.py | 1 + tests/api_connexion/endpoints/test_auth_endpoint.py | 4 +++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 52ee3fbd388da..e74a8e8f70c81 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -73,8 +73,9 @@ def auth_dblogin(): data = login_form_schema.load(body) except ValidationError as err: raise BadRequest(detail=str(err.messages)) - user = security_manager.auth_user_db(data['username'], data['password']) - if not user: + if auth_type == AUTH_DB: + user = security_manager.auth_user_db(data['username'], data['password']) + else: user = security_manager.auth_user_ldap(data['username'], data['password']) if not user: raise NotFound(detail="Invalid login") diff --git a/setup.py b/setup.py index 9331d1c6b9b0a..5f3d2393d1d14 100644 --- a/setup.py +++ b/setup.py @@ -495,6 +495,7 @@ def get_sphinx_theme_version() -> str: 'jira', 'jsonpath-ng', 'jsondiff', + 'mockldap>=0.3.0', 'mongomock', 'moto~=2.0', 'mypy==0.770', diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 44ebf7934ee7b..7ef04dc9902bc 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -18,6 +18,7 @@ import pytest from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH +from mockldap import MockLdap from tests.test_utils.api_connexion_utils import delete_user from tests.test_utils.fab_utils import create_user @@ -89,6 +90,7 @@ def test_auth_type_must_be_db(self): class TestLDAPLoginEndpoint(TestLoginEndpoint): def test_user_can_login(self): self.auth_type(AUTH_LDAP) + self.app.config["AUTH_LDAP_ALLOW_SELF_SIGNED"] = False payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.json['username'] == 'test' @@ -121,7 +123,7 @@ def test_post_body_conforms(self): assert response.status_code == 400 assert response.json['detail'] == "{'password': ['Missing data for required field.']}" - def test_auth_type_must_be_db(self): + def test_auth_type_must_be_ldap(self): self.auth_type(AUTH_OAUTH) payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) From 015171492537b97d2ad41b9f841d19b46095b653 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Sun, 28 Mar 2021 14:43:44 +0100 Subject: [PATCH 09/53] improve test for ldap --- .../api_connexion/endpoints/test_auth_endpoint.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 7ef04dc9902bc..c63a30c22e62f 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -18,7 +18,6 @@ import pytest from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH -from mockldap import MockLdap from tests.test_utils.api_connexion_utils import delete_user from tests.test_utils.fab_utils import create_user @@ -44,6 +43,10 @@ def auth_type(self, auth): self.app.config['AUTH_TYPE'] = auth +def auth_type(self, auth): + self.app.config['AUTH_TYPE'] = auth + + class TestDBLoginEndpoint(TestLoginEndpoint): def test_user_can_login(self): self.auth_type(AUTH_DB) @@ -90,9 +93,12 @@ def test_auth_type_must_be_db(self): class TestLDAPLoginEndpoint(TestLoginEndpoint): def test_user_can_login(self): self.auth_type(AUTH_LDAP) - self.app.config["AUTH_LDAP_ALLOW_SELF_SIGNED"] = False + self.app.appbuilder.sm.auth_user_ldap = mock.Mock() + user = self.app.appbuilder.sm.find_user(username='test') + self.app.appbuilder.sm.auth_user_ldap.return_value = user payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.json['username'] == 'test' cookie = next( (cookie for cookie in self.client.cookie_jar if cookie.name == "access_token_cookie"), None @@ -102,6 +108,9 @@ def test_user_can_login(self): def test_logged_in_user_cant_relogin(self): self.auth_type(AUTH_LDAP) + self.app.appbuilder.sm.auth_user_ldap = mock.Mock() + user = self.app.appbuilder.sm.find_user(username='test') + self.app.appbuilder.sm.auth_user_ldap.return_value = user payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.json['username'] == 'test' @@ -111,6 +120,8 @@ def test_logged_in_user_cant_relogin(self): def test_incorrect_username_raises(self): self.auth_type(AUTH_LDAP) + self.app.appbuilder.sm.auth_user_ldap = mock.Mock() + self.app.appbuilder.sm.auth_user_ldap.return_value = None payload = {"username": "tests", "password": "test"} response = self.client.post('api/v1/auth-dblogin', json=payload) assert response.status_code == 404 From 936e83f1466a9b52dcf92a6462f49283132ac74e Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Sun, 28 Mar 2021 21:55:48 +0100 Subject: [PATCH 10/53] More tests for authorize oauth --- setup.py | 1 - .../endpoints/test_auth_endpoint.py | 33 ++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 5f3d2393d1d14..9331d1c6b9b0a 100644 --- a/setup.py +++ b/setup.py @@ -495,7 +495,6 @@ def get_sphinx_theme_version() -> str: 'jira', 'jsonpath-ng', 'jsondiff', - 'mockldap>=0.3.0', 'mongomock', 'moto~=2.0', 'mypy==0.770', diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index c63a30c22e62f..8ef2f8a1dd528 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -192,4 +192,35 @@ def test_user_refused_sign_in_request(self): assert response.json['detail'] == "You denied the request to sign in" def test_wrong_state_signature_raises(self): - pass + self.auth_type(AUTH_OAUTH) + mock_twitter_auth_provider = mock.Mock() + self.app.appbuilder.sm.oauth_remotes = { + "twitter": mock_twitter_auth_provider, + } + mock_twitter_auth_provider.authorize_access_token.return_value = mock.MagicMock() + response = self.client.get('api/v1/oauth-authorized/twitter?state=state') + assert response.status_code == 400 + assert response.json['detail'] == "State signature is not valid!" + + @mock.patch("airflow.api_connexion.endpoints.auth_endpoint.jwt.decode") + def test_successful_authorization(self, mock_jwt_decode): + self.auth_type(AUTH_OAUTH) + mock_jwt_decode.return_value = {'some': 'payload'} + mock_twitter_auth_provider = mock.Mock() + mock_oauth_session = mock.MagicMock() + mock_user_info = mock.MagicMock() + mock_user_oauth = mock.MagicMock() + self.app.appbuilder.sm.set_oauth_session = mock_oauth_session + self.app.appbuilder.sm.oauth_user_info = mock_user_info + self.app.appbuilder.sm.auth_user_oauth = mock_user_oauth + user = self.app.appbuilder.sm.find_user(username='test') + self.app.appbuilder.sm.auth_user_oauth.return_value = user + self.app.appbuilder.sm.oauth_remotes = { + "twitter": mock_twitter_auth_provider, + } + mock_authorized = mock.MagicMock() + mock_twitter_auth_provider.authorize_access_token.return_value = mock_authorized + self.client.get('api/v1/oauth-authorized/twitter?state=state') + mock_oauth_session.assert_called_once_with('twitter', mock_authorized) + mock_user_info.assert_called_once_with('twitter', mock_authorized) + mock_user_oauth.assert_called_once_with(mock_user_info.return_value) From 1fe3aea832c0def84bea9809316d8c5d2a211f61 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 29 Mar 2021 01:13:11 +0100 Subject: [PATCH 11/53] add tests for remote user --- .../endpoints/test_auth_endpoint.py | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 8ef2f8a1dd528..29cabba4d00df 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -17,7 +17,7 @@ from unittest import mock import pytest -from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH +from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_REMOTE_USER from tests.test_utils.api_connexion_utils import delete_user from tests.test_utils.fab_utils import create_user @@ -224,3 +224,31 @@ def test_successful_authorization(self, mock_jwt_decode): mock_oauth_session.assert_called_once_with('twitter', mock_authorized) mock_user_info.assert_called_once_with('twitter', mock_authorized) mock_user_oauth.assert_called_once_with(mock_user_info.return_value) + + +class TestRemoteUserLoginEndpoint(TestLoginEndpoint): + def test_remote_user_can_login(self): + self.auth_type(AUTH_REMOTE_USER) + response = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) + assert response.status_code == 200 + assert response.json['username'] == 'test' + + def test_remote_user_cant_relogin(self): + self.auth_type(AUTH_REMOTE_USER) + response = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) + assert response.status_code == 200 + response = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) + assert response.status_code == 401 + assert response.json['detail'] == "Client already authenticated" + + def test_incorrect_username_raises(self): + self.auth_type(AUTH_REMOTE_USER) + response = self.client.post('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "tes"}) + assert response.status_code == 404 + assert response.json['detail'] == 'Invalid login' + + def test_incorrect_auth_type_raises(self): + self.auth_type(AUTH_DB) + resp = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) + assert resp.status_code == 400 + assert resp.json['detail'] == "Authentication type do not match" From c7d34355ba486c365a9385505cbd9cab583c3d0a Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 29 Mar 2021 15:01:36 +0100 Subject: [PATCH 12/53] Fix test and remove openid provision for now --- .../api_connexion/endpoints/auth_endpoint.py | 4 ---- airflow/api_connexion/schemas/auth_schema.py | 17 ----------------- .../endpoints/test_auth_endpoint.py | 2 +- 3 files changed, 1 insertion(+), 22 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index e74a8e8f70c81..e43ad877c21dc 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -29,10 +29,6 @@ log = logging.getLogger(__name__) -def refresh(): - """Refresh token""" - - def get_auth_info(): """Get site authentication info""" security_manager = current_app.appbuilder.sm diff --git a/airflow/api_connexion/schemas/auth_schema.py b/airflow/api_connexion/schemas/auth_schema.py index 6a215a875a831..12b7a6d248d6b 100644 --- a/airflow/api_connexion/schemas/auth_schema.py +++ b/airflow/api_connexion/schemas/auth_schema.py @@ -48,22 +48,5 @@ class LoginForm(Schema): password = fields.String(required=True) -class LoginOpenIDForm(Schema): - """Used to load credentials""" - - username = fields.String() - openid = fields.String(required=True) - remember_me = fields.Boolean(default=False) - - -class JwtTokenSchema(Schema): - """Response object after successful authentication""" - - token = fields.String() - refresh_token = fields.String() - - info_schema = InfoSchema() login_form_schema = LoginForm() -login_openid_form = LoginOpenIDForm() -jwt_token_schema = JwtTokenSchema() diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 29cabba4d00df..72f76fff96960 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -243,7 +243,7 @@ def test_remote_user_cant_relogin(self): def test_incorrect_username_raises(self): self.auth_type(AUTH_REMOTE_USER) - response = self.client.post('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "tes"}) + response = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "tes"}) assert response.status_code == 404 assert response.json['detail'] == 'Invalid login' From b8caa9aa7a7abf3d6902d5c2b82b1a918e00e213 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 29 Mar 2021 17:27:48 +0100 Subject: [PATCH 13/53] Add logout and fix incorrect user loading --- .../api_connexion/endpoints/auth_endpoint.py | 10 +++++++++- airflow/api_connexion/openapi/v1.yaml | 20 +++++++++++++++++++ airflow/www/extensions/init_security.py | 4 +++- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index e43ad877c21dc..abbb1dc4285ee 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -18,8 +18,9 @@ import logging import jwt -from flask import current_app, g, request, session +from flask import current_app, g, jsonify, request, session from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER +from flask_jwt_extended import unset_jwt_cookies from flask_login import login_user from marshmallow import ValidationError @@ -162,3 +163,10 @@ def auth_remoteuser(): else: raise NotFound(detail="Invalid login") return appbuilder.sm.create_access_token_and_dump_user() + + +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 f8831b3d329c6..94b103acc6c0f 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1670,6 +1670,26 @@ paths: '401': $ref: '#/components/responses/Unauthenticated' + /logout: + get: + summary: Webserver logout + description: Logout user by unsetting cookies + x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint + operationId: logout + tags: [Authentication] + responses: + '200': + description: Success + content: + application/json: + schema: + properties: + logged_out: + type: boolean + description: Indicate user is logged out + '400': + $ref: '#/components/responses/BadRequest' + components: # Reusable schemas (data models) diff --git a/airflow/www/extensions/init_security.py b/airflow/www/extensions/init_security.py index a1b40b0b71efb..159fb053a7542 100644 --- a/airflow/www/extensions/init_security.py +++ b/airflow/www/extensions/init_security.py @@ -76,13 +76,15 @@ def init_jwt_auth(app): app.config.setdefault('JWT_COOKIE_SECURE', False) jwt_manager = JWTManager() jwt_manager.init_app(app) - jwt_manager.user_loader_callback_loader(app.appbuilder.sm.load_user_jwt) def refresh_expiring_jwts(response): try: exp_timestamp = get_raw_jwt()["exp"] + print(exp_timestamp) now = datetime.now(timezone.utc) target_timestamp = datetime.timestamp(now + timedelta(minutes=3)) # Change this when ready + print(target_timestamp) + print(target_timestamp > exp_timestamp) if target_timestamp > exp_timestamp: access_token = create_access_token(identity=get_jwt_identity()) set_access_cookies(response, access_token) From 73f5e366b630079340459bae3db2f548122c73e6 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 29 Mar 2021 18:33:10 +0100 Subject: [PATCH 14/53] add tests for logout --- .../endpoints/test_auth_endpoint.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 72f76fff96960..bd8ca8b346b3c 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -252,3 +252,22 @@ def test_incorrect_auth_type_raises(self): resp = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) assert resp.status_code == 400 assert resp.json['detail'] == "Authentication type do not match" + + +class TestLogoutEndpoint(TestLoginEndpoint): + def test_logout(self): + self.auth_type(AUTH_DB) + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth-dblogin', json=payload) + assert response.json['username'] == '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) + response = self.client.get('api/v1/logout') + assert response.json == {"logged_out": True} + cookie = next( + (cookie for cookie in self.client.cookie_jar if cookie.name == "access_token_cookie"), None + ) + assert cookie is None From 4871a38737ce81576fd9e1a9543393b55ab51055 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 29 Mar 2021 18:39:33 +0100 Subject: [PATCH 15/53] remove jwt schema --- airflow/api_connexion/openapi/v1.yaml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 94b103acc6c0f..e7409806a8081 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -2658,15 +2658,6 @@ components: example: complex-password type: string - JwtToken: - description: Response object after successful authentication - type: object - properties: - token: - type: string - refresh_token: - type: string - AuthorizationUrl: description: Oauth authorization URl type: object From 0f69cd7177e0227b546eccc8673bf6c84aab0648 Mon Sep 17 00:00:00 2001 From: Ephraim Anierobi Date: Tue, 30 Mar 2021 12:25:19 +0100 Subject: [PATCH 16/53] Apply suggestions from code review Co-authored-by: Ash Berlin-Taylor --- airflow/api_connexion/openapi/v1.yaml | 2 +- airflow/api_connexion/schemas/auth_schema.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index e7409806a8081..a9957f73ec15f 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1586,7 +1586,7 @@ paths: schema: $ref: '#/components/schemas/AuthInfo' - /auth-dblogin: + /auth/login: post: summary: Login user x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint diff --git a/airflow/api_connexion/schemas/auth_schema.py b/airflow/api_connexion/schemas/auth_schema.py index 12b7a6d248d6b..5aa12de47f2a1 100644 --- a/airflow/api_connexion/schemas/auth_schema.py +++ b/airflow/api_connexion/schemas/auth_schema.py @@ -26,7 +26,7 @@ class OAUTHSchema(Schema): icon = fields.String() -class OPENIDSchema(Schema): +class OpenIDSchema(Schema): """A little information needed for the UI""" name = fields.String() From 5c7474992d4f84668b24f874eb00abb95c5d41d4 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 30 Mar 2021 13:45:02 +0100 Subject: [PATCH 17/53] add token to response body --- .../api_connexion/endpoints/auth_endpoint.py | 20 ++++-- airflow/api_connexion/openapi/v1.yaml | 47 ++++++++++++-- airflow/api_connexion/schemas/auth_schema.py | 13 +++- airflow/www/app.py | 8 +-- airflow/www/extensions/init_security.py | 39 ------------ airflow/www/security.py | 26 +++++--- .../endpoints/test_auth_endpoint.py | 62 +++++++------------ 7 files changed, 111 insertions(+), 104 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index abbb1dc4285ee..06acf4129de43 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -20,7 +20,7 @@ import jwt from flask import current_app, g, jsonify, request, session from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER -from flask_jwt_extended import unset_jwt_cookies +from flask_jwt_extended import create_access_token, get_jwt_identity, jwt_refresh_token_required from flask_login import login_user from marshmallow import ValidationError @@ -57,7 +57,7 @@ def get_auth_info(): ) -def auth_dblogin(): +def auth_login(): """Handle DB login""" security_manager = current_app.appbuilder.sm auth_type = security_manager.auth_type @@ -77,7 +77,7 @@ def auth_dblogin(): if not user: raise NotFound(detail="Invalid login") login_user(user, remember=False) - return security_manager.create_access_token_and_dump_user() + return security_manager.create_tokens_and_dump(user) def auth_oauthlogin(provider, register=None, redirect_url=None): @@ -143,7 +143,7 @@ def authorize_oauth(provider, state): if user is None: raise NotFound(detail="Invalid login") login_user(user) - return appbuilder.sm.create_access_token_and_dump_user() + return appbuilder.sm.create_tokens_and_dump(user) def auth_remoteuser(): @@ -162,11 +162,19 @@ def auth_remoteuser(): login_user(user) else: raise NotFound(detail="Invalid login") - return appbuilder.sm.create_access_token_and_dump_user() + return appbuilder.sm.create_tokens_and_dump(user) + + +@jwt_refresh_token_required +def refresh_token(): + """Refresh token""" + current_user = get_jwt_identity() + ret = {'access_token': create_access_token(identity=current_user)} + return jsonify(ret), 200 def logout(): """Sign out""" resp = jsonify({'logged_out': True}) - unset_jwt_cookies(resp) + # TODO: logout user return resp, 200 diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index a9957f73ec15f..3bc25aecccde1 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1590,7 +1590,7 @@ paths: post: summary: Login user x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint - operationId: auth_dblogin + operationId: auth_login tags: [Authentication] requestBody: required: true @@ -1604,7 +1604,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/JwtAuthAndUser' '400': $ref: '#/components/responses/BadRequest' '401': @@ -1645,7 +1645,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/JwtAuthAndUser' '400': $ref: '#/components/responses/BadRequest' '401': @@ -1664,12 +1664,29 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/JwtAuthAndUser' '400': $ref: '#/components/responses/BadRequest' '401': $ref: '#/components/responses/Unauthenticated' + /refresh: + post: + summary: Refresh a token + description: Accepts fresh token and creates access token + x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint + operationId: refresh_token + tags: [Authentication] + responses: + '200': + description: Success + content: + application/json: + schema: + $ref: '#/components/schemas/AccessToken' + '400': + $ref: '#/components/responses/BadRequest' + /logout: get: summary: Webserver logout @@ -2665,6 +2682,28 @@ components: auth_url: type: string + JwtAuthAndUser: + description: Data returned after user authentication + type: object + allOf: + - type: object + properties: + token: + type: string + description: Authentication token + refresh_token: + type: string + description: Authentication refresh token + - $ref: '#/components/schemas/UserCollectionItem' + + AccessToken: + description: AccessToken + type: object + properties: + access_token: + type: string + description: authentication token + # Configuration ConfigOption: type: object diff --git a/airflow/api_connexion/schemas/auth_schema.py b/airflow/api_connexion/schemas/auth_schema.py index 5aa12de47f2a1..9c31d761f81b5 100644 --- a/airflow/api_connexion/schemas/auth_schema.py +++ b/airflow/api_connexion/schemas/auth_schema.py @@ -18,6 +18,8 @@ from marshmallow import fields from marshmallow.schema import Schema +from airflow.api_connexion.schemas.user_schema import UserCollectionItemSchema + class OAUTHSchema(Schema): """A little information needed for UI""" @@ -38,7 +40,7 @@ class InfoSchema(Schema): auth_type = fields.String() oauth_providers = fields.List(fields.Nested(OAUTHSchema)) - openid_providers = fields.List(fields.Nested(OPENIDSchema)) + openid_providers = fields.List(fields.Nested(OpenIDSchema)) class LoginForm(Schema): @@ -48,5 +50,14 @@ class LoginForm(Schema): password = fields.String(required=True) +class AuthSchema(Schema): + """Schema for data returned after successful authentication""" + + token = fields.String() + refresh_token = fields.String() + user = fields.Nested(UserCollectionItemSchema) + + info_schema = InfoSchema() login_form_schema = LoginForm() +auth_schema = AuthSchema() diff --git a/airflow/www/app.py b/airflow/www/app.py index d3a8e5eace2fd..3dc93267a4808 100644 --- a/airflow/www/app.py +++ b/airflow/www/app.py @@ -34,11 +34,8 @@ 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_jwt_auth, - init_xframe_protection, -) +from airflow.www.extensions.init_security import init_api_experimental_auth, init_xframe_protection + from airflow.www.extensions.init_session import init_airflow_session_interface, init_permanent_session from airflow.www.extensions.init_views import ( init_api_connexion, @@ -133,7 +130,6 @@ def create_app(config=None, testing=False): init_error_handlers(flask_app) init_api_connexion(flask_app) init_api_experimental(flask_app) - init_jwt_auth(flask_app) sync_appbuilder_roles(flask_app) init_jinja_globals(flask_app) diff --git a/airflow/www/extensions/init_security.py b/airflow/www/extensions/init_security.py index 159fb053a7542..544deebeb3af4 100644 --- a/airflow/www/extensions/init_security.py +++ b/airflow/www/extensions/init_security.py @@ -15,20 +15,10 @@ # specific language governing permissions and limitations # under the License. import logging -from datetime import datetime, timedelta from importlib import import_module -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__) @@ -65,32 +55,3 @@ 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 Jwt manager""" - app.config['JWT_TOKEN_LOCATION'] = ['cookies'] - app.config.setdefault('JWT_ACCESS_TOKEN_EXPIRES', timedelta(minutes=5)) # Change this - app.config['JWT_CSRF_IN_COOKIES'] = True - app.config['JWT_COOKIE_CSRF_PROTECT'] = True - app.config.setdefault('JWT_COOKIE_SECURE', False) - jwt_manager = JWTManager() - jwt_manager.init_app(app) - - def refresh_expiring_jwts(response): - try: - exp_timestamp = get_raw_jwt()["exp"] - print(exp_timestamp) - now = datetime.now(timezone.utc) - target_timestamp = datetime.timestamp(now + timedelta(minutes=3)) # Change this when ready - print(target_timestamp) - print(target_timestamp > exp_timestamp) - 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 response - return response - - app.after_request(refresh_expiring_jwts) diff --git a/airflow/www/security.py b/airflow/www/security.py index 111bde36f603e..098cf3aa39604 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -18,17 +18,18 @@ # import warnings +from datetime import timedelta from typing import Dict, Optional, Sequence, Set, Tuple -from flask import current_app, g, jsonify +from flask import current_app, g 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, set_access_cookies +from flask_jwt_extended import JWTManager, create_access_token, create_refresh_token from sqlalchemy import or_ from sqlalchemy.orm import joinedload -from airflow.api_connexion.schemas.user_schema import user_collection_item_schema +from airflow.api_connexion.schemas.auth_schema import auth_schema from airflow.exceptions import AirflowException from airflow.models import DagBag, DagModel from airflow.security import permissions @@ -732,13 +733,20 @@ def check_authorization( return True - def create_access_token_and_dump_user(self): - """Creates access token, set token in session and return user data""" - user = self.current_user + def create_jwt_manager(self, app) -> JWTManager: + """JWT Manager""" + jwt_manager = JWTManager() + app.config["JWT_ACCESS_TOKEN_EXPIRES"] = timedelta(hours=1) + app.config["JWT_REFRESH_TOKEN_EXPIRES"] = timedelta(days=30) + jwt_manager.init_app(app) + jwt_manager.user_loader_callback_loader(self.load_user_jwt) + return jwt_manager + + def create_tokens_and_dump(self, user): + """Creates access token, return user data alongside tokens""" token = create_access_token(user.id) - resp = jsonify(user_collection_item_schema.dump(user)) - set_access_cookies(resp, token) - return resp + refresh_token = create_refresh_token(user.id) + return auth_schema.dump({'user': user, 'token': token, 'refresh_token': refresh_token}) class ApplessAirflowSecurityManager(AirflowSecurityManager): diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index bd8ca8b346b3c..41958d2db1c6a 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -51,41 +51,38 @@ class TestDBLoginEndpoint(TestLoginEndpoint): def test_user_can_login(self): self.auth_type(AUTH_DB) payload = {"username": "test", "password": "test"} - response = self.client.post('api/v1/auth-dblogin', json=payload) - assert response.json['username'] == '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) + response = self.client.post('api/v1/auth/login', json=payload) + assert response.json['user']['username'] == 'test' + assert isinstance(response.json['token'], str) + assert isinstance(response.json['refresh_token'], str) def test_logged_in_user_cant_relogin(self): self.auth_type(AUTH_DB) payload = {"username": "test", "password": "test"} - response = self.client.post('api/v1/auth-dblogin', json=payload) - assert response.json['username'] == 'test' - response = self.client.post('api/v1/auth-dblogin', json=payload) + response = self.client.post('api/v1/auth/login', json=payload) + assert response.json['user']['username'] == 'test' + response = self.client.post('api/v1/auth/login', json=payload) assert response.status_code == 401 assert response.json['detail'] == "Client already authenticated" def test_incorrect_username_raises(self): self.auth_type(AUTH_DB) payload = {"username": "tests", "password": "test"} - response = self.client.post('api/v1/auth-dblogin', json=payload) + response = self.client.post('api/v1/auth/login', json=payload) assert response.status_code == 404 assert response.json['detail'] == 'Invalid login' def test_post_body_conforms(self): self.auth_type(AUTH_DB) payload = {"username": "tests"} - response = self.client.post('api/v1/auth-dblogin', json=payload) + response = self.client.post('api/v1/auth/login', json=payload) assert response.status_code == 400 assert response.json['detail'] == "{'password': ['Missing data for required field.']}" def test_auth_type_must_be_db(self): self.auth_type(AUTH_OAUTH) payload = {"username": "test", "password": "test"} - response = self.client.post('api/v1/auth-dblogin', json=payload) + response = self.client.post('api/v1/auth/login', json=payload) assert response.status_code == 400 assert response.json['detail'] == 'Authentication type do not match' @@ -97,14 +94,10 @@ def test_user_can_login(self): user = self.app.appbuilder.sm.find_user(username='test') self.app.appbuilder.sm.auth_user_ldap.return_value = user payload = {"username": "test", "password": "test"} - response = self.client.post('api/v1/auth-dblogin', json=payload) - - assert response.json['username'] == '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) + response = self.client.post('api/v1/auth/login', json=payload) + assert response.json['user']['username'] == 'test' + assert isinstance(response.json['token'], str) + assert isinstance(response.json['refresh_token'], str) def test_logged_in_user_cant_relogin(self): self.auth_type(AUTH_LDAP) @@ -112,9 +105,9 @@ def test_logged_in_user_cant_relogin(self): user = self.app.appbuilder.sm.find_user(username='test') self.app.appbuilder.sm.auth_user_ldap.return_value = user payload = {"username": "test", "password": "test"} - response = self.client.post('api/v1/auth-dblogin', json=payload) - assert response.json['username'] == 'test' - response = self.client.post('api/v1/auth-dblogin', json=payload) + response = self.client.post('api/v1/auth/login', json=payload) + assert response.json['user']['username'] == 'test' + response = self.client.post('api/v1/auth/login', json=payload) assert response.status_code == 401 assert response.json['detail'] == "Client already authenticated" @@ -123,21 +116,21 @@ def test_incorrect_username_raises(self): self.app.appbuilder.sm.auth_user_ldap = mock.Mock() self.app.appbuilder.sm.auth_user_ldap.return_value = None payload = {"username": "tests", "password": "test"} - response = self.client.post('api/v1/auth-dblogin', json=payload) + response = self.client.post('api/v1/auth/login', json=payload) assert response.status_code == 404 assert response.json['detail'] == 'Invalid login' def test_post_body_conforms(self): self.auth_type(AUTH_LDAP) payload = {"username": "tests"} - response = self.client.post('api/v1/auth-dblogin', json=payload) + response = self.client.post('api/v1/auth/login', json=payload) assert response.status_code == 400 assert response.json['detail'] == "{'password': ['Missing data for required field.']}" def test_auth_type_must_be_ldap(self): self.auth_type(AUTH_OAUTH) payload = {"username": "test", "password": "test"} - response = self.client.post('api/v1/auth-dblogin', json=payload) + response = self.client.post('api/v1/auth/login', json=payload) assert response.status_code == 400 assert response.json['detail'] == 'Authentication type do not match' @@ -231,7 +224,7 @@ def test_remote_user_can_login(self): self.auth_type(AUTH_REMOTE_USER) response = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) assert response.status_code == 200 - assert response.json['username'] == 'test' + assert response.json['user']['username'] == 'test' def test_remote_user_cant_relogin(self): self.auth_type(AUTH_REMOTE_USER) @@ -258,16 +251,7 @@ class TestLogoutEndpoint(TestLoginEndpoint): def test_logout(self): self.auth_type(AUTH_DB) payload = {"username": "test", "password": "test"} - response = self.client.post('api/v1/auth-dblogin', json=payload) - assert response.json['username'] == '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) + response = self.client.post('api/v1/auth/login', json=payload) + assert response.json['user']['username'] == 'test' response = self.client.get('api/v1/logout') assert response.json == {"logged_out": True} - cookie = next( - (cookie for cookie in self.client.cookie_jar if cookie.name == "access_token_cookie"), None - ) - assert cookie is None From 209ccfdbb6875389fa51c2ea2d9b717a41b9f4fb Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 2 Apr 2021 12:13:09 +0100 Subject: [PATCH 18/53] add token to response body --- .../api_connexion/endpoints/auth_endpoint.py | 10 +-- .../endpoints/test_auth_endpoint.py | 68 ++++++++++++++++++- 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 06acf4129de43..491ab742eab13 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -33,8 +33,7 @@ def get_auth_info(): """Get site authentication info""" security_manager = current_app.appbuilder.sm - oauth_providers = None - openid_providers = None + config = current_app.config auth_type = security_manager.auth_type type_mapping = { AUTH_DB: "auth_db", @@ -43,11 +42,8 @@ def get_auth_info(): AUTH_OAUTH: "auth_oauth", AUTH_REMOTE_USER: "auth_remote_user", } - - if auth_type == AUTH_OAUTH: - oauth_providers = security_manager.oauth_providers - if auth_type == AUTH_OID: - openid_providers = security_manager.openid_providers + oauth_providers = config.get("OAUTH_PROVIDERS", None) + openid_providers = config.get("OPENID_PROVIDERS", None) return info_schema.dump( { "auth_type": type_mapping[auth_type], diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 41958d2db1c6a..c07e2d16faffa 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -17,11 +17,33 @@ from unittest import mock import pytest -from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_REMOTE_USER +from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER from tests.test_utils.api_connexion_utils import delete_user from tests.test_utils.fab_utils import create_user +OAUTH_PROVIDERS = [ + { + 'name': 'google', + 'token_key': 'access_token', + 'icon': 'fa-google', + 'remote_app': { + 'api_base_url': 'https://www.googleapis.com/oauth2/v2/', + 'client_kwargs': {'scope': 'email profile'}, + 'access_token_url': 'https://accounts.google.com/o/oauth2/token', + 'authorize_url': 'https://accounts.google.com/o/oauth2/auth', + 'request_token_url': None, + 'client_id': "GOOGLE_KEY", + 'client_secret': "GOOGLE_SECRET_KEY", + }, + } +] + +OPENID_PROVIDERS = [ + {'name': 'Yahoo', 'url': 'https://me.yahoo.com'}, + {'name': 'AOL', 'url': 'http://openid.aol.com/'}, +] + @pytest.fixture(scope="module") def configured_app(minimal_app_for_api): @@ -41,10 +63,50 @@ def setup_attrs(self, configured_app) -> None: def auth_type(self, auth): self.app.config['AUTH_TYPE'] = auth + if auth == AUTH_OAUTH: + self.app.config['OAUTH_PROVIDERS'] = OAUTH_PROVIDERS + self.app.config['OPENID_PROVIDERS'] = None + elif auth == AUTH_OID: + self.app.config['OPENID_PROVIDERS'] = OPENID_PROVIDERS + self.app.config['OAUTH_PROVIDERS'] = None + else: + self.app.config['OPENID_PROVIDERS'] = None + self.app.config['OAUTH_PROVIDERS'] = None + + +class TestAuthInfo(TestLoginEndpoint): + def test_auth_db_info(self): + self.auth_type(AUTH_DB) + response = self.client.get("api/v1/auth-info") + assert response.json == {"auth_type": "auth_db", "oauth_providers": None, "openid_providers": None} + def test_auth_ldap_info(self): + self.auth_type(AUTH_LDAP) + response = self.client.get("api/v1/auth-info") + assert response.json == {"auth_type": "auth_ldap", "oauth_providers": None, "openid_providers": None} -def auth_type(self, auth): - self.app.config['AUTH_TYPE'] = auth + def test_auth_oath_info(self): + self.auth_type(AUTH_OAUTH) + response = self.client.get("api/v1/auth-info") + assert response.json == { + 'auth_type': 'auth_oauth', + 'oauth_providers': [ # Only data necessary to build forms are returned + {'icon': 'fa-google', 'name': 'google'} + ], + 'openid_providers': None, + } + + def test_auth_oid_info(self): + self.auth_type(AUTH_OID) + response = self.client.get("api/v1/auth-info") + assert response.json == { + 'auth_type': 'auth_oid', + 'oauth_providers': None, + 'openid_providers': [ + {'name': 'Yahoo', 'url': 'https://me.yahoo.com'}, + {'name': 'AOL', 'url': 'http://openid.aol.com/'}, + ], + } class TestDBLoginEndpoint(TestLoginEndpoint): From 00e28e46998cc6a4ff76194f61694e28db5b2662 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 2 Apr 2021 12:54:47 +0100 Subject: [PATCH 19/53] Return 401 for every auth failure --- .../api_connexion/endpoints/auth_endpoint.py | 24 +++++++++---------- .../endpoints/test_auth_endpoint.py | 22 ++++++++--------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 491ab742eab13..59d56a1e1529c 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -24,7 +24,7 @@ from flask_login import login_user from marshmallow import ValidationError -from airflow.api_connexion.exceptions import BadRequest, NotFound, Unauthenticated +from airflow.api_connexion.exceptions import Unauthenticated from airflow.api_connexion.schemas.auth_schema import info_schema, login_form_schema log = logging.getLogger(__name__) @@ -60,18 +60,18 @@ def auth_login(): if g.user is not None and g.user.is_authenticated: raise Unauthenticated(detail="Client already authenticated") # For security if auth_type not in (AUTH_DB, AUTH_LDAP): - raise BadRequest(detail="Authentication type do not match") + raise Unauthenticated(detail="Authentication type do not match") body = request.json try: data = login_form_schema.load(body) except ValidationError as err: - raise BadRequest(detail=str(err.messages)) + raise Unauthenticated(detail=str(err.messages)) if auth_type == AUTH_DB: user = security_manager.auth_user_db(data['username'], data['password']) else: user = security_manager.auth_user_ldap(data['username'], data['password']) if not user: - raise NotFound(detail="Invalid login") + raise Unauthenticated(detail="Invalid login") login_user(user, remember=False) return security_manager.create_tokens_and_dump(user) @@ -82,7 +82,7 @@ def auth_oauthlogin(provider, register=None, redirect_url=None): if g.user is not None and g.user.is_authenticated: raise Unauthenticated(detail="Client already authenticated") if appbuilder.sm.auth_type != AUTH_OAUTH: - raise BadRequest(detail="Authentication type do not match") + raise Unauthenticated(detail="Authentication type do not match") state = jwt.encode( request.args.to_dict(flat=False), appbuilder.app.config["SECRET_KEY"], @@ -106,7 +106,7 @@ def auth_oauthlogin(provider, register=None, redirect_url=None): auth_provider.save_authorize_data(request, redirect_uri=redirect_url, **auth_data) return dict(auth_url=auth_data['url']) except Exception as err: # pylint: disable=broad-except - raise NotFound(detail=str(err)) + raise Unauthenticated(detail=str(err)) def authorize_oauth(provider, state): @@ -114,7 +114,7 @@ def authorize_oauth(provider, state): appbuilder = current_app.appbuilder resp = appbuilder.sm.oauth_remotes[provider].authorize_access_token() if resp is None: - raise BadRequest(detail="You denied the request to sign in") + raise Unauthenticated(detail="You denied the request to sign in") log.debug("OAUTH Authorized resp: %s", resp) # Verify state try: @@ -124,7 +124,7 @@ def authorize_oauth(provider, state): algorithms=["HS256"], ) except jwt.InvalidTokenError: - raise BadRequest(detail="State signature is not valid!") + raise Unauthenticated(detail="State signature is not valid!") # Retrieves specific user info from the provider try: appbuilder.sm.set_oauth_session(provider, resp) @@ -137,7 +137,7 @@ def authorize_oauth(provider, state): user = appbuilder.sm.auth_user_oauth(userinfo) if user is None: - raise NotFound(detail="Invalid login") + raise Unauthenticated(detail="Invalid login") login_user(user) return appbuilder.sm.create_tokens_and_dump(user) @@ -149,15 +149,15 @@ def auth_remoteuser(): if g.user is not None and g.user.is_authenticated: raise Unauthenticated(detail="Client already authenticated") if appbuilder.sm.auth_type != AUTH_REMOTE_USER: - raise BadRequest(detail="Authentication type do not match") + raise Unauthenticated(detail="Authentication type do not match") if username: user = appbuilder.sm.auth_user_remote_user(username) if user is None: - raise NotFound(detail="Invalid login") + raise Unauthenticated(detail="Invalid login") else: login_user(user) else: - raise NotFound(detail="Invalid login") + raise Unauthenticated(detail="Invalid login") return appbuilder.sm.create_tokens_and_dump(user) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index c07e2d16faffa..fa47369abc38c 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -131,21 +131,21 @@ def test_incorrect_username_raises(self): self.auth_type(AUTH_DB) payload = {"username": "tests", "password": "test"} response = self.client.post('api/v1/auth/login', json=payload) - assert response.status_code == 404 + assert response.status_code == 401 assert response.json['detail'] == 'Invalid login' def test_post_body_conforms(self): self.auth_type(AUTH_DB) payload = {"username": "tests"} response = self.client.post('api/v1/auth/login', json=payload) - assert response.status_code == 400 + assert response.status_code == 401 assert response.json['detail'] == "{'password': ['Missing data for required field.']}" def test_auth_type_must_be_db(self): self.auth_type(AUTH_OAUTH) payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth/login', json=payload) - assert response.status_code == 400 + assert response.status_code == 401 assert response.json['detail'] == 'Authentication type do not match' @@ -179,21 +179,21 @@ def test_incorrect_username_raises(self): self.app.appbuilder.sm.auth_user_ldap.return_value = None payload = {"username": "tests", "password": "test"} response = self.client.post('api/v1/auth/login', json=payload) - assert response.status_code == 404 + assert response.status_code == 401 assert response.json['detail'] == 'Invalid login' def test_post_body_conforms(self): self.auth_type(AUTH_LDAP) payload = {"username": "tests"} response = self.client.post('api/v1/auth/login', json=payload) - assert response.status_code == 400 + assert response.status_code == 401 assert response.json['detail'] == "{'password': ['Missing data for required field.']}" def test_auth_type_must_be_ldap(self): self.auth_type(AUTH_OAUTH) payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth/login', json=payload) - assert response.status_code == 400 + assert response.status_code == 401 assert response.json['detail'] == 'Authentication type do not match' @@ -230,7 +230,7 @@ def test_incorrect_auth_type_raises(self): self.auth_type(AUTH_DB) redirect_url = "http://localhost:8080" resp = self.client.get(f'api/v1/auth-oauth/google?register=True&redirect_url={redirect_url}') - assert resp.status_code == 400 + assert resp.status_code == 401 assert resp.json['detail'] == "Authentication type do not match" @@ -243,7 +243,7 @@ def test_user_refused_sign_in_request(self): } mock_twitter_auth_provider.authorize_access_token.return_value = None response = self.client.get('api/v1/oauth-authorized/twitter?state=state') - assert response.status_code == 400 + assert response.status_code == 401 assert response.json['detail'] == "You denied the request to sign in" def test_wrong_state_signature_raises(self): @@ -254,7 +254,7 @@ def test_wrong_state_signature_raises(self): } mock_twitter_auth_provider.authorize_access_token.return_value = mock.MagicMock() response = self.client.get('api/v1/oauth-authorized/twitter?state=state') - assert response.status_code == 400 + assert response.status_code == 401 assert response.json['detail'] == "State signature is not valid!" @mock.patch("airflow.api_connexion.endpoints.auth_endpoint.jwt.decode") @@ -299,13 +299,13 @@ def test_remote_user_cant_relogin(self): def test_incorrect_username_raises(self): self.auth_type(AUTH_REMOTE_USER) response = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "tes"}) - assert response.status_code == 404 + assert response.status_code == 401 assert response.json['detail'] == 'Invalid login' def test_incorrect_auth_type_raises(self): self.auth_type(AUTH_DB) resp = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) - assert resp.status_code == 400 + assert resp.status_code == 401 assert resp.json['detail'] == "Authentication type do not match" From 69a2c3eaef1136e3cb42c00fdfcaff9e2f703ab1 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 2 Apr 2021 13:31:09 +0100 Subject: [PATCH 20/53] Add blocklist model --- airflow/models/auth.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 airflow/models/auth.py diff --git a/airflow/models/auth.py b/airflow/models/auth.py new file mode 100644 index 0000000000000..ac17a1207741c --- /dev/null +++ b/airflow/models/auth.py @@ -0,0 +1,31 @@ +# 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 sqlalchemy import Column, DateTime, Integer, String + +from airflow.models.base import Base +from airflow.utils.log.logging_mixin import LoggingMixin + + +class TokenBlocklist(Base, LoggingMixin): + """Token blobk list""" + + __tablename__ = 'tokenblocklist' + id = Column(Integer, primary_key=True) + jti = Column(String(50), nullable=False) + reason = Column(String(100)) + created_at = Column(DateTime, nullable=False) From 041805ccb6e6f71a23a428aea5db96ca3a0475c8 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 6 Apr 2021 09:46:02 +0100 Subject: [PATCH 21/53] add tokens table --- .../api_connexion/endpoints/auth_endpoint.py | 31 ++++++--- airflow/api_connexion/openapi/v1.yaml | 35 +++++++--- airflow/api_connexion/security.py | 68 +++++++++++++++++-- .../versions/43c2bf7117bd_add_tokens_table.py | 55 +++++++++++++++ airflow/models/auth.py | 56 +++++++++++++-- airflow/www/security.py | 19 ++++-- .../endpoints/test_auth_endpoint.py | 10 --- 7 files changed, 230 insertions(+), 44 deletions(-) create mode 100644 airflow/migrations/versions/43c2bf7117bd_add_tokens_table.py diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 59d56a1e1529c..ab955e2b7884c 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -20,12 +20,15 @@ import jwt from flask import current_app, g, jsonify, request, session from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER -from flask_jwt_extended import create_access_token, get_jwt_identity, jwt_refresh_token_required +from flask_jwt_extended import create_access_token, get_jwt_identity from flask_login import login_user from marshmallow import ValidationError from airflow.api_connexion.exceptions import Unauthenticated from airflow.api_connexion.schemas.auth_schema import info_schema, login_form_schema +from airflow.api_connexion.security import jwt_refresh_token_required_ +from airflow.models.auth import Tokens +from airflow.utils.session import provide_session log = logging.getLogger(__name__) @@ -161,16 +164,28 @@ def auth_remoteuser(): return appbuilder.sm.create_tokens_and_dump(user) -@jwt_refresh_token_required -def refresh_token(): +@jwt_refresh_token_required_ +@provide_session +def refresh_token(session): """Refresh token""" + app = current_app current_user = get_jwt_identity() - ret = {'access_token': create_access_token(identity=current_user)} + token = Tokens( + jti=create_access_token(identity=current_user), expiry_date=app.config.get("JWT_ACCESS_TOKEN_EXPIRES") + ) + session.add(token) + session.commit() + ret = {'access_token': token.jti} return jsonify(ret), 200 -def logout(): +def logout(token, refresh_token): """Sign out""" - resp = jsonify({'logged_out': True}) - # TODO: logout user - return resp, 200 + exist = Tokens.get_token(token) + if exist: + Tokens.delete_token(token) + exist = Tokens.get_token(refresh_token) + if exist: + Tokens.delete_token(token) + resp = {"logged_out": True} + return jsonify(resp), 200 diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 3bc25aecccde1..8097f61d90cca 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1605,8 +1605,6 @@ paths: application/json: schema: $ref: '#/components/schemas/JwtAuthAndUser' - '400': - $ref: '#/components/responses/BadRequest' '401': $ref: '#/components/responses/Unauthenticated' @@ -1629,6 +1627,9 @@ paths: schema: $ref: '#/components/schemas/AuthorizationUrl' + '401': + $ref: '#/components/responses/Unauthenticated' + /oauth-authorized/{provider}: parameters: - $ref: '#/components/parameters/OauthProvider' @@ -1646,8 +1647,6 @@ paths: application/json: schema: $ref: '#/components/schemas/JwtAuthAndUser' - '400': - $ref: '#/components/responses/BadRequest' '401': $ref: '#/components/responses/Unauthenticated' @@ -1665,8 +1664,6 @@ paths: application/json: schema: $ref: '#/components/schemas/JwtAuthAndUser' - '400': - $ref: '#/components/responses/BadRequest' '401': $ref: '#/components/responses/Unauthenticated' @@ -1684,16 +1681,22 @@ paths: application/json: schema: $ref: '#/components/schemas/AccessToken' - '400': - $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthenticated' /logout: - get: + post: summary: Webserver logout description: Logout user by unsetting cookies x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint operationId: logout tags: [Authentication] + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/AccessRefresh' responses: '200': description: Success @@ -1704,8 +1707,8 @@ paths: logged_out: type: boolean description: Indicate user is logged out - '400': - $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthenticated' components: @@ -2750,6 +2753,16 @@ components: nullable: true # Form + AccessRefresh: + type: object + properties: + token: + description: Access token + type: string + refresh_token: + description: Refresh token + type: string + ClearTaskInstance: type: object properties: diff --git a/airflow/api_connexion/security.py b/airflow/api_connexion/security.py index 57df760933fa7..1a4f82f2fc181 100644 --- a/airflow/api_connexion/security.py +++ b/airflow/api_connexion/security.py @@ -14,25 +14,85 @@ # 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, Optional, Sequence, Tuple, TypeVar, cast -from flask import Response, current_app -from flask_jwt_extended import verify_jwt_in_request +try: + from flask import _app_ctx_stack as ctx_stack +except ImportError: # pragma: no cover + from flask import _request_ctx_stack as ctx_stack +from flask import Response, current_app, request +from flask_jwt_extended.config import config +from flask_jwt_extended.utils import verify_token_claims +from flask_jwt_extended.view_decorators import _decode_jwt_from_request, _load_user from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated +from airflow.models.auth import Tokens T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name +def verify_jwt_access_token_(): + """Verify JWT in request""" + if request.method not in config.exempt_methods: + jwt_data, jwt_header = _decode_jwt_from_request(request_type='access') + ctx_stack.top.jwt = jwt_data + ctx_stack.top.jwt_header = jwt_header + verify_token_claims(jwt_data) + _load_user(jwt_data[config.identity_claim_key]) + token = Tokens.get_token(jwt_data) + if token and token.is_revoked: + raise Unauthenticated(detail="Token revoked") + if token and token.expiry_delta < datetime.now() + timedelta(minutes=1): + Tokens.delete_token(token) + raise Unauthenticated(detail="Token expired and we have deleted it") + if not token: + raise Unauthenticated(detail="Token Unknown") + + +def verify_jwt_refresh_token_in_request_(): + """ + Ensure that the requester has a valid refresh token. Raises an appropiate + exception if there is no token or the token is invalid. + """ + if request.method not in config.exempt_methods: + jwt_data, jwt_header = _decode_jwt_from_request(request_type='refresh') + ctx_stack.top.jwt = jwt_data + ctx_stack.top.jwt_header = jwt_header + _load_user(jwt_data[config.identity_claim_key]) + token = Tokens.get_token(jwt_data) + if token and token.is_revoked: + raise Unauthenticated(detail="Token revoked") + if token and token.expiry_delta < datetime.now() + timedelta(minutes=1): + Tokens.delete_token(token) + raise Unauthenticated(detail="Token expired and we have deleted it") + if not token: + raise Unauthenticated(detail="Token Unknown") + + +def jwt_refresh_token_required_(fn): + """ + A decorator to protect a an endpoint. + If you decorate an endpoint with this, it will ensure that the requester + has a valid refresh token before allowing the endpoint to be called. + """ + + @wraps(fn) + def wrapper(*args, **kwargs): + verify_jwt_refresh_token_in_request_() + return fn(*args, **kwargs) + + return wrapper + + 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 try: - verify_jwt_in_request() + verify_jwt_access_token_() return except Exception: # pylint: disable=broad-except pass diff --git a/airflow/migrations/versions/43c2bf7117bd_add_tokens_table.py b/airflow/migrations/versions/43c2bf7117bd_add_tokens_table.py new file mode 100644 index 0000000000000..c6ae69282dc27 --- /dev/null +++ b/airflow/migrations/versions/43c2bf7117bd_add_tokens_table.py @@ -0,0 +1,55 @@ +# +# 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. + +"""Add token blocklist table + +Revision ID: 43c2bf7117bd +Revises: 2e42bb497a22 +Create Date: 2021-04-05 13:04:23.339826 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = '43c2bf7117bd' +down_revision = '2e42bb497a22' +branch_labels = None +depends_on = None + + +def upgrade(): + """Apply Add token blocklist table""" + op.create_table( + "tokens", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("jti", sa.Text(), nullable=False), + sa.Column("is_revoked", sa.Boolean(name="is_revoked"), server_default="0"), + sa.Column("refresh", sa.Boolean(name="refresh"), server_default="0"), + sa.Column("revoke_reason", sa.String(100)), + sa.Column("revoked_by", sa.String(50)), + sa.Column("date_revoked", sa.DateTime), + sa.Column("expiry_delta", sa.Interval, nullable=False), + sa.Column("created_at", sa.DateTime, nullable=False), + ) + + +def downgrade(): # noqa: D103 + """Unapply Add token blocklist table""" + op.drop_table('tokens') diff --git a/airflow/models/auth.py b/airflow/models/auth.py index ac17a1207741c..924d679324e08 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -14,18 +14,60 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from typing import List -from sqlalchemy import Column, DateTime, Integer, String +from sqlalchemy import Boolean, Column, DateTime, Integer, Interval, String, Text, func from airflow.models.base import Base from airflow.utils.log.logging_mixin import LoggingMixin +from airflow.utils.session import provide_session -class TokenBlocklist(Base, LoggingMixin): - """Token blobk list""" +class Tokens(Base, LoggingMixin): + """Token list""" - __tablename__ = 'tokenblocklist' + __tablename__ = 'tokens' id = Column(Integer, primary_key=True) - jti = Column(String(50), nullable=False) - reason = Column(String(100)) - created_at = Column(DateTime, nullable=False) + jti = Column(Text(), nullable=False) + refresh = Column(Boolean(name="refresh"), default=False) + is_revoked = Column(Boolean(name='revoked'), default=False) + revoke_reason = Column(String(100)) + revoked_by = Column(String(50)) + date_revoked = Column(DateTime) + expiry_delta = Column(Interval, nullable=False) + created_at = Column(DateTime, default=func.now()) + + def __init__( + self, + jti, + expiry_delta, + refresh=False, + is_revoked=False, + revoked_reason=None, + revoked_by=None, + date_revoked=None, + created_at=None, + ): + super().__init__() + self.jti = jti + self.expiry_delta = expiry_delta + self.refresh = refresh + self.is_revoked = is_revoked + self.revoke_reason = revoked_reason + self.revoked_by = revoked_by + self.date_revoked = date_revoked + self.created_at = created_at + + @classmethod + @provide_session + def get_token(cls, token, session=None): + tkn = session.query(cls).filter(cls.jti == token).first() + return tkn + + @classmethod + @provide_session + def delete_token(cls, token, session=None): + tkn = session.query(cls).filter(cls.jti == token).first() + if tkn: + session.delete(tkn) + session.commit() diff --git a/airflow/www/security.py b/airflow/www/security.py index 098cf3aa39604..8e384de7fd14a 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -32,6 +32,7 @@ from airflow.api_connexion.schemas.auth_schema import auth_schema from airflow.exceptions import AirflowException from airflow.models import DagBag, DagModel +from airflow.models.auth import Tokens from airflow.security import permissions from airflow.utils.log.logging_mixin import LoggingMixin from airflow.utils.session import provide_session @@ -742,11 +743,21 @@ def create_jwt_manager(self, app) -> JWTManager: jwt_manager.user_loader_callback_loader(self.load_user_jwt) return jwt_manager - def create_tokens_and_dump(self, user): + @provide_session + def create_tokens_and_dump(self, user, session=None): """Creates access token, return user data alongside tokens""" - token = create_access_token(user.id) - refresh_token = create_refresh_token(user.id) - return auth_schema.dump({'user': user, 'token': token, 'refresh_token': refresh_token}) + app = current_app + token = Tokens( + jti=create_access_token(user.id), expiry_delta=app.config.get("JWT_ACCESS_TOKEN_EXPIRES") + ) + refresh_token = Tokens( + jti=create_refresh_token(user.id), + expiry_delta=app.config.get("JWT_REFRESH_TOKEN_EXPIRES"), + refresh=True, + ) + session.add_all([token, refresh_token]) + session.commit() + return auth_schema.dump({'user': user, 'token': token.jti, 'refresh_token': refresh_token.jti}) class ApplessAirflowSecurityManager(AirflowSecurityManager): diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index fa47369abc38c..f47089dd19344 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -307,13 +307,3 @@ def test_incorrect_auth_type_raises(self): resp = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) assert resp.status_code == 401 assert resp.json['detail'] == "Authentication type do not match" - - -class TestLogoutEndpoint(TestLoginEndpoint): - def test_logout(self): - self.auth_type(AUTH_DB) - payload = {"username": "test", "password": "test"} - response = self.client.post('api/v1/auth/login', json=payload) - assert response.json['user']['username'] == 'test' - response = self.client.get('api/v1/logout') - assert response.json == {"logged_out": True} From 0463660fc54543eae32d13e9b7d0fd3af816b6f0 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 6 Apr 2021 09:50:24 +0100 Subject: [PATCH 22/53] Apply suggestion from review --- airflow/api_connexion/openapi/v1.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 8097f61d90cca..d38cb2e2e4667 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1670,7 +1670,7 @@ paths: /refresh: post: summary: Refresh a token - description: Accepts fresh token and creates access token + description: Accepts refresh token and creates access token x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint operationId: refresh_token tags: [Authentication] From 4ab7cb0572eff3b387b31a0b821180d5e90eaa43 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 6 Apr 2021 13:22:07 +0100 Subject: [PATCH 23/53] add tests for token model --- .../api_connexion/endpoints/auth_endpoint.py | 12 ++-- airflow/api_connexion/security.py | 10 +-- ...ble.py => 43c2bf7117bd_add_token_table.py} | 2 +- airflow/models/auth.py | 5 +- airflow/www/security.py | 6 +- tests/models/test_auth.py | 61 +++++++++++++++++++ 6 files changed, 78 insertions(+), 18 deletions(-) rename airflow/migrations/versions/{43c2bf7117bd_add_tokens_table.py => 43c2bf7117bd_add_token_table.py} (99%) create mode 100644 tests/models/test_auth.py diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index ab955e2b7884c..757686f2fdcd9 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -27,7 +27,7 @@ from airflow.api_connexion.exceptions import Unauthenticated from airflow.api_connexion.schemas.auth_schema import info_schema, login_form_schema from airflow.api_connexion.security import jwt_refresh_token_required_ -from airflow.models.auth import Tokens +from airflow.models.auth import Token from airflow.utils.session import provide_session log = logging.getLogger(__name__) @@ -170,7 +170,7 @@ def refresh_token(session): """Refresh token""" app = current_app current_user = get_jwt_identity() - token = Tokens( + token = Token( jti=create_access_token(identity=current_user), expiry_date=app.config.get("JWT_ACCESS_TOKEN_EXPIRES") ) session.add(token) @@ -181,11 +181,11 @@ def refresh_token(session): def logout(token, refresh_token): """Sign out""" - exist = Tokens.get_token(token) + exist = Token.get_token(token) if exist: - Tokens.delete_token(token) - exist = Tokens.get_token(refresh_token) + Token.delete_token(token) + exist = Token.get_token(refresh_token) if exist: - Tokens.delete_token(token) + Token.delete_token(token) resp = {"logged_out": True} return jsonify(resp), 200 diff --git a/airflow/api_connexion/security.py b/airflow/api_connexion/security.py index 1a4f82f2fc181..a01ed43ce6711 100644 --- a/airflow/api_connexion/security.py +++ b/airflow/api_connexion/security.py @@ -28,7 +28,7 @@ from flask_jwt_extended.view_decorators import _decode_jwt_from_request, _load_user from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated -from airflow.models.auth import Tokens +from airflow.models.auth import Token T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name @@ -41,11 +41,11 @@ def verify_jwt_access_token_(): ctx_stack.top.jwt_header = jwt_header verify_token_claims(jwt_data) _load_user(jwt_data[config.identity_claim_key]) - token = Tokens.get_token(jwt_data) + token = Token.get_token(jwt_data) if token and token.is_revoked: raise Unauthenticated(detail="Token revoked") if token and token.expiry_delta < datetime.now() + timedelta(minutes=1): - Tokens.delete_token(token) + Token.delete_token(token) raise Unauthenticated(detail="Token expired and we have deleted it") if not token: raise Unauthenticated(detail="Token Unknown") @@ -61,11 +61,11 @@ def verify_jwt_refresh_token_in_request_(): ctx_stack.top.jwt = jwt_data ctx_stack.top.jwt_header = jwt_header _load_user(jwt_data[config.identity_claim_key]) - token = Tokens.get_token(jwt_data) + token = Token.get_token(jwt_data) if token and token.is_revoked: raise Unauthenticated(detail="Token revoked") if token and token.expiry_delta < datetime.now() + timedelta(minutes=1): - Tokens.delete_token(token) + Token.delete_token(token) raise Unauthenticated(detail="Token expired and we have deleted it") if not token: raise Unauthenticated(detail="Token Unknown") diff --git a/airflow/migrations/versions/43c2bf7117bd_add_tokens_table.py b/airflow/migrations/versions/43c2bf7117bd_add_token_table.py similarity index 99% rename from airflow/migrations/versions/43c2bf7117bd_add_tokens_table.py rename to airflow/migrations/versions/43c2bf7117bd_add_token_table.py index c6ae69282dc27..c99fe3eaf333c 100644 --- a/airflow/migrations/versions/43c2bf7117bd_add_tokens_table.py +++ b/airflow/migrations/versions/43c2bf7117bd_add_token_table.py @@ -37,7 +37,7 @@ def upgrade(): """Apply Add token blocklist table""" op.create_table( - "tokens", + "token", sa.Column("id", sa.Integer(), primary_key=True), sa.Column("jti", sa.Text(), nullable=False), sa.Column("is_revoked", sa.Boolean(name="is_revoked"), server_default="0"), diff --git a/airflow/models/auth.py b/airflow/models/auth.py index 924d679324e08..cedde8ec15d3c 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -14,7 +14,6 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from typing import List from sqlalchemy import Boolean, Column, DateTime, Integer, Interval, String, Text, func @@ -23,10 +22,10 @@ from airflow.utils.session import provide_session -class Tokens(Base, LoggingMixin): +class Token(Base, LoggingMixin): """Token list""" - __tablename__ = 'tokens' + __tablename__ = 'token' id = Column(Integer, primary_key=True) jti = Column(Text(), nullable=False) refresh = Column(Boolean(name="refresh"), default=False) diff --git a/airflow/www/security.py b/airflow/www/security.py index 8e384de7fd14a..30b4ddd9c3a11 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -32,7 +32,7 @@ from airflow.api_connexion.schemas.auth_schema import auth_schema from airflow.exceptions import AirflowException from airflow.models import DagBag, DagModel -from airflow.models.auth import Tokens +from airflow.models.auth import Token from airflow.security import permissions from airflow.utils.log.logging_mixin import LoggingMixin from airflow.utils.session import provide_session @@ -747,10 +747,10 @@ def create_jwt_manager(self, app) -> JWTManager: def create_tokens_and_dump(self, user, session=None): """Creates access token, return user data alongside tokens""" app = current_app - token = Tokens( + token = Token( jti=create_access_token(user.id), expiry_delta=app.config.get("JWT_ACCESS_TOKEN_EXPIRES") ) - refresh_token = Tokens( + refresh_token = Token( jti=create_refresh_token(user.id), expiry_delta=app.config.get("JWT_REFRESH_TOKEN_EXPIRES"), refresh=True, diff --git a/tests/models/test_auth.py b/tests/models/test_auth.py new file mode 100644 index 0000000000000..a188beb66b8e7 --- /dev/null +++ b/tests/models/test_auth.py @@ -0,0 +1,61 @@ +# 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 datetime import timedelta + +from airflow.models.auth import Token +from airflow.utils.session import provide_session + + +class TestToken(unittest.TestCase): + def tearDown(self) -> None: + self.delete_tokens() + + @provide_session + def delete_tokens(self, session=None): + tokens = session.query(Token).all() + for i in tokens: + session.delete(i) + + @provide_session + def create_token(self, session=None): + token = Token(jti="token", expiry_delta=timedelta(minutes=2)) + session.add(token) + session.commit() + return token + + @provide_session + def test_create_token(self, session=None): + self.create_token() + token = session.query(Token).all() + assert len(token) == 1 + + def test_get_token_method(self): + token = self.create_token() + token2 = Token.get_token(token.jti) + assert token2.jti == token.jti + assert token2.expiry_delta == token.expiry_delta + + @provide_session + def test_delete_token_method(self, session): + tkn = self.create_token() + token = Token.get_token(tkn.jti) + assert token is not None + Token.delete_token(token.jti) + token = Token.get_token(token.jti) + assert token is None From 7a94db59c02d9cc16b7a945962a6060aa3742c23 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 6 Apr 2021 16:53:30 +0100 Subject: [PATCH 24/53] improve auth flow and tests --- .../api_connexion/endpoints/auth_endpoint.py | 32 +++++---- airflow/api_connexion/schemas/auth_schema.py | 8 +++ airflow/api_connexion/security.py | 26 ++++---- .../versions/43c2bf7117bd_add_token_table.py | 6 +- airflow/models/auth.py | 12 ++-- airflow/www/security.py | 22 ++++--- .../endpoints/test_auth_endpoint.py | 66 +++++++++++++++++++ 7 files changed, 128 insertions(+), 44 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 757686f2fdcd9..8d919263e2d19 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -20,12 +20,12 @@ import jwt from flask import current_app, g, jsonify, request, session from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER -from flask_jwt_extended import create_access_token, get_jwt_identity +from flask_jwt_extended import create_access_token, decode_token, get_jti, get_jwt_identity from flask_login import login_user from marshmallow import ValidationError -from airflow.api_connexion.exceptions import Unauthenticated -from airflow.api_connexion.schemas.auth_schema import info_schema, login_form_schema +from airflow.api_connexion.exceptions import BadRequest, Unauthenticated +from airflow.api_connexion.schemas.auth_schema import info_schema, login_form_schema, logout_schema from airflow.api_connexion.security import jwt_refresh_token_required_ from airflow.models.auth import Token from airflow.utils.session import provide_session @@ -168,24 +168,30 @@ def auth_remoteuser(): @provide_session def refresh_token(session): """Refresh token""" - app = current_app current_user = get_jwt_identity() - token = Token( - jti=create_access_token(identity=current_user), expiry_date=app.config.get("JWT_ACCESS_TOKEN_EXPIRES") - ) + access_token = create_access_token(identity=current_user) + decoded = decode_token(access_token) + token = Token(jti=decoded['jti'], expiry_delta=decoded['exp'], created_delta=decoded['iat']) session.add(token) session.commit() - ret = {'access_token': token.jti} + ret = {'access_token': access_token} return jsonify(ret), 200 -def logout(token, refresh_token): +def logout(): """Sign out""" - exist = Token.get_token(token) + body = request.json + try: + data = logout_schema.load(body) + except ValidationError as err: + raise BadRequest(detail=str(err.messages)) + token_jti = get_jti(data['token']) + exist = Token.get_token(token_jti) if exist: - Token.delete_token(token) - exist = Token.get_token(refresh_token) + Token.delete_token(token_jti) + refresh_jti = get_jti(data['refresh_token']) + exist = Token.get_token(refresh_jti) if exist: - Token.delete_token(token) + Token.delete_token(refresh_jti) resp = {"logged_out": True} return jsonify(resp), 200 diff --git a/airflow/api_connexion/schemas/auth_schema.py b/airflow/api_connexion/schemas/auth_schema.py index 9c31d761f81b5..70df6d102ddd9 100644 --- a/airflow/api_connexion/schemas/auth_schema.py +++ b/airflow/api_connexion/schemas/auth_schema.py @@ -58,6 +58,14 @@ class AuthSchema(Schema): user = fields.Nested(UserCollectionItemSchema) +class LogoutSchema(Schema): + """Schema for logout""" + + token = fields.String() + refresh_token = fields.String() + + info_schema = InfoSchema() login_form_schema = LoginForm() auth_schema = AuthSchema() +logout_schema = LogoutSchema() diff --git a/airflow/api_connexion/security.py b/airflow/api_connexion/security.py index a01ed43ce6711..cddcef1ecf411 100644 --- a/airflow/api_connexion/security.py +++ b/airflow/api_connexion/security.py @@ -14,15 +14,11 @@ # 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 datetime import datetime from functools import wraps from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast -try: - from flask import _app_ctx_stack as ctx_stack -except ImportError: # pragma: no cover - from flask import _request_ctx_stack as ctx_stack -from flask import Response, current_app, request +from flask import Response, _app_ctx_stack as ctx_stack, current_app, request from flask_jwt_extended.config import config from flask_jwt_extended.utils import verify_token_claims from flask_jwt_extended.view_decorators import _decode_jwt_from_request, _load_user @@ -32,6 +28,10 @@ T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name +# Because we are using flask_jwt_extended <4.0, the two functions here have +# to be created due to language matters. When we upgrade to >=4.0, we would +# not need these functions, rather we would use token_in_blocklist callback + def verify_jwt_access_token_(): """Verify JWT in request""" @@ -41,11 +41,12 @@ def verify_jwt_access_token_(): ctx_stack.top.jwt_header = jwt_header verify_token_claims(jwt_data) _load_user(jwt_data[config.identity_claim_key]) - token = Token.get_token(jwt_data) + token = Token.get_token(jwt_data['jti']) if token and token.is_revoked: raise Unauthenticated(detail="Token revoked") - if token and token.expiry_delta < datetime.now() + timedelta(minutes=1): - Token.delete_token(token) + exp = datetime.timestamp(datetime.now()) + if token and token.expiry_delta < exp: + Token.delete_token(token.jti) raise Unauthenticated(detail="Token expired and we have deleted it") if not token: raise Unauthenticated(detail="Token Unknown") @@ -61,11 +62,12 @@ def verify_jwt_refresh_token_in_request_(): ctx_stack.top.jwt = jwt_data ctx_stack.top.jwt_header = jwt_header _load_user(jwt_data[config.identity_claim_key]) - token = Token.get_token(jwt_data) + token = Token.get_token(jwt_data['jti']) if token and token.is_revoked: raise Unauthenticated(detail="Token revoked") - if token and token.expiry_delta < datetime.now() + timedelta(minutes=1): - Token.delete_token(token) + exp = datetime.timestamp(datetime.now()) + if token and token.expiry_delta < exp: + Token.delete_token(token.jti) raise Unauthenticated(detail="Token expired and we have deleted it") if not token: raise Unauthenticated(detail="Token Unknown") diff --git a/airflow/migrations/versions/43c2bf7117bd_add_token_table.py b/airflow/migrations/versions/43c2bf7117bd_add_token_table.py index c99fe3eaf333c..fce0b2e9400e1 100644 --- a/airflow/migrations/versions/43c2bf7117bd_add_token_table.py +++ b/airflow/migrations/versions/43c2bf7117bd_add_token_table.py @@ -39,14 +39,14 @@ def upgrade(): op.create_table( "token", sa.Column("id", sa.Integer(), primary_key=True), - sa.Column("jti", sa.Text(), nullable=False), + sa.Column("jti", sa.String(50), nullable=False), sa.Column("is_revoked", sa.Boolean(name="is_revoked"), server_default="0"), sa.Column("refresh", sa.Boolean(name="refresh"), server_default="0"), sa.Column("revoke_reason", sa.String(100)), sa.Column("revoked_by", sa.String(50)), sa.Column("date_revoked", sa.DateTime), - sa.Column("expiry_delta", sa.Interval, nullable=False), - sa.Column("created_at", sa.DateTime, nullable=False), + sa.Column("expiry_delta", sa.Integer, nullable=False), + sa.Column("created_delta", sa.Integer, nullable=False), ) diff --git a/airflow/models/auth.py b/airflow/models/auth.py index cedde8ec15d3c..2e528d9fb6b73 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. -from sqlalchemy import Boolean, Column, DateTime, Integer, Interval, String, Text, func +from sqlalchemy import Boolean, Column, DateTime, Integer, String, Text, func from airflow.models.base import Base from airflow.utils.log.logging_mixin import LoggingMixin @@ -27,14 +27,14 @@ class Token(Base, LoggingMixin): __tablename__ = 'token' id = Column(Integer, primary_key=True) - jti = Column(Text(), nullable=False) + jti = Column(String(50), nullable=False) refresh = Column(Boolean(name="refresh"), default=False) is_revoked = Column(Boolean(name='revoked'), default=False) revoke_reason = Column(String(100)) revoked_by = Column(String(50)) date_revoked = Column(DateTime) - expiry_delta = Column(Interval, nullable=False) - created_at = Column(DateTime, default=func.now()) + expiry_delta = Column(Integer, nullable=False) + created_delta = Column(Integer, default=func.now()) def __init__( self, @@ -45,7 +45,7 @@ def __init__( revoked_reason=None, revoked_by=None, date_revoked=None, - created_at=None, + created_delta=None, ): super().__init__() self.jti = jti @@ -55,7 +55,7 @@ def __init__( self.revoke_reason = revoked_reason self.revoked_by = revoked_by self.date_revoked = date_revoked - self.created_at = created_at + self.created_delta = created_delta @classmethod @provide_session diff --git a/airflow/www/security.py b/airflow/www/security.py index 30b4ddd9c3a11..38d46dd680432 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -25,7 +25,7 @@ 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 JWTManager, create_access_token, create_refresh_token +from flask_jwt_extended import JWTManager, create_access_token, create_refresh_token, decode_token, get_jti from sqlalchemy import or_ from sqlalchemy.orm import joinedload @@ -746,18 +746,20 @@ def create_jwt_manager(self, app) -> JWTManager: @provide_session def create_tokens_and_dump(self, user, session=None): """Creates access token, return user data alongside tokens""" - app = current_app - token = Token( - jti=create_access_token(user.id), expiry_delta=app.config.get("JWT_ACCESS_TOKEN_EXPIRES") - ) - refresh_token = Token( - jti=create_refresh_token(user.id), - expiry_delta=app.config.get("JWT_REFRESH_TOKEN_EXPIRES"), + access_token = create_access_token(user.id) + decoded = decode_token(access_token) + token = Token(jti=decoded['jti'], expiry_delta=decoded['exp'], created_delta=decoded['iat']) + refresh_token = create_refresh_token(user.id) + decoded = decode_token(refresh_token) + r_token = Token( + jti=decoded['jti'], + expiry_delta=decoded['exp'], + created_delta=decoded['iat'], refresh=True, ) - session.add_all([token, refresh_token]) + session.add_all([token, r_token]) session.commit() - return auth_schema.dump({'user': user, 'token': token.jti, 'refresh_token': refresh_token.jti}) + return auth_schema.dump({'user': user, 'token': access_token, 'refresh_token': refresh_token}) class ApplessAirflowSecurityManager(AirflowSecurityManager): diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index f47089dd19344..077cc0231f454 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -18,7 +18,10 @@ import pytest from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER +from sqlalchemy import func +from airflow.models.auth import Token +from airflow.utils.session import provide_session from tests.test_utils.api_connexion_utils import delete_user from tests.test_utils.fab_utils import create_user @@ -45,6 +48,13 @@ ] +@provide_session +def delete_tokens(session=None): + tokens = session.query(Token).all() + for token in tokens: + Token.delete_token(token.jti, session) + + @pytest.fixture(scope="module") def configured_app(minimal_app_for_api): app = minimal_app_for_api @@ -60,6 +70,7 @@ class TestLoginEndpoint: 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 auth_type(self, auth): self.app.config['AUTH_TYPE'] = auth @@ -73,6 +84,12 @@ def auth_type(self, auth): self.app.config['OPENID_PROVIDERS'] = None self.app.config['OAUTH_PROVIDERS'] = None + def teardown_method(self): + tokens = self.session.query(Token).all() + for token in tokens: + self.session.delete(token) + self.session.commit() + class TestAuthInfo(TestLoginEndpoint): def test_auth_db_info(self): @@ -307,3 +324,52 @@ def test_incorrect_auth_type_raises(self): resp = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) assert resp.status_code == 401 assert resp.json['detail'] == "Authentication type do not match" + + +class TestRefreshTokenEndpoint(TestLoginEndpoint): + @provide_session + def test_creates_access_token(self, session=None): + self.auth_type(AUTH_DB) + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth/login', json=payload) + token = response.json['token'] + refresh = response.json['refresh_token'] + total_tokens_in_db = session.query(func.count(Token.id)).scalar() + + assert total_tokens_in_db == 2 + assert token is not None + response2 = self.client.post("api/v1/refresh", headers={"Authorization": f"Bearer {refresh}"}) + + assert response2.json['access_token'] is not None + total_tokens_in_db = session.query(func.count(Token.id)).scalar() + assert total_tokens_in_db == 3 + + @provide_session + def test_access_token_cant_access_endpoint(self, session): + self.auth_type(AUTH_DB) + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth/login', json=payload) + token = response.json['token'] + total_tokens_in_db = session.query(func.count(Token.id)).scalar() + assert total_tokens_in_db == 2 + assert token is not None + response2 = self.client.post("api/v1/refresh", headers={"Authorization": f"Bearer {token}"}) + assert response2.status_code == 422 + assert response2.json['msg'] == 'Only refresh tokens are allowed' + + +class TestLogoutEndpoint(TestLoginEndpoint): + @provide_session + def test_logout_works(self, session): + self.auth_type(AUTH_DB) + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth/login', json=payload) + token = response.json['token'] + refresh = response.json['refresh_token'] + total_tokens_in_db = session.query(func.count(Token.id)).scalar() + assert total_tokens_in_db == 2 + assert token is not None + response2 = self.client.post("api/v1/logout", json={"token": token, "refresh_token": refresh}) + assert response2.json == {'logged_out': True} + total_tokens_in_db = session.query(func.count(Token.id)).scalar() + assert total_tokens_in_db == 0 From 22a2d3070497d8bb07636427601cbcf04a50dc1d Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 6 Apr 2021 20:50:13 +0100 Subject: [PATCH 25/53] add token revoke endpoint and improve tests --- .../api_connexion/endpoints/auth_endpoint.py | 44 +++++++++++++++---- airflow/api_connexion/openapi/v1.yaml | 38 +++++++++++++++- airflow/api_connexion/schemas/auth_schema.py | 8 ++++ .../endpoints/test_auth_endpoint.py | 23 +++++++--- 4 files changed, 99 insertions(+), 14 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 8d919263e2d19..cc632dec20b00 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -16,16 +16,22 @@ # under the License. import logging +from datetime import datetime import jwt -from flask import current_app, g, jsonify, request, session +from flask import current_app, g, jsonify, request, session as c_session from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER from flask_jwt_extended import create_access_token, decode_token, get_jti, get_jwt_identity -from flask_login import login_user +from flask_login import current_user, login_user from marshmallow import ValidationError -from airflow.api_connexion.exceptions import BadRequest, Unauthenticated -from airflow.api_connexion.schemas.auth_schema import info_schema, login_form_schema, logout_schema +from airflow.api_connexion.exceptions import BadRequest, NotFound, Unauthenticated +from airflow.api_connexion.schemas.auth_schema import ( + info_schema, + login_form_schema, + logout_schema, + token_schema, +) from airflow.api_connexion.security import jwt_refresh_token_required_ from airflow.models.auth import Token from airflow.utils.session import provide_session @@ -94,7 +100,7 @@ def auth_oauthlogin(provider, register=None, redirect_url=None): try: auth_provider = appbuilder.sm.oauth_remotes[provider] if register: - session["register"] = True + c_session["register"] = True if provider == "twitter": redirect_uri = redirect_url + f"&state={state}" auth_data = auth_provider.create_authorization_url(redirect_uri=redirect_uri) @@ -166,10 +172,10 @@ def auth_remoteuser(): @jwt_refresh_token_required_ @provide_session -def refresh_token(session): +def refresh_token(session=None): """Refresh token""" - current_user = get_jwt_identity() - access_token = create_access_token(identity=current_user) + user = get_jwt_identity() + access_token = create_access_token(identity=user) decoded = decode_token(access_token) token = Token(jti=decoded['jti'], expiry_delta=decoded['exp'], created_delta=decoded['iat']) session.add(token) @@ -195,3 +201,25 @@ def logout(): Token.delete_token(refresh_jti) resp = {"logged_out": True} return jsonify(resp), 200 + + +@provide_session +def revoke_token(session=None): + """Revoke a token""" + body = request.json + try: + data = token_schema.load(body) + except ValidationError as err: + raise BadRequest(detail=str(err.messages)) + jti = get_jti(data['token']) + token = Token.get_token(jti) + if token: + token.is_revoked = True + token.revoked_by = current_user.username + token.revoke_reason = data['reason'] + token.date_revoked = datetime.now() + session.merge(token) + session.commit() + resp = jsonify({"revoked": True}) + return resp + raise NotFound(detail="Token not found") diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index d38cb2e2e4667..40e4c5cf53060 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1710,6 +1710,31 @@ paths: '401': $ref: '#/components/responses/Unauthenticated' + /revoke: + post: + summary: Revoke a token + description: Revoke a refresh or access token + x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint + operationId: revoke_token + tags: [Authentication] + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/Revoke' + responses: + '200': + description: Success + content: + application/json: + schema: + properties: + revoked: + type: boolean + description: Indicate token is revoked + '404': + $ref: '#/components/responses/NotFound' components: # Reusable schemas (data models) @@ -2700,13 +2725,24 @@ components: - $ref: '#/components/schemas/UserCollectionItem' AccessToken: - description: AccessToken + description: Access Token type: object properties: access_token: type: string description: authentication token + Revoke: + description: Revoke a token + type: object + properties: + token: + type: string + description: The token to be revoked + reason: + type: string + description: Reason for the revoke + # Configuration ConfigOption: type: object diff --git a/airflow/api_connexion/schemas/auth_schema.py b/airflow/api_connexion/schemas/auth_schema.py index 70df6d102ddd9..cfc4b2cf76a1e 100644 --- a/airflow/api_connexion/schemas/auth_schema.py +++ b/airflow/api_connexion/schemas/auth_schema.py @@ -65,7 +65,15 @@ class LogoutSchema(Schema): refresh_token = fields.String() +class RevokeTokenSchema(Schema): + """Schema to revoke token""" + + token = fields.String() + reason = fields.String() + + info_schema = InfoSchema() login_form_schema = LoginForm() auth_schema = AuthSchema() logout_schema = LogoutSchema() +token_schema = RevokeTokenSchema() diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 077cc0231f454..c7144f62bbb2f 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -344,15 +344,11 @@ def test_creates_access_token(self, session=None): total_tokens_in_db = session.query(func.count(Token.id)).scalar() assert total_tokens_in_db == 3 - @provide_session - def test_access_token_cant_access_endpoint(self, session): + def test_access_token_cant_access_endpoint(self): self.auth_type(AUTH_DB) payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth/login', json=payload) token = response.json['token'] - total_tokens_in_db = session.query(func.count(Token.id)).scalar() - assert total_tokens_in_db == 2 - assert token is not None response2 = self.client.post("api/v1/refresh", headers={"Authorization": f"Bearer {token}"}) assert response2.status_code == 422 assert response2.json['msg'] == 'Only refresh tokens are allowed' @@ -373,3 +369,20 @@ def test_logout_works(self, session): assert response2.json == {'logged_out': True} total_tokens_in_db = session.query(func.count(Token.id)).scalar() assert total_tokens_in_db == 0 + + +class TestTokenRevokeEndpoint(TestLoginEndpoint): + @provide_session + def test_revoke_token_works(self, session): + self.auth_type(AUTH_DB) + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth/login', json=payload) + token = response.json['token'] + refresh = response.json['refresh_token'] + total_tokens_in_db = session.query(func.count(Token.id)).scalar() + assert total_tokens_in_db == 2 + self.client.post("api/v1/revoke", json={"token": token, "reason": "test"}) + self.client.post("api/v1/revoke", json={"token": refresh, "reason": "test"}) + tokens = session.query(Token).all() + for token in tokens: + assert token.is_revoked is True From 21595c34cc4d28eca10e33bbfa2bc4fe6ac7f4e0 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 6 Apr 2021 21:25:59 +0100 Subject: [PATCH 26/53] import from api-connnection utils --- tests/api_connexion/endpoints/test_auth_endpoint.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index c7144f62bbb2f..0db49e18a5f73 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -22,8 +22,7 @@ from airflow.models.auth import Token from airflow.utils.session import provide_session -from tests.test_utils.api_connexion_utils import delete_user -from tests.test_utils.fab_utils import create_user +from tests.test_utils.api_connexion_utils import create_user, delete_user OAUTH_PROVIDERS = [ { From 47509af3fc3e8eb5ffaded2af56e5f02d3b20b66 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 6 Apr 2021 21:55:24 +0100 Subject: [PATCH 27/53] add object type to response object --- airflow/api_connexion/openapi/v1.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 40e4c5cf53060..9141b20b2b3f0 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1703,6 +1703,7 @@ paths: content: application/json: schema: + type: object properties: logged_out: type: boolean @@ -1729,6 +1730,7 @@ paths: content: application/json: schema: + type: object properties: revoked: type: boolean From b4eb910133fb96b7b326a1d1f244b10584355bd4 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 7 Apr 2021 07:49:03 +0100 Subject: [PATCH 28/53] Add bearer auth to security scheme, we can change to using cookie anytime --- airflow/api_connexion/openapi/v1.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 9141b20b2b3f0..af25188e7f433 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -3655,10 +3655,10 @@ components: Kerberos: type: http scheme: negotiate - JwtCookieAuth: - type: apiKey - in: cookie - name: access_token_cookie + bearerAuth: + type: http + scheme: bearer + bearerFormat: JWT # The API will provide support for plugins to support various authorization mechanisms. # Detailed information will be available in the plugin specification. From 9c85de1c76d3641256dbbb6f650c5d02dd470e79 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 7 Apr 2021 11:12:45 +0100 Subject: [PATCH 29/53] Fix model doc and pylint --- airflow/models/auth.py | 41 +++++++++++++++++++++++++++++---------- airflow/www/security.py | 2 +- tests/models/test_auth.py | 3 +-- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/airflow/models/auth.py b/airflow/models/auth.py index 2e528d9fb6b73..c3c2614686768 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -14,8 +14,10 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from datetime import datetime +from typing import Optional -from sqlalchemy import Boolean, Column, DateTime, Integer, String, Text, func +from sqlalchemy import Boolean, Column, DateTime, Integer, String, func from airflow.models.base import Base from airflow.utils.log.logging_mixin import LoggingMixin @@ -23,7 +25,24 @@ class Token(Base, LoggingMixin): - """Token list""" + """ + A model for recording decoded token information + + :param jti: The token jti(JWT ID) + :type jti: str + :param refresh: Whether the token is a refresh token or not + :type refresh: bool + :param is_revoked: Whether the token is revoked + :type is_revoked: bool + :param revoked_by: The username of the user who revoked the token + :type revoked_by: str + :param date_revoked: The date the token was revoked + :type date_revoked: datetime + :param expiry_delta: The timestamp of when the token will expire. Token exp + :type expiry_delta: int + :param created_delta: The timestamp of when the token was created. The token's iat + :type created_delta: int + """ __tablename__ = 'token' id = Column(Integer, primary_key=True) @@ -38,14 +57,14 @@ class Token(Base, LoggingMixin): def __init__( self, - jti, - expiry_delta, - refresh=False, - is_revoked=False, - revoked_reason=None, - revoked_by=None, - date_revoked=None, - created_delta=None, + jti: str, + expiry_delta: int, + refresh: Optional[bool] = False, + is_revoked: Optional[bool] = False, + revoked_reason: Optional[str] = None, + revoked_by: Optional[str] = None, + date_revoked: Optional[datetime] = None, + created_delta: Optional[int] = None, ): super().__init__() self.jti = jti @@ -60,12 +79,14 @@ def __init__( @classmethod @provide_session def get_token(cls, token, session=None): + """Get a token""" tkn = session.query(cls).filter(cls.jti == token).first() return tkn @classmethod @provide_session def delete_token(cls, token, session=None): + """Delete a token""" tkn = session.query(cls).filter(cls.jti == token).first() if tkn: session.delete(tkn) diff --git a/airflow/www/security.py b/airflow/www/security.py index 38d46dd680432..f02404f86e3fd 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -25,7 +25,7 @@ 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 JWTManager, create_access_token, create_refresh_token, decode_token, get_jti +from flask_jwt_extended import JWTManager, create_access_token, create_refresh_token, decode_token from sqlalchemy import or_ from sqlalchemy.orm import joinedload diff --git a/tests/models/test_auth.py b/tests/models/test_auth.py index a188beb66b8e7..65bd3cf34d3dc 100644 --- a/tests/models/test_auth.py +++ b/tests/models/test_auth.py @@ -51,8 +51,7 @@ def test_get_token_method(self): assert token2.jti == token.jti assert token2.expiry_delta == token.expiry_delta - @provide_session - def test_delete_token_method(self, session): + def test_delete_token_method(self): tkn = self.create_token() token = Token.get_token(tkn.jti) assert token is not None From cb8a6164286eae1ad6cf3d7af1fcc3ec23eb167a Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 7 Apr 2021 12:09:07 +0100 Subject: [PATCH 30/53] add words to spelling --- docs/spelling_wordlist.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 464db0cb14e4d..7ed3d5f8acf98 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -194,6 +194,7 @@ InspectContentResponse InspectTemplate Investorise JPype +JWT Jakob Jarek Jdbc @@ -920,6 +921,7 @@ joygao js json jthomas +jti jupyter jupytercmd kaxil From 0af79b3e0fde80e11cc50448dedb34edec2b2cee Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 7 Apr 2021 12:49:16 +0100 Subject: [PATCH 31/53] fixup! add words to spelling --- docs/spelling_wordlist.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 7ed3d5f8acf98..8458bb3899574 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -750,6 +750,7 @@ evo exasol execvp exitcode +exp explict exportingmultiple extensibility @@ -859,6 +860,7 @@ hyperparameter iPython iTerm iam +iat idempotence idempotency ie From aa1131426f8d0d5e57c8932a666b4757f70f9585 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 7 Apr 2021 13:52:26 +0100 Subject: [PATCH 32/53] Rename token table --- .../api_connexion/endpoints/auth_endpoint.py | 14 +++++------ airflow/api_connexion/security.py | 10 ++++---- ....py => 43c2bf7117bd_add_jwttoken_table.py} | 2 +- airflow/models/auth.py | 4 ++-- airflow/www/security.py | 6 ++--- .../endpoints/test_auth_endpoint.py | 23 +++++++------------ tests/models/test_auth.py | 16 ++++++------- 7 files changed, 34 insertions(+), 41 deletions(-) rename airflow/migrations/versions/{43c2bf7117bd_add_token_table.py => 43c2bf7117bd_add_jwttoken_table.py} (98%) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index cc632dec20b00..70fe97fa2319c 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -33,7 +33,7 @@ token_schema, ) from airflow.api_connexion.security import jwt_refresh_token_required_ -from airflow.models.auth import Token +from airflow.models.auth import JwtToken from airflow.utils.session import provide_session log = logging.getLogger(__name__) @@ -177,7 +177,7 @@ def refresh_token(session=None): user = get_jwt_identity() access_token = create_access_token(identity=user) decoded = decode_token(access_token) - token = Token(jti=decoded['jti'], expiry_delta=decoded['exp'], created_delta=decoded['iat']) + token = JwtToken(jti=decoded['jti'], expiry_delta=decoded['exp'], created_delta=decoded['iat']) session.add(token) session.commit() ret = {'access_token': access_token} @@ -192,13 +192,13 @@ def logout(): except ValidationError as err: raise BadRequest(detail=str(err.messages)) token_jti = get_jti(data['token']) - exist = Token.get_token(token_jti) + exist = JwtToken.get_token(token_jti) if exist: - Token.delete_token(token_jti) + JwtToken.delete_token(token_jti) refresh_jti = get_jti(data['refresh_token']) - exist = Token.get_token(refresh_jti) + exist = JwtToken.get_token(refresh_jti) if exist: - Token.delete_token(refresh_jti) + JwtToken.delete_token(refresh_jti) resp = {"logged_out": True} return jsonify(resp), 200 @@ -212,7 +212,7 @@ def revoke_token(session=None): except ValidationError as err: raise BadRequest(detail=str(err.messages)) jti = get_jti(data['token']) - token = Token.get_token(jti) + token = JwtToken.get_token(jti) if token: token.is_revoked = True token.revoked_by = current_user.username diff --git a/airflow/api_connexion/security.py b/airflow/api_connexion/security.py index cddcef1ecf411..92bd00d208b9b 100644 --- a/airflow/api_connexion/security.py +++ b/airflow/api_connexion/security.py @@ -24,7 +24,7 @@ from flask_jwt_extended.view_decorators import _decode_jwt_from_request, _load_user from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated -from airflow.models.auth import Token +from airflow.models.auth import JwtToken T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name @@ -41,12 +41,12 @@ def verify_jwt_access_token_(): ctx_stack.top.jwt_header = jwt_header verify_token_claims(jwt_data) _load_user(jwt_data[config.identity_claim_key]) - token = Token.get_token(jwt_data['jti']) + token = JwtToken.get_token(jwt_data['jti']) if token and token.is_revoked: raise Unauthenticated(detail="Token revoked") exp = datetime.timestamp(datetime.now()) if token and token.expiry_delta < exp: - Token.delete_token(token.jti) + JwtToken.delete_token(token.jti) raise Unauthenticated(detail="Token expired and we have deleted it") if not token: raise Unauthenticated(detail="Token Unknown") @@ -62,12 +62,12 @@ def verify_jwt_refresh_token_in_request_(): ctx_stack.top.jwt = jwt_data ctx_stack.top.jwt_header = jwt_header _load_user(jwt_data[config.identity_claim_key]) - token = Token.get_token(jwt_data['jti']) + token = JwtToken.get_token(jwt_data['jti']) if token and token.is_revoked: raise Unauthenticated(detail="Token revoked") exp = datetime.timestamp(datetime.now()) if token and token.expiry_delta < exp: - Token.delete_token(token.jti) + JwtToken.delete_token(token.jti) raise Unauthenticated(detail="Token expired and we have deleted it") if not token: raise Unauthenticated(detail="Token Unknown") diff --git a/airflow/migrations/versions/43c2bf7117bd_add_token_table.py b/airflow/migrations/versions/43c2bf7117bd_add_jwttoken_table.py similarity index 98% rename from airflow/migrations/versions/43c2bf7117bd_add_token_table.py rename to airflow/migrations/versions/43c2bf7117bd_add_jwttoken_table.py index fce0b2e9400e1..e441073d26ac4 100644 --- a/airflow/migrations/versions/43c2bf7117bd_add_token_table.py +++ b/airflow/migrations/versions/43c2bf7117bd_add_jwttoken_table.py @@ -37,7 +37,7 @@ def upgrade(): """Apply Add token blocklist table""" op.create_table( - "token", + "jwt_token", sa.Column("id", sa.Integer(), primary_key=True), sa.Column("jti", sa.String(50), nullable=False), sa.Column("is_revoked", sa.Boolean(name="is_revoked"), server_default="0"), diff --git a/airflow/models/auth.py b/airflow/models/auth.py index c3c2614686768..cf16146516b25 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -24,7 +24,7 @@ from airflow.utils.session import provide_session -class Token(Base, LoggingMixin): +class JwtToken(Base, LoggingMixin): """ A model for recording decoded token information @@ -44,7 +44,7 @@ class Token(Base, LoggingMixin): :type created_delta: int """ - __tablename__ = 'token' + __tablename__ = 'jwt_token' id = Column(Integer, primary_key=True) jti = Column(String(50), nullable=False) refresh = Column(Boolean(name="refresh"), default=False) diff --git a/airflow/www/security.py b/airflow/www/security.py index f02404f86e3fd..0952fdfa5a9dc 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -32,7 +32,7 @@ from airflow.api_connexion.schemas.auth_schema import auth_schema from airflow.exceptions import AirflowException from airflow.models import DagBag, DagModel -from airflow.models.auth import Token +from airflow.models.auth import JwtToken from airflow.security import permissions from airflow.utils.log.logging_mixin import LoggingMixin from airflow.utils.session import provide_session @@ -748,10 +748,10 @@ def create_tokens_and_dump(self, user, session=None): """Creates access token, return user data alongside tokens""" access_token = create_access_token(user.id) decoded = decode_token(access_token) - token = Token(jti=decoded['jti'], expiry_delta=decoded['exp'], created_delta=decoded['iat']) + token = JwtToken(jti=decoded['jti'], expiry_delta=decoded['exp'], created_delta=decoded['iat']) refresh_token = create_refresh_token(user.id) decoded = decode_token(refresh_token) - r_token = Token( + r_token = JwtToken( jti=decoded['jti'], expiry_delta=decoded['exp'], created_delta=decoded['iat'], diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 0db49e18a5f73..ad0bb79d5e285 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -20,7 +20,7 @@ from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER from sqlalchemy import func -from airflow.models.auth import Token +from airflow.models.auth import JwtToken from airflow.utils.session import provide_session from tests.test_utils.api_connexion_utils import create_user, delete_user @@ -47,13 +47,6 @@ ] -@provide_session -def delete_tokens(session=None): - tokens = session.query(Token).all() - for token in tokens: - Token.delete_token(token.jti, session) - - @pytest.fixture(scope="module") def configured_app(minimal_app_for_api): app = minimal_app_for_api @@ -84,7 +77,7 @@ def auth_type(self, auth): self.app.config['OAUTH_PROVIDERS'] = None def teardown_method(self): - tokens = self.session.query(Token).all() + tokens = self.session.query(JwtToken).all() for token in tokens: self.session.delete(token) self.session.commit() @@ -333,14 +326,14 @@ def test_creates_access_token(self, session=None): response = self.client.post('api/v1/auth/login', json=payload) token = response.json['token'] refresh = response.json['refresh_token'] - total_tokens_in_db = session.query(func.count(Token.id)).scalar() + total_tokens_in_db = session.query(func.count(JwtToken.id)).scalar() assert total_tokens_in_db == 2 assert token is not None response2 = self.client.post("api/v1/refresh", headers={"Authorization": f"Bearer {refresh}"}) assert response2.json['access_token'] is not None - total_tokens_in_db = session.query(func.count(Token.id)).scalar() + total_tokens_in_db = session.query(func.count(JwtToken.id)).scalar() assert total_tokens_in_db == 3 def test_access_token_cant_access_endpoint(self): @@ -361,12 +354,12 @@ def test_logout_works(self, session): response = self.client.post('api/v1/auth/login', json=payload) token = response.json['token'] refresh = response.json['refresh_token'] - total_tokens_in_db = session.query(func.count(Token.id)).scalar() + total_tokens_in_db = session.query(func.count(JwtToken.id)).scalar() assert total_tokens_in_db == 2 assert token is not None response2 = self.client.post("api/v1/logout", json={"token": token, "refresh_token": refresh}) assert response2.json == {'logged_out': True} - total_tokens_in_db = session.query(func.count(Token.id)).scalar() + total_tokens_in_db = session.query(func.count(JwtToken.id)).scalar() assert total_tokens_in_db == 0 @@ -378,10 +371,10 @@ def test_revoke_token_works(self, session): response = self.client.post('api/v1/auth/login', json=payload) token = response.json['token'] refresh = response.json['refresh_token'] - total_tokens_in_db = session.query(func.count(Token.id)).scalar() + total_tokens_in_db = session.query(func.count(JwtToken.id)).scalar() assert total_tokens_in_db == 2 self.client.post("api/v1/revoke", json={"token": token, "reason": "test"}) self.client.post("api/v1/revoke", json={"token": refresh, "reason": "test"}) - tokens = session.query(Token).all() + tokens = session.query(JwtToken).all() for token in tokens: assert token.is_revoked is True diff --git a/tests/models/test_auth.py b/tests/models/test_auth.py index 65bd3cf34d3dc..23811487f8b83 100644 --- a/tests/models/test_auth.py +++ b/tests/models/test_auth.py @@ -18,7 +18,7 @@ import unittest from datetime import timedelta -from airflow.models.auth import Token +from airflow.models.auth import JwtToken from airflow.utils.session import provide_session @@ -28,13 +28,13 @@ def tearDown(self) -> None: @provide_session def delete_tokens(self, session=None): - tokens = session.query(Token).all() + tokens = session.query(JwtToken).all() for i in tokens: session.delete(i) @provide_session def create_token(self, session=None): - token = Token(jti="token", expiry_delta=timedelta(minutes=2)) + token = JwtToken(jti="token", expiry_delta=timedelta(minutes=2)) session.add(token) session.commit() return token @@ -42,19 +42,19 @@ def create_token(self, session=None): @provide_session def test_create_token(self, session=None): self.create_token() - token = session.query(Token).all() + token = session.query(JwtToken).all() assert len(token) == 1 def test_get_token_method(self): token = self.create_token() - token2 = Token.get_token(token.jti) + token2 = JwtToken.get_token(token.jti) assert token2.jti == token.jti assert token2.expiry_delta == token.expiry_delta def test_delete_token_method(self): tkn = self.create_token() - token = Token.get_token(tkn.jti) + token = JwtToken.get_token(tkn.jti) assert token is not None - Token.delete_token(token.jti) - token = Token.get_token(token.jti) + JwtToken.delete_token(token.jti) + token = JwtToken.get_token(token.jti) assert token is None From 88c30ddce6f5277047cda50d44ccf0334ab753d4 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 7 Apr 2021 14:59:21 +0100 Subject: [PATCH 33/53] fixup! Rename token table --- airflow/models/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/airflow/models/__init__.py b/airflow/models/__init__.py index 61606ea86b601..493744a3f42f6 100644 --- a/airflow/models/__init__.py +++ b/airflow/models/__init__.py @@ -16,6 +16,7 @@ # specific language governing permissions and limitations # under the License. """Airflow models""" +from airflow.models.auth import JwtToken from airflow.models.base import ID_LEN, Base from airflow.models.baseoperator import BaseOperator, BaseOperatorLink from airflow.models.connection import Connection From f0f63c092aaf21d3174a088037296e3cf8fcf79f Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 7 Apr 2021 16:18:46 +0100 Subject: [PATCH 34/53] fix jwttoken model test --- airflow/models/auth.py | 4 ++-- tests/models/test_auth.py | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/airflow/models/auth.py b/airflow/models/auth.py index cf16146516b25..1f88f17c425c3 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -17,7 +17,7 @@ from datetime import datetime from typing import Optional -from sqlalchemy import Boolean, Column, DateTime, Integer, String, func +from sqlalchemy import Boolean, Column, DateTime, Integer, String from airflow.models.base import Base from airflow.utils.log.logging_mixin import LoggingMixin @@ -53,7 +53,7 @@ class JwtToken(Base, LoggingMixin): revoked_by = Column(String(50)) date_revoked = Column(DateTime) expiry_delta = Column(Integer, nullable=False) - created_delta = Column(Integer, default=func.now()) + created_delta = Column(Integer, default=datetime.timestamp(datetime.now())) def __init__( self, diff --git a/tests/models/test_auth.py b/tests/models/test_auth.py index 23811487f8b83..7f20a4a7c8b4c 100644 --- a/tests/models/test_auth.py +++ b/tests/models/test_auth.py @@ -16,7 +16,6 @@ # under the License. import unittest -from datetime import timedelta from airflow.models.auth import JwtToken from airflow.utils.session import provide_session @@ -34,7 +33,7 @@ def delete_tokens(self, session=None): @provide_session def create_token(self, session=None): - token = JwtToken(jti="token", expiry_delta=timedelta(minutes=2)) + token = JwtToken(jti="token", expiry_delta=62939233) session.add(token) session.commit() return token From f4488f2b2939d983f692b20fb0f681d47d8020ae Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 7 Apr 2021 18:43:27 +0100 Subject: [PATCH 35/53] Fix migration --- ....py => 09dd4b177b92_add_jwt_token_table.py} | 18 +++++++++--------- airflow/models/auth.py | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) rename airflow/migrations/versions/{43c2bf7117bd_add_jwttoken_table.py => 09dd4b177b92_add_jwt_token_table.py} (84%) diff --git a/airflow/migrations/versions/43c2bf7117bd_add_jwttoken_table.py b/airflow/migrations/versions/09dd4b177b92_add_jwt_token_table.py similarity index 84% rename from airflow/migrations/versions/43c2bf7117bd_add_jwttoken_table.py rename to airflow/migrations/versions/09dd4b177b92_add_jwt_token_table.py index e441073d26ac4..def1e8f1e78c4 100644 --- a/airflow/migrations/versions/43c2bf7117bd_add_jwttoken_table.py +++ b/airflow/migrations/versions/09dd4b177b92_add_jwt_token_table.py @@ -16,11 +16,11 @@ # specific language governing permissions and limitations # under the License. -"""Add token blocklist table +"""add jwt token table -Revision ID: 43c2bf7117bd -Revises: 2e42bb497a22 -Create Date: 2021-04-05 13:04:23.339826 +Revision ID: 09dd4b177b92 +Revises: 90d1635d7b86 +Create Date: 2021-04-07 17:06:42.061407 """ @@ -28,14 +28,14 @@ from alembic import op # revision identifiers, used by Alembic. -revision = '43c2bf7117bd' -down_revision = '2e42bb497a22' +revision = '09dd4b177b92' +down_revision = '90d1635d7b86' branch_labels = None depends_on = None def upgrade(): - """Apply Add token blocklist table""" + """Apply Add jwt token table""" op.create_table( "jwt_token", sa.Column("id", sa.Integer(), primary_key=True), @@ -51,5 +51,5 @@ def upgrade(): def downgrade(): # noqa: D103 - """Unapply Add token blocklist table""" - op.drop_table('tokens') + """Unapply Add jwt token table""" + op.drop_table('jwt_token') diff --git a/airflow/models/auth.py b/airflow/models/auth.py index 1f88f17c425c3..081e259307155 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -53,7 +53,7 @@ class JwtToken(Base, LoggingMixin): revoked_by = Column(String(50)) date_revoked = Column(DateTime) expiry_delta = Column(Integer, nullable=False) - created_delta = Column(Integer, default=datetime.timestamp(datetime.now())) + created_delta = Column(Integer, nullable=False, default=datetime.timestamp(datetime.now())) def __init__( self, From b5fe8aa5016ac8a0ddd0c217e012deca01c398ba Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 12 Apr 2021 00:21:35 +0100 Subject: [PATCH 36/53] Do not save oauth_token in session --- airflow/api_connexion/endpoints/auth_endpoint.py | 1 - tests/api_connexion/endpoints/test_auth_endpoint.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 70fe97fa2319c..e91303c969ffb 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -136,7 +136,6 @@ def authorize_oauth(provider, state): raise Unauthenticated(detail="State signature is not valid!") # Retrieves specific user info from the provider try: - appbuilder.sm.set_oauth_session(provider, resp) userinfo = appbuilder.sm.oauth_user_info(provider, resp) except Exception as e: # pylint: disable=broad-except log.error("Error returning OAuth user info: %s", e) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index ad0bb79d5e285..ef0312294df81 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -285,7 +285,7 @@ def test_successful_authorization(self, mock_jwt_decode): mock_authorized = mock.MagicMock() mock_twitter_auth_provider.authorize_access_token.return_value = mock_authorized self.client.get('api/v1/oauth-authorized/twitter?state=state') - mock_oauth_session.assert_called_once_with('twitter', mock_authorized) + mock_oauth_session.assert_not_called() mock_user_info.assert_called_once_with('twitter', mock_authorized) mock_user_oauth.assert_called_once_with(mock_user_info.return_value) From 8009eeafb1ca62aad532d1f7386c992f0c2471e6 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 14 Apr 2021 10:37:33 +0100 Subject: [PATCH 37/53] Use token blocklist and add necessary endpoints --- .pre-commit-config.yaml | 1 + .../api_connexion/endpoints/auth_endpoint.py | 124 +++++++++--------- airflow/api_connexion/openapi/v1.yaml | 55 +++++--- airflow/api_connexion/schemas/auth_schema.py | 5 +- airflow/api_connexion/security.py | 73 ++--------- .../09dd4b177b92_add_jwt_token_table.py | 14 +- airflow/models/__init__.py | 2 +- airflow/models/auth.py | 63 +++------ airflow/www/security.py | 31 +++-- .../endpoints/test_auth_endpoint.py | 59 +++++---- tests/models/test_auth.py | 35 +++-- 11 files changed, 196 insertions(+), 266 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b68511b7d43b2..f57fc6e6fe1f5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -315,6 +315,7 @@ repos: pass_filenames: true exclude: > (?x) + ^airflow/api_connexion/security.py$| ^airflow/providers/apache/cassandra/hooks/cassandra.py$| ^airflow/providers/apache/hive/operators/hive_stats.py$| ^airflow/providers/apache/hive/.*PROVIDER_CHANGES_*| diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index e91303c969ffb..d812fae351024 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -14,27 +14,25 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. - import logging -from datetime import datetime import jwt from flask import current_app, g, jsonify, request, session as c_session from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER -from flask_jwt_extended import create_access_token, decode_token, get_jti, get_jwt_identity -from flask_login import current_user, login_user +from flask_jwt_extended import ( + create_access_token, + decode_token, + get_jwt_identity, + get_raw_jwt, + jwt_refresh_token_required, + jwt_required, +) +from flask_login import login_user from marshmallow import ValidationError -from airflow.api_connexion.exceptions import BadRequest, NotFound, Unauthenticated -from airflow.api_connexion.schemas.auth_schema import ( - info_schema, - login_form_schema, - logout_schema, - token_schema, -) -from airflow.api_connexion.security import jwt_refresh_token_required_ -from airflow.models.auth import JwtToken -from airflow.utils.session import provide_session +from airflow.api_connexion.exceptions import BadRequest, Unauthenticated +from airflow.api_connexion.schemas.auth_schema import info_schema, login_form_schema, token_schema +from airflow.models.auth import TokenBlockList log = logging.getLogger(__name__) @@ -64,21 +62,13 @@ def get_auth_info(): def auth_login(): """Handle DB login""" - security_manager = current_app.appbuilder.sm - auth_type = security_manager.auth_type - if g.user is not None and g.user.is_authenticated: - raise Unauthenticated(detail="Client already authenticated") # For security - if auth_type not in (AUTH_DB, AUTH_LDAP): - raise Unauthenticated(detail="Authentication type do not match") body = request.json try: data = login_form_schema.load(body) except ValidationError as err: raise Unauthenticated(detail=str(err.messages)) - if auth_type == AUTH_DB: - user = security_manager.auth_user_db(data['username'], data['password']) - else: - user = security_manager.auth_user_ldap(data['username'], data['password']) + security_manager = current_app.appbuilder.sm + user = security_manager.api_login_with_username_and_password(data['username'], data['password']) if not user: raise Unauthenticated(detail="Invalid login") login_user(user, remember=False) @@ -169,56 +159,60 @@ def auth_remoteuser(): return appbuilder.sm.create_tokens_and_dump(user) -@jwt_refresh_token_required_ -@provide_session -def refresh_token(session=None): +@jwt_refresh_token_required +def refresh_token(): """Refresh token""" user = get_jwt_identity() access_token = create_access_token(identity=user) - decoded = decode_token(access_token) - token = JwtToken(jti=decoded['jti'], expiry_delta=decoded['exp'], created_delta=decoded['iat']) - session.add(token) - session.commit() ret = {'access_token': access_token} return jsonify(ret), 200 -def logout(): - """Sign out""" - body = request.json - try: - data = logout_schema.load(body) - except ValidationError as err: - raise BadRequest(detail=str(err.messages)) - token_jti = get_jti(data['token']) - exist = JwtToken.get_token(token_jti) - if exist: - JwtToken.delete_token(token_jti) - refresh_jti = get_jti(data['refresh_token']) - exist = JwtToken.get_token(refresh_jti) - if exist: - JwtToken.delete_token(refresh_jti) - resp = {"logged_out": True} - return jsonify(resp), 200 - - -@provide_session -def revoke_token(session=None): - """Revoke a token""" +@jwt_required +def revoke_token(): + """ + An endpoint for revoking both access and refresh token. + + This is intended for a case where a logged in user want to revoke + another user's tokens + """ + resp = jsonify({"revoked": True}) body = request.json try: data = token_schema.load(body) except ValidationError as err: raise BadRequest(detail=str(err.messages)) - jti = get_jti(data['token']) - token = JwtToken.get_token(jti) - if token: - token.is_revoked = True - token.revoked_by = current_user.username - token.revoke_reason = data['reason'] - token.date_revoked = datetime.now() - session.merge(token) - session.commit() - resp = jsonify({"revoked": True}) - return resp - raise NotFound(detail="Token not found") + token = decode_token(data['token']) + tkn = TokenBlockList.get_token(token['jti']) + + if not tkn: + TokenBlockList.add_token(jti=token['jti'], expiry_delta=token['exp']) + return resp + + +@jwt_required +def revoke_current_user_token(): + """ + An endpoint for revoking current user access token + This should be called during current user logout + """ + resp = jsonify({"revoked": True}) + raw_jwt = get_raw_jwt() + token = TokenBlockList.get_token(raw_jwt['jti']) + if not token: + TokenBlockList.add_token(jti=raw_jwt['jti'], expiry_delta=raw_jwt['exp']) + return resp + + +@jwt_refresh_token_required +def revoke_current_user_refresh_token(): + """ + An endpoint for revoking current user refresh token + This should be called during current user logout + """ + resp = jsonify({"revoked": True}) + raw_jwt = get_raw_jwt() + token = TokenBlockList.get_token(raw_jwt['jti']) + if not token: + TokenBlockList.add_token(jti=raw_jwt['jti'], expiry_delta=raw_jwt['exp']) + return resp diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index af25188e7f433..d7c0e498557f3 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1684,19 +1684,15 @@ paths: '401': $ref: '#/components/responses/Unauthenticated' - /logout: - post: - summary: Webserver logout - description: Logout user by unsetting cookies + /revoke_access_token: + get: + summary: Revoke current user access token + description: | + An endpoint for revoking current user access token + This should be called during current user logout x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint - operationId: logout + operationId: revoke_current_user_token tags: [Authentication] - requestBody: - required: true - content: - application/json: - schema: - $ref: '#/components/schemas/AccessRefresh' responses: '200': description: Success @@ -1705,7 +1701,30 @@ paths: schema: type: object properties: - logged_out: + revoked: + type: boolean + description: Indicate user is logged out + '401': + $ref: '#/components/responses/Unauthenticated' + + /revoke_refresh_token: + get: + summary: Revoke current user refresh token + description: | + An endpoint for revoking current user access token + This should be called during current user logout + x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint + operationId: revoke_current_user_refresh_token + tags: [Authentication] + responses: + '200': + description: Success + content: + application/json: + schema: + type: object + properties: + revoked: type: boolean description: Indicate user is logged out '401': @@ -1713,8 +1732,9 @@ paths: /revoke: post: - summary: Revoke a token - description: Revoke a refresh or access token + summary: Revoke access and refresh token + description: | + An endpoint for revoking both access and refresh token. x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint operationId: revoke_token tags: [Authentication] @@ -1737,6 +1757,10 @@ paths: description: Indicate token is revoked '404': $ref: '#/components/responses/NotFound' + '401': + $ref: '#/components/responses/Unauthenticated' + '400': + $ref: '#/components/responses/BadRequest' components: # Reusable schemas (data models) @@ -2741,9 +2765,6 @@ components: token: type: string description: The token to be revoked - reason: - type: string - description: Reason for the revoke # Configuration ConfigOption: diff --git a/airflow/api_connexion/schemas/auth_schema.py b/airflow/api_connexion/schemas/auth_schema.py index cfc4b2cf76a1e..8836220745e2b 100644 --- a/airflow/api_connexion/schemas/auth_schema.py +++ b/airflow/api_connexion/schemas/auth_schema.py @@ -66,10 +66,9 @@ class LogoutSchema(Schema): class RevokeTokenSchema(Schema): - """Schema to revoke token""" + """Schema to revoke tokens""" - token = fields.String() - reason = fields.String() + token = fields.String(required=True) info_schema = InfoSchema() diff --git a/airflow/api_connexion/security.py b/airflow/api_connexion/security.py index 92bd00d208b9b..86f209b1e038b 100644 --- a/airflow/api_connexion/security.py +++ b/airflow/api_connexion/security.py @@ -14,78 +14,23 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from datetime import datetime from functools import wraps from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast -from flask import Response, _app_ctx_stack as ctx_stack, current_app, request -from flask_jwt_extended.config import config -from flask_jwt_extended.utils import verify_token_claims -from flask_jwt_extended.view_decorators import _decode_jwt_from_request, _load_user +from flask import Response, current_app +from flask_jwt_extended import verify_jwt_in_request from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated -from airflow.models.auth import JwtToken +from airflow.models import TokenBlockList T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name -# Because we are using flask_jwt_extended <4.0, the two functions here have -# to be created due to language matters. When we upgrade to >=4.0, we would -# not need these functions, rather we would use token_in_blocklist callback - -def verify_jwt_access_token_(): - """Verify JWT in request""" - if request.method not in config.exempt_methods: - jwt_data, jwt_header = _decode_jwt_from_request(request_type='access') - ctx_stack.top.jwt = jwt_data - ctx_stack.top.jwt_header = jwt_header - verify_token_claims(jwt_data) - _load_user(jwt_data[config.identity_claim_key]) - token = JwtToken.get_token(jwt_data['jti']) - if token and token.is_revoked: - raise Unauthenticated(detail="Token revoked") - exp = datetime.timestamp(datetime.now()) - if token and token.expiry_delta < exp: - JwtToken.delete_token(token.jti) - raise Unauthenticated(detail="Token expired and we have deleted it") - if not token: - raise Unauthenticated(detail="Token Unknown") - - -def verify_jwt_refresh_token_in_request_(): - """ - Ensure that the requester has a valid refresh token. Raises an appropiate - exception if there is no token or the token is invalid. - """ - if request.method not in config.exempt_methods: - jwt_data, jwt_header = _decode_jwt_from_request(request_type='refresh') - ctx_stack.top.jwt = jwt_data - ctx_stack.top.jwt_header = jwt_header - _load_user(jwt_data[config.identity_claim_key]) - token = JwtToken.get_token(jwt_data['jti']) - if token and token.is_revoked: - raise Unauthenticated(detail="Token revoked") - exp = datetime.timestamp(datetime.now()) - if token and token.expiry_delta < exp: - JwtToken.delete_token(token.jti) - raise Unauthenticated(detail="Token expired and we have deleted it") - if not token: - raise Unauthenticated(detail="Token Unknown") - - -def jwt_refresh_token_required_(fn): - """ - A decorator to protect a an endpoint. - If you decorate an endpoint with this, it will ensure that the requester - has a valid refresh token before allowing the endpoint to be called. - """ - - @wraps(fn) - def wrapper(*args, **kwargs): - verify_jwt_refresh_token_in_request_() - return fn(*args, **kwargs) - - return wrapper +@current_app.appbuilder.sm.jwt_manager.token_in_blacklist_loader +def check_if_token_in_blacklist(decrypted_token): + """Checks if there's a blocked token""" + jti = decrypted_token['jti'] + return TokenBlockList.get_token(jti) is not None def check_authentication() -> None: @@ -94,7 +39,7 @@ def check_authentication() -> None: if response.status_code == 200: return try: - verify_jwt_access_token_() + verify_jwt_in_request() return except Exception: # pylint: disable=broad-except pass diff --git a/airflow/migrations/versions/09dd4b177b92_add_jwt_token_table.py b/airflow/migrations/versions/09dd4b177b92_add_jwt_token_table.py index def1e8f1e78c4..bb6b172bb9ba4 100644 --- a/airflow/migrations/versions/09dd4b177b92_add_jwt_token_table.py +++ b/airflow/migrations/versions/09dd4b177b92_add_jwt_token_table.py @@ -37,19 +37,13 @@ def upgrade(): """Apply Add jwt token table""" op.create_table( - "jwt_token", + "token_blocklist", sa.Column("id", sa.Integer(), primary_key=True), - sa.Column("jti", sa.String(50), nullable=False), - sa.Column("is_revoked", sa.Boolean(name="is_revoked"), server_default="0"), - sa.Column("refresh", sa.Boolean(name="refresh"), server_default="0"), - sa.Column("revoke_reason", sa.String(100)), - sa.Column("revoked_by", sa.String(50)), - sa.Column("date_revoked", sa.DateTime), - sa.Column("expiry_delta", sa.Integer, nullable=False), - sa.Column("created_delta", sa.Integer, nullable=False), + sa.Column("jti", sa.String(50), nullable=False, unique=True), + sa.Column("expiry_date", sa.DateTime, nullable=True, index=True), ) def downgrade(): # noqa: D103 """Unapply Add jwt token table""" - op.drop_table('jwt_token') + op.drop_table('token_blocklist') diff --git a/airflow/models/__init__.py b/airflow/models/__init__.py index 493744a3f42f6..e6650e03ffa85 100644 --- a/airflow/models/__init__.py +++ b/airflow/models/__init__.py @@ -16,7 +16,7 @@ # specific language governing permissions and limitations # under the License. """Airflow models""" -from airflow.models.auth import JwtToken +from airflow.models.auth import TokenBlockList from airflow.models.base import ID_LEN, Base from airflow.models.baseoperator import BaseOperator, BaseOperatorLink from airflow.models.connection import Connection diff --git a/airflow/models/auth.py b/airflow/models/auth.py index 081e259307155..7886b960bc013 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -14,67 +14,34 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from datetime import datetime -from typing import Optional -from sqlalchemy import Boolean, Column, DateTime, Integer, String +from pendulum import from_timestamp +from sqlalchemy import Column, DateTime, Integer, String from airflow.models.base import Base from airflow.utils.log.logging_mixin import LoggingMixin from airflow.utils.session import provide_session -class JwtToken(Base, LoggingMixin): +class TokenBlockList(Base, LoggingMixin): """ - A model for recording decoded token information + A model for recording blocked token :param jti: The token jti(JWT ID) :type jti: str - :param refresh: Whether the token is a refresh token or not - :type refresh: bool - :param is_revoked: Whether the token is revoked - :type is_revoked: bool - :param revoked_by: The username of the user who revoked the token - :type revoked_by: str - :param date_revoked: The date the token was revoked - :type date_revoked: datetime - :param expiry_delta: The timestamp of when the token will expire. Token exp - :type expiry_delta: int - :param created_delta: The timestamp of when the token was created. The token's iat - :type created_delta: int + :param expiry_date: When the token would expire + :type expiry_date: DateTime """ - __tablename__ = 'jwt_token' + __tablename__ = 'token_blocklist' id = Column(Integer, primary_key=True) - jti = Column(String(50), nullable=False) - refresh = Column(Boolean(name="refresh"), default=False) - is_revoked = Column(Boolean(name='revoked'), default=False) - revoke_reason = Column(String(100)) - revoked_by = Column(String(50)) - date_revoked = Column(DateTime) - expiry_delta = Column(Integer, nullable=False) - created_delta = Column(Integer, nullable=False, default=datetime.timestamp(datetime.now())) + jti = Column(String(50), unique=True, nullable=False) + expiry_date = Column(DateTime, nullable=False, index=True) - def __init__( - self, - jti: str, - expiry_delta: int, - refresh: Optional[bool] = False, - is_revoked: Optional[bool] = False, - revoked_reason: Optional[str] = None, - revoked_by: Optional[str] = None, - date_revoked: Optional[datetime] = None, - created_delta: Optional[int] = None, - ): + def __init__(self, jti: str, expiry_date: DateTime): super().__init__() self.jti = jti - self.expiry_delta = expiry_delta - self.refresh = refresh - self.is_revoked = is_revoked - self.revoke_reason = revoked_reason - self.revoked_by = revoked_by - self.date_revoked = date_revoked - self.created_delta = created_delta + self.expiry_date = expiry_date @classmethod @provide_session @@ -91,3 +58,11 @@ def delete_token(cls, token, session=None): if tkn: session.delete(tkn) session.commit() + + @classmethod + @provide_session + def add_token(cls, jti, expiry_delta, session=None): + """Add a token to blocklist""" + token = cls(jti=jti, expiry_date=from_timestamp(expiry_delta)) + session.add(token) + session.commit() diff --git a/airflow/www/security.py b/airflow/www/security.py index 0952fdfa5a9dc..f4681a1dd74d4 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -22,13 +22,15 @@ from typing import Dict, Optional, Sequence, Set, Tuple from flask import current_app, g +from flask_appbuilder.const import AUTH_DB, AUTH_LDAP 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 JWTManager, create_access_token, create_refresh_token, decode_token +from flask_jwt_extended import JWTManager, create_access_token, create_refresh_token from sqlalchemy import or_ from sqlalchemy.orm import joinedload +from airflow.api_connexion.exceptions import Unauthenticated from airflow.api_connexion.schemas.auth_schema import auth_schema from airflow.exceptions import AirflowException from airflow.models import DagBag, DagModel @@ -734,6 +736,19 @@ def check_authorization( return True + def api_login_with_username_and_password(self, username, password): + """Convenience method for user login through the API""" + if g.user is not None and g.user.is_authenticated: + raise Unauthenticated(detail="Client already authenticated") # For security + if self.auth_type not in (AUTH_DB, AUTH_LDAP): + raise Unauthenticated(detail="Authentication type do not match") + user = None + if self.auth_type == AUTH_DB: + user = self.auth_user_db(username, password) + elif self.auth_type == AUTH_LDAP: + user = self.auth_user_ldap(username, password) + return user + def create_jwt_manager(self, app) -> JWTManager: """JWT Manager""" jwt_manager = JWTManager() @@ -743,22 +758,10 @@ def create_jwt_manager(self, app) -> JWTManager: jwt_manager.user_loader_callback_loader(self.load_user_jwt) return jwt_manager - @provide_session - def create_tokens_and_dump(self, user, session=None): + def create_tokens_and_dump(self, user): """Creates access token, return user data alongside tokens""" access_token = create_access_token(user.id) - decoded = decode_token(access_token) - token = JwtToken(jti=decoded['jti'], expiry_delta=decoded['exp'], created_delta=decoded['iat']) refresh_token = create_refresh_token(user.id) - decoded = decode_token(refresh_token) - r_token = JwtToken( - jti=decoded['jti'], - expiry_delta=decoded['exp'], - created_delta=decoded['iat'], - refresh=True, - ) - session.add_all([token, r_token]) - session.commit() return auth_schema.dump({'user': user, 'token': access_token, 'refresh_token': refresh_token}) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index ef0312294df81..0fe6485cfcec5 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -20,7 +20,7 @@ from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER from sqlalchemy import func -from airflow.models.auth import JwtToken +from airflow.models.auth import TokenBlockList from airflow.utils.session import provide_session from tests.test_utils.api_connexion_utils import create_user, delete_user @@ -77,7 +77,7 @@ def auth_type(self, auth): self.app.config['OAUTH_PROVIDERS'] = None def teardown_method(self): - tokens = self.session.query(JwtToken).all() + tokens = self.session.query(TokenBlockList).all() for token in tokens: self.session.delete(token) self.session.commit() @@ -319,22 +319,14 @@ def test_incorrect_auth_type_raises(self): class TestRefreshTokenEndpoint(TestLoginEndpoint): - @provide_session - def test_creates_access_token(self, session=None): + def test_creates_access_token(self): self.auth_type(AUTH_DB) payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth/login', json=payload) - token = response.json['token'] refresh = response.json['refresh_token'] - total_tokens_in_db = session.query(func.count(JwtToken.id)).scalar() - - assert total_tokens_in_db == 2 - assert token is not None response2 = self.client.post("api/v1/refresh", headers={"Authorization": f"Bearer {refresh}"}) assert response2.json['access_token'] is not None - total_tokens_in_db = session.query(func.count(JwtToken.id)).scalar() - assert total_tokens_in_db == 3 def test_access_token_cant_access_endpoint(self): self.auth_type(AUTH_DB) @@ -346,21 +338,34 @@ def test_access_token_cant_access_endpoint(self): assert response2.json['msg'] == 'Only refresh tokens are allowed' -class TestLogoutEndpoint(TestLoginEndpoint): +class TestRevokeAccessTokenEndpoint(TestLoginEndpoint): @provide_session - def test_logout_works(self, session): + def test_revoke_current_user_access_token_works(self, session): self.auth_type(AUTH_DB) payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth/login', json=payload) token = response.json['token'] - refresh = response.json['refresh_token'] - total_tokens_in_db = session.query(func.count(JwtToken.id)).scalar() - assert total_tokens_in_db == 2 - assert token is not None - response2 = self.client.post("api/v1/logout", json={"token": token, "refresh_token": refresh}) - assert response2.json == {'logged_out': True} - total_tokens_in_db = session.query(func.count(JwtToken.id)).scalar() - assert total_tokens_in_db == 0 + response2 = self.client.get( + "api/v1/revoke_access_token", headers={"Authorization": f"Bearer {token}"} + ) + assert response2.json == {'revoked': True} + total_tokens_in_db = session.query(func.count(TokenBlockList.id)).scalar() + assert total_tokens_in_db == 1 + + +class TestRevokeRefreshTokenEndpoint(TestLoginEndpoint): + @provide_session + def test_revoke_current_user_access_token_works(self, session): + self.auth_type(AUTH_DB) + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth/login', json=payload) + refresh_token = response.json['refresh_token'] + response2 = self.client.get( + "api/v1/revoke_refresh_token", headers={"Authorization": f"Bearer {refresh_token}"} + ) + assert response2.json == {'revoked': True} + total_tokens_in_db = session.query(func.count(TokenBlockList.id)).scalar() + assert total_tokens_in_db == 1 class TestTokenRevokeEndpoint(TestLoginEndpoint): @@ -371,10 +376,8 @@ def test_revoke_token_works(self, session): response = self.client.post('api/v1/auth/login', json=payload) token = response.json['token'] refresh = response.json['refresh_token'] - total_tokens_in_db = session.query(func.count(JwtToken.id)).scalar() - assert total_tokens_in_db == 2 - self.client.post("api/v1/revoke", json={"token": token, "reason": "test"}) - self.client.post("api/v1/revoke", json={"token": refresh, "reason": "test"}) - tokens = session.query(JwtToken).all() - for token in tokens: - assert token.is_revoked is True + self.client.post( + "api/v1/revoke", json={"token": refresh}, headers={"Authorization": f"Bearer {token}"} + ) + total_tokens_in_db = session.query(func.count(TokenBlockList.id)).scalar() + assert total_tokens_in_db == 1 diff --git a/tests/models/test_auth.py b/tests/models/test_auth.py index 7f20a4a7c8b4c..7937330a680f8 100644 --- a/tests/models/test_auth.py +++ b/tests/models/test_auth.py @@ -16,10 +16,13 @@ # under the License. import unittest +from datetime import datetime -from airflow.models.auth import JwtToken +from airflow.models.auth import TokenBlockList from airflow.utils.session import provide_session +EXPIRY_DELTA = datetime.timestamp(datetime.now()) + class TestToken(unittest.TestCase): def tearDown(self) -> None: @@ -27,33 +30,25 @@ def tearDown(self) -> None: @provide_session def delete_tokens(self, session=None): - tokens = session.query(JwtToken).all() + tokens = session.query(TokenBlockList).all() for i in tokens: session.delete(i) @provide_session - def create_token(self, session=None): - token = JwtToken(jti="token", expiry_delta=62939233) - session.add(token) - session.commit() - return token - - @provide_session - def test_create_token(self, session=None): - self.create_token() - token = session.query(JwtToken).all() + def test_add_token(self, session=None): + TokenBlockList.add_token(jti="token", expiry_delta=EXPIRY_DELTA) + token = session.query(TokenBlockList).all() assert len(token) == 1 def test_get_token_method(self): - token = self.create_token() - token2 = JwtToken.get_token(token.jti) - assert token2.jti == token.jti - assert token2.expiry_delta == token.expiry_delta + TokenBlockList.add_token(jti="token", expiry_delta=EXPIRY_DELTA) + token2 = TokenBlockList.get_token("token") + assert token2.jti == "token" def test_delete_token_method(self): - tkn = self.create_token() - token = JwtToken.get_token(tkn.jti) + TokenBlockList.add_token(jti="token", expiry_delta=EXPIRY_DELTA) + token = TokenBlockList.get_token("token") assert token is not None - JwtToken.delete_token(token.jti) - token = JwtToken.get_token(token.jti) + TokenBlockList.delete_token("token") + token = TokenBlockList.get_token("token") assert token is None From b6c2b260ad9a540be89ffe8ebcb868facc99dfb2 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Sat, 17 Apr 2021 12:11:51 +0100 Subject: [PATCH 38/53] Move some common code to security manager --- .../api_connexion/endpoints/auth_endpoint.py | 76 ++--------------- airflow/www/security.py | 81 ++++++++++++++++++- 2 files changed, 85 insertions(+), 72 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index d812fae351024..50f6d9707a9de 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -16,8 +16,7 @@ # under the License. import logging -import jwt -from flask import current_app, g, jsonify, request, session as c_session +from flask import current_app, jsonify, request, session as c_session from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER from flask_jwt_extended import ( create_access_token, @@ -68,7 +67,7 @@ def auth_login(): except ValidationError as err: raise Unauthenticated(detail=str(err.messages)) security_manager = current_app.appbuilder.sm - user = security_manager.api_login_with_username_and_password(data['username'], data['password']) + user = security_manager.login_with_user_pass(data['username'], data['password']) if not user: raise Unauthenticated(detail="Invalid login") login_user(user, remember=False) @@ -76,67 +75,17 @@ def auth_login(): def auth_oauthlogin(provider, register=None, redirect_url=None): - """Handle Oauth login""" + """Returns OAUTH authorization url""" appbuilder = current_app.appbuilder - if g.user is not None and g.user.is_authenticated: - raise Unauthenticated(detail="Client already authenticated") - if appbuilder.sm.auth_type != AUTH_OAUTH: - raise Unauthenticated(detail="Authentication type do not match") - state = jwt.encode( - request.args.to_dict(flat=False), - appbuilder.app.config["SECRET_KEY"], - algorithm="HS256", - ) - try: - auth_provider = appbuilder.sm.oauth_remotes[provider] - if register: - c_session["register"] = True - if provider == "twitter": - redirect_uri = redirect_url + f"&state={state}" - auth_data = auth_provider.create_authorization_url(redirect_uri=redirect_uri) - auth_provider.save_authorize_data(request, redirect_uri=redirect_uri, **auth_data) - return dict(auth_url=auth_data['url']) - else: - state = state.decode("ascii") if isinstance(state, bytes) else state - auth_data = auth_provider.create_authorization_url( - redirect_uri=redirect_url, - state=state, - ) - auth_provider.save_authorize_data(request, redirect_uri=redirect_url, **auth_data) - return dict(auth_url=auth_data['url']) - except Exception as err: # pylint: disable=broad-except - raise Unauthenticated(detail=str(err)) + if register: + c_session["register"] = True + return appbuilder.sm.oauth_authorization_url(appbuilder.app, provider, redirect_url) def authorize_oauth(provider, state): """Callback to authorize Oauth.""" appbuilder = current_app.appbuilder - resp = appbuilder.sm.oauth_remotes[provider].authorize_access_token() - if resp is None: - raise Unauthenticated(detail="You denied the request to sign in") - log.debug("OAUTH Authorized resp: %s", resp) - # Verify state - try: - jwt.decode( - state, - appbuilder.app.config["SECRET_KEY"], - algorithms=["HS256"], - ) - except jwt.InvalidTokenError: - raise Unauthenticated(detail="State signature is not valid!") - # Retrieves specific user info from the provider - try: - userinfo = appbuilder.sm.oauth_user_info(provider, resp) - except Exception as e: # pylint: disable=broad-except - log.error("Error returning OAuth user info: %s", e) - user = None - else: - log.debug("User info retrieved from %s: %s", provider, userinfo) - user = appbuilder.sm.auth_user_oauth(userinfo) - - if user is None: - raise Unauthenticated(detail="Invalid login") - login_user(user) + user = appbuilder.sm.oauth_login_user(appbuilder.app, provider, state) return appbuilder.sm.create_tokens_and_dump(user) @@ -144,16 +93,8 @@ def auth_remoteuser(): """Handle remote user auth""" appbuilder = current_app.appbuilder username = request.environ.get("REMOTE_USER") - if g.user is not None and g.user.is_authenticated: - raise Unauthenticated(detail="Client already authenticated") - if appbuilder.sm.auth_type != AUTH_REMOTE_USER: - raise Unauthenticated(detail="Authentication type do not match") if username: - user = appbuilder.sm.auth_user_remote_user(username) - if user is None: - raise Unauthenticated(detail="Invalid login") - else: - login_user(user) + user = appbuilder.sm.login_remote_user(username) else: raise Unauthenticated(detail="Invalid login") return appbuilder.sm.create_tokens_and_dump(user) @@ -184,7 +125,6 @@ def revoke_token(): raise BadRequest(detail=str(err.messages)) token = decode_token(data['token']) tkn = TokenBlockList.get_token(token['jti']) - if not tkn: TokenBlockList.add_token(jti=token['jti'], expiry_delta=token['exp']) return resp diff --git a/airflow/www/security.py b/airflow/www/security.py index f4681a1dd74d4..4b890bd70faee 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -21,12 +21,14 @@ from datetime import timedelta from typing import Dict, Optional, Sequence, Set, Tuple -from flask import current_app, g -from flask_appbuilder.const import AUTH_DB, AUTH_LDAP +import jwt +from flask import current_app, g, request +from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_REMOTE_USER 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 JWTManager, create_access_token, create_refresh_token +from flask_login import login_user from sqlalchemy import or_ from sqlalchemy.orm import joinedload @@ -736,10 +738,15 @@ def check_authorization( return True - def api_login_with_username_and_password(self, username, password): - """Convenience method for user login through the API""" + # TODO: Whether to create APISecurityManager and move api related code to it? + def is_user_logged_in(self): + """Raise if user already logged in""" if g.user is not None and g.user.is_authenticated: raise Unauthenticated(detail="Client already authenticated") # For security + + def login_with_user_pass(self, username, password): + """Convenience method for user login through the API""" + self.is_user_logged_in() if self.auth_type not in (AUTH_DB, AUTH_LDAP): raise Unauthenticated(detail="Authentication type do not match") user = None @@ -749,6 +756,72 @@ def api_login_with_username_and_password(self, username, password): user = self.auth_user_ldap(username, password) return user + def oauth_authorization_url(self, app, provider, redirect_url): + """Get authorization url for oauth""" + self.is_user_logged_in() + if self.auth_type != AUTH_OAUTH: + raise Unauthenticated(detail="Authentication type do not match") + state = jwt.encode( + request.args.to_dict(flat=False), + app.config["SECRET_KEY"], + algorithm="HS256", + ) + auth_provider = self.oauth_remotes[provider] + try: + + if provider == "twitter": + redirect_uri = redirect_url + f"&state={state}" + auth_data = auth_provider.create_authorization_url(redirect_uri=redirect_uri) + auth_provider.save_authorize_data(request, redirect_uri=redirect_uri, **auth_data) + return dict(auth_url=auth_data['url']) + else: + state = state.decode("ascii") if isinstance(state, bytes) else state + auth_data = auth_provider.create_authorization_url( + redirect_uri=redirect_url, + state=state, + ) + auth_provider.save_authorize_data(request, redirect_uri=redirect_url, **auth_data) + return dict(auth_url=auth_data['url']) + except Exception as err: # pylint: disable=broad-except + raise Unauthenticated(detail=str(err)) + + def oauth_login_user(self, app, provider, state): + """Oauth login""" + resp = self.oauth_remotes[provider].authorize_access_token() + if resp is None: + raise Unauthenticated(detail="You denied the request to sign in") + # Verify state + try: + jwt.decode( + state, + app.config["SECRET_KEY"], + algorithms=["HS256"], + ) + except jwt.InvalidTokenError: + raise Unauthenticated(detail="State signature is not valid!") + # Retrieves specific user info from the provider + try: + userinfo = self.oauth_user_info(provider, resp) + except Exception: # pylint: disable=broad-except + user = None + else: + user = self.auth_user_oauth(userinfo) + if user is None: + raise Unauthenticated(detail="Invalid login") + login_user(user) + return user + + def login_remote_user(self, username): + """Login user using remote auth""" + self.is_user_logged_in() + if self.auth_type != AUTH_REMOTE_USER: + raise Unauthenticated(detail="Authentication type do not match") + user = self.auth_user_remote_user(username) + if user is None: + raise Unauthenticated(detail="Invalid login") + login_user(user) + return user + def create_jwt_manager(self, app) -> JWTManager: """JWT Manager""" jwt_manager = JWTManager() From 54fc35b6879735f3655e90bb743e421ac6a18bfe Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Sat, 17 Apr 2021 19:01:21 +0100 Subject: [PATCH 39/53] change token table migration and fix table --- ....py => 22ab4efd5674_add_token_blocklist.py} | 18 +++++++++--------- airflow/models/auth.py | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) rename airflow/migrations/versions/{09dd4b177b92_add_jwt_token_table.py => 22ab4efd5674_add_token_blocklist.py} (78%) diff --git a/airflow/migrations/versions/09dd4b177b92_add_jwt_token_table.py b/airflow/migrations/versions/22ab4efd5674_add_token_blocklist.py similarity index 78% rename from airflow/migrations/versions/09dd4b177b92_add_jwt_token_table.py rename to airflow/migrations/versions/22ab4efd5674_add_token_blocklist.py index bb6b172bb9ba4..1d94546f73e46 100644 --- a/airflow/migrations/versions/09dd4b177b92_add_jwt_token_table.py +++ b/airflow/migrations/versions/22ab4efd5674_add_token_blocklist.py @@ -16,11 +16,11 @@ # specific language governing permissions and limitations # under the License. -"""add jwt token table +"""add-token-blocklist -Revision ID: 09dd4b177b92 -Revises: 90d1635d7b86 -Create Date: 2021-04-07 17:06:42.061407 +Revision ID: 22ab4efd5674 +Revises: a13f7613ad25 +Create Date: 2021-04-17 18:16:31.019394 """ @@ -28,22 +28,22 @@ from alembic import op # revision identifiers, used by Alembic. -revision = '09dd4b177b92' -down_revision = '90d1635d7b86' +revision = '22ab4efd5674' +down_revision = 'a13f7613ad25' branch_labels = None depends_on = None def upgrade(): - """Apply Add jwt token table""" + """Apply Add token blocklist table""" op.create_table( "token_blocklist", sa.Column("id", sa.Integer(), primary_key=True), sa.Column("jti", sa.String(50), nullable=False, unique=True), - sa.Column("expiry_date", sa.DateTime, nullable=True, index=True), + sa.Column("expiry_date", sa.DateTime(), nullable=False, index=True), ) def downgrade(): # noqa: D103 - """Unapply Add jwt token table""" + """Unapply Add token blocklist table""" op.drop_table('token_blocklist') diff --git a/airflow/models/auth.py b/airflow/models/auth.py index 7886b960bc013..686af77f1b4e2 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -36,7 +36,7 @@ class TokenBlockList(Base, LoggingMixin): __tablename__ = 'token_blocklist' id = Column(Integer, primary_key=True) jti = Column(String(50), unique=True, nullable=False) - expiry_date = Column(DateTime, nullable=False, index=True) + expiry_date = Column(DateTime(), nullable=False, index=True) def __init__(self, jti: str, expiry_date: DateTime): super().__init__() From 924bea38ef1a2e93345b397ce52689b95ad4aa19 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 21 Apr 2021 08:23:25 +0100 Subject: [PATCH 40/53] Apply suggestions from code review --- .../api_connexion/endpoints/auth_endpoint.py | 28 +++++++++---------- .../22ab4efd5674_add_token_blocklist.py | 14 ++++++---- airflow/models/auth.py | 19 ++++--------- airflow/www/security.py | 10 +++---- 4 files changed, 33 insertions(+), 38 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 50f6d9707a9de..7893b67839e64 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -28,6 +28,7 @@ ) from flask_login import login_user from marshmallow import ValidationError +from sqlalchemy.exc import IntegrityError from airflow.api_connexion.exceptions import BadRequest, Unauthenticated from airflow.api_connexion.schemas.auth_schema import info_schema, login_form_schema, token_schema @@ -105,8 +106,7 @@ def refresh_token(): """Refresh token""" user = get_jwt_identity() access_token = create_access_token(identity=user) - ret = {'access_token': access_token} - return jsonify(ret), 200 + return {'access_token': access_token} @jwt_required @@ -117,17 +117,17 @@ def revoke_token(): This is intended for a case where a logged in user want to revoke another user's tokens """ - resp = jsonify({"revoked": True}) body = request.json try: data = token_schema.load(body) except ValidationError as err: raise BadRequest(detail=str(err.messages)) token = decode_token(data['token']) - tkn = TokenBlockList.get_token(token['jti']) - if not tkn: + try: TokenBlockList.add_token(jti=token['jti'], expiry_delta=token['exp']) - return resp + except IntegrityError: + pass + return jsonify({"revoked": True}) @jwt_required @@ -136,12 +136,12 @@ def revoke_current_user_token(): An endpoint for revoking current user access token This should be called during current user logout """ - resp = jsonify({"revoked": True}) raw_jwt = get_raw_jwt() - token = TokenBlockList.get_token(raw_jwt['jti']) - if not token: + try: TokenBlockList.add_token(jti=raw_jwt['jti'], expiry_delta=raw_jwt['exp']) - return resp + except IntegrityError: + pass + return jsonify({"revoked": True}) @jwt_refresh_token_required @@ -150,9 +150,9 @@ def revoke_current_user_refresh_token(): An endpoint for revoking current user refresh token This should be called during current user logout """ - resp = jsonify({"revoked": True}) raw_jwt = get_raw_jwt() - token = TokenBlockList.get_token(raw_jwt['jti']) - if not token: + try: TokenBlockList.add_token(jti=raw_jwt['jti'], expiry_delta=raw_jwt['exp']) - return resp + except IntegrityError: + pass + return jsonify({"revoked": True}) diff --git a/airflow/migrations/versions/22ab4efd5674_add_token_blocklist.py b/airflow/migrations/versions/22ab4efd5674_add_token_blocklist.py index 1d94546f73e46..9d190a9e39214 100644 --- a/airflow/migrations/versions/22ab4efd5674_add_token_blocklist.py +++ b/airflow/migrations/versions/22ab4efd5674_add_token_blocklist.py @@ -33,17 +33,21 @@ branch_labels = None depends_on = None +INDEX_NAME = "idx_expiry_date_token_blocklist" +TABLE_NAME = "token_blocklist" + def upgrade(): """Apply Add token blocklist table""" op.create_table( - "token_blocklist", - sa.Column("id", sa.Integer(), primary_key=True), - sa.Column("jti", sa.String(50), nullable=False, unique=True), - sa.Column("expiry_date", sa.DateTime(), nullable=False, index=True), + TABLE_NAME, + sa.Column("jti", sa.String(50), nullable=False, primary_key=True), + sa.Column("expiry_date", sa.DateTime(), nullable=False), ) + op.create_index(INDEX_NAME, TABLE_NAME, ['expiry_date'], unique=False) def downgrade(): # noqa: D103 """Unapply Add token blocklist table""" - op.drop_table('token_blocklist') + op.drop_index(INDEX_NAME, table_name=TABLE_NAME) + op.drop_table(TABLE_NAME) diff --git a/airflow/models/auth.py b/airflow/models/auth.py index 686af77f1b4e2..413754328e80a 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -16,7 +16,7 @@ # under the License. from pendulum import from_timestamp -from sqlalchemy import Column, DateTime, Integer, String +from sqlalchemy import Column, DateTime, String from airflow.models.base import Base from airflow.utils.log.logging_mixin import LoggingMixin @@ -34,35 +34,26 @@ class TokenBlockList(Base, LoggingMixin): """ __tablename__ = 'token_blocklist' - id = Column(Integer, primary_key=True) - jti = Column(String(50), unique=True, nullable=False) + jti = Column(String(50), nullable=False, primary_key=True) expiry_date = Column(DateTime(), nullable=False, index=True) - def __init__(self, jti: str, expiry_date: DateTime): - super().__init__() - self.jti = jti - self.expiry_date = expiry_date - @classmethod @provide_session def get_token(cls, token, session=None): """Get a token""" - tkn = session.query(cls).filter(cls.jti == token).first() - return tkn + return session.query(cls).get(token) @classmethod @provide_session def delete_token(cls, token, session=None): """Delete a token""" - tkn = session.query(cls).filter(cls.jti == token).first() - if tkn: - session.delete(tkn) - session.commit() + session.query(cls).get(token).delete() @classmethod @provide_session def add_token(cls, jti, expiry_delta, session=None): """Add a token to blocklist""" + # pylint: disable=unexpected-keyword-arg token = cls(jti=jti, expiry_date=from_timestamp(expiry_delta)) session.add(token) session.commit() diff --git a/airflow/www/security.py b/airflow/www/security.py index 4b890bd70faee..9d14b49503756 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -748,7 +748,7 @@ def login_with_user_pass(self, username, password): """Convenience method for user login through the API""" self.is_user_logged_in() if self.auth_type not in (AUTH_DB, AUTH_LDAP): - raise Unauthenticated(detail="Authentication type do not match") + raise Unauthenticated(detail="Authentication type does not match") user = None if self.auth_type == AUTH_DB: user = self.auth_user_db(username, password) @@ -760,7 +760,7 @@ def oauth_authorization_url(self, app, provider, redirect_url): """Get authorization url for oauth""" self.is_user_logged_in() if self.auth_type != AUTH_OAUTH: - raise Unauthenticated(detail="Authentication type do not match") + raise Unauthenticated(detail="Authentication type does not match") state = jwt.encode( request.args.to_dict(flat=False), app.config["SECRET_KEY"], @@ -815,7 +815,7 @@ def login_remote_user(self, username): """Login user using remote auth""" self.is_user_logged_in() if self.auth_type != AUTH_REMOTE_USER: - raise Unauthenticated(detail="Authentication type do not match") + raise Unauthenticated(detail="Authentication type does not match") user = self.auth_user_remote_user(username) if user is None: raise Unauthenticated(detail="Invalid login") @@ -823,7 +823,7 @@ def login_remote_user(self, username): return user def create_jwt_manager(self, app) -> JWTManager: - """JWT Manager""" + """Called by FAB for us when it wants a configured JWT manager""" jwt_manager = JWTManager() app.config["JWT_ACCESS_TOKEN_EXPIRES"] = timedelta(hours=1) app.config["JWT_REFRESH_TOKEN_EXPIRES"] = timedelta(days=30) @@ -832,7 +832,7 @@ def create_jwt_manager(self, app) -> JWTManager: return jwt_manager def create_tokens_and_dump(self, user): - """Creates access token, return user data alongside tokens""" + """Creates access and refresh token, return user data alongside tokens""" access_token = create_access_token(user.id) refresh_token = create_refresh_token(user.id) return auth_schema.dump({'user': user, 'token': access_token, 'refresh_token': refresh_token}) From a01cabd74290fd9dac0c731ebcce9dcea5ad0380 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 21 Apr 2021 08:38:01 +0100 Subject: [PATCH 41/53] Move auth_schema out of security manager and fix tests --- .../api_connexion/endpoints/auth_endpoint.py | 13 ++++++++---- airflow/www/security.py | 14 ++++++------- .../endpoints/test_auth_endpoint.py | 21 ++++++++++--------- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 7893b67839e64..033a7d1c061ad 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -31,7 +31,12 @@ from sqlalchemy.exc import IntegrityError from airflow.api_connexion.exceptions import BadRequest, Unauthenticated -from airflow.api_connexion.schemas.auth_schema import info_schema, login_form_schema, token_schema +from airflow.api_connexion.schemas.auth_schema import ( + auth_schema, + info_schema, + login_form_schema, + token_schema, +) from airflow.models.auth import TokenBlockList log = logging.getLogger(__name__) @@ -72,7 +77,7 @@ def auth_login(): if not user: raise Unauthenticated(detail="Invalid login") login_user(user, remember=False) - return security_manager.create_tokens_and_dump(user) + return security_manager.create_tokens_and_dump(user, auth_schema) def auth_oauthlogin(provider, register=None, redirect_url=None): @@ -87,7 +92,7 @@ def authorize_oauth(provider, state): """Callback to authorize Oauth.""" appbuilder = current_app.appbuilder user = appbuilder.sm.oauth_login_user(appbuilder.app, provider, state) - return appbuilder.sm.create_tokens_and_dump(user) + return appbuilder.sm.create_tokens_and_dump(user, auth_schema) def auth_remoteuser(): @@ -98,7 +103,7 @@ def auth_remoteuser(): user = appbuilder.sm.login_remote_user(username) else: raise Unauthenticated(detail="Invalid login") - return appbuilder.sm.create_tokens_and_dump(user) + return appbuilder.sm.create_tokens_and_dump(user, auth_schema) @jwt_refresh_token_required diff --git a/airflow/www/security.py b/airflow/www/security.py index 9d14b49503756..9111d5a21bc7c 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -33,10 +33,8 @@ from sqlalchemy.orm import joinedload from airflow.api_connexion.exceptions import Unauthenticated -from airflow.api_connexion.schemas.auth_schema import auth_schema from airflow.exceptions import AirflowException from airflow.models import DagBag, DagModel -from airflow.models.auth import JwtToken from airflow.security import permissions from airflow.utils.log.logging_mixin import LoggingMixin from airflow.utils.session import provide_session @@ -65,6 +63,8 @@ 'Public', } +AUTH_TYPE_MISMATCH_MESSAGE = "Authentication type does not match" + class AirflowSecurityManager(SecurityManager, LoggingMixin): # pylint: disable=too-many-public-methods """Custom security manager, which introduces a permission model adapted to Airflow""" @@ -748,7 +748,7 @@ def login_with_user_pass(self, username, password): """Convenience method for user login through the API""" self.is_user_logged_in() if self.auth_type not in (AUTH_DB, AUTH_LDAP): - raise Unauthenticated(detail="Authentication type does not match") + raise Unauthenticated(detail=AUTH_TYPE_MISMATCH_MESSAGE) user = None if self.auth_type == AUTH_DB: user = self.auth_user_db(username, password) @@ -760,7 +760,7 @@ def oauth_authorization_url(self, app, provider, redirect_url): """Get authorization url for oauth""" self.is_user_logged_in() if self.auth_type != AUTH_OAUTH: - raise Unauthenticated(detail="Authentication type does not match") + raise Unauthenticated(detail=AUTH_TYPE_MISMATCH_MESSAGE) state = jwt.encode( request.args.to_dict(flat=False), app.config["SECRET_KEY"], @@ -815,7 +815,7 @@ def login_remote_user(self, username): """Login user using remote auth""" self.is_user_logged_in() if self.auth_type != AUTH_REMOTE_USER: - raise Unauthenticated(detail="Authentication type does not match") + raise Unauthenticated(detail=AUTH_TYPE_MISMATCH_MESSAGE) user = self.auth_user_remote_user(username) if user is None: raise Unauthenticated(detail="Invalid login") @@ -831,11 +831,11 @@ def create_jwt_manager(self, app) -> JWTManager: jwt_manager.user_loader_callback_loader(self.load_user_jwt) return jwt_manager - def create_tokens_and_dump(self, user): + def create_tokens_and_dump(self, user, schema): """Creates access and refresh token, return user data alongside tokens""" access_token = create_access_token(user.id) refresh_token = create_refresh_token(user.id) - return auth_schema.dump({'user': user, 'token': access_token, 'refresh_token': refresh_token}) + return schema.dump({'user': user, 'token': access_token, 'refresh_token': refresh_token}) class ApplessAirflowSecurityManager(AirflowSecurityManager): diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 0fe6485cfcec5..29d74403df464 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -22,6 +22,7 @@ from airflow.models.auth import TokenBlockList from airflow.utils.session import provide_session +from airflow.www.security import AUTH_TYPE_MISMATCH_MESSAGE from tests.test_utils.api_connexion_utils import create_user, delete_user OAUTH_PROVIDERS = [ @@ -155,7 +156,7 @@ def test_auth_type_must_be_db(self): payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth/login', json=payload) assert response.status_code == 401 - assert response.json['detail'] == 'Authentication type do not match' + assert response.json['detail'] == AUTH_TYPE_MISMATCH_MESSAGE class TestLDAPLoginEndpoint(TestLoginEndpoint): @@ -203,11 +204,11 @@ def test_auth_type_must_be_ldap(self): payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth/login', json=payload) assert response.status_code == 401 - assert response.json['detail'] == 'Authentication type do not match' + assert response.json['detail'] == AUTH_TYPE_MISMATCH_MESSAGE class TestOauthAuthorizationURLEndpoint(TestLoginEndpoint): - @mock.patch("airflow.api_connexion.endpoints.auth_endpoint.jwt.encode") + @mock.patch("airflow.www.security.jwt.encode") def test_can_generate_authorization_url(self, mock_jwt_encode): self.auth_type(AUTH_OAUTH) mock_jwt_encode.return_value = "state" @@ -221,7 +222,7 @@ def test_can_generate_authorization_url(self, mock_jwt_encode): self.client.get(f'api/v1/auth-oauth/google?register=True&redirect_url={redirect_url}') mock_auth.assert_called_once_with(redirect_uri=redirect_url, state="state") - @mock.patch("airflow.api_connexion.endpoints.auth_endpoint.jwt.encode") + @mock.patch("airflow.www.security.jwt.encode") def test_can_generate_authorization_url_for_twitter(self, mock_jwt_encode): self.auth_type(AUTH_OAUTH) mock_jwt_encode.return_value = "state" @@ -240,7 +241,7 @@ def test_incorrect_auth_type_raises(self): redirect_url = "http://localhost:8080" resp = self.client.get(f'api/v1/auth-oauth/google?register=True&redirect_url={redirect_url}') assert resp.status_code == 401 - assert resp.json['detail'] == "Authentication type do not match" + assert resp.json['detail'] == AUTH_TYPE_MISMATCH_MESSAGE class TestAuthorizeOauth(TestLoginEndpoint): @@ -266,7 +267,7 @@ def test_wrong_state_signature_raises(self): assert response.status_code == 401 assert response.json['detail'] == "State signature is not valid!" - @mock.patch("airflow.api_connexion.endpoints.auth_endpoint.jwt.decode") + @mock.patch("airflow.www.security.jwt.decode") def test_successful_authorization(self, mock_jwt_decode): self.auth_type(AUTH_OAUTH) mock_jwt_decode.return_value = {'some': 'payload'} @@ -315,7 +316,7 @@ def test_incorrect_auth_type_raises(self): self.auth_type(AUTH_DB) resp = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) assert resp.status_code == 401 - assert resp.json['detail'] == "Authentication type do not match" + assert resp.json['detail'] == AUTH_TYPE_MISMATCH_MESSAGE class TestRefreshTokenEndpoint(TestLoginEndpoint): @@ -349,7 +350,7 @@ def test_revoke_current_user_access_token_works(self, session): "api/v1/revoke_access_token", headers={"Authorization": f"Bearer {token}"} ) assert response2.json == {'revoked': True} - total_tokens_in_db = session.query(func.count(TokenBlockList.id)).scalar() + total_tokens_in_db = session.query(func.count(TokenBlockList.jti)).scalar() assert total_tokens_in_db == 1 @@ -364,7 +365,7 @@ def test_revoke_current_user_access_token_works(self, session): "api/v1/revoke_refresh_token", headers={"Authorization": f"Bearer {refresh_token}"} ) assert response2.json == {'revoked': True} - total_tokens_in_db = session.query(func.count(TokenBlockList.id)).scalar() + total_tokens_in_db = session.query(func.count(TokenBlockList.jti)).scalar() assert total_tokens_in_db == 1 @@ -379,5 +380,5 @@ def test_revoke_token_works(self, session): self.client.post( "api/v1/revoke", json={"token": refresh}, headers={"Authorization": f"Bearer {token}"} ) - total_tokens_in_db = session.query(func.count(TokenBlockList.id)).scalar() + total_tokens_in_db = session.query(func.count(TokenBlockList.jti)).scalar() assert total_tokens_in_db == 1 From 92247fef82b379285eaa74e3fe4a4cb2f02589ec Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 21 Apr 2021 09:29:22 +0100 Subject: [PATCH 42/53] Removing check for if user already logged in, since we no longer set session on api --- airflow/www/security.py | 8 ----- .../endpoints/test_auth_endpoint.py | 29 ------------------- 2 files changed, 37 deletions(-) diff --git a/airflow/www/security.py b/airflow/www/security.py index 9111d5a21bc7c..be77e47237bd5 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -739,14 +739,8 @@ def check_authorization( return True # TODO: Whether to create APISecurityManager and move api related code to it? - def is_user_logged_in(self): - """Raise if user already logged in""" - if g.user is not None and g.user.is_authenticated: - raise Unauthenticated(detail="Client already authenticated") # For security - def login_with_user_pass(self, username, password): """Convenience method for user login through the API""" - self.is_user_logged_in() if self.auth_type not in (AUTH_DB, AUTH_LDAP): raise Unauthenticated(detail=AUTH_TYPE_MISMATCH_MESSAGE) user = None @@ -758,7 +752,6 @@ def login_with_user_pass(self, username, password): def oauth_authorization_url(self, app, provider, redirect_url): """Get authorization url for oauth""" - self.is_user_logged_in() if self.auth_type != AUTH_OAUTH: raise Unauthenticated(detail=AUTH_TYPE_MISMATCH_MESSAGE) state = jwt.encode( @@ -813,7 +806,6 @@ def oauth_login_user(self, app, provider, state): def login_remote_user(self, username): """Login user using remote auth""" - self.is_user_logged_in() if self.auth_type != AUTH_REMOTE_USER: raise Unauthenticated(detail=AUTH_TYPE_MISMATCH_MESSAGE) user = self.auth_user_remote_user(username) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 29d74403df464..99891d4bed3d6 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -128,15 +128,6 @@ def test_user_can_login(self): assert isinstance(response.json['token'], str) assert isinstance(response.json['refresh_token'], str) - def test_logged_in_user_cant_relogin(self): - self.auth_type(AUTH_DB) - payload = {"username": "test", "password": "test"} - response = self.client.post('api/v1/auth/login', json=payload) - assert response.json['user']['username'] == 'test' - response = self.client.post('api/v1/auth/login', json=payload) - assert response.status_code == 401 - assert response.json['detail'] == "Client already authenticated" - def test_incorrect_username_raises(self): self.auth_type(AUTH_DB) payload = {"username": "tests", "password": "test"} @@ -171,18 +162,6 @@ def test_user_can_login(self): assert isinstance(response.json['token'], str) assert isinstance(response.json['refresh_token'], str) - def test_logged_in_user_cant_relogin(self): - self.auth_type(AUTH_LDAP) - self.app.appbuilder.sm.auth_user_ldap = mock.Mock() - user = self.app.appbuilder.sm.find_user(username='test') - self.app.appbuilder.sm.auth_user_ldap.return_value = user - payload = {"username": "test", "password": "test"} - response = self.client.post('api/v1/auth/login', json=payload) - assert response.json['user']['username'] == 'test' - response = self.client.post('api/v1/auth/login', json=payload) - assert response.status_code == 401 - assert response.json['detail'] == "Client already authenticated" - def test_incorrect_username_raises(self): self.auth_type(AUTH_LDAP) self.app.appbuilder.sm.auth_user_ldap = mock.Mock() @@ -298,14 +277,6 @@ def test_remote_user_can_login(self): assert response.status_code == 200 assert response.json['user']['username'] == 'test' - def test_remote_user_cant_relogin(self): - self.auth_type(AUTH_REMOTE_USER) - response = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) - assert response.status_code == 200 - response = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) - assert response.status_code == 401 - assert response.json['detail'] == "Client already authenticated" - def test_incorrect_username_raises(self): self.auth_type(AUTH_REMOTE_USER) response = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "tes"}) From 8ca6889f68e6f051632c3e0c1a092db6def13c3e Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 21 Apr 2021 13:20:15 +0100 Subject: [PATCH 43/53] Allow storing token in header or cookie. --- .../api_connexion/endpoints/auth_endpoint.py | 27 ++++++--- airflow/api_connexion/openapi/v1.yaml | 2 +- airflow/api_connexion/schemas/auth_schema.py | 8 --- .../default_webserver_config.py | 13 +++++ airflow/www/security.py | 30 ++++++---- .../endpoints/test_auth_endpoint.py | 58 ++++++++++++++++++- 6 files changed, 109 insertions(+), 29 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 033a7d1c061ad..451b2ac828af5 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -25,6 +25,8 @@ get_raw_jwt, jwt_refresh_token_required, jwt_required, + set_access_cookies, + unset_jwt_cookies, ) from flask_login import login_user from marshmallow import ValidationError @@ -67,17 +69,18 @@ def get_auth_info(): def auth_login(): """Handle DB login""" + appbuilder = current_app.appbuilder body = request.json try: data = login_form_schema.load(body) except ValidationError as err: raise Unauthenticated(detail=str(err.messages)) - security_manager = current_app.appbuilder.sm + security_manager = appbuilder.sm user = security_manager.login_with_user_pass(data['username'], data['password']) if not user: raise Unauthenticated(detail="Invalid login") login_user(user, remember=False) - return security_manager.create_tokens_and_dump(user, auth_schema) + return security_manager.create_tokens_and_dump(appbuilder.app, user, auth_schema) def auth_oauthlogin(provider, register=None, redirect_url=None): @@ -85,14 +88,14 @@ def auth_oauthlogin(provider, register=None, redirect_url=None): appbuilder = current_app.appbuilder if register: c_session["register"] = True - return appbuilder.sm.oauth_authorization_url(appbuilder.app, provider, redirect_url) + return appbuilder.sm.oauth_authorization_url(current_app.appbuilder.app, provider, redirect_url) def authorize_oauth(provider, state): """Callback to authorize Oauth.""" appbuilder = current_app.appbuilder user = appbuilder.sm.oauth_login_user(appbuilder.app, provider, state) - return appbuilder.sm.create_tokens_and_dump(user, auth_schema) + return appbuilder.sm.create_tokens_and_dump(appbuilder.app, user, auth_schema) def auth_remoteuser(): @@ -103,7 +106,7 @@ def auth_remoteuser(): user = appbuilder.sm.login_remote_user(username) else: raise Unauthenticated(detail="Invalid login") - return appbuilder.sm.create_tokens_and_dump(user, auth_schema) + return appbuilder.sm.create_tokens_and_dump(appbuilder.app, user, auth_schema) @jwt_refresh_token_required @@ -111,6 +114,10 @@ def refresh_token(): """Refresh token""" user = get_jwt_identity() access_token = create_access_token(identity=user) + if 'cookies' in current_app.appbuilder.app.config['JWT_TOKEN_LOCATION']: + resp = jsonify({'access_token': ""}) # empty string to comply to spec + set_access_cookies(resp, access_token) + return resp return {'access_token': access_token} @@ -146,7 +153,10 @@ def revoke_current_user_token(): TokenBlockList.add_token(jti=raw_jwt['jti'], expiry_delta=raw_jwt['exp']) except IntegrityError: pass - return jsonify({"revoked": True}) + resp = jsonify({"revoked": True}) + if 'cookies' in current_app.appbuilder.app.config['JWT_TOKEN_LOCATION']: + unset_jwt_cookies(resp) + return resp @jwt_refresh_token_required @@ -160,4 +170,7 @@ def revoke_current_user_refresh_token(): TokenBlockList.add_token(jti=raw_jwt['jti'], expiry_delta=raw_jwt['exp']) except IntegrityError: pass - return jsonify({"revoked": True}) + resp = jsonify({"revoked": True}) + if 'cookies' in current_app.appbuilder.app.config['JWT_TOKEN_LOCATION']: + unset_jwt_cookies(resp) + return resp diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index d7c0e498557f3..1e6b99443e7e8 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1668,7 +1668,7 @@ paths: $ref: '#/components/responses/Unauthenticated' /refresh: - post: + get: summary: Refresh a token description: Accepts refresh token and creates access token x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint diff --git a/airflow/api_connexion/schemas/auth_schema.py b/airflow/api_connexion/schemas/auth_schema.py index 8836220745e2b..e7f82da14df5f 100644 --- a/airflow/api_connexion/schemas/auth_schema.py +++ b/airflow/api_connexion/schemas/auth_schema.py @@ -58,13 +58,6 @@ class AuthSchema(Schema): user = fields.Nested(UserCollectionItemSchema) -class LogoutSchema(Schema): - """Schema for logout""" - - token = fields.String() - refresh_token = fields.String() - - class RevokeTokenSchema(Schema): """Schema to revoke tokens""" @@ -74,5 +67,4 @@ class RevokeTokenSchema(Schema): info_schema = InfoSchema() login_form_schema = LoginForm() auth_schema = AuthSchema() -logout_schema = LogoutSchema() token_schema = RevokeTokenSchema() diff --git a/airflow/config_templates/default_webserver_config.py b/airflow/config_templates/default_webserver_config.py index 7f19ff129bd4d..51ec9974c7284 100644 --- a/airflow/config_templates/default_webserver_config.py +++ b/airflow/config_templates/default_webserver_config.py @@ -17,6 +17,7 @@ # under the License. """Default configuration for the Airflow webserver""" import os +from datetime import timedelta from flask_appbuilder.security.manager import AUTH_DB @@ -126,3 +127,15 @@ # APP_THEME = "superhero.css" # APP_THEME = "united.css" # APP_THEME = "yeti.css" +# ---------------------------------------------------- +# JWT CONFIGURATION +# ---------------------------------------------------- +# Airflow uses flask_jwt_extended for handling JWT Authentication. +# Please refer to https://flask-jwt-extended.readthedocs.io/en/3.0.0_release/options/ +# for more configuration options. +# Note that storing token in query string is not supported +JWT_TOKEN_LOCATION = ['headers'] +JWT_HEADER_NAME = "Authorization" +JWT_HEADER_TYPE = 'Bearer' +JWT_ACCESS_TOKEN_EXPIRES = timedelta(minutes=30) +JWT_REFRESH_TOKEN_EXPIRES = timedelta(days=30) diff --git a/airflow/www/security.py b/airflow/www/security.py index be77e47237bd5..dd839f0efff7b 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -18,16 +18,21 @@ # import warnings -from datetime import timedelta from typing import Dict, Optional, Sequence, Set, Tuple import jwt -from flask import current_app, g, request +from flask import current_app, g, jsonify, request from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_REMOTE_USER 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 JWTManager, create_access_token, create_refresh_token +from flask_jwt_extended import ( + JWTManager, + create_access_token, + create_refresh_token, + set_access_cookies, + set_refresh_cookies, +) from flask_login import login_user from sqlalchemy import or_ from sqlalchemy.orm import joinedload @@ -754,10 +759,11 @@ def oauth_authorization_url(self, app, provider, redirect_url): """Get authorization url for oauth""" if self.auth_type != AUTH_OAUTH: raise Unauthenticated(detail=AUTH_TYPE_MISMATCH_MESSAGE) + secret_key = app.config['SECRET_KEY'] state = jwt.encode( request.args.to_dict(flat=False), - app.config["SECRET_KEY"], - algorithm="HS256", + app.config.get("JWT_SECRET_KEY", secret_key), + algorithm=app.config["JWT_ALGORITHM"], ) auth_provider = self.oauth_remotes[provider] try: @@ -784,11 +790,12 @@ def oauth_login_user(self, app, provider, state): if resp is None: raise Unauthenticated(detail="You denied the request to sign in") # Verify state + secret_key = app.config['SECRET_KEY'] try: jwt.decode( state, - app.config["SECRET_KEY"], - algorithms=["HS256"], + app.config.get("JWT_SECRET_KEY", secret_key), + algorithms=app.config["JWT_ALGORITHM"], ) except jwt.InvalidTokenError: raise Unauthenticated(detail="State signature is not valid!") @@ -817,16 +824,19 @@ def login_remote_user(self, username): def create_jwt_manager(self, app) -> JWTManager: """Called by FAB for us when it wants a configured JWT manager""" jwt_manager = JWTManager() - app.config["JWT_ACCESS_TOKEN_EXPIRES"] = timedelta(hours=1) - app.config["JWT_REFRESH_TOKEN_EXPIRES"] = timedelta(days=30) jwt_manager.init_app(app) jwt_manager.user_loader_callback_loader(self.load_user_jwt) return jwt_manager - def create_tokens_and_dump(self, user, schema): + def create_tokens_and_dump(self, app, user, schema): """Creates access and refresh token, return user data alongside tokens""" access_token = create_access_token(user.id) refresh_token = create_refresh_token(user.id) + if 'cookies' in app.config['JWT_TOKEN_LOCATION']: + resp = jsonify(schema.dump({'user': user})) + set_access_cookies(resp, access_token) + set_refresh_cookies(resp, refresh_token) + return resp return schema.dump({'user': user, 'token': access_token, 'refresh_token': refresh_token}) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 99891d4bed3d6..a42b3c7c546bf 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -18,9 +18,11 @@ import pytest from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER +from flask_jwt_extended.utils import create_access_token, create_refresh_token from sqlalchemy import func from airflow.models.auth import TokenBlockList +from airflow.security import permissions from airflow.utils.session import provide_session from airflow.www.security import AUTH_TYPE_MISMATCH_MESSAGE from tests.test_utils.api_connexion_utils import create_user, delete_user @@ -51,7 +53,14 @@ @pytest.fixture(scope="module") def configured_app(minimal_app_for_api): app = minimal_app_for_api - create_user(app, username="test", role_name="Test") # type: ignore + create_user( + app, + username="test", + role_name="Test", + permissions=[ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_POOL), + ], + ) # type: ignore yield app @@ -296,7 +305,7 @@ def test_creates_access_token(self): payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth/login', json=payload) refresh = response.json['refresh_token'] - response2 = self.client.post("api/v1/refresh", headers={"Authorization": f"Bearer {refresh}"}) + response2 = self.client.get("api/v1/refresh", headers={"Authorization": f"Bearer {refresh}"}) assert response2.json['access_token'] is not None @@ -305,7 +314,7 @@ def test_access_token_cant_access_endpoint(self): payload = {"username": "test", "password": "test"} response = self.client.post('api/v1/auth/login', json=payload) token = response.json['token'] - response2 = self.client.post("api/v1/refresh", headers={"Authorization": f"Bearer {token}"}) + response2 = self.client.get("api/v1/refresh", headers={"Authorization": f"Bearer {token}"}) assert response2.status_code == 422 assert response2.json['msg'] == 'Only refresh tokens are allowed' @@ -353,3 +362,46 @@ def test_revoke_token_works(self, session): ) total_tokens_in_db = session.query(func.count(TokenBlockList.jti)).scalar() assert total_tokens_in_db == 1 + + +class TestTokenCanBeStoredInCookies(TestLoginEndpoint): + def test_token_stored_in_cookie_in_db_login(self): + self.app.config['JWT_TOKEN_LOCATION'] = ['cookies'] + self.auth_type(AUTH_DB) + payload = {"username": "test", "password": "test"} + self.client.post('api/v1/auth/login', json=payload) + + assert all(cookie.name != "session" for cookie in self.client.cookie_jar) + expected = ['access_token_cookie', 'refresh_token_cookie', 'csrf_access_token', 'csrf_refresh_token'] + result = [cookie.name for cookie in self.client.cookie_jar] + assert sorted(expected) == sorted(result) + + def test_token_not_in_response_if_token_in_cookie(self): + self.app.config['JWT_TOKEN_LOCATION'] = ['cookies'] + self.auth_type(AUTH_DB) + payload = {"username": "test", "password": "test"} + response = self.client.post('api/v1/auth/login', json=payload) + assert not response.json.get('refresh_token') + assert not response.json.get('access_token') + # Ensure cookie was set + assert [cookie.name for cookie in self.client.cookie_jar] + + def test_token_can_view_other_endpoints_if_token_in_cookie(self): + self.app.config['JWT_TOKEN_LOCATION'] = ['cookies'] + self.auth_type(AUTH_DB) + user = self.app.appbuilder.sm.find_user(username='test') + with self.app.app_context(): + token = create_access_token(user.id) + self.client.set_cookie("localhost", 'access_token_cookie', token) + response = self.client.get("/api/v1/pools") + assert response.status_code == 200 + + def test_refresh_works_for_token_in_cookie(self): + self.app.config['JWT_TOKEN_LOCATION'] = ['cookies'] + self.auth_type(AUTH_DB) + user = self.app.appbuilder.sm.find_user(username='test') + with self.app.app_context(): + token = create_refresh_token(user.id) + self.client.set_cookie("localhost", 'refresh_token_cookie', token) + response = self.client.get("/api/v1/refresh") + assert response.status_code == 200 From e71a685a066eebdab2013178e6455421af48a357 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 21 Apr 2021 13:27:50 +0100 Subject: [PATCH 44/53] fixup! Allow storing token in header or cookie. --- airflow/www/app.py | 1 - 1 file changed, 1 deletion(-) diff --git a/airflow/www/app.py b/airflow/www/app.py index 3dc93267a4808..99f1dbad6a318 100644 --- a/airflow/www/app.py +++ b/airflow/www/app.py @@ -35,7 +35,6 @@ 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_session import init_airflow_session_interface, init_permanent_session from airflow.www.extensions.init_views import ( init_api_connexion, From fad8be298b1be30042e97e2ddb8ab89fcc205ed3 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 21 Apr 2021 16:31:23 +0100 Subject: [PATCH 45/53] fixup! fixup! Allow storing token in header or cookie. --- airflow/api_connexion/endpoints/auth_endpoint.py | 5 ++--- docs/spelling_wordlist.txt | 1 + 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 451b2ac828af5..a0093f09f89be 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -102,10 +102,9 @@ def auth_remoteuser(): """Handle remote user auth""" appbuilder = current_app.appbuilder username = request.environ.get("REMOTE_USER") - if username: - user = appbuilder.sm.login_remote_user(username) - else: + if not username: raise Unauthenticated(detail="Invalid login") + user = appbuilder.sm.login_remote_user(username) return appbuilder.sm.create_tokens_and_dump(appbuilder.app, user, auth_schema) diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 8458bb3899574..99442e3f17c94 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -529,6 +529,7 @@ behaviours bigquery bigtable bitshift +blocklist bolkedebruin booktabs boolean From 09658c62f9ebd3662fedeae23cb8437249563130 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Wed, 21 Apr 2021 22:12:36 +0100 Subject: [PATCH 46/53] Make Oauth redirect happen inside the server --- .pre-commit-config.yaml | 1 + .../api_connexion/endpoints/auth_endpoint.py | 15 +++-- airflow/api_connexion/openapi/v1.yaml | 35 +++-------- airflow/www/security.py | 58 ++++++++++++------- .../endpoints/test_auth_endpoint.py | 41 +++++++------ 5 files changed, 77 insertions(+), 73 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f57fc6e6fe1f5..78067d9a60b07 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -315,6 +315,7 @@ repos: pass_filenames: true exclude: > (?x) + ^airflow/www/security.py$| ^airflow/api_connexion/security.py$| ^airflow/providers/apache/cassandra/hooks/cassandra.py$| ^airflow/providers/apache/hive/operators/hive_stats.py$| diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index a0093f09f89be..353f5ea35ec62 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -16,7 +16,7 @@ # under the License. import logging -from flask import current_app, jsonify, request, session as c_session +from flask import current_app, jsonify, request from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER from flask_jwt_extended import ( create_access_token, @@ -83,16 +83,19 @@ def auth_login(): return security_manager.create_tokens_and_dump(appbuilder.app, user, auth_schema) -def auth_oauthlogin(provider, register=None, redirect_url=None): +def auth_oauthlogin(provider): """Returns OAUTH authorization url""" appbuilder = current_app.appbuilder - if register: - c_session["register"] = True - return appbuilder.sm.oauth_authorization_url(current_app.appbuilder.app, provider, redirect_url) + return appbuilder.sm.oauth_authorize_redirect(current_app.appbuilder.app, provider) -def authorize_oauth(provider, state): +def authorize_oauth(): """Callback to authorize Oauth.""" + args = request.args + provider = args.get('provider', None) + state = args.get('state', None) + if not (provider and state): + raise BadRequest(detail="Missing required parameters: provider and state") appbuilder = current_app.appbuilder user = appbuilder.sm.oauth_login_user(appbuilder.app, provider, state) return appbuilder.sm.create_tokens_and_dump(appbuilder.app, user, auth_schema) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 1e6b99443e7e8..8d8e25b09698d 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1618,28 +1618,24 @@ paths: parameters: - $ref: '#/components/parameters/ShouldRegister' - $ref: '#/components/parameters/OauthProvider' - - $ref: '#/components/parameters/RedirectUrl' responses: - '200': - description: Success. - content: - application/json: + '302': + description: Redirect. + headers: + Location: schema: - $ref: '#/components/schemas/AuthorizationUrl' + type: string + format: uri '401': $ref: '#/components/responses/Unauthenticated' - /oauth-authorized/{provider}: - parameters: - - $ref: '#/components/parameters/OauthProvider' + /oauth-authorized: get: summary: Oauth authentication x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint operationId: authorize_oauth tags: [Authentication] - parameters: - - $ref: '#/components/parameters/AuthState' responses: '200': description: Success. @@ -3542,23 +3538,6 @@ components: required: true description: The name of the provider to authenticate - AuthState: - in: query - name: state - schema: - type: string - required: true - description: The auth state from provider - - RedirectUrl: - in: query - name: redirect_url - schema: - type: string - format: url - required: true - description: The oauth redirect url from the frontend - ShouldRegister: in: query name: register diff --git a/airflow/www/security.py b/airflow/www/security.py index dd839f0efff7b..f20971c4de0f4 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -16,7 +16,8 @@ # specific language governing permissions and limitations # under the License. # - +import logging +import re import warnings from typing import Dict, Optional, Sequence, Set, Tuple @@ -38,6 +39,7 @@ from sqlalchemy.orm import joinedload from airflow.api_connexion.exceptions import Unauthenticated +from airflow.configuration import conf from airflow.exceptions import AirflowException from airflow.models import DagBag, DagModel from airflow.security import permissions @@ -70,6 +72,8 @@ AUTH_TYPE_MISMATCH_MESSAGE = "Authentication type does not match" +log = logging.getLogger(__name__) + class AirflowSecurityManager(SecurityManager, LoggingMixin): # pylint: disable=too-many-public-methods """Custom security manager, which introduces a permission model adapted to Airflow""" @@ -755,60 +759,74 @@ def login_with_user_pass(self, username, password): user = self.auth_user_ldap(username, password) return user - def oauth_authorization_url(self, app, provider, redirect_url): + def oauth_authorize_redirect(self, app, provider): """Get authorization url for oauth""" + api_base_path = "/api/v1/" + base_url = conf.get("webserver", "base_url") + api_base_path + redirect_uri = base_url + "oauth-authorized?provider=" + provider if self.auth_type != AUTH_OAUTH: raise Unauthenticated(detail=AUTH_TYPE_MISMATCH_MESSAGE) - secret_key = app.config['SECRET_KEY'] state = jwt.encode( request.args.to_dict(flat=False), - app.config.get("JWT_SECRET_KEY", secret_key), - algorithm=app.config["JWT_ALGORITHM"], + key=app.config['SECRET_KEY'], + algorithm='HS256', ) auth_provider = self.oauth_remotes[provider] try: - if provider == "twitter": - redirect_uri = redirect_url + f"&state={state}" - auth_data = auth_provider.create_authorization_url(redirect_uri=redirect_uri) - auth_provider.save_authorize_data(request, redirect_uri=redirect_uri, **auth_data) - return dict(auth_url=auth_data['url']) + redirect_uri = redirect_uri + f"&state={state}" + + return auth_provider.authorize_redirect(redirect_uri=redirect_uri) else: state = state.decode("ascii") if isinstance(state, bytes) else state - auth_data = auth_provider.create_authorization_url( - redirect_uri=redirect_url, + return auth_provider.authorize_redirect( + redirect_uri=redirect_uri, state=state, ) - auth_provider.save_authorize_data(request, redirect_uri=redirect_url, **auth_data) - return dict(auth_url=auth_data['url']) except Exception as err: # pylint: disable=broad-except + log.error("Error on OAuth authorize: %s", err) raise Unauthenticated(detail=str(err)) def oauth_login_user(self, app, provider, state): """Oauth login""" + log.debug("Authorized init") resp = self.oauth_remotes[provider].authorize_access_token() if resp is None: raise Unauthenticated(detail="You denied the request to sign in") # Verify state - secret_key = app.config['SECRET_KEY'] try: jwt.decode( state, - app.config.get("JWT_SECRET_KEY", secret_key), - algorithms=app.config["JWT_ALGORITHM"], + key=app.config['SECRET_KEY'], + algorithms='HS256', ) except jwt.InvalidTokenError: raise Unauthenticated(detail="State signature is not valid!") + log.debug("OAUTH Authorized resp: %s", resp) # Retrieves specific user info from the provider - try: + try: # pylint: disable=too-many-nested-blocks userinfo = self.oauth_user_info(provider, resp) except Exception: # pylint: disable=broad-except user = None else: + log.debug("User info retrieved from %s: %s", provider, userinfo) + # User email is not whitelisted + if provider in self.oauth_whitelists: + whitelist = self.oauth_whitelists[provider] + allow = False + for e in whitelist: + if re.search(e, userinfo["email"]): + allow = True + break + if not allow: + raise Unauthenticated(detail="You are not authorized.") + else: + log.debug("No allow list for OAuth provider") user = self.auth_user_oauth(userinfo) + log.debug("User is %s", user) if user is None: raise Unauthenticated(detail="Invalid login") - login_user(user) + login_user(user, remember=False) return user def login_remote_user(self, username): @@ -818,7 +836,7 @@ def login_remote_user(self, username): user = self.auth_user_remote_user(username) if user is None: raise Unauthenticated(detail="Invalid login") - login_user(user) + login_user(user, remember=False) return user def create_jwt_manager(self, app) -> JWTManager: diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index a42b3c7c546bf..06bcba9e9e67e 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -26,6 +26,7 @@ from airflow.utils.session import provide_session from airflow.www.security import AUTH_TYPE_MISMATCH_MESSAGE from tests.test_utils.api_connexion_utils import create_user, delete_user +from tests.test_utils.config import conf_vars OAUTH_PROVIDERS = [ { @@ -195,39 +196,40 @@ def test_auth_type_must_be_ldap(self): assert response.json['detail'] == AUTH_TYPE_MISMATCH_MESSAGE -class TestOauthAuthorizationURLEndpoint(TestLoginEndpoint): +class TestOauthAuthorizationEndpoint(TestLoginEndpoint): + @conf_vars({("webserver", 'base_url'): 'http://localhost:8080'}) @mock.patch("airflow.www.security.jwt.encode") - def test_can_generate_authorization_url(self, mock_jwt_encode): + def test_can_redirect_for_google(self, mock_jwt_encode): self.auth_type(AUTH_OAUTH) mock_jwt_encode.return_value = "state" mock_google_auth_provider = mock.Mock() self.app.appbuilder.sm.oauth_remotes = { "google": mock_google_auth_provider, } - mock_auth = mock.MagicMock(return_value={"state": "state", "url": "authurl"}) - mock_google_auth_provider.create_authorization_url = mock_auth - redirect_url = "http://localhost:8080" - self.client.get(f'api/v1/auth-oauth/google?register=True&redirect_url={redirect_url}') - mock_auth.assert_called_once_with(redirect_uri=redirect_url, state="state") + mock_auth = mock.MagicMock(return_value="test-val") + mock_google_auth_provider.authorize_redirect = mock_auth + redirect_url = "http://localhost:8080/api/v1/oauth-authorized?provider=google" + self.client.get('api/v1/auth-oauth/google?register=True') + mock_auth.assert_called_once_with(redirect_uri=redirect_url, state='state') + @conf_vars({("webserver", 'base_url'): 'http://localhost:8080'}) @mock.patch("airflow.www.security.jwt.encode") - def test_can_generate_authorization_url_for_twitter(self, mock_jwt_encode): + def test_can_redirect_for_twitter(self, mock_jwt_encode): self.auth_type(AUTH_OAUTH) mock_jwt_encode.return_value = "state" mock_twitter_auth_provider = mock.Mock() self.app.appbuilder.sm.oauth_remotes = { "twitter": mock_twitter_auth_provider, } - mock_auth = mock.MagicMock(return_value={"state": "state", "url": "authurl"}) - mock_twitter_auth_provider.create_authorization_url = mock_auth - redirect_url = "http://localhost:8080" - self.client.get(f'api/v1/auth-oauth/twitter?register=True&redirect_url={redirect_url}') + mock_auth = mock.MagicMock(return_value='val') + mock_twitter_auth_provider.authorize_redirect = mock_auth + redirect_url = "http://localhost:8080/api/v1/oauth-authorized?provider=twitter" + self.client.get('api/v1/auth-oauth/twitter?register=True') mock_auth.assert_called_once_with(redirect_uri=redirect_url + "&state=state") def test_incorrect_auth_type_raises(self): self.auth_type(AUTH_DB) - redirect_url = "http://localhost:8080" - resp = self.client.get(f'api/v1/auth-oauth/google?register=True&redirect_url={redirect_url}') + resp = self.client.get('api/v1/auth-oauth/google?register=True') assert resp.status_code == 401 assert resp.json['detail'] == AUTH_TYPE_MISMATCH_MESSAGE @@ -240,7 +242,7 @@ def test_user_refused_sign_in_request(self): "twitter": mock_twitter_auth_provider, } mock_twitter_auth_provider.authorize_access_token.return_value = None - response = self.client.get('api/v1/oauth-authorized/twitter?state=state') + response = self.client.get('api/v1/oauth-authorized?provider=twitter&state=state') assert response.status_code == 401 assert response.json['detail'] == "You denied the request to sign in" @@ -251,7 +253,7 @@ def test_wrong_state_signature_raises(self): "twitter": mock_twitter_auth_provider, } mock_twitter_auth_provider.authorize_access_token.return_value = mock.MagicMock() - response = self.client.get('api/v1/oauth-authorized/twitter?state=state') + response = self.client.get('api/v1/oauth-authorized?provider=twitter&state=state') assert response.status_code == 401 assert response.json['detail'] == "State signature is not valid!" @@ -273,7 +275,7 @@ def test_successful_authorization(self, mock_jwt_decode): } mock_authorized = mock.MagicMock() mock_twitter_auth_provider.authorize_access_token.return_value = mock_authorized - self.client.get('api/v1/oauth-authorized/twitter?state=state') + self.client.get('api/v1/oauth-authorized?provider=twitter&state=state') mock_oauth_session.assert_not_called() mock_user_info.assert_called_once_with('twitter', mock_authorized) mock_user_oauth.assert_called_once_with(mock_user_info.return_value) @@ -288,13 +290,14 @@ def test_remote_user_can_login(self): def test_incorrect_username_raises(self): self.auth_type(AUTH_REMOTE_USER) - response = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "tes"}) + self.app.config['AUTH_USER_REGISTRATION'] = False + response = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "fakeuser"}) assert response.status_code == 401 assert response.json['detail'] == 'Invalid login' def test_incorrect_auth_type_raises(self): self.auth_type(AUTH_DB) - resp = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "test"}) + resp = self.client.get('api/v1/auth-remoteuser', environ_overrides={"REMOTE_USER": "tes"}) assert resp.status_code == 401 assert resp.json['detail'] == AUTH_TYPE_MISMATCH_MESSAGE From 22223e4f250fec2944bee63aa91f1404e20effde Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Thu, 22 Apr 2021 09:47:55 +0100 Subject: [PATCH 47/53] fixup! Make Oauth redirect happen inside the server --- airflow/www/security.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/airflow/www/security.py b/airflow/www/security.py index f20971c4de0f4..48f8b7aa4ef0f 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -22,7 +22,7 @@ from typing import Dict, Optional, Sequence, Set, Tuple import jwt -from flask import current_app, g, jsonify, request +from flask import current_app, g, jsonify from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_REMOTE_USER from flask_appbuilder.security.sqla import models as sqla_models from flask_appbuilder.security.sqla.manager import SecurityManager @@ -759,15 +759,16 @@ def login_with_user_pass(self, username, password): user = self.auth_user_ldap(username, password) return user - def oauth_authorize_redirect(self, app, provider): + def oauth_authorize_redirect(self, app, provider, register=False): """Get authorization url for oauth""" api_base_path = "/api/v1/" base_url = conf.get("webserver", "base_url") + api_base_path redirect_uri = base_url + "oauth-authorized?provider=" + provider if self.auth_type != AUTH_OAUTH: raise Unauthenticated(detail=AUTH_TYPE_MISMATCH_MESSAGE) + arg = {'register': [str(register)]} if register else {} state = jwt.encode( - request.args.to_dict(flat=False), + arg, key=app.config['SECRET_KEY'], algorithm='HS256', ) @@ -851,9 +852,9 @@ def create_tokens_and_dump(self, app, user, schema): access_token = create_access_token(user.id) refresh_token = create_refresh_token(user.id) if 'cookies' in app.config['JWT_TOKEN_LOCATION']: - resp = jsonify(schema.dump({'user': user})) - set_access_cookies(resp, access_token) - set_refresh_cookies(resp, refresh_token) + resp = schema.dump({'user': user}) + set_access_cookies(jsonify(resp), access_token) + set_refresh_cookies(jsonify(resp), refresh_token) return resp return schema.dump({'user': user, 'token': access_token, 'refresh_token': refresh_token}) From 9866f128e3cec0dbaffa708657c8206d852fe9fc Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 23 Apr 2021 15:48:33 +0100 Subject: [PATCH 48/53] move some api related stuff from security manager --- .pre-commit-config.yaml | 2 +- .../api_connexion/endpoints/auth_endpoint.py | 92 ++++++++++++++++-- airflow/www/security.py | 93 +------------------ .../endpoints/test_auth_endpoint.py | 15 +-- 4 files changed, 94 insertions(+), 108 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 78067d9a60b07..1b201005ba7fd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -315,7 +315,7 @@ repos: pass_filenames: true exclude: > (?x) - ^airflow/www/security.py$| + ^airflow/api_connexion/endpoints/auth_endpoint.py$| ^airflow/api_connexion/security.py$| ^airflow/providers/apache/cassandra/hooks/cassandra.py$| ^airflow/providers/apache/hive/operators/hive_stats.py$| diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 353f5ea35ec62..787d32166aa5f 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -15,7 +15,9 @@ # specific language governing permissions and limitations # under the License. import logging +import re +import jwt from flask import current_app, jsonify, request from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER from flask_jwt_extended import ( @@ -39,6 +41,7 @@ login_form_schema, token_schema, ) +from airflow.configuration import conf from airflow.models.auth import TokenBlockList log = logging.getLogger(__name__) @@ -74,8 +77,10 @@ def auth_login(): try: data = login_form_schema.load(body) except ValidationError as err: - raise Unauthenticated(detail=str(err.messages)) + raise BadRequest(detail=str(err.messages)) security_manager = appbuilder.sm + if security_manager.auth_type not in [AUTH_DB, AUTH_LDAP]: + raise Unauthenticated(detail="Authentication type does not match") user = security_manager.login_with_user_pass(data['username'], data['password']) if not user: raise Unauthenticated(detail="Invalid login") @@ -83,10 +88,36 @@ def auth_login(): return security_manager.create_tokens_and_dump(appbuilder.app, user, auth_schema) -def auth_oauthlogin(provider): - """Returns OAUTH authorization url""" +def auth_oauthlogin(provider, register=None): + """Authenticates user and redirect to callback endpoint""" appbuilder = current_app.appbuilder - return appbuilder.sm.oauth_authorize_redirect(current_app.appbuilder.app, provider) + security_manager = appbuilder.sm + if security_manager.auth_type != AUTH_OAUTH: + raise Unauthenticated(detail="Authentication type does not match") + api_base_path = "/api/v1/" + base_url = conf.get("webserver", "base_url") + api_base_path + redirect_uri = base_url + "oauth-authorized?provider=" + provider + + arg = {'register': [str(register)]} if register else {} + state = jwt.encode( + arg, + key=appbuilder.app.config['SECRET_KEY'], + algorithm='HS256', + ) + auth_provider = security_manager.oauth_remotes[provider] + try: + if provider == "twitter": + redirect_uri = redirect_uri + f"&state={state}" + return auth_provider.authorize_redirect(redirect_uri=redirect_uri) + else: + state = state.decode("ascii") if isinstance(state, bytes) else state + return auth_provider.authorize_redirect( + redirect_uri=redirect_uri, + state=state, + ) + except Exception as err: # pylint: disable=broad-except + log.error("Error on OAuth authorize: %s", err) + raise Unauthenticated(detail=str(err)) def authorize_oauth(): @@ -97,18 +128,63 @@ def authorize_oauth(): if not (provider and state): raise BadRequest(detail="Missing required parameters: provider and state") appbuilder = current_app.appbuilder - user = appbuilder.sm.oauth_login_user(appbuilder.app, provider, state) - return appbuilder.sm.create_tokens_and_dump(appbuilder.app, user, auth_schema) + security_manager = appbuilder.sm + log.debug("Authorized init") + resp = security_manager.oauth_remotes[provider].authorize_access_token() + if resp is None: + raise Unauthenticated(detail="You denied the request to sign in") + # Verify state + try: + jwt.decode( + state, + key=appbuilder.app.config['SECRET_KEY'], + algorithms='HS256', + ) + except jwt.InvalidTokenError: + raise BadRequest(detail="State signature is not valid!") + log.debug("OAUTH Authorized resp: %s", resp) + # Retrieves specific user info from the provider + try: # pylint: disable=too-many-nested-blocks + userinfo = security_manager.oauth_user_info(provider, resp) + except Exception as err: # pylint: disable=broad-except + log.debug("Error retrieving user info. Error =>%s", err) + user = None + else: + log.debug("User info retrieved from %s: %s", provider, userinfo) + # User email is not whitelisted + if provider in security_manager.oauth_whitelists: + whitelist = security_manager.oauth_whitelists[provider] + allow = False + for e in whitelist: + if re.search(e, userinfo["email"]): + allow = True + break + if not allow: + raise Unauthenticated(detail="You are not authorized.") + else: + log.debug("No allow list for OAuth provider") + user = security_manager.auth_user_oauth(userinfo) + log.debug("User is %s", user) + if user is None: + raise Unauthenticated(detail="Invalid login") + login_user(user, remember=False) + return security_manager.create_tokens_and_dump(appbuilder.app, user, auth_schema) def auth_remoteuser(): """Handle remote user auth""" appbuilder = current_app.appbuilder + security_manager = appbuilder.sm + if security_manager.auth_type != AUTH_REMOTE_USER: + raise Unauthenticated(detail="Authentication type does not match") username = request.environ.get("REMOTE_USER") if not username: raise Unauthenticated(detail="Invalid login") - user = appbuilder.sm.login_remote_user(username) - return appbuilder.sm.create_tokens_and_dump(appbuilder.app, user, auth_schema) + user = security_manager.auth_user_remote_user(username) + if user is None: + raise Unauthenticated(detail="Invalid login") + login_user(user, remember=False) + return security_manager.create_tokens_and_dump(appbuilder.app, user, auth_schema) @jwt_refresh_token_required diff --git a/airflow/www/security.py b/airflow/www/security.py index 48f8b7aa4ef0f..b86b04337ba6a 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -17,13 +17,11 @@ # under the License. # import logging -import re import warnings from typing import Dict, Optional, Sequence, Set, Tuple -import jwt from flask import current_app, g, jsonify -from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_REMOTE_USER +from flask_appbuilder.const import AUTH_DB, AUTH_LDAP 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 @@ -34,12 +32,9 @@ set_access_cookies, set_refresh_cookies, ) -from flask_login import login_user from sqlalchemy import or_ from sqlalchemy.orm import joinedload -from airflow.api_connexion.exceptions import Unauthenticated -from airflow.configuration import conf from airflow.exceptions import AirflowException from airflow.models import DagBag, DagModel from airflow.security import permissions @@ -70,8 +65,6 @@ 'Public', } -AUTH_TYPE_MISMATCH_MESSAGE = "Authentication type does not match" - log = logging.getLogger(__name__) @@ -747,11 +740,8 @@ def check_authorization( return True - # TODO: Whether to create APISecurityManager and move api related code to it? def login_with_user_pass(self, username, password): """Convenience method for user login through the API""" - if self.auth_type not in (AUTH_DB, AUTH_LDAP): - raise Unauthenticated(detail=AUTH_TYPE_MISMATCH_MESSAGE) user = None if self.auth_type == AUTH_DB: user = self.auth_user_db(username, password) @@ -759,87 +749,6 @@ def login_with_user_pass(self, username, password): user = self.auth_user_ldap(username, password) return user - def oauth_authorize_redirect(self, app, provider, register=False): - """Get authorization url for oauth""" - api_base_path = "/api/v1/" - base_url = conf.get("webserver", "base_url") + api_base_path - redirect_uri = base_url + "oauth-authorized?provider=" + provider - if self.auth_type != AUTH_OAUTH: - raise Unauthenticated(detail=AUTH_TYPE_MISMATCH_MESSAGE) - arg = {'register': [str(register)]} if register else {} - state = jwt.encode( - arg, - key=app.config['SECRET_KEY'], - algorithm='HS256', - ) - auth_provider = self.oauth_remotes[provider] - try: - if provider == "twitter": - redirect_uri = redirect_uri + f"&state={state}" - - return auth_provider.authorize_redirect(redirect_uri=redirect_uri) - else: - state = state.decode("ascii") if isinstance(state, bytes) else state - return auth_provider.authorize_redirect( - redirect_uri=redirect_uri, - state=state, - ) - except Exception as err: # pylint: disable=broad-except - log.error("Error on OAuth authorize: %s", err) - raise Unauthenticated(detail=str(err)) - - def oauth_login_user(self, app, provider, state): - """Oauth login""" - log.debug("Authorized init") - resp = self.oauth_remotes[provider].authorize_access_token() - if resp is None: - raise Unauthenticated(detail="You denied the request to sign in") - # Verify state - try: - jwt.decode( - state, - key=app.config['SECRET_KEY'], - algorithms='HS256', - ) - except jwt.InvalidTokenError: - raise Unauthenticated(detail="State signature is not valid!") - log.debug("OAUTH Authorized resp: %s", resp) - # Retrieves specific user info from the provider - try: # pylint: disable=too-many-nested-blocks - userinfo = self.oauth_user_info(provider, resp) - except Exception: # pylint: disable=broad-except - user = None - else: - log.debug("User info retrieved from %s: %s", provider, userinfo) - # User email is not whitelisted - if provider in self.oauth_whitelists: - whitelist = self.oauth_whitelists[provider] - allow = False - for e in whitelist: - if re.search(e, userinfo["email"]): - allow = True - break - if not allow: - raise Unauthenticated(detail="You are not authorized.") - else: - log.debug("No allow list for OAuth provider") - user = self.auth_user_oauth(userinfo) - log.debug("User is %s", user) - if user is None: - raise Unauthenticated(detail="Invalid login") - login_user(user, remember=False) - return user - - def login_remote_user(self, username): - """Login user using remote auth""" - if self.auth_type != AUTH_REMOTE_USER: - raise Unauthenticated(detail=AUTH_TYPE_MISMATCH_MESSAGE) - user = self.auth_user_remote_user(username) - if user is None: - raise Unauthenticated(detail="Invalid login") - login_user(user, remember=False) - return user - def create_jwt_manager(self, app) -> JWTManager: """Called by FAB for us when it wants a configured JWT manager""" jwt_manager = JWTManager() diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index 06bcba9e9e67e..d60789e56712a 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -24,10 +24,11 @@ from airflow.models.auth import TokenBlockList from airflow.security import permissions from airflow.utils.session import provide_session -from airflow.www.security import AUTH_TYPE_MISMATCH_MESSAGE from tests.test_utils.api_connexion_utils import create_user, delete_user from tests.test_utils.config import conf_vars +AUTH_TYPE_MISMATCH_MESSAGE = "Authentication type does not match" + OAUTH_PROVIDERS = [ { 'name': 'google', @@ -149,7 +150,7 @@ def test_post_body_conforms(self): self.auth_type(AUTH_DB) payload = {"username": "tests"} response = self.client.post('api/v1/auth/login', json=payload) - assert response.status_code == 401 + assert response.status_code == 400 assert response.json['detail'] == "{'password': ['Missing data for required field.']}" def test_auth_type_must_be_db(self): @@ -185,7 +186,7 @@ def test_post_body_conforms(self): self.auth_type(AUTH_LDAP) payload = {"username": "tests"} response = self.client.post('api/v1/auth/login', json=payload) - assert response.status_code == 401 + assert response.status_code == 400 assert response.json['detail'] == "{'password': ['Missing data for required field.']}" def test_auth_type_must_be_ldap(self): @@ -198,7 +199,7 @@ def test_auth_type_must_be_ldap(self): class TestOauthAuthorizationEndpoint(TestLoginEndpoint): @conf_vars({("webserver", 'base_url'): 'http://localhost:8080'}) - @mock.patch("airflow.www.security.jwt.encode") + @mock.patch("airflow.api_connexion.endpoints.auth_endpoint.jwt.encode") def test_can_redirect_for_google(self, mock_jwt_encode): self.auth_type(AUTH_OAUTH) mock_jwt_encode.return_value = "state" @@ -213,7 +214,7 @@ def test_can_redirect_for_google(self, mock_jwt_encode): mock_auth.assert_called_once_with(redirect_uri=redirect_url, state='state') @conf_vars({("webserver", 'base_url'): 'http://localhost:8080'}) - @mock.patch("airflow.www.security.jwt.encode") + @mock.patch("airflow.api_connexion.endpoints.auth_endpoint.jwt.encode") def test_can_redirect_for_twitter(self, mock_jwt_encode): self.auth_type(AUTH_OAUTH) mock_jwt_encode.return_value = "state" @@ -254,10 +255,10 @@ def test_wrong_state_signature_raises(self): } mock_twitter_auth_provider.authorize_access_token.return_value = mock.MagicMock() response = self.client.get('api/v1/oauth-authorized?provider=twitter&state=state') - assert response.status_code == 401 + assert response.status_code == 400 assert response.json['detail'] == "State signature is not valid!" - @mock.patch("airflow.www.security.jwt.decode") + @mock.patch("airflow.api_connexion.endpoints.auth_endpoint.jwt.decode") def test_successful_authorization(self, mock_jwt_decode): self.auth_type(AUTH_OAUTH) mock_jwt_decode.return_value = {'some': 'payload'} From 47e46cdc334e0f804fa558daada6675010a8a5c9 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 23 Apr 2021 16:00:51 +0100 Subject: [PATCH 49/53] Drop support for token in cookie --- .../api_connexion/endpoints/auth_endpoint.py | 22 +++------- .../default_webserver_config.py | 2 +- airflow/www/security.py | 17 ++----- .../endpoints/test_auth_endpoint.py | 44 ------------------- 4 files changed, 9 insertions(+), 76 deletions(-) diff --git a/airflow/api_connexion/endpoints/auth_endpoint.py b/airflow/api_connexion/endpoints/auth_endpoint.py index 787d32166aa5f..f4515df0ad4b0 100644 --- a/airflow/api_connexion/endpoints/auth_endpoint.py +++ b/airflow/api_connexion/endpoints/auth_endpoint.py @@ -27,8 +27,6 @@ get_raw_jwt, jwt_refresh_token_required, jwt_required, - set_access_cookies, - unset_jwt_cookies, ) from flask_login import login_user from marshmallow import ValidationError @@ -85,7 +83,7 @@ def auth_login(): if not user: raise Unauthenticated(detail="Invalid login") login_user(user, remember=False) - return security_manager.create_tokens_and_dump(appbuilder.app, user, auth_schema) + return security_manager.create_tokens_and_dump(user, auth_schema) def auth_oauthlogin(provider, register=None): @@ -168,7 +166,7 @@ def authorize_oauth(): if user is None: raise Unauthenticated(detail="Invalid login") login_user(user, remember=False) - return security_manager.create_tokens_and_dump(appbuilder.app, user, auth_schema) + return security_manager.create_tokens_and_dump(user, auth_schema) def auth_remoteuser(): @@ -184,7 +182,7 @@ def auth_remoteuser(): if user is None: raise Unauthenticated(detail="Invalid login") login_user(user, remember=False) - return security_manager.create_tokens_and_dump(appbuilder.app, user, auth_schema) + return security_manager.create_tokens_and_dump(user, auth_schema) @jwt_refresh_token_required @@ -192,10 +190,6 @@ def refresh_token(): """Refresh token""" user = get_jwt_identity() access_token = create_access_token(identity=user) - if 'cookies' in current_app.appbuilder.app.config['JWT_TOKEN_LOCATION']: - resp = jsonify({'access_token': ""}) # empty string to comply to spec - set_access_cookies(resp, access_token) - return resp return {'access_token': access_token} @@ -231,10 +225,7 @@ def revoke_current_user_token(): TokenBlockList.add_token(jti=raw_jwt['jti'], expiry_delta=raw_jwt['exp']) except IntegrityError: pass - resp = jsonify({"revoked": True}) - if 'cookies' in current_app.appbuilder.app.config['JWT_TOKEN_LOCATION']: - unset_jwt_cookies(resp) - return resp + return jsonify({"revoked": True}) @jwt_refresh_token_required @@ -248,7 +239,4 @@ def revoke_current_user_refresh_token(): TokenBlockList.add_token(jti=raw_jwt['jti'], expiry_delta=raw_jwt['exp']) except IntegrityError: pass - resp = jsonify({"revoked": True}) - if 'cookies' in current_app.appbuilder.app.config['JWT_TOKEN_LOCATION']: - unset_jwt_cookies(resp) - return resp + return jsonify({"revoked": True}) diff --git a/airflow/config_templates/default_webserver_config.py b/airflow/config_templates/default_webserver_config.py index 51ec9974c7284..7605438a77326 100644 --- a/airflow/config_templates/default_webserver_config.py +++ b/airflow/config_templates/default_webserver_config.py @@ -133,7 +133,7 @@ # Airflow uses flask_jwt_extended for handling JWT Authentication. # Please refer to https://flask-jwt-extended.readthedocs.io/en/3.0.0_release/options/ # for more configuration options. -# Note that storing token in query string is not supported +# Note that storing token in query_string or cookies is not supported JWT_TOKEN_LOCATION = ['headers'] JWT_HEADER_NAME = "Authorization" JWT_HEADER_TYPE = 'Bearer' diff --git a/airflow/www/security.py b/airflow/www/security.py index b86b04337ba6a..e68b38c3b872d 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -20,18 +20,12 @@ import warnings from typing import Dict, Optional, Sequence, Set, Tuple -from flask import current_app, g, jsonify +from flask import current_app, g from flask_appbuilder.const import AUTH_DB, AUTH_LDAP 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 ( - JWTManager, - create_access_token, - create_refresh_token, - set_access_cookies, - set_refresh_cookies, -) +from flask_jwt_extended import JWTManager, create_access_token, create_refresh_token from sqlalchemy import or_ from sqlalchemy.orm import joinedload @@ -756,15 +750,10 @@ def create_jwt_manager(self, app) -> JWTManager: jwt_manager.user_loader_callback_loader(self.load_user_jwt) return jwt_manager - def create_tokens_and_dump(self, app, user, schema): + def create_tokens_and_dump(self, user, schema): """Creates access and refresh token, return user data alongside tokens""" access_token = create_access_token(user.id) refresh_token = create_refresh_token(user.id) - if 'cookies' in app.config['JWT_TOKEN_LOCATION']: - resp = schema.dump({'user': user}) - set_access_cookies(jsonify(resp), access_token) - set_refresh_cookies(jsonify(resp), refresh_token) - return resp return schema.dump({'user': user, 'token': access_token, 'refresh_token': refresh_token}) diff --git a/tests/api_connexion/endpoints/test_auth_endpoint.py b/tests/api_connexion/endpoints/test_auth_endpoint.py index d60789e56712a..afc65cff0068b 100644 --- a/tests/api_connexion/endpoints/test_auth_endpoint.py +++ b/tests/api_connexion/endpoints/test_auth_endpoint.py @@ -18,7 +18,6 @@ import pytest from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, AUTH_REMOTE_USER -from flask_jwt_extended.utils import create_access_token, create_refresh_token from sqlalchemy import func from airflow.models.auth import TokenBlockList @@ -366,46 +365,3 @@ def test_revoke_token_works(self, session): ) total_tokens_in_db = session.query(func.count(TokenBlockList.jti)).scalar() assert total_tokens_in_db == 1 - - -class TestTokenCanBeStoredInCookies(TestLoginEndpoint): - def test_token_stored_in_cookie_in_db_login(self): - self.app.config['JWT_TOKEN_LOCATION'] = ['cookies'] - self.auth_type(AUTH_DB) - payload = {"username": "test", "password": "test"} - self.client.post('api/v1/auth/login', json=payload) - - assert all(cookie.name != "session" for cookie in self.client.cookie_jar) - expected = ['access_token_cookie', 'refresh_token_cookie', 'csrf_access_token', 'csrf_refresh_token'] - result = [cookie.name for cookie in self.client.cookie_jar] - assert sorted(expected) == sorted(result) - - def test_token_not_in_response_if_token_in_cookie(self): - self.app.config['JWT_TOKEN_LOCATION'] = ['cookies'] - self.auth_type(AUTH_DB) - payload = {"username": "test", "password": "test"} - response = self.client.post('api/v1/auth/login', json=payload) - assert not response.json.get('refresh_token') - assert not response.json.get('access_token') - # Ensure cookie was set - assert [cookie.name for cookie in self.client.cookie_jar] - - def test_token_can_view_other_endpoints_if_token_in_cookie(self): - self.app.config['JWT_TOKEN_LOCATION'] = ['cookies'] - self.auth_type(AUTH_DB) - user = self.app.appbuilder.sm.find_user(username='test') - with self.app.app_context(): - token = create_access_token(user.id) - self.client.set_cookie("localhost", 'access_token_cookie', token) - response = self.client.get("/api/v1/pools") - assert response.status_code == 200 - - def test_refresh_works_for_token_in_cookie(self): - self.app.config['JWT_TOKEN_LOCATION'] = ['cookies'] - self.auth_type(AUTH_DB) - user = self.app.appbuilder.sm.find_user(username='test') - with self.app.app_context(): - token = create_refresh_token(user.id) - self.client.set_cookie("localhost", 'refresh_token_cookie', token) - response = self.client.get("/api/v1/refresh") - assert response.status_code == 200 From da25fa163f149f2e7da0cfb49d5d60c4f8ddc054 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 23 Apr 2021 16:37:05 +0100 Subject: [PATCH 50/53] fix test --- airflow/models/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow/models/auth.py b/airflow/models/auth.py index 413754328e80a..4ac782f77d70a 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -47,7 +47,7 @@ def get_token(cls, token, session=None): @provide_session def delete_token(cls, token, session=None): """Delete a token""" - session.query(cls).get(token).delete() + session.query(cls).filter(cls.jti == token).delete() @classmethod @provide_session From 7dcd433dba1a08e605bf27500763c32e605b6d4c Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Fri, 23 Apr 2021 18:06:25 +0100 Subject: [PATCH 51/53] fixup! fix test --- airflow/models/auth.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/airflow/models/auth.py b/airflow/models/auth.py index 4ac782f77d70a..a295e56b210dc 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -16,7 +16,7 @@ # under the License. from pendulum import from_timestamp -from sqlalchemy import Column, DateTime, String +from sqlalchemy import Column, DateTime, Index, String from airflow.models.base import Base from airflow.utils.log.logging_mixin import LoggingMixin @@ -35,7 +35,8 @@ class TokenBlockList(Base, LoggingMixin): __tablename__ = 'token_blocklist' jti = Column(String(50), nullable=False, primary_key=True) - expiry_date = Column(DateTime(), nullable=False, index=True) + expiry_date = Column(DateTime(), nullable=False) + __table_args__ = (Index('idx_expiry_date_token_blocklist', expiry_date),) @classmethod @provide_session From a507911ff84d41f6fbeae957f43f16babc2858eb Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 26 Apr 2021 17:12:37 +0100 Subject: [PATCH 52/53] add method to delete expired tokens --- airflow/models/auth.py | 8 ++++++++ tests/models/test_auth.py | 9 +++++++++ 2 files changed, 17 insertions(+) diff --git a/airflow/models/auth.py b/airflow/models/auth.py index a295e56b210dc..130ddaa5504f3 100644 --- a/airflow/models/auth.py +++ b/airflow/models/auth.py @@ -15,6 +15,8 @@ # specific language governing permissions and limitations # under the License. +from datetime import datetime + from pendulum import from_timestamp from sqlalchemy import Column, DateTime, Index, String @@ -58,3 +60,9 @@ def add_token(cls, jti, expiry_delta, session=None): token = cls(jti=jti, expiry_date=from_timestamp(expiry_delta)) session.add(token) session.commit() + + @classmethod + @provide_session + def delete_expired_tokens(cls, session=None): + """Delete all expired tokens""" + session.query(cls).filter(cls.expiry_date <= datetime.now()).delete() diff --git a/tests/models/test_auth.py b/tests/models/test_auth.py index 7937330a680f8..8ddeed0e6e177 100644 --- a/tests/models/test_auth.py +++ b/tests/models/test_auth.py @@ -52,3 +52,12 @@ def test_delete_token_method(self): TokenBlockList.delete_token("token") token = TokenBlockList.get_token("token") assert token is None + + def test_delete_expired_tokens(self): + TokenBlockList.add_token(jti="token", expiry_delta=EXPIRY_DELTA) + TokenBlockList.add_token(jti="token2", expiry_delta=EXPIRY_DELTA) + assert TokenBlockList.get_token('token') + assert TokenBlockList.get_token('token2') + TokenBlockList.delete_expired_tokens() + assert TokenBlockList.get_token('token') is None + assert TokenBlockList.get_token('token2') is None From 60988fbf32f037beb8758a8c80b2916dcbb9defb Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 27 Apr 2021 20:06:59 +0100 Subject: [PATCH 53/53] Delete expired tokens at startup --- airflow/www/extensions/init_views.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/airflow/www/extensions/init_views.py b/airflow/www/extensions/init_views.py index 14e532b7ca9be..6226bdc1d118a 100644 --- a/airflow/www/extensions/init_views.py +++ b/airflow/www/extensions/init_views.py @@ -25,6 +25,7 @@ from airflow.api_connexion.exceptions import common_error_handler from airflow.configuration import conf +from airflow.models import TokenBlockList from airflow.security import permissions from airflow.www.views import lazy_add_provider_discovered_options_to_connection_form @@ -156,6 +157,11 @@ def set_cors_headers_on_response(response): return response +def delete_expired_tokens(): + """Function called before first API request""" + TokenBlockList.delete_expired_tokens() + + def init_api_connexion(app: Flask) -> None: """Initialize Stable API""" base_path = '/api/v1' @@ -185,6 +191,7 @@ def _handle_api_error(ex): app.after_request_funcs.setdefault(api_bp.name, []).append(set_cors_headers_on_response) app.register_error_handler(ProblemException, common_error_handler) app.extensions['csrf'].exempt(api_bp) + app.before_first_request(delete_expired_tokens) def init_api_experimental(app):