-
-
Notifications
You must be signed in to change notification settings - Fork 434
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 auth_oauth_ropc #493
Add auth_oauth_ropc #493
Conversation
2c8499f
to
6f41774
Compare
|
||
https://learn.microsoft.com/en-us/azure/active-directory/develop/v2-oauth-ropc | ||
|
||
Microsoft recommends you do not use the ROPC flow. In most scenarios, more secure alternatives are available and recommended. This flow requires a very high degree of trust in the application, and carries risks that are not present in other flows. You should only use this flow when other more secure flows aren't viable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think this is not specific to Azure. This looks like oauth2 Resource Owner Password Credentials Grant (aka Direct Access Grant in keycloak).
So you could perhaps rename it to auth_oauth_ropc
, and give the Azure config as an example?
Also it might be worth mentioning why it is necessary in the README (i.e. the Odoo mobile app as you explained to me :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sbidoul Done
a6b06d8
to
617d8b3
Compare
auth_oauth_ropc/models/res_users.py
Outdated
if passwd_allowed and self.env.user.active: | ||
ropc_providers = self.env["oauth.ropc.provider"].sudo().search([]) | ||
for conf in ropc_providers: | ||
if conf._authenticate(self.env.user.login, password): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sending the user/password to several providers is going to leak the credential to providers that should not receive them? I tend to think we should allow only one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch
cad310c
to
e7925ba
Compare
@sbidoul Done |
e7925ba
to
93f99f4
Compare
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
@OCA/server-environment-maintainers Could you reopen this one please ? |
/ocabot merge nobump |
On my way to merge this fine PR! |
Congratulations, your PR was merged at 83060f7. Thanks a lot for contributing to OCA. ❤️ |
Syncing from upstream OCA/server-auth (14.0)
This module add the possibility to login with OAuth Resource Owner Password Credentials Grant
https://datatracker.ietf.org/doc/html/rfc6749#section-4.3
In most scenarios, more secure alternatives are available and recommended. This flow requires a very high degree of trust in the application, and carries risks that are not present in other flows. You should only use this flow when other more secure flows aren't viable.
This module is usefull for the Odoo mobile application, which only supports user/password authentication.