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

Add support to Client Credentials Flow (OAuth2) #5052

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Riuzaky77
Copy link

issue : #774

@codecov
Copy link

codecov bot commented Jun 18, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0bb8920) 100.00% compared to head (7090cdb) 100.00%.
Report is 1198 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##            master     #5052    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          535       537     +2     
  Lines        13825     13963   +138     
==========================================
+ Hits         13825     13963   +138     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link
Contributor

📝 Docs preview for commit 48611ac at: https://62adefeee6a1801161daafce--fastapi.netlify.app

Copy link
Contributor

@JarroVGIT JarroVGIT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All for this! I do see you didn't provide any tests though, is that on the planning?

HTTPException(status_code=400, detail="Client credentials not provided")
pass

This will allow the client server send credential either header or body. But the recommended
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence is wrangled. Proposal: "This will allow the client to send its credentials either via headers or body with the request for a token."

auto_error=False, scheme_name="oAuth2ClientCredentials"
)

@router.post("/login")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Client Credentials flow typically goes to a '/token' endpoint. Normally, a '/login' endpoint is for the Resource Owner Password Credentials flow.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good! will fix the comments and add some tests too!

@Riuzaky77
Copy link
Author

All for this! I do see you didn't provide any tests though, is that on the planning?

Will add test yep, sorry!

@github-actions
Copy link
Contributor

📝 Docs preview for commit 6b5a4cc at: https://62b2119df868f015e2b47298--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit b07569d at: https://62b7196e1aa56f7e7005c960--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit 9c3dda9 at: https://62b71fa51aa56f036105c86c--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit 232394d at: https://62b720b7f57baf090dfb812e--fastapi.netlify.app

@Riuzaky77
Copy link
Author

Ready to go!

@github-actions
Copy link
Contributor

📝 Docs preview for commit cb0305e at: https://62b722f3e2b0760a373ddeb6--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit f02c4aa at: https://62d18ce54624d43f5615452b--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit a1893f0 at: https://62e29b96e3bee326f645bf5b--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit 7090cdb at: https://630e1bf89f0bdf6afee85fc5--fastapi.netlify.app

@kokolem
Copy link

kokolem commented Oct 20, 2022

Hey @JarroVGIT, just wondering, is there a chance to get this merged? I'm willing to pick this up if there's any additional work needed to be done before merging.

@JarroVGIT
Copy link
Contributor

@kokolem I am not the maintainer of this project; I just do some reviews now and then and help people out with issues. @tiangolo is the one that determines what is merged and not, but as you can see there is quite a backlog he is going through. Eventually he'll get to this, I'm sure :)

@petri
Copy link

petri commented Nov 22, 2022

could some docs be added to the PR?

@Ryandaydev
Copy link
Sponsor

Would it be appropriate to add some info about using this flow on the advanced/security/oauth2-scopes/ docs page?

@tiangolo tiangolo added feature New feature or request p3 and removed investigate labels Jan 15, 2024
@nkukard
Copy link

nkukard commented Feb 21, 2024

@tiangolo is there any plans to implement client credentials flow?

@aidanwardman
Copy link

Also looking for a good client credentials flow. Is this still going?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request p3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants