-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Non-confidential apps should always re-prompt for user consent #1589
Comments
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I'm was expected a core dev to chime about the potentially security related RFC deviation? |
Non-confidential applications should not be able to skip the authorization stop, even if they have an existing matching_token. From the [issue](doorkeeper-gem#1589): > According to RFC 8252 section 8.6, the authentication server should re-prompt for user consent, since the client's identity cannot be assured simply from the client_id parameter Fixes doorkeeper-gem#1589
Fixed in https://github.com/doorkeeper-gem/doorkeeper/releases/tag/v5.6.6 @nbulaj is there a CVE for this issue? It seems an example of "CWE-287: Improper Authentication" https://cwe.mitre.org/data/definitions/287.html
In this case, the actor is an OAuth client claiming the identity of some other public OAuth client |
No we don't have it @hickford |
@nbulaj what do you think about opening a CVE? |
@hickford could you please do it? Never did it before, not sure where and what to do |
@nbulaj I'm not sure either. Admins should be able to record security vulnerabilities at https://github.com/doorkeeper-gem/doorkeeper/security
It looks like @stefansundin has done it before. Stefan, can you help? Previous vulnerabilities https://osv.dev/list?ecosystem=RubyGems&q=doorkeeper |
Uhh. I think @nbulaj did in fact create the one I helped with. This documentation page says admin permissions are required, so I couldn't possibly have done it. Probably someone with admin permissions just have to go to https://github.com/doorkeeper-gem/doorkeeper/security and there's probably a green button to create a new one. 🤷 What I did do was create a proper CVE identifier. However it was a total pain so I don't recommend getting one. I submitted the form that they had but no one got back to me. A few days I got tired of waiting and I emailed someone and they replied back with a super short message ("Use CVE-2020-10187.").. so not the best experience lol. Just create a GitHub advisory and be satisfied with that. :) |
@stefansundin thanks for your reply. According to https://github.blog/2022-04-22-removing-the-stigma-of-a-cve/ these days it's easier to assign a CVE to a GHSA. |
Admin can open a GHSA at https://github.com/doorkeeper-gem/doorkeeper/security/advisories/new |
@nbudin right now only admin can draft a GHSA at https://github.com/doorkeeper-gem/doorkeeper/security/advisories/new but if you enable "Private vulnerability reporting" at https://github.com/doorkeeper-gem/doorkeeper/security then non-admins should be able to draft a GHSA for admins to review |
Enabled @hickford , thanks 🙇 |
@nbulaj thanks. Ready to publish at GHSA-7w2c-w47h-789w
|
(Mentioning @rgammans, since he told me about this issue and wanted to be notified about replies.)
Steps to reproduce
resource_owner_authenticator
check)authorization_code
flow with PKCEExpected behavior
According to RFC 8252 section 8.6, the authentication server should re-prompt for user consent, since the client's identity cannot be assured simply from the client_id parameter. (In Doorkeeper, I think this means that it should redirect to
AuthorizationsController#new
.)Actual behavior
The authentication server immediately redirects with a code parameter.
System configuration
You can help us to understand your problem if you will share some very
useful information about your project environment (don't forget to
remove any confidential data if it exists).
Doorkeeper initializer:
Ruby version:
3.1.2
Gemfile.lock:
Gemfile.lock content
The text was updated successfully, but these errors were encountered: