diff --git a/README.md b/README.md index 5b74bff..da04c31 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,9 @@ Golang OAuth2 server library OSIN is an OAuth2 server library for the Go language, as specified at http://tools.ietf.org/html/rfc6749 and http://tools.ietf.org/html/draft-ietf-oauth-v2-10. +It also includes support for PKCE, as specified at https://tools.ietf.org/html/rfc7636, +which increases security for code-exchange flows for public OAuth clients. + Using it, you can build your own OAuth2 authentication service. The library implements the majority of the specification, like authorization and token endpoints, and authorization code, implicit, resource owner and client credentials grant types. diff --git a/access.go b/access.go index 6e8a24c..bff10a9 100644 --- a/access.go +++ b/access.go @@ -1,6 +1,8 @@ package osin import ( + "crypto/sha256" + "encoding/base64" "errors" "net/http" "strings" @@ -50,6 +52,9 @@ type AccessRequest struct { // HttpRequest *http.Request for special use HttpRequest *http.Request + + // Optional code_verifier as described in rfc7636 + CodeVerifier string } // AccessData represents an access grant (tokens, expiration, client, etc) @@ -158,6 +163,7 @@ func (s *Server) handleAuthorizationCodeRequest(w *Response, r *http.Request) *A ret := &AccessRequest{ Type: AUTHORIZATION_CODE, Code: r.Form.Get("code"), + CodeVerifier: r.Form.Get("code_verifier"), RedirectUri: r.Form.Get("redirect_uri"), GenerateRefresh: true, Expiration: s.Config.AccessExpiration, @@ -221,6 +227,34 @@ func (s *Server) handleAuthorizationCodeRequest(w *Response, r *http.Request) *A return nil } + // Verify PKCE, if present in the authorization data + if len(ret.AuthorizeData.CodeChallenge) > 0 { + // https://tools.ietf.org/html/rfc7636#section-4.1 + if matched := pkceMatcher.MatchString(ret.CodeVerifier); !matched { + w.SetError(E_INVALID_REQUEST, "code_verifier invalid (rfc7636)") + w.InternalError = errors.New("invalid format") + return nil + } + + // https: //tools.ietf.org/html/rfc7636#section-4.6 + codeVerifier := "" + switch ret.AuthorizeData.CodeChallengeMethod { + case "", PKCE_PLAIN: + codeVerifier = ret.CodeVerifier + case PKCE_S256: + hash := sha256.Sum256([]byte(ret.CodeVerifier)) + codeVerifier = base64.RawURLEncoding.EncodeToString(hash[:]) + default: + w.SetError(E_INVALID_REQUEST, "code_challenge_method transform algorithm not supported (rfc7636)") + return nil + } + if codeVerifier != ret.AuthorizeData.CodeChallenge { + w.SetError(E_INVALID_GRANT, "code_verifier invalid (rfc7636)") + w.InternalError = errors.New("failed comparison with code_challenge") + return nil + } + } + // set rest of data ret.Scope = ret.AuthorizeData.Scope ret.UserData = ret.AuthorizeData.UserData diff --git a/access_test.go b/access_test.go index 1d773a1..750cc0e 100644 --- a/access_test.go +++ b/access_test.go @@ -4,6 +4,7 @@ import ( "net/http" "net/url" "testing" + "time" ) func TestAccessAuthorizationCode(t *testing.T) { @@ -313,3 +314,93 @@ func TestGetClientSecretMatcher(t *testing.T) { } } } + +func TestAccessAuthorizationCodePKCE(t *testing.T) { + testcases := map[string]struct { + Challenge string + ChallengeMethod string + Verifier string + ExpectedError string + }{ + "good, plain": { + Challenge: "12345678901234567890123456789012345678901234567890", + Verifier: "12345678901234567890123456789012345678901234567890", + }, + "bad, plain": { + Challenge: "12345678901234567890123456789012345678901234567890", + Verifier: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", + ExpectedError: "invalid_grant", + }, + "good, S256": { + Challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + ChallengeMethod: "S256", + Verifier: "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk", + }, + "bad, S256": { + Challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + ChallengeMethod: "S256", + Verifier: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", + ExpectedError: "invalid_grant", + }, + "missing from storage": { + Challenge: "", + ChallengeMethod: "", + Verifier: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", + }, + } + + for k, test := range testcases { + testStorage := NewTestingStorage() + sconfig := NewServerConfig() + sconfig.AllowedAccessTypes = AllowedAccessType{AUTHORIZATION_CODE} + server := NewServer(sconfig, testStorage) + server.AccessTokenGen = &TestingAccessTokenGen{} + server.Storage.SaveAuthorize(&AuthorizeData{ + Client: testStorage.clients["public-client"], + Code: "pkce-code", + ExpiresIn: 3600, + CreatedAt: time.Now(), + RedirectUri: "http://localhost:14000/appauth", + CodeChallenge: test.Challenge, + CodeChallengeMethod: test.ChallengeMethod, + }) + resp := server.NewResponse() + + req, err := http.NewRequest("POST", "http://localhost:14000/appauth", nil) + if err != nil { + t.Fatal(err) + } + + req.SetBasicAuth("public-client", "") + + req.Form = make(url.Values) + req.Form.Set("grant_type", string(AUTHORIZATION_CODE)) + req.Form.Set("code", "pkce-code") + req.Form.Set("state", "a") + req.Form.Set("code_verifier", test.Verifier) + req.PostForm = make(url.Values) + + if ar := server.HandleAccessRequest(resp, req); ar != nil { + ar.Authorized = true + server.FinishAccessRequest(resp, req, ar) + } + + if resp.IsError { + if test.ExpectedError == "" || test.ExpectedError != resp.ErrorId { + t.Errorf("%s: unexpected error: %v, %v", k, resp.ErrorId, resp.StatusText) + continue + } + } + if test.ExpectedError == "" { + if resp.Type != DATA { + t.Fatalf("%s: Response should be data", k) + } + if d := resp.Output["access_token"]; d != "1" { + t.Fatalf("%s: Unexpected access token: %s", k, d) + } + if d := resp.Output["refresh_token"]; d != "r1" { + t.Fatalf("%s: Unexpected refresh token: %s", k, d) + } + } + } +} diff --git a/authorize.go b/authorize.go index 0318ed2..fe319ab 100644 --- a/authorize.go +++ b/authorize.go @@ -3,6 +3,7 @@ package osin import ( "net/http" "net/url" + "regexp" "time" ) @@ -12,6 +13,13 @@ type AuthorizeRequestType string const ( CODE AuthorizeRequestType = "code" TOKEN AuthorizeRequestType = "token" + + PKCE_PLAIN = "plain" + PKCE_S256 = "S256" +) + +var ( + pkceMatcher = regexp.MustCompile("^[a-zA-Z0-9~._-]{43,128}$") ) // Authorize request information @@ -34,6 +42,11 @@ type AuthorizeRequest struct { // HttpRequest *http.Request for special use HttpRequest *http.Request + + // Optional code_challenge as described in rfc7636 + CodeChallenge string + // Optional code_challenge_method as described in rfc7636 + CodeChallengeMethod string } // Authorization data @@ -61,6 +74,11 @@ type AuthorizeData struct { // Data to be passed to storage. Not used by the library. UserData interface{} + + // Optional code_challenge as described in rfc7636 + CodeChallenge string + // Optional code_challenge_method as described in rfc7636 + CodeChallengeMethod string } // IsExpired is true if authorization expired @@ -140,6 +158,36 @@ func (s *Server) HandleAuthorizeRequest(w *Response, r *http.Request) *Authorize case CODE: ret.Type = CODE ret.Expiration = s.Config.AuthorizationExpiration + + // Optional PKCE support (https://tools.ietf.org/html/rfc7636) + if codeChallenge := r.Form.Get("code_challenge"); len(codeChallenge) == 0 { + if s.Config.RequirePKCEForPublicClients && CheckClientSecret(ret.Client, "") { + // https://tools.ietf.org/html/rfc7636#section-4.4.1 + w.SetErrorState(E_INVALID_REQUEST, "code_challenge (rfc7636) required for public clients", ret.State) + return nil + } + } else { + codeChallengeMethod := r.Form.Get("code_challenge_method") + // allowed values are "plain" (default) and "S256", per https://tools.ietf.org/html/rfc7636#section-4.3 + if len(codeChallengeMethod) == 0 { + codeChallengeMethod = PKCE_PLAIN + } + if codeChallengeMethod != PKCE_PLAIN && codeChallengeMethod != PKCE_S256 { + // https://tools.ietf.org/html/rfc7636#section-4.4.1 + w.SetErrorState(E_INVALID_REQUEST, "code_challenge_method transform algorithm not supported (rfc7636)", ret.State) + return nil + } + + // https://tools.ietf.org/html/rfc7636#section-4.2 + if matched := pkceMatcher.MatchString(codeChallenge); !matched { + w.SetErrorState(E_INVALID_REQUEST, "code_challenge invalid (rfc7636)", ret.State) + return nil + } + + ret.CodeChallenge = codeChallenge + ret.CodeChallengeMethod = codeChallengeMethod + } + case TOKEN: ret.Type = TOKEN ret.Expiration = s.Config.AccessExpiration @@ -191,6 +239,9 @@ func (s *Server) FinishAuthorizeRequest(w *Response, r *http.Request, ar *Author State: ar.State, Scope: ar.Scope, UserData: ar.UserData, + // Optional PKCE challenge + CodeChallenge: ar.CodeChallenge, + CodeChallengeMethod: ar.CodeChallengeMethod, } // generate token code diff --git a/authorize_test.go b/authorize_test.go index af695d6..d8e0ad2 100644 --- a/authorize_test.go +++ b/authorize_test.go @@ -3,6 +3,7 @@ package osin import ( "net/http" "net/url" + "strings" "testing" ) @@ -86,3 +87,171 @@ func TestAuthorizeToken(t *testing.T) { t.Fatalf("Unexpected access token: %s", d) } } + +func TestAuthorizeCodePKCERequired(t *testing.T) { + sconfig := NewServerConfig() + sconfig.RequirePKCEForPublicClients = true + sconfig.AllowedAuthorizeTypes = AllowedAuthorizeType{CODE} + server := NewServer(sconfig, NewTestingStorage()) + server.AuthorizeTokenGen = &TestingAuthorizeTokenGen{} + + // Public client returns an error + { + resp := server.NewResponse() + req, err := http.NewRequest("GET", "http://localhost:14000/appauth", nil) + if err != nil { + t.Fatal(err) + } + req.Form = make(url.Values) + req.Form.Set("response_type", string(CODE)) + req.Form.Set("state", "a") + req.Form.Set("client_id", "public-client") + if ar := server.HandleAuthorizeRequest(resp, req); ar != nil { + ar.Authorized = true + server.FinishAuthorizeRequest(resp, req, ar) + } + if !resp.IsError || resp.ErrorId != "invalid_request" || strings.Contains(resp.StatusText, "code_challenge") { + t.Errorf("Expected invalid_request error describing the code_challenge required, got %#v", resp) + } + } + + // Confidential client works without PKCE + { + resp := server.NewResponse() + req, err := http.NewRequest("GET", "http://localhost:14000/appauth", nil) + if err != nil { + t.Fatal(err) + } + req.Form = make(url.Values) + req.Form.Set("response_type", string(CODE)) + req.Form.Set("state", "a") + req.Form.Set("client_id", "1234") + if ar := server.HandleAuthorizeRequest(resp, req); ar != nil { + ar.Authorized = true + server.FinishAuthorizeRequest(resp, req, ar) + } + if resp.IsError && resp.InternalError != nil { + t.Fatalf("Error in response: %s", resp.InternalError) + } + if resp.IsError { + t.Fatalf("Should not be an error") + } + if resp.Type != REDIRECT { + t.Fatalf("Response should be a redirect") + } + if d := resp.Output["code"]; d != "1" { + t.Fatalf("Unexpected authorization code: %s", d) + } + } +} + +func TestAuthorizeCodePKCEPlain(t *testing.T) { + challenge := "12345678901234567890123456789012345678901234567890" + + sconfig := NewServerConfig() + sconfig.AllowedAuthorizeTypes = AllowedAuthorizeType{CODE} + server := NewServer(sconfig, NewTestingStorage()) + server.AuthorizeTokenGen = &TestingAuthorizeTokenGen{} + resp := server.NewResponse() + + req, err := http.NewRequest("GET", "http://localhost:14000/appauth", nil) + if err != nil { + t.Fatal(err) + } + req.Form = make(url.Values) + req.Form.Set("response_type", string(CODE)) + req.Form.Set("client_id", "1234") + req.Form.Set("state", "a") + req.Form.Set("code_challenge", challenge) + + if ar := server.HandleAuthorizeRequest(resp, req); ar != nil { + ar.Authorized = true + server.FinishAuthorizeRequest(resp, req, ar) + } + + //fmt.Printf("%+v", resp) + + if resp.IsError && resp.InternalError != nil { + t.Fatalf("Error in response: %s", resp.InternalError) + } + + if resp.IsError { + t.Fatalf("Should not be an error") + } + + if resp.Type != REDIRECT { + t.Fatalf("Response should be a redirect") + } + + code, ok := resp.Output["code"].(string) + if !ok || code != "1" { + t.Fatalf("Unexpected authorization code: %s", code) + } + + token, err := server.Storage.LoadAuthorize(code) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + if token.CodeChallenge != challenge { + t.Errorf("Expected stored code_challenge %s, got %s", challenge, token.CodeChallenge) + } + if token.CodeChallengeMethod != "plain" { + t.Errorf("Expected stored code_challenge plain, got %s", token.CodeChallengeMethod) + } +} + +func TestAuthorizeCodePKCES256(t *testing.T) { + challenge := "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM" + + sconfig := NewServerConfig() + sconfig.AllowedAuthorizeTypes = AllowedAuthorizeType{CODE} + server := NewServer(sconfig, NewTestingStorage()) + server.AuthorizeTokenGen = &TestingAuthorizeTokenGen{} + resp := server.NewResponse() + + req, err := http.NewRequest("GET", "http://localhost:14000/appauth", nil) + if err != nil { + t.Fatal(err) + } + req.Form = make(url.Values) + req.Form.Set("response_type", string(CODE)) + req.Form.Set("client_id", "1234") + req.Form.Set("state", "a") + req.Form.Set("code_challenge", challenge) + req.Form.Set("code_challenge_method", "S256") + + if ar := server.HandleAuthorizeRequest(resp, req); ar != nil { + ar.Authorized = true + server.FinishAuthorizeRequest(resp, req, ar) + } + + //fmt.Printf("%+v", resp) + + if resp.IsError && resp.InternalError != nil { + t.Fatalf("Error in response: %s", resp.InternalError) + } + + if resp.IsError { + t.Fatalf("Should not be an error") + } + + if resp.Type != REDIRECT { + t.Fatalf("Response should be a redirect") + } + + code, ok := resp.Output["code"].(string) + if !ok || code != "1" { + t.Fatalf("Unexpected authorization code: %s", code) + } + + token, err := server.Storage.LoadAuthorize(code) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + if token.CodeChallenge != challenge { + t.Errorf("Expected stored code_challenge %s, got %s", challenge, token.CodeChallenge) + } + if token.CodeChallengeMethod != "S256" { + t.Errorf("Expected stored code_challenge S256, got %s", token.CodeChallengeMethod) + } +} diff --git a/config.go b/config.go index 8029c81..5b8929e 100644 --- a/config.go +++ b/config.go @@ -54,6 +54,9 @@ type ServerConfig struct { // If true allows access request using GET, else only POST - default false AllowGetAccessRequest bool + // Require PKCE for code flows for public OAuth clients - default false + RequirePKCEForPublicClients bool + // Separator to support multiple URIs in Client.GetRedirectUri(). // If blank (the default), don't allow multiple URIs. RedirectUriSeparator string diff --git a/storage_test.go b/storage_test.go index f1ad669..ce2a3d7 100644 --- a/storage_test.go +++ b/storage_test.go @@ -27,6 +27,11 @@ func NewTestingStorage() *TestingStorage { RedirectUri: "http://localhost:14000/appauth", } + r.clients["public-client"] = &DefaultClient{ + Id: "public-client", + RedirectUri: "http://localhost:14000/appauth", + } + r.authorize["9999"] = &AuthorizeData{ Client: r.clients["1234"], Code: "9999",