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 implementation #1784

Merged
merged 35 commits into from
Oct 26, 2020
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
2879801
Basic implementation of PKCE
Teeed Feb 13, 2020
551a292
@mfmarche on 24 Feb: when code_verifier is set, don't check client_se…
HEllRZA Jul 22, 2020
2688931
@deric on 16 Jun: return invalid_grant when wrong code_verifier
HEllRZA Jul 22, 2020
3c47734
Enforce PKCE flow on /token when PKCE flow was started on /auth
HEllRZA Jul 22, 2020
d0fafb0
fixed error messages when mixed PKCE/no PKCE flow.
HEllRZA Jul 24, 2020
492fecf
server_test.go: Added PKCE error cases on /token endpoint
HEllRZA Jul 24, 2020
0c80328
cleanup: extracted method checkErrorResponse and type TestDefinition
HEllRZA Jul 24, 2020
118ab10
/token endpoint: skip client_secret verification only for grand type …
HEllRZA Jul 28, 2020
60b0ec8
Merge branch 'master' of github.com:dexidp/dex into faro-upstream/PKCE
HEllRZA Aug 25, 2020
c58264d
Allow "Authorization" header in CORS handlers
HEllRZA Aug 25, 2020
739beef
Add "code_challenge_methods_supported" to discovery endpoint
HEllRZA Aug 31, 2020
b79b265
Merge branch 'master' of github.com:dexidp/dex into faro-upstream/PKCE
HEllRZA Aug 31, 2020
06f40be
Merge pull request #4 from faro-oss/faro-upstream/feature/PKCE
HEllRZA Aug 31, 2020
1683a17
Updated tests (mixed-up comments), added a PKCE test
HEllRZA Sep 10, 2020
369674f
Merge pull request #5 from faro-oss/faro-upstream/feature/PKCE
HEllRZA Sep 10, 2020
a1aab00
remove redefinition of providedCodeVerifier, fixed spelling (#6)
HEllRZA Sep 16, 2020
7251cd6
Rename struct CodeChallenge to PKCE
HEllRZA Sep 17, 2020
b24e4d5
PKCE: Check clientSecret when available
HEllRZA Sep 17, 2020
9faf988
Enable PKCE with public: true
HEllRZA Sep 17, 2020
a17bfc1
Redirect error on unsupported code_challenge_method
HEllRZA Sep 17, 2020
f78dc9d
Reverted go.mod and go.sum to the state of master
HEllRZA Sep 17, 2020
9b02f80
Merge pull request #7 from faro-oss/faro-upstream/feature/PKCE
HEllRZA Sep 17, 2020
1059ba7
Don't omit client secret check for PKCE
HEllRZA Sep 21, 2020
d305bc4
Merge pull request #8 from faro-oss/faro-upstream/feature/PKCE
HEllRZA Sep 22, 2020
b6e297b
Allow public clients (e.g. with PKCE) to have redirect URIs configured
heidemn-faro Oct 5, 2020
46c6d9d
Remove "Authorization" as Accepted Headers on CORS, small fixes
HEllRZA Oct 5, 2020
3e86bb6
Merge pull request #9 from faro-oss/faro-upstream/feature/PKCE
HEllRZA Oct 5, 2020
5435741
Revert "Allow public clients (e.g. with PKCE) to have redirect URIs c…
heidemn-faro Oct 5, 2020
7fcf960
Merge pull request #10 from faro-oss/faro-upstream/feature/PKCE
heidemn-faro Oct 5, 2020
dd6de36
PKCE on client_secret client error message
HEllRZA Oct 14, 2020
0063431
Merge pull request #11 from faro-oss/faro-upstream/feature/PKCE
HEllRZA Oct 14, 2020
a3ed229
Output info message when PKCE without client_secret used on confident…
HEllRZA Oct 15, 2020
3b1f1a5
Merge pull request #12 from faro-oss/faro-upstream/feature/PKCE
HEllRZA Oct 15, 2020
7b369a6
General missing/invalid client_secret message on token endpoint
HEllRZA Oct 16, 2020
cbc646f
Merge pull request #13 from faro-oss/faro-upstream/feature/PKCE
HEllRZA Oct 16, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 71 additions & 24 deletions server/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package server

import (
"context"
"crypto/sha256"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
Expand All @@ -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 {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -767,6 +777,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")
Expand All @@ -783,6 +805,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)
HEllRZA marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
16 changes: 16 additions & 0 deletions server/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
72 changes: 72 additions & 0 deletions server/oauth2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading