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

PKCE is not supported #161

Open
ghost opened this issue Aug 9, 2020 · 7 comments
Open

PKCE is not supported #161

ghost opened this issue Aug 9, 2020 · 7 comments

Comments

@ghost
Copy link

ghost commented Aug 9, 2020

No description provided.

@Ekliptor
Copy link

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

@maximkosov
Copy link

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

@muir
Copy link

muir commented Jun 13, 2022

Should this be marked resolved now?

@jarlandre
Copy link

jarlandre commented Jan 18, 2023

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
Copy link

jarlandre commented Jan 18, 2023

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
Copy link

jarlandre commented Jan 18, 2023

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
Copy link

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

#230

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants