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

Support for OAuth public clients (native apps) #21299

Closed
hickford opened this issue Sep 29, 2022 · 3 comments · Fixed by #21316
Closed

Support for OAuth public clients (native apps) #21299

hickford opened this issue Sep 29, 2022 · 3 comments · Fixed by #21316
Labels
type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@hickford
Copy link
Contributor

hickford commented Sep 29, 2022

The OAuth spec defines two types of client, confidential and public, however Gitea assumes all clients to be confidential.

OAuth defines two client types, based on their ability to authenticate securely with the authorization server (i.e., ability to
maintain the confidentiality of their client credentials):

confidential
Clients capable of maintaining the confidentiality of their credentials (e.g., client implemented on a secure server with
restricted access to the client credentials), or capable of secure client authentication using other means.

public
Clients incapable of maintaining the confidentiality of their credentials (e.g., clients executing on the device used by the resource owner, such as an installed native application or a web browser-based application), and incapable of secure client authentication via any other means.

The client type designation is based on the authorization server's definition of secure authentication and its acceptable exposure levels of client credentials. The authorization server SHOULD NOT make assumptions about the client type.

This is a barrier to native apps using OAuth https://datatracker.ietf.org/doc/html/rfc8252

Some OAuth server implementations that assume all clients are confidential web clients will need to add an
understanding of public native app clients and the types of redirect URIs they use to support this best practice.

In particular, Gitea should record the client type https://datatracker.ietf.org/doc/html/rfc8252#section-8.4

Authorization servers MUST record the client type in the client registration details in order to identify and process requests
accordingly.

and require PKCE for public clients: https://datatracker.ietf.org/doc/html/rfc8252#section-8.1

Authorization servers SHOULD reject authorization requests from native apps that don't use PKCE by returning an error message

@hickford hickford added type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Sep 29, 2022
@hickford
Copy link
Contributor Author

hickford commented Sep 29, 2022

First step would be to add client type field to type OAuth2Application https://github.com/go-gitea/gitea/blob/main/models/auth/oauth2.go and associated forms

GitLab has a confidential option:

Confidential
Enable only for confidential applications exclusively used by a trusted backend server that can securely store the client secret. Do not enable for native-mobile, single-page, or other JavaScript applications because they cannot keep the client secret confidential.

image

@hickford
Copy link
Contributor Author

hickford commented Sep 29, 2022

Secondly I think handleAuthorizationCode would have to implement the logic described in https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.3:

ensure that the authorization code was issued to the authenticated confidential client, or if the client is public, ensure that the code was issued to "client_id" in the request,

Right now when my native app tries to authenticate, it gets error "unauthorized_client: invalid client secret".

@hickford
Copy link
Contributor Author

@jonasfranz who originally added OAuth support #5378 , do you have any insight?

lunny added a commit that referenced this issue Oct 24, 2022
The OAuth spec [defines two types of
client](https://datatracker.ietf.org/doc/html/rfc6749#section-2.1),
confidential and public. Previously Gitea assumed all clients to be
confidential.

> OAuth defines two client types, based on their ability to authenticate
securely with the authorization server (i.e., ability to
>   maintain the confidentiality of their client credentials):
>
>   confidential
> Clients capable of maintaining the confidentiality of their
credentials (e.g., client implemented on a secure server with
> restricted access to the client credentials), or capable of secure
client authentication using other means.
>
>   **public
> Clients incapable of maintaining the confidentiality of their
credentials (e.g., clients executing on the device used by the resource
owner, such as an installed native application or a web browser-based
application), and incapable of secure client authentication via any
other means.**
>
> The client type designation is based on the authorization server's
definition of secure authentication and its acceptable exposure levels
of client credentials. The authorization server SHOULD NOT make
assumptions about the client type.

 https://datatracker.ietf.org/doc/html/rfc8252#section-8.4

> Authorization servers MUST record the client type in the client
registration details in order to identify and process requests
accordingly.

Require PKCE for public clients:
https://datatracker.ietf.org/doc/html/rfc8252#section-8.1

> Authorization servers SHOULD reject authorization requests from native
apps that don't use PKCE by returning an error message

Fixes #21299

Co-authored-by: wxiaoguang <wxiaoguang@gmail.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
type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant