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 2020/08/19 20:08:28 UTC
[GitHub] [airflow] jhtimmins opened a new pull request #10409: WIP: Add permission controls to new API.
jhtimmins opened a new pull request #10409:
URL: https://github.com/apache/airflow/pull/10409
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ 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 pull request #10409: WIP: Add permission controls to stable API.
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #10409:
URL: https://github.com/apache/airflow/pull/10409#issuecomment-676853055
@houqp I want to delete the requires_authorization decorator and configure authorizations on a different layer for the stable API.
----------------------------------------------------------------
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 pull request #10409: WIP: Add permission controls to stable API.
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on pull request #10409:
URL: https://github.com/apache/airflow/pull/10409#issuecomment-683131565
@mik-laj Yeah, that branch is built on top of this one. I'll close this out.
----------------------------------------------------------------
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 closed pull request #10409: WIP: Add permission controls to stable API.
Posted by GitBox <gi...@apache.org>.
jhtimmins closed pull request #10409:
URL: https://github.com/apache/airflow/pull/10409
----------------------------------------------------------------
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 #10409: WIP: Add permission controls to stable API.
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #10409:
URL: https://github.com/apache/airflow/pull/10409#issuecomment-676853561
@houqp Good idea.I support this idea.
----------------------------------------------------------------
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 removed a comment on pull request #10409: WIP: Add permission controls to stable API.
Posted by GitBox <gi...@apache.org>.
mik-laj removed a comment on pull request #10409:
URL: https://github.com/apache/airflow/pull/10409#issuecomment-676853055
@houqp I want to delete the requires_authorization decorator and configure authorizations on a different layer for the stable API.
----------------------------------------------------------------
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] houqp commented on pull request #10409: WIP: Add permission controls to stable API.
Posted by GitBox <gi...@apache.org>.
houqp commented on pull request #10409:
URL: https://github.com/apache/airflow/pull/10409#issuecomment-682085285
@jhtimmins let me know if there is anything I can help with to get this over the finish line :)
----------------------------------------------------------------
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 #10409: WIP: Add permission controls to new API.
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10409:
URL: https://github.com/apache/airflow/pull/10409#discussion_r473290872
##########
File path: airflow/api_connexion/security.py
##########
@@ -35,3 +35,23 @@ def decorated(*args, **kwargs):
return function(*args, **kwargs)
return cast(T, decorated)
+
+
+def requires_access(permissions: Sequence[Tuple[str, str]]) -> Callable[[T], T]:
+ """
+ Factory for decorator that checks current user's permissions against required permissions.
+ """
+
+ def requires_access_decorator(func: T):
+ @wraps(func)
+ def wrapped_function(*args, **kwargs):
+ appbuilder = current_app.appbuilder
+ for permission in permissions:
+ if not appbuilder.sm.has_access(*permission):
+ return Response("Forbidden", 403)
Review comment:
```suggestion
raise PermissionDenied("Forbidden", 403)
```
It returns a [Problem Details](https://tools.ietf.org/html/draft-ietf-appsawg-http-problem-00) error response
----------------------------------------------------------------
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] houqp commented on pull request #10409: WIP: Add permission controls to stable API.
Posted by GitBox <gi...@apache.org>.
houqp commented on pull request #10409:
URL: https://github.com/apache/airflow/pull/10409#issuecomment-676840020
should we combine `@security.requires_access` and `@security.requires_authentication` into one? will there be a case where one would only use require_access without requires_authentication? this doesn't even make sense semantically right?
----------------------------------------------------------------
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 #10409: WIP: Add permission controls to stable API.
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #10409:
URL: https://github.com/apache/airflow/pull/10409#issuecomment-682738625
Duplicate of: https://github.com/apache/airflow/pull/10594?
@houqp Can you look here? https://github.com/apache/airflow/pull/10594
----------------------------------------------------------------
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 edited a comment on pull request #10409: WIP: Add permission controls to stable API.
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #10409:
URL: https://github.com/apache/airflow/pull/10409#issuecomment-676853561
@houqp Good idea.I support it.
----------------------------------------------------------------
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 #10409: WIP: Add permission controls to new API.
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10409:
URL: https://github.com/apache/airflow/pull/10409#discussion_r473291847
##########
File path: airflow/api_connexion/security.py
##########
@@ -16,7 +16,7 @@
# under the License.
from functools import wraps
-from typing import Callable, TypeVar, cast
+from typing import Callable, Sequence, Tuple, TypeVar, cast
from flask import Response, current_app
Review comment:
Missing import - PermissionDenied
----------------------------------------------------------------
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