Skip to content
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

OpenAPI docs for scopes defined on a single route #1592

Conversation

andrewsnowden
Copy link

Scopes that are defined on an individual route as part of a Security Dependency were not being returned on the OpenAPI JSON that is generated.

This seems to be because when the dependencies are flattened the security_scopes are not being flattened (unlike security_requirements and others which are). This sets the default scopes to an empty list and flattens the scopes of sub dependencies. I've added a test which confirms this in the OAuth2 test.

I'm brand new to the FastAPI code base so I am unsure if there are any unintended consequences with this approach - but all the tests seem to pass and the new behaviour makes sense in my understanding of what the code should do.

@codecov
Copy link

codecov bot commented Jun 18, 2020

Codecov Report

Merging #1592 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1592   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          236       236           
  Lines         7068      7076    +8     
=========================================
+ Hits          7068      7076    +8     
Impacted Files Coverage Δ
fastapi/dependencies/models.py 100.00% <100.00%> (ø)
fastapi/dependencies/utils.py 100.00% <100.00%> (ø)
tests/test_security_oauth2.py 100.00% <100.00%> (ø)

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 b87072b...ca7770b. Read the comment docs.

@tiangolo
Copy link
Member

tiangolo commented Aug 8, 2020

Hey @andrewsnowden , thanks for your contribution! Do you have an example code that shows the current problem?

I think there are things to update in the way scopes are currently handled, but I ran your test and it seems to pass with the current version, so I'm not sure this PR would fix it.

Could you write a simple self-contained example that shows the problem?

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2021

Assuming the original need was handled, this will be automatically closed now. But feel free to add more comments or create new issues or PRs.

@github-actions github-actions bot closed this Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants