You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/02/13 00:30:26 UTC
[GitHub] [airflow] ephraimbuddy opened a new pull request #14219: Provide login endpoint for the REST API with JWT authentication method
ephraimbuddy opened a new pull request #14219:
URL: https://github.com/apache/airflow/pull/14219
This PR seeks to add `/login` endpoint with jwt authentication backend. This will help us to build a new UI using the REST API.
The JWT authentication works alongside other auth_backends.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583983148
##########
File path: airflow/api_connexion/webserver_auth.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from datetime import datetime, timedelta
+from functools import wraps
+from typing import Callable, TypeVar, cast
+
+import jwt
+from flask import Response, current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_login import login_user
+
+from airflow.configuration import conf
+
+SECRET = conf.get("webserver", "secret_key")
+T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
+
+
+def encode_auth_token(user_id):
+ """Generate authentication token"""
+ expire_time = conf.getint("webserver", "session_lifetime_minutes")
+ payload = {
+ 'exp': datetime.utcnow() + timedelta(minutes=expire_time),
+ 'iat': datetime.utcnow(),
+ 'sub': user_id,
+ }
+ return jwt.encode(payload, SECRET, algorithm='HS256')
+
+
+def decode_auth_token(auth_token):
+ """Decode authentication token"""
+ try:
+ payload = jwt.decode(auth_token, SECRET, algorithms=['HS256'])
+ return payload['sub']
+ except (jwt.ExpiredSignatureError, jwt.InvalidTokenError):
+ # Do not raise exception here due to auth_backend auth
+ return None
+
+
+def auth_current_user():
+ """Checks the authentication and return the current user"""
+ auth_header = request.headers.get("Authorization", None)
+ ab_security_manager = current_app.appbuilder.sm
+ token = None
+ user = None
+ if auth_header:
+ try:
+ token = auth_header.split(" ")[1]
+ except IndexError:
+ # Do not raise exception here due to auth_backend auth
+ return None
+ if auth_header and auth_header.startswith("Basic"):
+ auth = request.authorization
+ if auth is None or not (auth.username and auth.password):
+ return None
+ if ab_security_manager.auth_type == AUTH_LDAP:
+ user = ab_security_manager.auth_user_ldap(auth.username, auth.password)
+ if user is None:
+ user = ab_security_manager.auth_user_db(auth.username, auth.password)
+ if user is not None:
+ login_user(user, remember=False)
+ return user
+ if token:
Review comment:
```suggestion
if auth_header.startswith("Bearer"):
```
##########
File path: airflow/api_connexion/webserver_auth.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from datetime import datetime, timedelta
+from functools import wraps
+from typing import Callable, TypeVar, cast
+
+import jwt
+from flask import Response, current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_login import login_user
+
+from airflow.configuration import conf
+
+SECRET = conf.get("webserver", "secret_key")
+T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
+
+
+def encode_auth_token(user_id):
+ """Generate authentication token"""
+ expire_time = conf.getint("webserver", "session_lifetime_minutes")
+ payload = {
+ 'exp': datetime.utcnow() + timedelta(minutes=expire_time),
+ 'iat': datetime.utcnow(),
+ 'sub': user_id,
+ }
+ return jwt.encode(payload, SECRET, algorithm='HS256')
+
+
+def decode_auth_token(auth_token):
+ """Decode authentication token"""
+ try:
+ payload = jwt.decode(auth_token, SECRET, algorithms=['HS256'])
+ return payload['sub']
+ except (jwt.ExpiredSignatureError, jwt.InvalidTokenError):
+ # Do not raise exception here due to auth_backend auth
+ return None
+
+
+def auth_current_user():
+ """Checks the authentication and return the current user"""
+ auth_header = request.headers.get("Authorization", None)
+ ab_security_manager = current_app.appbuilder.sm
+ token = None
+ user = None
+ if auth_header:
+ try:
+ token = auth_header.split(" ")[1]
+ except IndexError:
+ # Do not raise exception here due to auth_backend auth
+ return None
+ if auth_header and auth_header.startswith("Basic"):
+ auth = request.authorization
+ if auth is None or not (auth.username and auth.password):
+ return None
+ if ab_security_manager.auth_type == AUTH_LDAP:
+ user = ab_security_manager.auth_user_ldap(auth.username, auth.password)
+ if user is None:
+ user = ab_security_manager.auth_user_db(auth.username, auth.password)
+ if user is not None:
+ login_user(user, remember=False)
+ return user
+ if token:
Review comment:
```suggestion
if auth_header.startswith("Bearer "):
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r576281594
##########
File path: airflow/api_connexion/security.py
##########
@@ -14,24 +14,27 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-
from functools import wraps
from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast
from flask import Response, current_app
from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated
+from airflow.api_connexion.webserver_auth import requires_authentication
T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
def check_authentication() -> None:
"""Checks that the request has valid authorization information."""
response = current_app.api_auth.requires_authentication(Response)()
- if response.status_code != 200:
+ jwt_response = requires_authentication(Response)()
Review comment:
I'm trying to figure out the best way to plug the jwt into the app. What I did here was to try both auth_backend and jwt_response. I have updated the failing path which made the auth_backend from 2.0.0/.1 unusable. Now both the old auth and the new one works. The major problem now is that all authentication failures now appear as 401. I'm still thinking around it because it'll be good to tell the user that the token expired, invalid token or malformed authorization code.
If you can check again to see that both now work. I also added a comment on why error is not raised for expired token
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-790869420
[The Workflow run](https://github.com/apache/airflow/actions/runs/622127230) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583971505
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/User'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
+ type: object
+ properties:
+ roles:
+ type: array
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+
+ PermissionCollectionItem:
+ description: Permission Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The permission ID
+ name:
+ type: string
+ description: The name of the permission
+ nullable: false
+
+ PermissionCollection:
+ description: Permission Collection
+ type: object
+ properties:
+ actions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionCollectionItem'
+
+ PermissionView:
+ description: Permission view item
+ type: object
+ properties:
+ id:
Review comment:
```suggestion
permission_id:
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r597874688
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -2364,73 +2473,6 @@ components:
$ref: '#/components/schemas/PluginCollectionItem'
- $ref: '#/components/schemas/CollectionInfo'
- Role:
- description: Role item
- type: object
- properties:
- name:
- type: string
- description: The name of the role
- actions:
- type: array
- items:
- $ref: '#/components/schemas/ActionResource'
-
- RoleCollection:
- description: Role Collections
- type: object
- allOf:
- - type: object
- properties:
- roles:
- type: array
- items:
- $ref: '#/components/schemas/Role'
- - $ref: '#/components/schemas/CollectionInfo'
-
- Action:
- description: Action Item
- type: object
- properties:
- name:
- type: string
- description: The name of the permission "action"
- nullable: false
-
- ActionCollection:
- description: Action Collection
- type: object
- allOf:
- - type: object
- properties:
- actions:
- type: array
- items:
- $ref: '#/components/schemas/Action'
- - $ref: '#/components/schemas/CollectionInfo'
-
- Resource:
- description: A "resource" on which permissions are granted.
- type: object
- properties:
- name:
- type: string
- description: The name of the resource
- nullable: false
-
- ActionResource:
- description: The Action-Resource item
- type: object
- properties:
- action:
- type: object
- $ref: '#/components/schemas/Action'
- description: The permission action
- resource:
- type: object
- $ref: '#/components/schemas/Resource'
- description: The permission resource
-
Review comment:
Moved Role definitions to database entities. Previously was under python objects
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-778767078
[The Workflow run](https://github.com/apache/airflow/actions/runs/565773219) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r576281594
##########
File path: airflow/api_connexion/security.py
##########
@@ -14,24 +14,27 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-
from functools import wraps
from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast
from flask import Response, current_app
from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated
+from airflow.api_connexion.webserver_auth import requires_authentication
T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
def check_authentication() -> None:
"""Checks that the request has valid authorization information."""
response = current_app.api_auth.requires_authentication(Response)()
- if response.status_code != 200:
+ jwt_response = requires_authentication(Response)()
Review comment:
I'm trying to figure out the best way to plug the jwt into the app. What I did here was to try both auth_backend and jwt_response. I have updated the failing path which made the auth_backend from 2.0.0/.1 unusable. Now both the old auth and the new one works. The major problem now is that all authentication failures now appear as 401. I'm still thinking around it because it'll be good to tell the user that the token expired, invalid token or malformed authorization code.
If you can check again to see that both now works. I also added a comment on why error is not raised for expired token
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-797140566
Depends on #14735 so we can properly model user schema
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r579382844
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,184 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/User'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
Review comment:
Is a RoleCollection different from a Role? Naming seems ambiguous
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583972702
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserLogin:
+ description: Login item
+ allOf:
Review comment:
Do we have to use `allOf` here? I think it will be more understandable if we just return one object with two keys user, token. The `allOf` property is part of a fairly new standard and is sometimes problematic. Besides, the user will now be able to easily overlook that a given endpoint does not return user data, but returns user + token.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r575793854
##########
File path: airflow/api_connexion/endpoints/user_endpoint.py
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from flask import current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_login import login_user
+from marshmallow import ValidationError
+
+from airflow.api_connexion.exceptions import BadRequest
+from airflow.api_connexion.schemas.user_schema import auth_login_shcema, logged_in_user_data_schema
+from airflow.api_connexion.webserver_auth import encode_auth_token
+
+
+def login():
+ """User login"""
+ body = request.json
+ try:
+ data = auth_login_shcema.load(body)
+ except ValidationError as err:
+ raise BadRequest(detail=str(err.messages))
+ username = data["username"]
+ password = data['password']
+ ab_security_manager = current_app.appbuilder.sm
+ user = None
+ if ab_security_manager.auth_type == AUTH_LDAP:
+ user = ab_security_manager.auth_user_ldap(username, password)
+ if user is None:
+ user = ab_security_manager.auth_user_db(username, password)
+ if user:
+ token = encode_auth_token(user.id)
+ login_user(user)
+ return logged_in_user_data_schema.dump(dict(user=user, token=token))
Review comment:
Done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy closed pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy closed pull request #14219:
URL: https://github.com/apache/airflow/pull/14219
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-778528892
[The Workflow run](https://github.com/apache/airflow/actions/runs/562624266) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-779536185
[The Workflow run](https://github.com/apache/airflow/actions/runs/570223176) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583090567
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,198 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Login'
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ UserCollectionItem:
+ description: >
+ User collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserCollection:
+ description: User collection
+ type: object
+ properties:
+ users:
+ type: array
+ items:
+ $ref: '#/components/schemas/UserCollectionItem'
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/UserCollectionItem'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
+ type: object
+ properties:
+ roles:
+ type: array
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+
+ PermissionCollectionItem:
+ description: Permission Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The permission ID
+ name:
+ type: string
+ description: The name of the permission
+ nullable: false
+
+ PermissionCollection:
+ description: Permission Collection
+ type: object
+ properties:
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionCollectionItem'
+
+ PermissionView:
+ description: Permission view item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The PermissionView ID
+ permission_id:
+ type: string
+ description: The permission ID
+ permission:
+ type: string
+ description: The name of the permission
+ view_menu_id:
+ type: string
+ description: The view menu id
+ view_menu_name:
+ type: string
+ description: The view menu name
+
+ ViewMenuCollectionItem:
+ description: ViewMenu Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The ViewMenu ID
+ name:
+ type: string
+ description: The name of the ViewMenu
+ nullable: false
+
+ ViewMenuCollection:
+ description: ViewMenu Collection
+ type: object
+ properties:
+ view_menus:
+ type: array
+ items:
+ $ref: '#/components/schemas/ViewMenuCollectionItem'
Review comment:
@jhtimmins what do you think about roles coming out like this:
```
"roles": [
{
"id": 1,
"name": "Admin",
"permissions": [
{
"action": {
"id": 3,
"name": "can_delete"
},
"id": 1,
"resource": {
"id": 1,
"name": "Connections"
}
},
{
"action": {
"id": 1,
"name": "can_read"
},
"id": 2,
"resource": {
"id": 1,
"name": "Connections"
}
},
]
}
]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-781643451
[The Workflow run](https://github.com/apache/airflow/actions/runs/579402990) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-790754604
[The Workflow run](https://github.com/apache/airflow/actions/runs/621661103) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r580911375
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,198 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Login'
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ UserCollectionItem:
+ description: >
+ User collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserCollection:
+ description: User collection
+ type: object
+ properties:
+ users:
+ type: array
+ items:
+ $ref: '#/components/schemas/UserCollectionItem'
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/UserCollectionItem'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
+ type: object
+ properties:
+ roles:
+ type: array
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+
+ PermissionCollectionItem:
+ description: Permission Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The permission ID
+ name:
+ type: string
+ description: The name of the permission
+ nullable: false
+
+ PermissionCollection:
+ description: Permission Collection
+ type: object
+ properties:
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionCollectionItem'
+
+ PermissionView:
+ description: Permission view item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The PermissionView ID
+ permission_id:
+ type: string
+ description: The permission ID
+ permission:
+ type: string
+ description: The name of the permission
+ view_menu_id:
+ type: string
+ description: The view menu id
+ view_menu_name:
+ type: string
+ description: The view menu name
+
+ ViewMenuCollectionItem:
+ description: ViewMenu Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The ViewMenu ID
+ name:
+ type: string
+ description: The name of the ViewMenu
+ nullable: false
+
+ ViewMenuCollection:
+ description: ViewMenu Collection
+ type: object
+ properties:
+ view_menus:
+ type: array
+ items:
+ $ref: '#/components/schemas/ViewMenuCollectionItem'
Review comment:
How can we model this in a schema @jhtimmins
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-782012804
Taking a look
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583972702
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserLogin:
+ description: Login item
+ allOf:
Review comment:
Do we have to use `allOf` here? I think it will be more understandable if we just return one object with two keys user, token. The `allOf` property is part of a fairly new standard and is sometimes problematic.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-808516140
closing this for #15042
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-782004419
Currently, I believe this can handle any type of authentication. The /login endpoint just looks for current user in security manager, and if there is, it generates a token.
If the endpoint is called with basic authentication header, it verifies and logs the user in and then generates the auth token. If not called with any authentication header, it checks for current_user which might have logged in through Oauth and then generate a token. If no user is logged in, it raises 404.
I have a unittest where I used `remote user` and it works fine.
Any thoughts on this implementation?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r586079000
##########
File path: tests/api_connexion/test_webserver_auth.py
##########
@@ -0,0 +1,129 @@
+# 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 base64 import b64encode
+from datetime import datetime, timedelta
+
+import jwt
+from flask_login import current_user
+from parameterized import parameterized
+
+from airflow.www.app import create_app
+from tests.test_utils.api_connexion_utils import assert_401
+from tests.test_utils.config import conf_vars
+from tests.test_utils.db import clear_db_pools
+
+
+class TestWebserverAuth(unittest.TestCase):
+ def setUp(self) -> None:
+ with conf_vars({("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend"}):
+ self.app = create_app(testing=True)
+
+ self.appbuilder = self.app.appbuilder # pylint: disable=no-member
+ role_admin = self.appbuilder.sm.find_role("Admin")
+ self.tester = self.appbuilder.sm.find_user(username="test")
+ if not self.tester:
+ self.appbuilder.sm.add_user(
+ username="test",
+ first_name="test",
+ last_name="test",
+ email="test@fab.org",
+ role=role_admin,
+ password="test",
+ )
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
Review comment:
In my tests, I keep getting several pools that I didn't expect, I decided to clear it, I will try again and see what is actually happening
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583371087
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,198 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Login'
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ UserCollectionItem:
+ description: >
+ User collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserCollection:
+ description: User collection
+ type: object
+ properties:
+ users:
+ type: array
+ items:
+ $ref: '#/components/schemas/UserCollectionItem'
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/UserCollectionItem'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
+ type: object
+ properties:
+ roles:
+ type: array
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+
+ PermissionCollectionItem:
+ description: Permission Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The permission ID
+ name:
+ type: string
+ description: The name of the permission
+ nullable: false
+
+ PermissionCollection:
+ description: Permission Collection
+ type: object
+ properties:
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionCollectionItem'
+
+ PermissionView:
+ description: Permission view item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The PermissionView ID
+ permission_id:
+ type: string
+ description: The permission ID
+ permission:
+ type: string
+ description: The name of the permission
+ view_menu_id:
+ type: string
+ description: The view menu id
+ view_menu_name:
+ type: string
+ description: The view menu name
+
+ ViewMenuCollectionItem:
+ description: ViewMenu Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The ViewMenu ID
+ name:
+ type: string
+ description: The name of the ViewMenu
+ nullable: false
+
+ ViewMenuCollection:
+ description: ViewMenu Collection
+ type: object
+ properties:
+ view_menus:
+ type: array
+ items:
+ $ref: '#/components/schemas/ViewMenuCollectionItem'
Review comment:
@ephraimbuddy This looks great!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r575621235
##########
File path: airflow/api_connexion/endpoints/user_endpoint.py
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from flask import current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_login import login_user
+from marshmallow import ValidationError
+
+from airflow.api_connexion.exceptions import BadRequest
+from airflow.api_connexion.schemas.user_schema import auth_login_shcema, logged_in_user_data_schema
+from airflow.api_connexion.webserver_auth import encode_auth_token
+
+
+def login():
+ """User login"""
+ body = request.json
+ try:
+ data = auth_login_shcema.load(body)
+ except ValidationError as err:
+ raise BadRequest(detail=str(err.messages))
+ username = data["username"]
+ password = data['password']
+ ab_security_manager = current_app.appbuilder.sm
+ user = None
+ if ab_security_manager.auth_type == AUTH_LDAP:
+ user = ab_security_manager.auth_user_ldap(username, password)
+ if user is None:
+ user = ab_security_manager.auth_user_db(username, password)
+ if user:
+ token = encode_auth_token(user.id)
+ login_user(user)
+ return logged_in_user_data_schema.dump(dict(user=user, token=token))
Review comment:
What will happened if `user` is `None`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-790682313
Can someone review the authentication once more? I feel it's OK now except for the user data schemas.
The way I was writing the schemas before was to emulate how it is as SQLAlchemy model with the consciousness that those schemas can be used in other endpoints e.g Role listing, adding, and updating, same with User endpoints=> Lists, Add, Edit etc.
That made me to map them out as RoleCollectionItem(for adding and updating role endpoints), RoleCollection(for listing all roles), UserCollectionItem(for dumping user info), User(with sensitive data for creating new user), UserCollections(for get users endpoint), etc. But I think it can be modified when we work on those endpoints
So, I have modified the schemas with only the things necessary for this authentication and also added @jhtimmins suggestion of using Action in place of Permission, Resource in place of View_Menu.
Please take a look.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r579407959
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,184 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/User'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
Review comment:
RoleCollection is just a list of Roles. I used it because most components are named similarly. ConnectionCollection, PoolCollection etc.
What do you suggest?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-779253145
[The Workflow run](https://github.com/apache/airflow/actions/runs/568756160) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583982975
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/User'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
Review comment:
I think it's needed since we need a list of Roles sometimes and also need the ability to create a new role/edit role. List/add/edit kind of thing. It's not implemented in this PR but it'll be needed for roles and permissions endpoint.
What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r579390290
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,198 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Login'
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ UserCollectionItem:
+ description: >
+ User collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserCollection:
+ description: User collection
+ type: object
+ properties:
+ users:
+ type: array
+ items:
+ $ref: '#/components/schemas/UserCollectionItem'
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/UserCollectionItem'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
+ type: object
+ properties:
+ roles:
+ type: array
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+
+ PermissionCollectionItem:
+ description: Permission Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The permission ID
+ name:
+ type: string
+ description: The name of the permission
+ nullable: false
+
+ PermissionCollection:
+ description: Permission Collection
+ type: object
+ properties:
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionCollectionItem'
+
+ PermissionView:
+ description: Permission view item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The PermissionView ID
+ permission_id:
+ type: string
+ description: The permission ID
+ permission:
+ type: string
+ description: The name of the permission
+ view_menu_id:
+ type: string
+ description: The view menu id
+ view_menu_name:
+ type: string
+ description: The view menu name
+
+ ViewMenuCollectionItem:
+ description: ViewMenu Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The ViewMenu ID
+ name:
+ type: string
+ description: The name of the ViewMenu
+ nullable: false
+
+ ViewMenuCollection:
+ description: ViewMenu Collection
+ type: object
+ properties:
+ view_menus:
+ type: array
+ items:
+ $ref: '#/components/schemas/ViewMenuCollectionItem'
Review comment:
In the long run, I think it will make sense to get away from FAB and Flask altogether. To do that we can abstract away the security side of FAB into its own module (so yeah we'll basically have to copy/redo parts of FAB), then we can use the custom security module to whatever new framework we use. The existing architecture and updated names should give a solid base for this transition.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-778770832
[The Workflow run](https://github.com/apache/airflow/actions/runs/565857210) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-790754193
[The Workflow run](https://github.com/apache/airflow/actions/runs/621651041) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-790868876
[The Workflow run](https://github.com/apache/airflow/actions/runs/622105358) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-781394134
[The Workflow run](https://github.com/apache/airflow/actions/runs/578371326) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r599451442
##########
File path: airflow/config_templates/config.yml
##########
@@ -728,6 +728,27 @@
type: string
example: ~
default: "airflow.api.auth.backend.deny_all"
+ - name: jwt_access_token_expires
+ description: How long an access token should live before it expires(in minutes).
+ version_added: 2.0.2
Review comment:
2.1 for new features.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-779911104
[The Workflow run](https://github.com/apache/airflow/actions/runs/571938198) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583971433
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/User'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
+ type: object
+ properties:
+ roles:
+ type: array
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+
+ PermissionCollectionItem:
+ description: Permission Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The permission ID
+ name:
+ type: string
+ description: The name of the permission
+ nullable: false
+
+ PermissionCollection:
+ description: Permission Collection
+ type: object
+ properties:
+ actions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionCollectionItem'
+
+ PermissionView:
+ description: Permission view item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The PermissionView ID
+ action:
+ type: string
+ description: The name of the permission
+ resource:
+ type: string
+ description: The resource name
+
+ ResourceCollectionItem:
+ description: Resource Collection Item
+ type: object
+ properties:
+ id:
Review comment:
```suggestion
resource_id:
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583970536
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
Review comment:
```suggestion
user_id:
```
For consistency.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583985209
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -2910,6 +3077,7 @@ tags:
- name: TaskInstance
- name: Variable
- name: XCom
+ - name: User
externalDocs:
Review comment:
Should we also update security section?
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#securityRequirementObject
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-791232030
[The Workflow run](https://github.com/apache/airflow/actions/runs/623738817) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r575792101
##########
File path: airflow/api_connexion/endpoints/user_endpoint.py
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from flask import current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_login import login_user
+from marshmallow import ValidationError
+
+from airflow.api_connexion.exceptions import BadRequest
+from airflow.api_connexion.schemas.user_schema import auth_login_shcema, logged_in_user_data_schema
+from airflow.api_connexion.webserver_auth import encode_auth_token
+
+
+def login():
+ """User login"""
+ body = request.json
+ try:
+ data = auth_login_shcema.load(body)
+ except ValidationError as err:
+ raise BadRequest(detail=str(err.messages))
+ username = data["username"]
+ password = data['password']
+ ab_security_manager = current_app.appbuilder.sm
+ user = None
+ if ab_security_manager.auth_type == AUTH_LDAP:
+ user = ab_security_manager.auth_user_ldap(username, password)
+ if user is None:
+ user = ab_security_manager.auth_user_db(username, password)
+ if user:
+ token = encode_auth_token(user.id)
+ login_user(user)
+ return logged_in_user_data_schema.dump(dict(user=user, token=token))
Review comment:
Thanks, a commit is on the way. :+1:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-794574040
[The Workflow run](https://github.com/apache/airflow/actions/runs/637386344) is cancelling this PR. Building images for the PR has failed. Follow the workflow link to check the reason.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r579390290
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,198 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Login'
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ UserCollectionItem:
+ description: >
+ User collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserCollection:
+ description: User collection
+ type: object
+ properties:
+ users:
+ type: array
+ items:
+ $ref: '#/components/schemas/UserCollectionItem'
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/UserCollectionItem'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
+ type: object
+ properties:
+ roles:
+ type: array
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+
+ PermissionCollectionItem:
+ description: Permission Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The permission ID
+ name:
+ type: string
+ description: The name of the permission
+ nullable: false
+
+ PermissionCollection:
+ description: Permission Collection
+ type: object
+ properties:
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionCollectionItem'
+
+ PermissionView:
+ description: Permission view item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The PermissionView ID
+ permission_id:
+ type: string
+ description: The permission ID
+ permission:
+ type: string
+ description: The name of the permission
+ view_menu_id:
+ type: string
+ description: The view menu id
+ view_menu_name:
+ type: string
+ description: The view menu name
+
+ ViewMenuCollectionItem:
+ description: ViewMenu Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The ViewMenu ID
+ name:
+ type: string
+ description: The name of the ViewMenu
+ nullable: false
+
+ ViewMenuCollection:
+ description: ViewMenu Collection
+ type: object
+ properties:
+ view_menus:
+ type: array
+ items:
+ $ref: '#/components/schemas/ViewMenuCollectionItem'
Review comment:
In the long run, I think it will make sense to get away from FAB and Flask altogether. To do that we can abstract away the security side of FAB into its own module (so yeah we'll basically have to copy/redo parts of FAB), then we can use the custom security module in whatever new framework we use. The existing architecture and updated names should give a solid base for this transition.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r584519426
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserLogin:
+ description: Login item
+ allOf:
Review comment:
I'm lost at what to return for the user part. The way I was thinking about this was to return all necessary info about the user so it can be used to check if the user has permissions on certain items on the UI, and also used to update some user info in the UI. I'm thinking about it towards the new UI.
Would be glad to know what to return for the user part and your thoughts on what I'm thinking above
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-789407426
I have switched to using flask_jwt_extension. This offered a lot of suggestions on how to implement jwt auth which are better than what I did before. I'm following this [advise](https://flask-jwt-extended.readthedocs.io/en/3.0.0_release/tokens_in_cookies/ ) of storing the token in cookies.
I currently removed user data and will add it back once I got this correctly
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r586077844
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/User'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
Review comment:
Noted
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r588481070
##########
File path: tests/api_connexion/endpoints/test_auth_endpoint.py
##########
@@ -0,0 +1,96 @@
+# 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 base64 import b64encode
+
+from airflow.www import app
+from tests.test_utils.api_connexion_utils import create_user, delete_user
+from tests.test_utils.config import conf_vars
+
+TEST_USERNAME = "test"
+
+
+class TestLoginEndpoint(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls) -> None:
+ super().setUpClass()
+ with conf_vars(
+ {
+ ("webserver", "session_lifetime_minutes"): "1",
Review comment:
```suggestion
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583976602
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/User'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
Review comment:
Is this suffix `Item`/`CollectionIItem` needed? I think `Role` is enough. In other places, we've used suffixes consistently. For list, we used `Collection` suffix. For a simplified representation of an object, which also had full representations, we used the `CollectionItem` suffix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583982852
##########
File path: airflow/api_connexion/webserver_auth.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from datetime import datetime, timedelta
+from functools import wraps
+from typing import Callable, TypeVar, cast
+
+import jwt
+from flask import Response, current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_login import login_user
+
+from airflow.configuration import conf
+
+SECRET = conf.get("webserver", "secret_key")
+T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
+
+
+def encode_auth_token(user_id):
+ """Generate authentication token"""
+ expire_time = conf.getint("webserver", "session_lifetime_minutes")
+ payload = {
+ 'exp': datetime.utcnow() + timedelta(minutes=expire_time),
+ 'iat': datetime.utcnow(),
+ 'sub': user_id,
+ }
+ return jwt.encode(payload, SECRET, algorithm='HS256')
+
+
+def decode_auth_token(auth_token):
+ """Decode authentication token"""
+ try:
+ payload = jwt.decode(auth_token, SECRET, algorithms=['HS256'])
+ return payload['sub']
+ except (jwt.ExpiredSignatureError, jwt.InvalidTokenError):
+ # Do not raise exception here due to auth_backend auth
+ return None
+
+
+def auth_current_user():
+ """Checks the authentication and return the current user"""
+ auth_header = request.headers.get("Authorization", None)
+ ab_security_manager = current_app.appbuilder.sm
+ token = None
+ user = None
+ if auth_header:
+ try:
+ token = auth_header.split(" ")[1]
+ except IndexError:
+ # Do not raise exception here due to auth_backend auth
+ return None
+ if auth_header and auth_header.startswith("Basic"):
Review comment:
``if auth_token`` This is repeated twice.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r599465261
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1490,6 +1490,48 @@ paths:
'404':
$ref: '#/components/responses/NotFound'
+ /login:
+ post:
+ summary: Webserver login
+ description: |
+ Verify user and set jwt token in cookies
+ x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint
+ operationId: login
+ tags: [WebserverAuth]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/User'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
+
+ /logout:
+ post:
+ summary: Webserver logout
+ description: Logout user by unsetting cookies
+ x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint
+ operationId: logout
+ tags: [WebserverAuth]
Review comment:
```suggestion
tags: [Authentication]
```
##########
File path: airflow/api_connexion/webserver_auth.py
##########
@@ -0,0 +1,57 @@
+# 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 logging
+from typing import Callable, TypeVar
+
+from flask import current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_jwt_extended import verify_jwt_in_request
+from flask_login import login_user
+
+from airflow.configuration import conf
+
+SECRET = conf.get("webserver", "secret_key")
+T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
+
+log = logging.getLogger(__name__)
+
+
+def auth_current_user():
Review comment:
This isn't strictly only tied to webserver use of the API, so lets have this function live in security.py
##########
File path: airflow/api_connexion/security.py
##########
@@ -14,24 +14,27 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-
from functools import wraps
from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast
from flask import Response, current_app
from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated
+from airflow.api_connexion.webserver_auth import auth_current_user
T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
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:
- # since this handler only checks authentication, not authorization,
- # we should always return 401
- raise Unauthenticated(headers=response.headers)
+ if response.status_code == 200:
+ return
+ if auth_current_user():
Review comment:
By having this here, it means that any/all requests would accept and process `Authorization` headers, which is probably not what we want?
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1490,6 +1490,48 @@ paths:
'404':
$ref: '#/components/responses/NotFound'
+ /login:
+ post:
+ summary: Webserver login
+ description: |
+ Verify user and set jwt token in cookies
+ x-openapi-router-controller: airflow.api_connexion.endpoints.auth_endpoint
+ operationId: login
+ tags: [WebserverAuth]
Review comment:
```suggestion
tags: [Authentication]
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-778530070
[The Workflow run](https://github.com/apache/airflow/actions/runs/562637354) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-779253456
[The Workflow run](https://github.com/apache/airflow/actions/runs/568770817) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r581525397
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,198 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Login'
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ UserCollectionItem:
+ description: >
+ User collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserCollection:
+ description: User collection
+ type: object
+ properties:
+ users:
+ type: array
+ items:
+ $ref: '#/components/schemas/UserCollectionItem'
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/UserCollectionItem'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
+ type: object
+ properties:
+ roles:
+ type: array
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+
+ PermissionCollectionItem:
+ description: Permission Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The permission ID
+ name:
+ type: string
+ description: The name of the permission
+ nullable: false
+
+ PermissionCollection:
+ description: Permission Collection
+ type: object
+ properties:
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionCollectionItem'
+
+ PermissionView:
+ description: Permission view item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The PermissionView ID
+ permission_id:
+ type: string
+ description: The permission ID
+ permission:
+ type: string
+ description: The name of the permission
+ view_menu_id:
+ type: string
+ description: The view menu id
+ view_menu_name:
+ type: string
+ description: The view menu name
+
+ ViewMenuCollectionItem:
+ description: ViewMenu Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The ViewMenu ID
+ name:
+ type: string
+ description: The name of the ViewMenu
+ nullable: false
+
+ ViewMenuCollection:
+ description: ViewMenu Collection
+ type: object
+ properties:
+ view_menus:
+ type: array
+ items:
+ $ref: '#/components/schemas/ViewMenuCollectionItem'
Review comment:
@ephraimbuddy I think in the schema we can just change `ViewMenu` to `Resource` and `Permission` to `Action`. I can make a PR to update the documentation so we use the new names.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-781458129
[The Workflow run](https://github.com/apache/airflow/actions/runs/578705221) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r584756617
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/User'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
Review comment:
I agree with Kamil here, I think just calling this `Role` would be clearer (and a RoleCollection has a array of Role items in it) -- don't think we need to separation really.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r586079849
##########
File path: tests/api_connexion/test_webserver_auth.py
##########
@@ -0,0 +1,129 @@
+# 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 base64 import b64encode
+from datetime import datetime, timedelta
+
+import jwt
+from flask_login import current_user
+from parameterized import parameterized
+
+from airflow.www.app import create_app
+from tests.test_utils.api_connexion_utils import assert_401
+from tests.test_utils.config import conf_vars
+from tests.test_utils.db import clear_db_pools
+
+
+class TestWebserverAuth(unittest.TestCase):
+ def setUp(self) -> None:
+ with conf_vars({("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend"}):
+ self.app = create_app(testing=True)
+
+ self.appbuilder = self.app.appbuilder # pylint: disable=no-member
+ role_admin = self.appbuilder.sm.find_role("Admin")
+ self.tester = self.appbuilder.sm.find_user(username="test")
+ if not self.tester:
+ self.appbuilder.sm.add_user(
+ username="test",
+ first_name="test",
+ last_name="test",
+ email="test@fab.org",
+ role=role_admin,
+ password="test",
+ )
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
+
+ def test_successful_login(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ with self.app.test_client() as test_client:
+ response = test_client.post("api/v1/login", headers={"Authorization": token})
+ assert isinstance(response.json["token"], str)
+ assert response.json["user"]['email'] == "test@fab.org"
+
+ def test_can_view_other_endpoints(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ with self.app.test_client() as test_client:
+ response = test_client.post("api/v1/login", headers={"Authorization": token})
+ assert current_user.email == "test@fab.org"
+ token = response.json["token"]
+ response2 = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + token})
+ assert response2.status_code == 200
+ assert response2.json == {
+ "pools": [
+ {
+ "name": "default_pool",
+ "slots": 128,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 128,
+ },
+ ],
+ "total_entries": 1,
+ }
+
+ def test_raises_for_the_none_algorithm(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ payload = {
+ 'exp': datetime.utcnow() + timedelta(minutes=10),
+ 'iat': datetime.utcnow(),
+ 'sub': self.tester.id,
+ }
+ forgedtoken = jwt.encode(payload, key=None, algorithm=None).decode()
+ with self.app.test_client() as test_client:
+ test_client.post("api/v1/login", headers={"Authorization": token})
+ assert current_user.email == "test@fab.org"
+ response = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + forgedtoken})
+ assert response.status_code == 401
+
+ @parameterized.expand(
+ [
+ ("basic",),
+ ("basic ",),
+ ("bearer",),
+ ("test:test",),
+ (b64encode(b"test:test").decode(),),
+ ("bearer ",),
+ ("basic: ",),
+ ("basic 123",),
+ ]
+ )
+ def test_malformed_headers(self, token):
+ with self.app.test_client() as test_client:
+ response = test_client.get("/api/v1/pools", headers={"Authorization": token})
+ assert response.status_code == 401
Review comment:
Yes, it's more appropriate but because of auth_backends, every error is returned as 401 in check_authorization
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-778762478
> Can you add a little more docs?
Hi @mik-laj , I have provided more info. Let me know what you think. Thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-778593388
Can you add a little more docs?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r579385287
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,198 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Login'
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ UserCollectionItem:
+ description: >
+ User collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserCollection:
+ description: User collection
+ type: object
+ properties:
+ users:
+ type: array
+ items:
+ $ref: '#/components/schemas/UserCollectionItem'
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/UserCollectionItem'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
+ type: object
+ properties:
+ roles:
+ type: array
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+
+ PermissionCollectionItem:
+ description: Permission Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The permission ID
+ name:
+ type: string
+ description: The name of the permission
+ nullable: false
+
+ PermissionCollection:
+ description: Permission Collection
+ type: object
+ properties:
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionCollectionItem'
+
+ PermissionView:
+ description: Permission view item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The PermissionView ID
+ permission_id:
+ type: string
+ description: The permission ID
+ permission:
+ type: string
+ description: The name of the permission
+ view_menu_id:
+ type: string
+ description: The view menu id
+ view_menu_name:
+ type: string
+ description: The view menu name
+
+ ViewMenuCollectionItem:
+ description: ViewMenu Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The ViewMenu ID
+ name:
+ type: string
+ description: The name of the ViewMenu
+ nullable: false
+
+ ViewMenuCollection:
+ description: ViewMenu Collection
+ type: object
+ properties:
+ view_menus:
+ type: array
+ items:
+ $ref: '#/components/schemas/ViewMenuCollectionItem'
Review comment:
@ashb Structurally, I think FAB has a good model. Roles are made up of discrete permissions, and users can have multiple roles. It's a common design and I don't have a problem with us using it indefinitely.
I don't think it makes sense to use FAB names though. `ViewMenu` and `Permission` really only makes sense in a FAB UI-centric world. I've switched (somewhat inconsistently, I'll admit) to using `Resource` instead of `ViewMenu` and `Action` instead of `Permission`. With the new naming, a `Permission = Resource + Action`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r576173777
##########
File path: airflow/api_connexion/endpoints/user_endpoint.py
##########
@@ -0,0 +1,47 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from flask import current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_login import login_user
+from marshmallow import ValidationError
+
+from airflow.api_connexion.exceptions import BadRequest, NotFound
+from airflow.api_connexion.schemas.user_schema import auth_login_shcema, logged_in_user_data_schema
+from airflow.api_connexion.webserver_auth import encode_auth_token
+
+
+def login():
+ """User login"""
+ body = request.json
+ try:
+ data = auth_login_shcema.load(body)
+ except ValidationError as err:
+ raise BadRequest(detail=str(err.messages))
+ username = data["username"]
+ password = data['password']
+ ab_security_manager = current_app.appbuilder.sm
+ user = None
+ if ab_security_manager.auth_type == AUTH_LDAP:
+ user = ab_security_manager.auth_user_ldap(username, password)
+ if user is None:
+ user = ab_security_manager.auth_user_db(username, password)
Review comment:
This should not be handled by us -- there is likely a generic method in the SecurityManager that we can use instead.
(The problem here is that the Auth manager has ~5 different auth backends, we have only supported two of them.)
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,198 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Login'
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ UserCollectionItem:
+ description: >
+ User collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserCollection:
+ description: User collection
+ type: object
+ properties:
+ users:
+ type: array
+ items:
+ $ref: '#/components/schemas/UserCollectionItem'
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/UserCollectionItem'
Review comment:
```suggestion
- $ref: '#/components/schemas/User'
```
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,198 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Login'
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ UserCollectionItem:
+ description: >
+ User collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserCollection:
+ description: User collection
+ type: object
+ properties:
+ users:
+ type: array
+ items:
+ $ref: '#/components/schemas/UserCollectionItem'
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/UserCollectionItem'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
+ type: object
+ properties:
+ roles:
+ type: array
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+
+ PermissionCollectionItem:
+ description: Permission Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The permission ID
+ name:
+ type: string
+ description: The name of the permission
+ nullable: false
+
+ PermissionCollection:
+ description: Permission Collection
+ type: object
+ properties:
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionCollectionItem'
+
+ PermissionView:
+ description: Permission view item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The PermissionView ID
+ permission_id:
+ type: string
+ description: The permission ID
+ permission:
+ type: string
+ description: The name of the permission
+ view_menu_id:
+ type: string
+ description: The view menu id
+ view_menu_name:
+ type: string
+ description: The view menu name
+
+ ViewMenuCollectionItem:
+ description: ViewMenu Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The ViewMenu ID
+ name:
+ type: string
+ description: The name of the ViewMenu
+ nullable: false
+
+ ViewMenuCollection:
+ description: ViewMenu Collection
+ type: object
+ properties:
+ view_menus:
+ type: array
+ items:
+ $ref: '#/components/schemas/ViewMenuCollectionItem'
Review comment:
@ryanahamilton @jhtimmins Does this unduly tie is un to using FABs permissions model forever?
Do we perhaps want to re-think this and choose what/how we expose permissions without just blindly copying FAB?
##########
File path: airflow/api_connexion/security.py
##########
@@ -14,24 +14,27 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-
from functools import wraps
from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast
from flask import Response, current_app
from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated
+from airflow.api_connexion.webserver_auth import requires_authentication
T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
def check_authentication() -> None:
"""Checks that the request has valid authorization information."""
response = current_app.api_auth.requires_authentication(Response)()
- if response.status_code != 200:
+ jwt_response = requires_authentication(Response)()
Review comment:
This is not going to be accepted like this, sorry.
It means you _have_ to log in to API before you can use it which represents a massive breaking change in the behaviour to the API from 2.0.0/.1.
##########
File path: airflow/api_connexion/schemas/view_menu_schema.py
##########
@@ -0,0 +1,48 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from typing import List, NamedTuple
+
+from flask_appbuilder.security.sqla.models import ViewMenu
+from marshmallow import Schema, fields
+from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field
+
+
+class ViewMenuCollectionItemSchema(SQLAlchemySchema):
Review comment:
I'm not a huge fan of so many files -- I think all the permissions schemas (View, Role, Permissions, etc) should be in a single `schemas/permissions_schemas` or similar file.
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,198 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Login'
Review comment:
This is only ever used here, do we should just "inline" it, rather than making a schema for it.
_If_ we even have parameters for it, rather than just let the auth backend handle it (i.e. the `Authorization` header)
##########
File path: tests/api_connexion/test_webserver_auth.py
##########
@@ -0,0 +1,75 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import unittest
+
+from flask_login import current_user
+
+from airflow.www.app import create_app
+from tests.test_utils.config import conf_vars
+
+
+class TestWebserverAuth(unittest.TestCase):
+ def setUp(self) -> None:
+ with conf_vars(
+ {("webserver", "session_lifetime_minutes"): "1", ("webserver", "secret_key"): "secret"}
+ ):
+ self.app = create_app(testing=True)
+
+ self.appbuilder = self.app.appbuilder # pylint: disable=no-member
+ role_admin = self.appbuilder.sm.find_role("Admin")
+ self.tester = self.appbuilder.sm.find_user(username="test")
+ if not self.tester:
+ self.appbuilder.sm.add_user(
+ username="test",
+ first_name="test",
+ last_name="test",
+ email="test@fab.org",
+ role=role_admin,
+ password="test",
+ )
+
+ def test_successful_login(self):
+ payload = {"username": "test", "password": "test"}
+ with self.app.test_client() as test_client:
+ response = test_client.post("api/v1/login", json=payload)
+ assert isinstance(response.json["token"], str)
+ assert response.json["user"]['email'] == "test@fab.org"
+
+ def test_can_view_other_endpoints(self):
+ payload = {"username": "test", "password": "test"}
+ with self.app.test_client() as test_client:
+ response = test_client.post("api/v1/login", json=payload)
+ token = response.json["token"]
+
+ response2 = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + token})
+ assert current_user.email == "test@fab.org"
+
+ assert response2.status_code == 200
+ assert response2.json == {
+ "pools": [
+ {
+ "name": "default_pool",
+ "slots": 128,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 128,
+ },
+ ],
+ "total_entries": 1,
+ }
Review comment:
Please add a test that _ensures_ that we never accept a JWT header that uses the `none` algorithm https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,198 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Login'
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ UserCollectionItem:
+ description: >
+ User collection item
Review comment:
```suggestion
User:
description: >
A User object
```
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,198 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Login'
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ UserCollectionItem:
+ description: >
+ User collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserCollection:
+ description: User collection
+ type: object
+ properties:
+ users:
+ type: array
+ items:
+ $ref: '#/components/schemas/UserCollectionItem'
+
Review comment:
```suggestion
```
Not used in this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583982636
##########
File path: airflow/api_connexion/webserver_auth.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from datetime import datetime, timedelta
+from functools import wraps
+from typing import Callable, TypeVar, cast
+
+import jwt
+from flask import Response, current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_login import login_user
+
+from airflow.configuration import conf
+
+SECRET = conf.get("webserver", "secret_key")
+T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
+
+
+def encode_auth_token(user_id):
+ """Generate authentication token"""
+ expire_time = conf.getint("webserver", "session_lifetime_minutes")
Review comment:
The default is 30 days. It's a bit too long for an access token. The access token should be generated for a short period of time, e.g. 1 hour, and then refreshed regulary with a refresh token. My main concerns are what happens if the token is stolen and the user changes the password? Will this token still work?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r597874688
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -2364,73 +2473,6 @@ components:
$ref: '#/components/schemas/PluginCollectionItem'
- $ref: '#/components/schemas/CollectionInfo'
- Role:
- description: Role item
- type: object
- properties:
- name:
- type: string
- description: The name of the role
- actions:
- type: array
- items:
- $ref: '#/components/schemas/ActionResource'
-
- RoleCollection:
- description: Role Collections
- type: object
- allOf:
- - type: object
- properties:
- roles:
- type: array
- items:
- $ref: '#/components/schemas/Role'
- - $ref: '#/components/schemas/CollectionInfo'
-
- Action:
- description: Action Item
- type: object
- properties:
- name:
- type: string
- description: The name of the permission "action"
- nullable: false
-
- ActionCollection:
- description: Action Collection
- type: object
- allOf:
- - type: object
- properties:
- actions:
- type: array
- items:
- $ref: '#/components/schemas/Action'
- - $ref: '#/components/schemas/CollectionInfo'
-
- Resource:
- description: A "resource" on which permissions are granted.
- type: object
- properties:
- name:
- type: string
- description: The name of the resource
- nullable: false
-
- ActionResource:
- description: The Action-Resource item
- type: object
- properties:
- action:
- type: object
- $ref: '#/components/schemas/Action'
- description: The permission action
- resource:
- type: object
- $ref: '#/components/schemas/Resource'
- description: The permission resource
-
Review comment:
Moved Role definitions under database entities. Previously was under python objects
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r583984421
##########
File path: airflow/api_connexion/webserver_auth.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from datetime import datetime, timedelta
+from functools import wraps
+from typing import Callable, TypeVar, cast
+
+import jwt
+from flask import Response, current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_login import login_user
+
+from airflow.configuration import conf
+
+SECRET = conf.get("webserver", "secret_key")
+T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
+
+
+def encode_auth_token(user_id):
Review comment:
What do you think about flask_jwt_extended library?
https://github.com/apache/airflow/blob/d9c72c6ebfc015fbe5a41630483fc3fad6a4244d/constraints-3.8.txt#L9
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r584772584
##########
File path: airflow/api_connexion/webserver_auth.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from datetime import datetime, timedelta
+from functools import wraps
+from typing import Callable, TypeVar, cast
+
+import jwt
+from flask import Response, current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_login import login_user
+
+from airflow.configuration import conf
+
+SECRET = conf.get("webserver", "secret_key")
+T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
+
+
+def encode_auth_token(user_id):
+ """Generate authentication token"""
+ expire_time = conf.getint("webserver", "session_lifetime_minutes")
+ payload = {
+ 'exp': datetime.utcnow() + timedelta(minutes=expire_time),
+ 'iat': datetime.utcnow(),
+ 'sub': user_id,
+ }
+ return jwt.encode(payload, SECRET, algorithm='HS256')
+
+
+def decode_auth_token(auth_token):
+ """Decode authentication token"""
+ try:
+ payload = jwt.decode(auth_token, SECRET, algorithms=['HS256'])
+ return payload['sub']
+ except (jwt.ExpiredSignatureError, jwt.InvalidTokenError):
+ # Do not raise exception here due to auth_backend auth
+ return None
+
+
+def auth_current_user():
+ """Checks the authentication and return the current user"""
+ auth_header = request.headers.get("Authorization", None)
+ ab_security_manager = current_app.appbuilder.sm
+ token = None
+ user = None
+ if auth_header:
+ try:
+ token = auth_header.split(" ")[1]
+ except IndexError:
+ # Do not raise exception here due to auth_backend auth
+ return None
+ if auth_header and auth_header.startswith("Basic"):
Review comment:
Something about the way this is integrated still doesn't sit right with me.
This implicitly allows basic auth logins, with no way to disable it, which isn't good.
Also the way this interacts with the _other_ auth backend configured in the app feels a big clunky :(
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
Review comment:
UserLogin is only ever going to be used here isn't it? If so just inline to object here.
##########
File path: tests/api_connexion/test_webserver_auth.py
##########
@@ -0,0 +1,129 @@
+# 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 base64 import b64encode
+from datetime import datetime, timedelta
+
+import jwt
+from flask_login import current_user
+from parameterized import parameterized
+
+from airflow.www.app import create_app
+from tests.test_utils.api_connexion_utils import assert_401
+from tests.test_utils.config import conf_vars
+from tests.test_utils.db import clear_db_pools
+
+
+class TestWebserverAuth(unittest.TestCase):
+ def setUp(self) -> None:
+ with conf_vars({("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend"}):
+ self.app = create_app(testing=True)
+
+ self.appbuilder = self.app.appbuilder # pylint: disable=no-member
+ role_admin = self.appbuilder.sm.find_role("Admin")
+ self.tester = self.appbuilder.sm.find_user(username="test")
+ if not self.tester:
+ self.appbuilder.sm.add_user(
+ username="test",
+ first_name="test",
+ last_name="test",
+ email="test@fab.org",
+ role=role_admin,
+ password="test",
+ )
Review comment:
This should be in setupClass
##########
File path: airflow/api_connexion/webserver_auth.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from datetime import datetime, timedelta
+from functools import wraps
+from typing import Callable, TypeVar, cast
+
+import jwt
+from flask import Response, current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_login import login_user
+
+from airflow.configuration import conf
+
+SECRET = conf.get("webserver", "secret_key")
+T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
+
+
+def encode_auth_token(user_id):
+ """Generate authentication token"""
+ expire_time = conf.getint("webserver", "session_lifetime_minutes")
Review comment:
Yeah, having the password change def should invalidate the token -- that could only work for "local" (in DB) users I think, but it's still worth doing.
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
Review comment:
Should this just be a list fo Role _names_?
I also don't think this should be nullable.
✅ `roles: []`
❎ `roles: null`
##########
File path: tests/api_connexion/test_webserver_auth.py
##########
@@ -0,0 +1,129 @@
+# 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 base64 import b64encode
+from datetime import datetime, timedelta
+
+import jwt
+from flask_login import current_user
+from parameterized import parameterized
+
+from airflow.www.app import create_app
+from tests.test_utils.api_connexion_utils import assert_401
+from tests.test_utils.config import conf_vars
+from tests.test_utils.db import clear_db_pools
+
+
+class TestWebserverAuth(unittest.TestCase):
+ def setUp(self) -> None:
+ with conf_vars({("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend"}):
+ self.app = create_app(testing=True)
+
+ self.appbuilder = self.app.appbuilder # pylint: disable=no-member
+ role_admin = self.appbuilder.sm.find_role("Admin")
+ self.tester = self.appbuilder.sm.find_user(username="test")
+ if not self.tester:
+ self.appbuilder.sm.add_user(
+ username="test",
+ first_name="test",
+ last_name="test",
+ email="test@fab.org",
+ role=role_admin,
+ password="test",
+ )
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
+
+ def test_successful_login(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ with self.app.test_client() as test_client:
+ response = test_client.post("api/v1/login", headers={"Authorization": token})
+ assert isinstance(response.json["token"], str)
+ assert response.json["user"]['email'] == "test@fab.org"
+
+ def test_can_view_other_endpoints(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ with self.app.test_client() as test_client:
+ response = test_client.post("api/v1/login", headers={"Authorization": token})
+ assert current_user.email == "test@fab.org"
+ token = response.json["token"]
Review comment:
Ideally you should get the token by calling a python method directly -- by calling this endpoint you are "retesting" the login endpoint, which it would be better to avoid.
##########
File path: airflow/api_connexion/security.py
##########
@@ -14,24 +14,27 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-
from functools import wraps
from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast
from flask import Response, current_app
from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated
+from airflow.api_connexion.webserver_auth import requires_authentication
T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
def check_authentication() -> None:
"""Checks that the request has valid authorization information."""
response = current_app.api_auth.requires_authentication(Response)()
- if response.status_code != 200:
+ jwt_response = requires_authentication(Response)()
+ if response.status_code != 200 and jwt_response.status_code != 200:
# since this handler only checks authentication, not authorization,
# we should always return 401
- raise Unauthenticated(headers=response.headers)
+ if response.status_code != 200:
+ raise Unauthenticated(headers=response.headers)
+ raise Unauthenticated(headers=jwt_response.headers)
Review comment:
This is a bit complex, and we check multiple auths. We should instead do something like:
```python
response = current_app.api_auth.requires_authentication(Response)()
if response.status_code == 200:
return
jwt_response = requires_authentication(Response)()
if response.status_code == 200:
return
raise Unauthenticated(headers=response.headers)
```
Less nesting, less checking of auth headers that we don't need to do.
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1381,11 +1381,178 @@ paths:
schema:
$ref: '#/components/schemas/VersionInfo'
+ /login:
+ post:
+ summary: User login
+ description: |
+ Verify user and return a user object and JWT token as well
+ x-openapi-router-controller: airflow.api_connexion.endpoints.user_endpoint
+ operationId: login
+ tags: [User]
+
+ responses:
+ '200':
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/UserLogin'
+ '400':
+ $ref: '#/components/responses/BadRequest'
+ '401':
+ $ref: '#/components/responses/Unauthenticated'
components:
# Reusable schemas (data models)
schemas:
# Database entities
+ User:
+ description: >
+ A user object
+ type: object
+ properties:
+ id:
+ type: string
+ description: The user id
+ readOnly: true
+ first_name:
+ type: string
+ description: The user firstname
+ last_name:
+ type: string
+ description: The user lastname
+ username:
+ type: string
+ description: The username
+ email:
+ type: string
+ description: The user's email
+ active:
+ type: boolean
+ description: Whether the user is active
+ last_login:
+ type: string
+ format: datetime
+ description: The last user login
+ readOnly: true
+ login_count:
+ type: integer
+ description: The login count
+ readOnly: true
+ failed_login_count:
+ type: integer
+ description: The number of times the login failed
+ readOnly: true
+ roles:
+ type: array
+ description: User roles
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+ readOnly: true
+ nullable: true
+ created_on:
+ type: string
+ format: datetime
+ description: The date user was created
+ readOnly: true
+ changed_on:
+ type: string
+ format: datetime
+ description: The date user was changed
+ readOnly: true
+
+ UserLogin:
+ description: Login item
+ allOf:
+ - $ref: '#/components/schemas/User'
+ - type: object
+ properties:
+ token:
+ type: string
+ nullable: false
+ description: JWT token
+
+ RoleCollectionItem:
+ description: Role collection item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The role ID
+ name:
+ type: string
+ description: The name of the role
+ permissions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionView'
+
+ RoleCollection:
+ description: Role Collections
+ type: object
+ properties:
+ roles:
+ type: array
+ items:
+ $ref: '#/components/schemas/RoleCollectionItem'
+
+ PermissionCollectionItem:
+ description: Permission Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The permission ID
+ name:
+ type: string
+ description: The name of the permission
+ nullable: false
+
+ PermissionCollection:
+ description: Permission Collection
+ type: object
+ properties:
+ actions:
+ type: array
+ items:
+ $ref: '#/components/schemas/PermissionCollectionItem'
+
+ PermissionView:
+ description: Permission view item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The PermissionView ID
+ action:
+ type: string
+ description: The name of the permission
+ resource:
+ type: string
+ description: The resource name
+
+ ResourceCollectionItem:
+ description: Resource Collection Item
+ type: object
+ properties:
+ id:
+ type: string
+ description: The Resource ID
+ name:
+ type: string
+ description: The name of the Resource
+ nullable: false
+
+ ResourceCollection:
Review comment:
Where is this used?
##########
File path: tests/api_connexion/test_webserver_auth.py
##########
@@ -0,0 +1,129 @@
+# 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 base64 import b64encode
+from datetime import datetime, timedelta
+
+import jwt
+from flask_login import current_user
+from parameterized import parameterized
+
+from airflow.www.app import create_app
+from tests.test_utils.api_connexion_utils import assert_401
+from tests.test_utils.config import conf_vars
+from tests.test_utils.db import clear_db_pools
+
+
+class TestWebserverAuth(unittest.TestCase):
+ def setUp(self) -> None:
+ with conf_vars({("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend"}):
+ self.app = create_app(testing=True)
+
+ self.appbuilder = self.app.appbuilder # pylint: disable=no-member
+ role_admin = self.appbuilder.sm.find_role("Admin")
+ self.tester = self.appbuilder.sm.find_user(username="test")
+ if not self.tester:
+ self.appbuilder.sm.add_user(
+ username="test",
+ first_name="test",
+ last_name="test",
+ email="test@fab.org",
+ role=role_admin,
+ password="test",
+ )
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
+
+ def test_successful_login(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ with self.app.test_client() as test_client:
+ response = test_client.post("api/v1/login", headers={"Authorization": token})
+ assert isinstance(response.json["token"], str)
+ assert response.json["user"]['email'] == "test@fab.org"
+
+ def test_can_view_other_endpoints(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ with self.app.test_client() as test_client:
+ response = test_client.post("api/v1/login", headers={"Authorization": token})
+ assert current_user.email == "test@fab.org"
+ token = response.json["token"]
+ response2 = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + token})
+ assert response2.status_code == 200
+ assert response2.json == {
+ "pools": [
+ {
+ "name": "default_pool",
+ "slots": 128,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 128,
+ },
+ ],
+ "total_entries": 1,
+ }
+
+ def test_raises_for_the_none_algorithm(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ payload = {
+ 'exp': datetime.utcnow() + timedelta(minutes=10),
+ 'iat': datetime.utcnow(),
+ 'sub': self.tester.id,
+ }
+ forgedtoken = jwt.encode(payload, key=None, algorithm=None).decode()
+ with self.app.test_client() as test_client:
+ test_client.post("api/v1/login", headers={"Authorization": token})
+ assert current_user.email == "test@fab.org"
Review comment:
```suggestion
```
Not needed I don't think.
##########
File path: tests/api_connexion/test_webserver_auth.py
##########
@@ -0,0 +1,129 @@
+# 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 base64 import b64encode
+from datetime import datetime, timedelta
+
+import jwt
+from flask_login import current_user
+from parameterized import parameterized
+
+from airflow.www.app import create_app
+from tests.test_utils.api_connexion_utils import assert_401
+from tests.test_utils.config import conf_vars
+from tests.test_utils.db import clear_db_pools
+
+
+class TestWebserverAuth(unittest.TestCase):
+ def setUp(self) -> None:
+ with conf_vars({("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend"}):
+ self.app = create_app(testing=True)
+
+ self.appbuilder = self.app.appbuilder # pylint: disable=no-member
+ role_admin = self.appbuilder.sm.find_role("Admin")
+ self.tester = self.appbuilder.sm.find_user(username="test")
+ if not self.tester:
+ self.appbuilder.sm.add_user(
+ username="test",
+ first_name="test",
+ last_name="test",
+ email="test@fab.org",
+ role=role_admin,
+ password="test",
+ )
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
Review comment:
I don't think we need to clear pools.
##########
File path: tests/api_connexion/test_webserver_auth.py
##########
@@ -0,0 +1,129 @@
+# 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 base64 import b64encode
+from datetime import datetime, timedelta
+
+import jwt
+from flask_login import current_user
+from parameterized import parameterized
+
+from airflow.www.app import create_app
+from tests.test_utils.api_connexion_utils import assert_401
+from tests.test_utils.config import conf_vars
+from tests.test_utils.db import clear_db_pools
+
+
+class TestWebserverAuth(unittest.TestCase):
+ def setUp(self) -> None:
+ with conf_vars({("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend"}):
+ self.app = create_app(testing=True)
+
+ self.appbuilder = self.app.appbuilder # pylint: disable=no-member
+ role_admin = self.appbuilder.sm.find_role("Admin")
+ self.tester = self.appbuilder.sm.find_user(username="test")
+ if not self.tester:
+ self.appbuilder.sm.add_user(
+ username="test",
+ first_name="test",
+ last_name="test",
+ email="test@fab.org",
+ role=role_admin,
+ password="test",
+ )
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
+
+ def test_successful_login(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ with self.app.test_client() as test_client:
+ response = test_client.post("api/v1/login", headers={"Authorization": token})
+ assert isinstance(response.json["token"], str)
+ assert response.json["user"]['email'] == "test@fab.org"
+
+ def test_can_view_other_endpoints(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ with self.app.test_client() as test_client:
+ response = test_client.post("api/v1/login", headers={"Authorization": token})
+ assert current_user.email == "test@fab.org"
+ token = response.json["token"]
+ response2 = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + token})
+ assert response2.status_code == 200
+ assert response2.json == {
+ "pools": [
+ {
+ "name": "default_pool",
+ "slots": 128,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 128,
+ },
+ ],
+ "total_entries": 1,
+ }
+
+ def test_raises_for_the_none_algorithm(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ payload = {
+ 'exp': datetime.utcnow() + timedelta(minutes=10),
+ 'iat': datetime.utcnow(),
+ 'sub': self.tester.id,
+ }
+ forgedtoken = jwt.encode(payload, key=None, algorithm=None).decode()
+ with self.app.test_client() as test_client:
+ test_client.post("api/v1/login", headers={"Authorization": token})
+ assert current_user.email == "test@fab.org"
+ response = test_client.get("/api/v1/pools", headers={"Authorization": "Bearer " + forgedtoken})
+ assert response.status_code == 401
+
+ @parameterized.expand(
+ [
+ ("basic",),
+ ("basic ",),
+ ("bearer",),
+ ("test:test",),
+ (b64encode(b"test:test").decode(),),
+ ("bearer ",),
+ ("basic: ",),
+ ("basic 123",),
+ ]
+ )
+ def test_malformed_headers(self, token):
+ with self.app.test_client() as test_client:
+ response = test_client.get("/api/v1/pools", headers={"Authorization": token})
+ assert response.status_code == 401
Review comment:
Is 400 (Client Error) more appropriate here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r588303765
##########
File path: airflow/api_connexion/webserver_auth.py
##########
@@ -0,0 +1,71 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import logging
+from functools import wraps
+from typing import Callable, TypeVar, cast
+
+from flask import Response, current_app, request
+from flask_appbuilder.const import AUTH_LDAP
+from flask_jwt_extended import verify_jwt_in_request
+from flask_login import login_user
+
+from airflow.configuration import conf
+
+SECRET = conf.get("webserver", "secret_key")
+T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
+
+log = logging.getLogger(__name__)
+
+
+def auth_current_user():
+ """Checks the authentication and return a value"""
+ db_login_disabled = conf.getboolean("api", "disable_db_login")
+ auth_header = request.headers.get("Authorization", None)
+ if auth_header and not db_login_disabled:
+ if auth_header.startswith("Basic"):
+ user = None
+ auth = request.authorization
+ if auth is None or not (auth.username and auth.password):
+ return None
+ ab_security_manager = current_app.appbuilder.sm
+ if ab_security_manager.auth_type == AUTH_LDAP:
+ user = ab_security_manager.auth_user_ldap(auth.username, auth.password)
+ if user is None:
+ user = ab_security_manager.auth_user_db(auth.username, auth.password)
+ if user is not None:
+ login_user(user, remember=False)
+ return user
+ try:
+ verify_jwt_in_request()
+ return 1
+ except Exception as err: # pylint: disable=broad-except
+ log.debug("Can't verify jwt: %s", str(err))
+ return None
+
+
+def requires_authentication(function: T):
+ """Decorator for functions that require authentication"""
+
+ @wraps(function)
+ def decorated(*args, **kwargs):
+ if auth_current_user() is not None:
+ return function(*args, **kwargs)
+ else:
+ return Response("Unauthorized", 401, {"WWW-Authenticate": "Bearer"})
+
+ return cast(T, decorated)
Review comment:
This one doesn't seem be be used as a decorator, so could be removed entirely, and
```python
if auth_current_user():
```
could be used in it's place (see next comment)
##########
File path: airflow/api_connexion/security.py
##########
@@ -14,24 +14,30 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-
from functools import wraps
from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast
from flask import Response, current_app
from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated
+from airflow.api_connexion.webserver_auth import requires_authentication
T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
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:
- # since this handler only checks authentication, not authorization,
- # we should always return 401
- raise Unauthenticated(headers=response.headers)
+ if response.status_code == 200:
+ return
+ jwt_response = requires_authentication(Response)()
+ if jwt_response.status_code == 200:
+ return
+ # since this handler only checks authentication, not authorization,
+ # we should always return 401
+ if jwt_response.status_code != 200:
+ raise Unauthenticated(headers=jwt_response.headers)
Review comment:
```suggestion
if auth_current_user():
return
raise Unauthenticated(headers={"WWW-Authenticate": "Bearer"})
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#discussion_r586079274
##########
File path: tests/api_connexion/test_webserver_auth.py
##########
@@ -0,0 +1,129 @@
+# 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 base64 import b64encode
+from datetime import datetime, timedelta
+
+import jwt
+from flask_login import current_user
+from parameterized import parameterized
+
+from airflow.www.app import create_app
+from tests.test_utils.api_connexion_utils import assert_401
+from tests.test_utils.config import conf_vars
+from tests.test_utils.db import clear_db_pools
+
+
+class TestWebserverAuth(unittest.TestCase):
+ def setUp(self) -> None:
+ with conf_vars({("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend"}):
+ self.app = create_app(testing=True)
+
+ self.appbuilder = self.app.appbuilder # pylint: disable=no-member
+ role_admin = self.appbuilder.sm.find_role("Admin")
+ self.tester = self.appbuilder.sm.find_user(username="test")
+ if not self.tester:
+ self.appbuilder.sm.add_user(
+ username="test",
+ first_name="test",
+ last_name="test",
+ email="test@fab.org",
+ role=role_admin,
+ password="test",
+ )
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
+
+ def test_successful_login(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ with self.app.test_client() as test_client:
+ response = test_client.post("api/v1/login", headers={"Authorization": token})
+ assert isinstance(response.json["token"], str)
+ assert response.json["user"]['email'] == "test@fab.org"
+
+ def test_can_view_other_endpoints(self):
+ token = "Basic " + b64encode(b"test:test").decode()
+ with self.app.test_client() as test_client:
+ response = test_client.post("api/v1/login", headers={"Authorization": token})
+ assert current_user.email == "test@fab.org"
+ token = response.json["token"]
Review comment:
Good. Thanks for this, will reimplement
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14219: Provide login endpoint for the REST API with JWT authentication method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14219:
URL: https://github.com/apache/airflow/pull/14219#issuecomment-790754452
[The Workflow run](https://github.com/apache/airflow/actions/runs/621652545) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org