go-oauth2 / oauth2

OAuth 2.0 server library for the Go programming language.
https://pkg.go.dev/github.com/go-oauth2/oauth2/v4
MIT License
3.31k stars 563 forks source link

fix server_test.go so it works + make code flow test pass with no secret #230

Closed jarlandre closed 1 year ago

jarlandre commented 1 year ago
  1. Remove query encoding of urls for redirect uri in server_test.go to make tests successfully hit the /oauth2 callback endpoint
  2. Then fix the token requests in the auth code flow tests
  3. Then fix the manager to allow for no client_secret for auth code flow with pkce where code verifier is at least 43 chars. EDIT: i see there might not be need to check its length and only its existence. Its already validated properly. Merely checking that that code verifier is not empty string will be enough to ensure that the request will fail without the proper code verifier.

the pkce spec states that code verifier should be minimum 43 chars (https://github.com/go-oauth2/oauth2/pull/230#issuecomment-1396556389), should this be verified earlier in the process?

jarlandre commented 1 year ago

☝️ @LyricTian

jarlandre commented 1 year ago

quote:

code_verifier — The code verifier should be a high-entropy cryptographic random string with a minimum of 43 characters and a maximum of 128 characters.

jarlandre commented 1 year ago
./go.test.sh
ok      github.com/go-oauth2/oauth2/v4  0.203s  coverage: 42.9% of statements
?       github.com/go-oauth2/oauth2/v4/errors   [no test files]
?       github.com/go-oauth2/oauth2/v4/example/client   [no test files]
?       github.com/go-oauth2/oauth2/v4/example/server   [no test files]
ok      github.com/go-oauth2/oauth2/v4/generates    0.219s  coverage: 77.8% of statements
ok      github.com/go-oauth2/oauth2/v4/manage   0.211s  coverage: 61.8% of statements
?       github.com/go-oauth2/oauth2/v4/models   [no test files]
ok      github.com/go-oauth2/oauth2/v4/server   0.237s  coverage: 53.6% of statements
ok      github.com/go-oauth2/oauth2/v4/store    2.314s  coverage: 87.2% of statements
DennisMuchiri commented 1 year ago

@jarlandre why was this not merged?

jarlandre commented 1 year ago

because it war replaced by another PR .. see mentions

jarlandre commented 1 year ago

should have rebased and kept it in this PR i guess .. live and learn

shynome commented 1 year ago

here is a tip: how to work with pcke , you need change the ClientInfoHandler to resolve invalid_client error

srv.SetClientInfoHandler(server.ClientFormHandler)