Allow a _least privilege_ model of automatic dependencies by "disabling" dependencies for individulal routers or endpoints.

[kristjanvalur]

Issue #2481 Mentions how it is difficult to remove a global dependency on a subset of endpoint. The proposed solution in that defect is to add global depdencies on individual APIRouter instances and combine a set of those into the app.

This PR proposes a complimentary solution. Anywhere where a Depends(foo) or Security(bar) is added as an item in a dependencies keyword argument, it can receive a disable=True keyword argument.

Once the final parameterless dependencies are gathered and added on a route, these are then processes and collapsed. This allows an APIRouter to disable a global dependency which has already been added on an app object.

This allows a least privilege approach to API design, where nothing is allowed by default unless explicitly allowed on a case by case basis: The app object can have a global dependency which requires all scopes, and then individual endpoints, for example those that deal with authentication, or other less privileged actions, can remove the more stringent scopes to allow access.

Additionally, multiple stacked calls to the same Security dependency are collapsed, merging all the provided scopes into a single list. This allows the creation of virtual scopes, for example, prefixing a scope with a "-" do disable it, handling the processing of the all the scopes to a single call. This is yet-another way disabling a scope requirement.

This is currently an undocumented feature request / proposal, containing tests.

[github-actions]

📝 Docs preview for commit 926eebf at: https://6331aa781b89db04dd18da52--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit 5961a72 at: https://6331b1d1f0221e0c947b1e6b--fastapi.netlify.app

[codecov]

Codecov Report

Base: 100.00% // Head: 90.53% // Decreases project coverage by -9.46% ⚠️

Coverage data is based on head (8107097) compared to base (cf73051).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 8107097 differs from pull request most recent head 344c1ed. Consider uploading reports for the commit 344c1ed to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##            master    #5433      +/-   ##
===========================================
- Coverage   100.00%   90.53%   -9.47%     
===========================================
  Files          540      505      -35     
  Lines        13969    13420     -549     
===========================================
- Hits         13969    12150    -1819     
- Misses           0     1270    +1270     

Impacted Files	Coverage Δ
fastapi/dependencies/utils.py	100.00% <100.00%> (ø)
fastapi/param_functions.py	100.00% <100.00%> (ø)
fastapi/params.py	100.00% <100.00%> (ø)
fastapi/routing.py	100.00% <100.00%> (ø)
tests/test_dependency_disable.py	100.00% <100.00%> (ø)
docs_src/app_testing/app_b_py310/main.py	0.00% <0.00%> (-100.00%)	⬇️
docs_src/sql_databases/sql_app_py39/crud.py	0.00% <0.00%> (-100.00%)	⬇️
docs_src/sql_databases/sql_app_py39/main.py	0.00% <0.00%> (-100.00%)	⬇️
docs_src/sql_databases/sql_app_py310/crud.py	0.00% <0.00%> (-100.00%)	⬇️
docs_src/sql_databases/sql_app_py310/main.py	0.00% <0.00%> (-100.00%)	⬇️
... and 96 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[github-actions]

📝 Docs preview for commit b4293ea at: https://6331b9fae87b5402c104aa66--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit 8107097 at: https://6331bd1572bbd2052c8296b9--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit 78804bd at: https://636948c803208f113158d77b--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit 344c1ed at: https://636e33f6bca4cf0d1b6dc265--fastapi.netlify.app

[tiangolo]

Thanks for the interest and effort @kristjanvalur ! ☕

This would add some significant complexity, in particular to explain it to new developers. And it would mean that adding a dependency somewhere would no longer guarantee that it would be executed everywhere down the line.

Given that I'll pass on this one, but thanks for the interest!

[AdrianSacristanPB]

@tiangolo then, there is not any alternative for this?

[tiangolo]

@AdrianSacristanPB nope. If we think about it, a least privilege model would really mean that you would add all the things that block and restrict at the top level in your app. Even if they are only related to a small section of it. And then you would have to enable access to most of those for most of the other unrelated parts.

It would also mean that to know if a section in your code is covered by some protection, you would have to check the whole chain of included routers to make sure none is disabling it. That's kinda linear complexity (O(n)) to verify if something would apply or not. This itself could be a security risk, as people could assume that if something was applied somewhere up the chain, then it would also apply here, but that's no longer the case.

So I prefer to have related things and restrictions closer to where they apply, with the related code close together.

Now, you can probably hack around using scopes, and relax permissions with scopes. I'm pretty sure it would be a quite complicated implementation (at least mentally), but maybe you could make it work.

[kristjanvalur]

The case I'm trying to solve is really: "Here I want almost the same restrictions as the rest of the app, but not quite". Doing it currently is quite inconvenient, with multiple routers.

The reverse solution is to set no rules at the top, and make sure that you apply them at every router/endpoint. that is too an O(n) problem and you have no guarantees there either, because you have to make sure that you don't forget to apply the rules individually.

The sort of middle ground is to have multiple routers, each with a different set of rules applied to them and join them in a top app with no rules. This works, but it can become messy and complex.

A different way to achieve the same, or similar, results is by using only Security and make sure to extend the scopes when passing down the inheritance chain. This PR can be modified to only join up Security scopes. This way, the application itself can decide how to interpret added scopes. An implementor can, for example, interpret scopes prefixed with '-' as disabling them in the security dependency. Please also see the #5622 which merges parameterless Securitiy dependences with parameter dependencies.

An additional benefit of merging Security scopes is that it reduces calling overhead in the endpoint. if you specify Security(foo, scopes=["a"]) at the top, and Security(foo, scopes=["b"]) lower down, the endpoint receives a single invocation of foo with scopes==["a", "b"] rather than two separate calls.

I can submit an alternate PR which only does that.