Skip to content

Get safely request argument #186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Get safely request argument #186

wants to merge 1 commit into from

Conversation

pcrespov
Copy link

@pcrespov pcrespov commented Sep 6, 2018

Problem

aiohttp-security decorators fail when router calls the handles using key-arguments instead of positional arguments.

In particular, this happens with the router class in aiohttp_apiset/swagger

  File "/home/devp/osparc-simcore/.venv/lib/python3.6/site-packages/aiohttp_session/__init__.py", line 149, in factory
    response = await handler(request)
  File "/home/devp/osparc-simcore/.venv/lib/python3.6/site-packages/aiohttp_apiset/middlewares.py", line 96, in process
    response = await handler(request)
  File "/home/devp/osparc-simcore/services/web/server/src/simcore_service_webserver/rest/middlewares.py", line 12, in handle_errors
    response = await handler(request)
  File "/home/devp/osparc-simcore/.venv/lib/python3.6/site-packages/aiohttp_apiset/swagger/route.py", line 90, in handler
    response = await self._handler(**parameters)
  File "/home/devp/osparc-simcore/.venv/lib/python3.6/site-packages/aiohttp_security/api.py", line 136, in wrapped
    request = args[-1]

Propose

safe check of request argument prioritizing key over positional arguments

@codecov
Copy link

codecov bot commented Sep 6, 2018

Codecov Report

Merging #186 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #186   +/-   ##
=======================================
  Coverage   94.75%   94.75%           
=======================================
  Files          12       12           
  Lines         591      591           
  Branches       21       21           
=======================================
  Hits          560      560           
  Misses         24       24           
  Partials        7        7
Impacted Files Coverage Δ
aiohttp_security/api.py 90.24% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca38b38...9a4d8fb. Read the comment docs.

@asvetlov
Copy link
Member

asvetlov commented Sep 6, 2018

Thank you for PR.
Unfortunately, I've chosen another solution: no more decorators but check_permission explicit call.

@asvetlov asvetlov closed this Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants