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

OAuth should require user consent for every public client authorization #25061

Closed
hickford opened this issue Jun 3, 2023 · 2 comments · Fixed by #30790
Closed

OAuth should require user consent for every public client authorization #25061

hickford opened this issue Jun 3, 2023 · 2 comments · Fixed by #30790
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!

Comments

@hickford
Copy link
Contributor

hickford commented Jun 3, 2023

Description

Following OAuth spec https://datatracker.ietf.org/doc/html/rfc8252#section-8.6

the authorization server SHOULD NOT process authorization requests automatically without user consent or interaction, except when the identity of the client can be assured [ie. client is a confidential client]. This includes the case where the user has previously approved an authorization request for a given client id

However Gitea's AuthorizeOAuth only asks for consent on the first authorization. This behaviour is the same for both public clients and confidential clients. Correct would be to ask for consent on the first authorization for confidential clients, and every authorisation for public clients.

grant, err := app.GetGrantByUserID(ctx, ctx.Doer.ID)
if err != nil {
handleServerError(ctx, form.State, form.RedirectURI)
return
}
// Redirect if user already granted access
if grant != nil {
code, err := grant.GenerateNewAuthorizationCode(ctx, form.RedirectURI, form.CodeChallenge, form.CodeChallengeMethod)
if err != nil {
handleServerError(ctx, form.State, form.RedirectURI)
return
}
redirect, err := code.GenerateRedirectURI(form.State)
if err != nil {
handleServerError(ctx, form.State, form.RedirectURI)
return
}
// Update nonce to reflect the new session
if len(form.Nonce) > 0 {
err := grant.SetNonce(ctx, form.Nonce)
if err != nil {
log.Error("Unable to update nonce: %v", err)
}
}
ctx.Redirect(redirect.String())
return
}
// show authorize page to grant access

Gitea Version

HEAD

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

reading code

Database

None

@hickford
Copy link
Contributor Author

This is equivalent to https://www.cve.org/CVERecord?id=CVE-2023-34246 in another OAuth implementation

@denyskon denyskon added topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! and removed type/bug labels Jul 11, 2023
@denyskon
Copy link
Member

I think this should be easy to resolve... I'll maybe open PR soon

wxiaoguang added a commit that referenced this issue May 2, 2024
This commit forces the resource owner (user) to always approve OAuth 2.0
authorization requests if the client is public (e.g. native
applications).

As detailed in [RFC 6749 Section 10.2](https://www.rfc-editor.org/rfc/rfc6749.html#section-10.2),

> The authorization server SHOULD NOT process repeated authorization
requests automatically (without active resource owner interaction)
without authenticating the client or relying on other measures to ensure
that the repeated request comes from the original client and not an
impersonator.

With the implementation prior to this patch, attackers with access to
the redirect URI (e.g., the loopback interface for
`git-credential-oauth`) can get access to the user account without any
user interaction if they can redirect the user to the
`/login/oauth/authorize` endpoint somehow (e.g., with `xdg-open` on
Linux).

Fixes #25061.

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue May 2, 2024
This commit forces the resource owner (user) to always approve OAuth 2.0
authorization requests if the client is public (e.g. native
applications).

As detailed in [RFC 6749 Section 10.2](https://www.rfc-editor.org/rfc/rfc6749.html#section-10.2),

> The authorization server SHOULD NOT process repeated authorization
requests automatically (without active resource owner interaction)
without authenticating the client or relying on other measures to ensure
that the repeated request comes from the original client and not an
impersonator.

With the implementation prior to this patch, attackers with access to
the redirect URI (e.g., the loopback interface for
`git-credential-oauth`) can get access to the user account without any
user interaction if they can redirect the user to the
`/login/oauth/authorize` endpoint somehow (e.g., with `xdg-open` on
Linux).

Fixes go-gitea#25061.

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue May 2, 2024
This commit forces the resource owner (user) to always approve OAuth 2.0
authorization requests if the client is public (e.g. native
applications).

As detailed in [RFC 6749 Section 10.2](https://www.rfc-editor.org/rfc/rfc6749.html#section-10.2),

> The authorization server SHOULD NOT process repeated authorization
requests automatically (without active resource owner interaction)
without authenticating the client or relying on other measures to ensure
that the repeated request comes from the original client and not an
impersonator.

With the implementation prior to this patch, attackers with access to
the redirect URI (e.g., the loopback interface for
`git-credential-oauth`) can get access to the user account without any
user interaction if they can redirect the user to the
`/login/oauth/authorize` endpoint somehow (e.g., with `xdg-open` on
Linux).

Fixes go-gitea#25061.

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
silverwind pushed a commit that referenced this issue May 2, 2024
Backport #30790 by archer-321

This commit forces the resource owner (user) to always approve OAuth 2.0
authorization requests if the client is public (e.g. native
applications).

As detailed in [RFC 6749 Section
10.2](https://www.rfc-editor.org/rfc/rfc6749.html#section-10.2),

> The authorization server SHOULD NOT process repeated authorization
requests automatically (without active resource owner interaction)
without authenticating the client or relying on other measures to ensure
that the repeated request comes from the original client and not an
impersonator.

With the implementation prior to this patch, attackers with access to
the redirect URI (e.g., the loopback interface for
`git-credential-oauth`) can get access to the user account without any
user interaction if they can redirect the user to the
`/login/oauth/authorize` endpoint somehow (e.g., with `xdg-open` on
Linux).

Fixes #25061.

Co-authored-by: Archer <archer@beezig.eu>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
silverwind pushed a commit that referenced this issue May 2, 2024
Backport #30790 by archer-321

This commit forces the resource owner (user) to always approve OAuth 2.0
authorization requests if the client is public (e.g. native
applications).

As detailed in [RFC 6749 Section
10.2](https://www.rfc-editor.org/rfc/rfc6749.html#section-10.2),

> The authorization server SHOULD NOT process repeated authorization
requests automatically (without active resource owner interaction)
without authenticating the client or relying on other measures to ensure
that the repeated request comes from the original client and not an
impersonator.

With the implementation prior to this patch, attackers with access to
the redirect URI (e.g., the loopback interface for
`git-credential-oauth`) can get access to the user account without any
user interaction if they can redirect the user to the
`/login/oauth/authorize` endpoint somehow (e.g., with `xdg-open` on
Linux).

Fixes #25061.

Co-authored-by: Archer <archer@beezig.eu>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants