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 refresh handler should require client authentication #21418

Closed
hickford opened this issue Oct 12, 2022 · 0 comments · Fixed by #21421
Closed

OAuth refresh handler should require client authentication #21418

hickford opened this issue Oct 12, 2022 · 0 comments · Fixed by #21421
Labels

Comments

@hickford
Copy link
Contributor

hickford commented Oct 12, 2022

The OAuth authorization_code handler authenticates the client by validating the client secret

if !app.ValidateClientSecret([]byte(form.ClientSecret)) {
errorDescription := "invalid client secret"
if form.ClientSecret == "" {
errorDescription = "invalid empty client secret"
}
handleAccessTokenError(ctx, AccessTokenError{
ErrorCode: AccessTokenErrorCodeUnauthorizedClient,
ErrorDescription: errorDescription,
})
return
}

According to the OAuth spec https://datatracker.ietf.org/doc/html/rfc6749#section-6 , this should also happen when "Refreshing an Access Token"

The authorization server MUST ... require client authentication for confidential clients

but handleRefreshToken doesn't do this

func handleRefreshToken(ctx *context.Context, form forms.AccessTokenForm, serverKey, clientKey oauth2.JWTSigningKey) {

lunny added a commit that referenced this issue Oct 23, 2022
According to the OAuth spec
https://datatracker.ietf.org/doc/html/rfc6749#section-6 when "Refreshing
an Access Token"

> The authorization server MUST ... require client authentication for
confidential clients


Fixes #21418

Co-authored-by: Gusted <williamzijl7@hotmail.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant