PKCE implementation

server/handlers.go

@@ -2,6 +2,8 @@ package server
 
 import (
 	"context"
+	"crypto/sha256"
+	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -23,6 +25,11 @@ import (
 	"github.com/dexidp/dex/storage"
 )
 
+const (
+	CodeChallengeMethodPlain = "plain"
+	CodeChallengeMethodS256  = "S256"
+)
+
 // newHealthChecker returns the healthz handler. The handler runs until the
 // provided context is canceled.
 func (s *Server) newHealthChecker(ctx context.Context) http.Handler {
@@ -148,34 +155,36 @@ func (s *Server) handlePublicKeys(w http.ResponseWriter, r *http.Request) {
 }
 
 type discovery struct {
-	Issuer         string   `json:"issuer"`
-	Auth           string   `json:"authorization_endpoint"`
-	Token          string   `json:"token_endpoint"`
-	Keys           string   `json:"jwks_uri"`
-	UserInfo       string   `json:"userinfo_endpoint"`
-	DeviceEndpoint string   `json:"device_authorization_endpoint"`
-	GrantTypes     []string `json:"grant_types_supported"`
-	ResponseTypes  []string `json:"response_types_supported"`
-	Subjects       []string `json:"subject_types_supported"`
-	IDTokenAlgs    []string `json:"id_token_signing_alg_values_supported"`
-	Scopes         []string `json:"scopes_supported"`
-	AuthMethods    []string `json:"token_endpoint_auth_methods_supported"`
-	Claims         []string `json:"claims_supported"`
+	Issuer            string   `json:"issuer"`
+	Auth              string   `json:"authorization_endpoint"`
+	Token             string   `json:"token_endpoint"`
+	Keys              string   `json:"jwks_uri"`
+	UserInfo          string   `json:"userinfo_endpoint"`
+	DeviceEndpoint    string   `json:"device_authorization_endpoint"`
+	GrantTypes        []string `json:"grant_types_supported"`
+	ResponseTypes     []string `json:"response_types_supported"`
+	Subjects          []string `json:"subject_types_supported"`
+	IDTokenAlgs       []string `json:"id_token_signing_alg_values_supported"`
+	CodeChallengeAlgs []string `json:"code_challenge_methods_supported"`
+	Scopes            []string `json:"scopes_supported"`
+	AuthMethods       []string `json:"token_endpoint_auth_methods_supported"`
+	Claims            []string `json:"claims_supported"`
 }
 
 func (s *Server) discoveryHandler() (http.HandlerFunc, error) {
 	d := discovery{
-		Issuer:         s.issuerURL.String(),
-		Auth:           s.absURL("/auth"),
-		Token:          s.absURL("/token"),
-		Keys:           s.absURL("/keys"),
-		UserInfo:       s.absURL("/userinfo"),
-		DeviceEndpoint: s.absURL("/device/code"),
-		Subjects:       []string{"public"},
-		GrantTypes:     []string{grantTypeAuthorizationCode, grantTypeRefreshToken, grantTypeDeviceCode},
-		IDTokenAlgs:    []string{string(jose.RS256)},
-		Scopes:         []string{"openid", "email", "groups", "profile", "offline_access"},
-		AuthMethods:    []string{"client_secret_basic"},
+		Issuer:            s.issuerURL.String(),
+		Auth:              s.absURL("/auth"),
+		Token:             s.absURL("/token"),
+		Keys:              s.absURL("/keys"),
+		UserInfo:          s.absURL("/userinfo"),
+		DeviceEndpoint:    s.absURL("/device/code"),
+		Subjects:          []string{"public"},
+		GrantTypes:        []string{grantTypeAuthorizationCode, grantTypeRefreshToken, grantTypeDeviceCode},
+		IDTokenAlgs:       []string{string(jose.RS256)},
+		CodeChallengeAlgs: []string{CodeChallengeMethodS256, CodeChallengeMethodPlain},
+		Scopes:            []string{"openid", "email", "groups", "profile", "offline_access"},
+		AuthMethods:       []string{"client_secret_basic"},
 		Claims: []string{
 			"aud", "email", "email_verified", "exp",
 			"iat", "iss", "locale", "name", "sub",
@@ -637,6 +646,7 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe
 				Expiry:        s.now().Add(time.Minute * 30),
 				RedirectURI:   authReq.RedirectURI,
 				ConnectorData: authReq.ConnectorData,
+				PKCE:          authReq.PKCE,
 			}
 			if err := s.storage.CreateAuthCode(code); err != nil {
 				s.logger.Errorf("Failed to create auth code: %v", err)
@@ -750,6 +760,11 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if client.Secret != clientSecret {
+		if clientSecret == "" {
+			s.logger.Infof("missing client_secret on token request for client: %s", client.ID)
+		} else {
+			s.logger.Infof("invalid client_secret on token request for client: %s", client.ID)
+		}
 		s.tokenErrHelper(w, errInvalidClient, "Invalid client credentials.", http.StatusUnauthorized)
 		return
 	}
@@ -767,6 +782,18 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+func (s *Server) calculateCodeChallenge(codeVerifier, codeChallengeMethod string) (string, error) {
+	switch codeChallengeMethod {
+	case CodeChallengeMethodPlain:
+		return codeVerifier, nil
+	case CodeChallengeMethodS256:
+		shaSum := sha256.Sum256([]byte(codeVerifier))
+		return base64.RawURLEncoding.EncodeToString(shaSum[:]), nil
+	default:
+		return "", fmt.Errorf("unknown challenge method (%v)", codeChallengeMethod)
+	}
+}
+
 // handle an access token request https://tools.ietf.org/html/rfc6749#section-4.1.3
 func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client storage.Client) {
 	code := r.PostFormValue("code")
@@ -783,6 +810,31 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s
 		return
 	}
 
+	// RFC 7636 (PKCE)
+	codeChallengeFromStorage := authCode.PKCE.CodeChallenge
+	providedCodeVerifier := r.PostFormValue("code_verifier")
+
+	if providedCodeVerifier != "" && codeChallengeFromStorage != "" {
+		calculatedCodeChallenge, err := s.calculateCodeChallenge(providedCodeVerifier, authCode.PKCE.CodeChallengeMethod)
+		if err != nil {
+			s.logger.Error(err)
+			s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
+			return
+		}
+		if codeChallengeFromStorage != calculatedCodeChallenge {
+			s.tokenErrHelper(w, errInvalidGrant, "Invalid code_verifier.", http.StatusBadRequest)
+			return
+		}
+	} else if providedCodeVerifier != "" {
+		// Received no code_challenge on /auth, but a code_verifier on /token
+		s.tokenErrHelper(w, errInvalidRequest, "No PKCE flow started. Cannot check code_verifier.", http.StatusBadRequest)
+		return
+	} else if codeChallengeFromStorage != "" {
+		// Received PKCE request on /auth, but no code_verifier on /token
+		s.tokenErrHelper(w, errInvalidGrant, "Expecting parameter code_verifier in PKCE flow.", http.StatusBadRequest)
+		return
+	}
+
 	if authCode.RedirectURI != redirectURI {
 		s.tokenErrHelper(w, errInvalidRequest, "redirect_uri did not match URI from initial request.", http.StatusBadRequest)
 		return

server/oauth2.go

@@ -413,6 +413,13 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques
 	scopes := strings.Fields(q.Get("scope"))
 	responseTypes := strings.Fields(q.Get("response_type"))
 
+	codeChallenge := q.Get("code_challenge")
+	codeChallengeMethod := q.Get("code_challenge_method")
+
+	if codeChallengeMethod == "" {
+		codeChallengeMethod = CodeChallengeMethodPlain
+	}
+
 	client, err := s.storage.GetClient(clientID)
 	if err != nil {
 		if err == storage.ErrNotFound {
@@ -446,6 +453,11 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques
 		return &authErr{state, redirectURI, typ, fmt.Sprintf(format, a...)}
 	}
 
+	if codeChallengeMethod != CodeChallengeMethodS256 && codeChallengeMethod != CodeChallengeMethodPlain {
+		description := fmt.Sprintf("Unsupported PKCE challenge method (%q).", codeChallengeMethod)
+		return nil, newErr(errInvalidRequest, description)
+	}
+
 	var (
 		unrecognized  []string
 		invalidScopes []string
@@ -541,6 +553,10 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques
 		RedirectURI:         redirectURI,
 		ResponseTypes:       responseTypes,
 		ConnectorID:         connectorID,
+		PKCE: storage.PKCE{
+			CodeChallenge:       codeChallenge,
+			CodeChallengeMethod: codeChallengeMethod,
+		},
 	}, nil
 }
 

server/oauth2_test.go

@@ -197,6 +197,78 @@ func TestParseAuthorizationRequest(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "PKCE code_challenge_method plain",
+			clients: []storage.Client{
+				{
+					ID:           "bar",
+					RedirectURIs: []string{"https://example.com/bar"},
+				},
+			},
+			supportedResponseTypes: []string{"code"},
+			queryParams: map[string]string{
+				"client_id":             "bar",
+				"redirect_uri":          "https://example.com/bar",
+				"response_type":         "code",
+				"code_challenge":        "123",
+				"code_challenge_method": "plain",
+				"scope":                 "openid email profile",
+			},
+		},
+		{
+			name: "PKCE code_challenge_method default plain",
+			clients: []storage.Client{
+				{
+					ID:           "bar",
+					RedirectURIs: []string{"https://example.com/bar"},
+				},
+			},
+			supportedResponseTypes: []string{"code"},
+			queryParams: map[string]string{
+				"client_id":      "bar",
+				"redirect_uri":   "https://example.com/bar",
+				"response_type":  "code",
+				"code_challenge": "123",
+				"scope":          "openid email profile",
+			},
+		},
+		{
+			name: "PKCE code_challenge_method S256",
+			clients: []storage.Client{
+				{
+					ID:           "bar",
+					RedirectURIs: []string{"https://example.com/bar"},
+				},
+			},
+			supportedResponseTypes: []string{"code"},
+			queryParams: map[string]string{
+				"client_id":             "bar",
+				"redirect_uri":          "https://example.com/bar",
+				"response_type":         "code",
+				"code_challenge":        "123",
+				"code_challenge_method": "S256",
+				"scope":                 "openid email profile",
+			},
+		},
+		{
+			name: "PKCE invalid code_challenge_method",
+			clients: []storage.Client{
+				{
+					ID:           "bar",
+					RedirectURIs: []string{"https://example.com/bar"},
+				},
+			},
+			supportedResponseTypes: []string{"code"},
+			queryParams: map[string]string{
+				"client_id":             "bar",
+				"redirect_uri":          "https://example.com/bar",
+				"response_type":         "code",
+				"code_challenge":        "123",
+				"code_challenge_method": "invalid_method",
+				"scope":                 "openid email profile",
+			},
+			wantErr: true,
+		},
 	}
 
 	for _, tc := range tests {