oauth2 icon indicating copy to clipboard operation
oauth2 copied to clipboard

PKCE is not supported

Open ghost opened this issue 4 years ago • 7 comments

ghost avatar Aug 09 '20 06:08 ghost

agreed. This would be really cool for react apps without a server (only static files). See https://oauth.net/2/pkce/

Ekliptor avatar Aug 17 '20 11:08 Ekliptor

Authentication for mobile apps also require PKCE. Is there any plans to support it?

maximkosov avatar Aug 19 '20 15:08 maximkosov

Should this be marked resolved now?

muir avatar Jun 13 '22 21:06 muir

authorisation code with flow PKCE without passing client_secret is hard to get working. Because nil or empty client_secret will cause the logic to not check client_credentials secret. because of:

func (m *Manager) GenerateAccessToken(ctx context.Context, gt oauth2.GrantType, tgr *oauth2.TokenGenerateRequest) (oauth2.TokenInfo, error) {
	cli, err := m.GetClient(ctx, tgr.ClientID)
	if err != nil {
		return nil, err
	}
	if cliPass, ok := cli.(oauth2.ClientPasswordVerifier); ok {
		if !cliPass.VerifyPassword(tgr.ClientSecret) {
			return nil, errors.ErrInvalidClient
		}
	} else if len(cli.GetSecret()) > 0 && tgr.ClientSecret != cli.GetSecret() { // <- because of this
		return nil, errors.ErrInvalidClient
	}
	.....
}

and because its used for both auth code flow AND client credentials as shown in

	switch gt {
	case oauth2.AuthorizationCode:
		ti, err := s.Manager.GenerateAccessToken(ctx, gt, tgr)
		if err != nil {
			switch err {
			case errors.ErrInvalidAuthorizeCode, errors.ErrInvalidCodeChallenge, errors.ErrMissingCodeChallenge:
				return nil, errors.ErrInvalidGrant
			case errors.ErrInvalidClient:
				return nil, errors.ErrInvalidClient
			default:
				return nil, err
			}
		}
		return ti, nil
	case oauth2.PasswordCredentials, oauth2.ClientCredentials:
		if fn := s.ClientScopeHandler; fn != nil {
			allowed, err := fn(tgr)
			if err != nil {
				return nil, err
			} else if !allowed {
				return nil, errors.ErrInvalidScope
			}
		}
		return s.Manager.GenerateAccessToken(ctx, gt, tgr)

So you cannot use both auth code flow with PKCE and client credentials grant on the same server, because it will allow to get access token for free without passing client secret to token endpoint when using client credentials grant.

jarlandre avatar Jan 18 '23 17:01 jarlandre

So to actually fix this problem, where we want to use auth code flow with PKCE without passing client_secret in post body, is to use different functions for getting access token for auth code flow and client credentials. Either by checking a flag on a struct when using auth code flow that tells the server if this client can be used with no client secret for auth code flow, or by some other means

jarlandre avatar Jan 18 '23 17:01 jarlandre

~ok, i can "fix" this on my end by implementing this method and setting it on the server~

	ClientScopeHandler func(tgr *oauth2.TokenGenerateRequest) (allowed bool, err error)

but i dont feel like it should be a programmer choice.

EDIT: Nope, this method is also called by both auth code flow and client credentials is there any other way to allow no client_secret for auth code flow with PKCE and still require it for client_credentials flow? @LyricTian ?

jarlandre avatar Jan 18 '23 17:01 jarlandre

made a PR here to fix the issue with client_secret being required in auth code flow with PKCE

https://github.com/go-oauth2/oauth2/pull/230

jarlandre avatar Jan 19 '23 05:01 jarlandre