refactor(openid): use pkce implementation from golang.org/x/oauth2

This commit is contained in:
Trong Huu Nguyen
2023-10-10 10:18:01 +02:00
parent 4f722d9041
commit af6642fe90
17 changed files with 152 additions and 223 deletions
+1 -1
View File
@@ -143,7 +143,7 @@ func (s *Standalone) Login(w http.ResponseWriter, r *http.Request) {
"redirect_after_login": canonicalRedirect,
}
mw.LogEntryFrom(r).WithFields(fields).Info("login: redirecting to identity provider")
http.Redirect(w, r, login.AuthCodeURL(), http.StatusFound)
http.Redirect(w, r, login.AuthCodeURL, http.StatusFound)
}
func (s *Standalone) LoginCallback(w http.ResponseWriter, r *http.Request) {
+3 -4
View File
@@ -16,13 +16,12 @@ import (
"github.com/lestrrat-go/jwx/v2/jwa"
"github.com/lestrrat-go/jwx/v2/jwk"
"github.com/lestrrat-go/jwx/v2/jwt"
"golang.org/x/oauth2"
"github.com/nais/wonderwall/pkg/config"
"github.com/nais/wonderwall/pkg/cookie"
"github.com/nais/wonderwall/pkg/crypto"
handlerpkg "github.com/nais/wonderwall/pkg/handler"
"github.com/nais/wonderwall/pkg/openid"
openidclient "github.com/nais/wonderwall/pkg/openid/client"
openidconfig "github.com/nais/wonderwall/pkg/openid/config"
scopespkg "github.com/nais/wonderwall/pkg/openid/scopes"
"github.com/nais/wonderwall/pkg/router"
@@ -311,7 +310,7 @@ func (ip *IdentityProviderHandler) Token(w http.ResponseWriter, r *http.Request)
return
}
grantType := r.PostForm.Get(openid.GrantType)
grantType := r.PostForm.Get("grant_type")
switch grantType {
case "authorization_code":
ip.TokenCodeGrant(w, r)
@@ -373,7 +372,7 @@ func (ip *IdentityProviderHandler) TokenCodeGrant(w http.ResponseWriter, r *http
return
}
expectedCodeChallenge := openidclient.CodeChallenge(codeVerifier)
expectedCodeChallenge := oauth2.S256ChallengeFromVerifier(codeVerifier)
if expectedCodeChallenge != auth.CodeChallenge {
w.WriteHeader(http.StatusBadRequest)
+8 -18
View File
@@ -146,17 +146,19 @@ func (c *Client) RefreshGrant(ctx context.Context, refreshToken string) (*openid
}
v := url.Values{}
v.Set(openid.GrantType, openid.RefreshTokenValue)
v.Set(openid.RefreshToken, refreshToken)
v.Set(openid.ClientID, c.cfg.Client().ClientID())
v.Set(openid.ClientAssertion, assertion)
v.Set(openid.ClientAssertionType, openid.ClientAssertionTypeJwtBearer)
v.Set("grant_type", "refresh_token")
v.Set("refresh_token", refreshToken)
v.Set("client_id", c.cfg.Client().ClientID())
for key, val := range openid.JwtAuthenticationParameters(assertion) {
v.Set(key, val)
}
r, err := http.NewRequestWithContext(ctx, http.MethodPost, c.cfg.Provider().TokenEndpoint(), strings.NewReader(v.Encode()))
if err != nil {
return nil, fmt.Errorf("creating request: %w", err)
}
r.Header.Add("Content-Type", "application/x-www-form-urlencoded")
r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
resp, err := c.httpClient.Do(r)
if err != nil {
@@ -186,15 +188,3 @@ func (c *Client) RefreshGrant(ctx context.Context, refreshToken string) (*openid
return &tokenResponse, nil
}
func StateMismatchError(expectedState, actualState string) error {
if len(actualState) <= 0 {
return fmt.Errorf("missing state parameter in request (possible csrf)")
}
if expectedState != actualState {
return fmt.Errorf("state parameter mismatch (possible csrf): expected %s, got %s", expectedState, actualState)
}
return nil
}
-16
View File
@@ -45,22 +45,6 @@ func TestMakeAssertion(t *testing.T) {
assert.True(t, assertion.Expiration().Before(time.Now().Add(expiry)))
}
func TestStateMismatchError(t *testing.T) {
for _, tt := range []struct {
name, expected, actual string
assertion assert.ErrorAssertionFunc
}{
{"missing actual state", "expected", "", assert.Error},
{"state mismatch", "match", "not-match", assert.Error},
{"state match", "match", "match", assert.NoError},
} {
t.Run(tt.name, func(t *testing.T) {
err := client.StateMismatchError(tt.expected, tt.actual)
tt.assertion(t, err)
})
}
}
func newTestClientWithConfig(config *mock.TestConfiguration) *client.Client {
jwksProvider := mock.NewTestJwksProvider()
return client.NewClient(config, jwksProvider)
+33 -103
View File
@@ -1,8 +1,6 @@
package client
import (
"crypto/sha256"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
@@ -10,21 +8,17 @@ import (
"golang.org/x/oauth2"
pkgcfg "github.com/nais/wonderwall/pkg/config"
cfg "github.com/nais/wonderwall/pkg/config"
"github.com/nais/wonderwall/pkg/cookie"
"github.com/nais/wonderwall/pkg/crypto"
"github.com/nais/wonderwall/pkg/openid"
"github.com/nais/wonderwall/pkg/strings"
urlpkg "github.com/nais/wonderwall/pkg/url"
"github.com/nais/wonderwall/pkg/url"
)
const (
LocaleURLParameter = "locale"
SecurityLevelURLParameter = "level"
ResponseModeQuery = "query"
CodeChallengeMethodS256 = "S256"
)
var (
@@ -34,13 +28,13 @@ var (
// LoginParameterMapping maps incoming login parameters to OpenID Connect parameters
LoginParameterMapping = map[string]string{
LocaleURLParameter: openid.UILocales,
SecurityLevelURLParameter: openid.ACRValues,
LocaleURLParameter: "ui_locales",
SecurityLevelURLParameter: "acr_values",
}
)
func NewLogin(c *Client, r *http.Request) (*Login, error) {
callbackURL, err := urlpkg.LoginCallback(r)
callbackURL, err := url.LoginCallback(r)
if err != nil {
return nil, fmt.Errorf("generating callback url: %w", err)
}
@@ -55,22 +49,27 @@ func NewLogin(c *Client, r *http.Request) (*Login, error) {
return nil, fmt.Errorf("%w: %w", ErrInvalidLocale, err)
}
params, err := newLoginParameters(acr, callbackURL)
nonce, err := strings.GenerateBase64(32)
if err != nil {
return nil, fmt.Errorf("generating parameters: %w", err)
return nil, fmt.Errorf("creating nonce: %w", err)
}
state, err := strings.GenerateBase64(32)
if err != nil {
return nil, fmt.Errorf("creating state: %w", err)
}
codeVerifier := oauth2.GenerateVerifier()
opts := []oauth2.AuthCodeOption{
oauth2.SetAuthURLParam(openid.Nonce, params.Nonce),
oauth2.SetAuthURLParam(openid.ResponseMode, ResponseModeQuery),
oauth2.SetAuthURLParam(openid.CodeChallenge, params.CodeChallenge),
oauth2.SetAuthURLParam(openid.CodeChallengeMethod, CodeChallengeMethodS256),
oauth2.SetAuthURLParam(openid.RedirectURI, callbackURL),
oauth2.SetAuthURLParam("nonce", nonce),
oauth2.SetAuthURLParam("response_mode", "query"),
oauth2.S256ChallengeOption(codeVerifier),
openid.RedirectURIOption(callbackURL),
}
resourceIndicator := c.cfg.Client().ResourceIndicator()
if resourceIndicator != "" {
opts = append(opts, oauth2.SetAuthURLParam(openid.Resource, resourceIndicator))
if resource := c.cfg.Client().ResourceIndicator(); resource != "" {
opts = append(opts, oauth2.SetAuthURLParam("resource", resource))
}
if len(acr) > 0 {
@@ -82,42 +81,26 @@ func NewLogin(c *Client, r *http.Request) (*Login, error) {
}
return &Login{
authCodeURL: c.oauth2Config.AuthCodeURL(params.State, opts...),
cookie: params.cookie(),
params: params,
AuthCodeURL: c.oauth2Config.AuthCodeURL(state, opts...),
LoginCookie: &openid.LoginCookie{
Acr: acr,
CodeVerifier: codeVerifier,
State: state,
Nonce: nonce,
RedirectURI: callbackURL,
},
}, nil
}
type Login struct {
authCodeURL string
cookie *openid.LoginCookie
params *loginParameters
}
func (l *Login) AuthCodeURL() string {
return l.authCodeURL
}
func (l *Login) CodeChallenge() string {
return l.params.CodeChallenge
}
func (l *Login) CodeVerifier() string {
return l.params.CodeVerifier
}
func (l *Login) Nonce() string {
return l.params.Nonce
}
func (l *Login) State() string {
return l.params.State
AuthCodeURL string
*openid.LoginCookie
}
func (l *Login) SetCookie(w http.ResponseWriter, opts cookie.Options, crypter crypto.Crypter, canonicalRedirect string) error {
l.cookie.Referer = canonicalRedirect
l.LoginCookie.Referer = canonicalRedirect
loginCookieJson, err := json.Marshal(l.cookie)
loginCookieJson, err := json.Marshal(l.LoginCookie)
if err != nil {
return fmt.Errorf("marshalling login cookie: %w", err)
}
@@ -138,51 +121,6 @@ func (l *Login) SetCookie(w http.ResponseWriter, opts cookie.Options, crypter cr
return nil
}
type loginParameters struct {
Acr string
CodeVerifier string
CodeChallenge string
Nonce string
RedirectURI string
State string
}
func newLoginParameters(acr, redirectUri string) (*loginParameters, error) {
codeVerifier, err := strings.GenerateBase64(64)
if err != nil {
return nil, fmt.Errorf("creating code verifier: %w", err)
}
nonce, err := strings.GenerateBase64(32)
if err != nil {
return nil, fmt.Errorf("creating nonce: %w", err)
}
state, err := strings.GenerateBase64(32)
if err != nil {
return nil, fmt.Errorf("creating state: %w", err)
}
return &loginParameters{
Acr: acr,
CodeVerifier: codeVerifier,
CodeChallenge: CodeChallenge(codeVerifier),
Nonce: nonce,
RedirectURI: redirectUri,
State: state,
}, nil
}
func (in *loginParameters) cookie() *openid.LoginCookie {
return &openid.LoginCookie{
Acr: in.Acr,
State: in.State,
Nonce: in.Nonce,
CodeVerifier: in.CodeVerifier,
RedirectURI: in.RedirectURI,
}
}
func getAcrParam(c *Client, r *http.Request) (string, error) {
defaultValue := c.cfg.Client().ACRValues()
if len(defaultValue) == 0 {
@@ -199,7 +137,7 @@ func getAcrParam(c *Client, r *http.Request) (string, error) {
return paramValue, nil
}
translatedAcr, ok := pkgcfg.IDPortenAcrMapping[paramValue]
translatedAcr, ok := cfg.IDPortenAcrMapping[paramValue]
if ok && supported.Contains(translatedAcr) {
return translatedAcr, nil
}
@@ -225,11 +163,3 @@ func getLocaleParam(c *Client, r *http.Request) (string, error) {
return "", fmt.Errorf("%w: invalid value for %s=%s (must be one of '%s')", ErrInvalidLoginParameter, LocaleURLParameter, paramValue, supported)
}
func CodeChallenge(codeVerifier string) string {
hasher := sha256.New()
hasher.Write([]byte(codeVerifier))
codeVerifierHash := hasher.Sum(nil)
return base64.RawURLEncoding.EncodeToString(codeVerifierHash)
}
+8 -12
View File
@@ -41,9 +41,9 @@ func NewLoginCallback(c *Client, r *http.Request, cookie *openid.LoginCookie) (*
}
func (in *LoginCallback) IdentityProviderError() error {
if in.requestParams.Get(openid.Error) != "" {
oauthError := in.requestParams.Get(openid.Error)
oauthErrorDescription := in.requestParams.Get(openid.ErrorDescription)
if in.requestParams.Get("error") != "" {
oauthError := in.requestParams.Get("error")
oauthErrorDescription := in.requestParams.Get("error_description")
return fmt.Errorf("error from identity provider: %s: %s", oauthError, oauthErrorDescription)
}
@@ -51,10 +51,7 @@ func (in *LoginCallback) IdentityProviderError() error {
}
func (in *LoginCallback) StateMismatchError() error {
expectedState := in.cookie.State
actualState := in.requestParams.Get(openid.State)
return StateMismatchError(expectedState, actualState)
return openid.StateMismatchError(in.requestParams, in.cookie.State)
}
func (in *LoginCallback) RedeemTokens(ctx context.Context) (*openid.Tokens, error) {
@@ -64,13 +61,12 @@ func (in *LoginCallback) RedeemTokens(ctx context.Context) (*openid.Tokens, erro
}
opts := []oauth2.AuthCodeOption{
oauth2.SetAuthURLParam(openid.CodeVerifier, in.cookie.CodeVerifier),
oauth2.SetAuthURLParam(openid.ClientAssertion, clientAssertion),
oauth2.SetAuthURLParam(openid.ClientAssertionType, openid.ClientAssertionTypeJwtBearer),
oauth2.SetAuthURLParam(openid.RedirectURI, in.cookie.RedirectURI),
openid.RedirectURIOption(in.cookie.RedirectURI),
oauth2.VerifierOption(in.cookie.CodeVerifier),
}
opts = openid.WithJwtAuthentication(opts, clientAssertion)
code := in.requestParams.Get(openid.Code)
code := in.requestParams.Get("code")
rawTokens, err := in.AuthCodeGrant(ctx, code, opts)
if err != nil {
return nil, fmt.Errorf("exchanging authorization code for token: %w", err)
+2 -1
View File
@@ -6,6 +6,7 @@ import (
"time"
"github.com/stretchr/testify/assert"
"golang.org/x/oauth2"
"github.com/nais/wonderwall/pkg/mock"
"github.com/nais/wonderwall/pkg/openid"
@@ -132,7 +133,7 @@ func newLoginCallback(t *testing.T, url string) (*mock.IdentityProvider, *client
"some-code": {
AcrLevel: "some-acr",
ClientID: idp.OpenIDConfig.Client().ClientID(),
CodeChallenge: client.CodeChallenge("some-verifier"),
CodeChallenge: oauth2.S256ChallengeFromVerifier("some-verifier"),
Nonce: "some-nonce",
RedirectUri: redirect,
},
+6 -7
View File
@@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/oauth2"
"github.com/nais/wonderwall/pkg/mock"
"github.com/nais/wonderwall/pkg/openid/client"
@@ -126,7 +127,7 @@ func TestLogin_URL(t *testing.T) {
} else {
require.NoError(t, err)
parsed, err := url.Parse(result.AuthCodeURL())
parsed, err := url.Parse(result.AuthCodeURL)
assert.NoError(t, err)
query := parsed.Query()
@@ -148,13 +149,11 @@ func TestLogin_URL(t *testing.T) {
assert.ElementsMatch(t, query["client_id"], []string{openidConfig.Client().ClientID()})
assert.ElementsMatch(t, query["redirect_uri"], []string{callbackURL})
assert.ElementsMatch(t, query["scope"], []string{openidConfig.Client().Scopes().String()})
assert.ElementsMatch(t, query["state"], []string{result.State()})
assert.ElementsMatch(t, query["nonce"], []string{result.Nonce()})
assert.ElementsMatch(t, query["state"], []string{result.State})
assert.ElementsMatch(t, query["nonce"], []string{result.Nonce})
assert.ElementsMatch(t, query["response_mode"], []string{"query"})
assert.ElementsMatch(t, query["code_challenge"], []string{result.CodeChallenge()})
assert.ElementsMatch(t, query["code_challenge_method"], []string{"S256"})
assert.Equal(t, client.CodeChallenge(result.CodeVerifier()), result.CodeChallenge())
assert.ElementsMatch(t, query["code_challenge"], []string{oauth2.S256ChallengeFromVerifier(result.CodeVerifier)})
if test.extraParams != nil {
for key, value := range test.extraParams {
@@ -187,7 +186,7 @@ func TestLoginURL_WithResourceIndicator(t *testing.T) {
result, err := c.Login(req)
assert.NoError(t, err)
assert.NotEmpty(t, result)
parsed, err := url.Parse(result.AuthCodeURL())
parsed, err := url.Parse(result.AuthCodeURL)
assert.NoError(t, err)
query := parsed.Query()
+3 -3
View File
@@ -43,11 +43,11 @@ func NewLogout(c *Client, r *http.Request) (*Logout, error) {
func (in *Logout) SingleLogoutURL(idToken string) string {
endSessionEndpoint := in.cfg.Provider().EndSessionEndpointURL()
v := endSessionEndpoint.Query()
v.Add(openid.PostLogoutRedirectURI, in.logoutCallbackURL)
v.Add(openid.State, in.Cookie.State)
v.Set("post_logout_redirect_uri", in.logoutCallbackURL)
v.Set("state", in.Cookie.State)
if len(idToken) > 0 {
v.Add(openid.IDTokenHint, idToken)
v.Set("id_token_hint", idToken)
}
endSessionEndpoint.RawQuery = v.Encode()
+1 -4
View File
@@ -47,8 +47,5 @@ func (in *LogoutCallback) stateMismatchError() error {
return fmt.Errorf("logout cookie is nil")
}
expectedState := in.cookie.State
actualState := in.request.URL.Query().Get(openid.State)
return StateMismatchError(expectedState, actualState)
return openid.StateMismatchError(in.request.URL.Query(), in.cookie.State)
}
+1 -3
View File
@@ -2,8 +2,6 @@ package client
import (
"net/http"
"github.com/nais/wonderwall/pkg/openid"
)
type LogoutFrontchannel struct {
@@ -12,7 +10,7 @@ type LogoutFrontchannel struct {
func NewLogoutFrontchannel(r *http.Request) *LogoutFrontchannel {
params := r.URL.Query()
sid := params.Get(openid.Sid)
sid := params.Get("sid")
return &LogoutFrontchannel{
sid: sid,
+56
View File
@@ -0,0 +1,56 @@
package openid
import (
"fmt"
"net/url"
"golang.org/x/oauth2"
)
// TokenResponse is the struct representing the HTTP response from authorization servers as defined in RFC 6749, section 5.1.
type TokenResponse struct {
AccessToken string `json:"access_token"`
ExpiresIn int64 `json:"expires_in"`
RefreshToken string `json:"refresh_token"`
TokenType string `json:"token_type"`
}
// TokenErrorResponse is the struct representing the HTTP error response returned from authorization servers as defined in RFC 6749, section 5.2.
type TokenErrorResponse struct {
Error string `json:"error"`
ErrorDescription string `json:"error_description"`
}
// JwtAuthenticationParameters returns a map of parameters to be sent to the authorization server when using a JWT for client authentication in RFC 7523, section 2.2.
func JwtAuthenticationParameters(clientAssertion string) map[string]string {
return map[string]string{
"client_assertion": clientAssertion,
"client_assertion_type": "urn:ietf:params:oauth:client-assertion-type:jwt-bearer",
}
}
func WithJwtAuthentication(opts []oauth2.AuthCodeOption, clientAssertion string) []oauth2.AuthCodeOption {
for k, v := range JwtAuthenticationParameters(clientAssertion) {
opts = append(opts, oauth2.SetAuthURLParam(k, v))
}
return opts
}
func RedirectURIOption(redirectUri string) oauth2.AuthCodeOption {
return oauth2.SetAuthURLParam("redirect_uri", redirectUri)
}
func StateMismatchError(queryParams url.Values, expectedState string) error {
actualState := queryParams.Get("state")
if len(actualState) <= 0 {
return fmt.Errorf("missing state parameter in request (possible csrf)")
}
if expectedState != actualState {
return fmt.Errorf("state parameter mismatch (possible csrf): expected %s, got %s", expectedState, actualState)
}
return nil
}
+28
View File
@@ -0,0 +1,28 @@
package openid
import (
"net/url"
"testing"
"github.com/stretchr/testify/assert"
)
func TestStateMismatchError(t *testing.T) {
for _, tt := range []struct {
name, expected, actual string
assertion assert.ErrorAssertionFunc
}{
{"missing actual state", "expected", "", assert.Error},
{"state mismatch", "match", "not-match", assert.Error},
{"state match", "match", "match", assert.NoError},
} {
t.Run(tt.name, func(t *testing.T) {
actual := url.Values{
"state": []string{tt.actual},
}
err := StateMismatchError(actual, tt.expected)
tt.assertion(t, err)
})
}
}
-26
View File
@@ -1,26 +0,0 @@
package openid
const (
ACRValues = "acr_values"
ClientAssertion = "client_assertion"
ClientAssertionType = "client_assertion_type"
ClientID = "client_id"
CodeChallenge = "code_challenge"
CodeChallengeMethod = "code_challenge_method"
Code = "code"
CodeVerifier = "code_verifier"
Error = "error"
ErrorDescription = "error_description"
GrantType = "grant_type"
IDTokenHint = "id_token_hint"
Nonce = "nonce"
PostLogoutRedirectURI = "post_logout_redirect_uri"
SessionState = "session_state"
Sid = "sid"
State = "state"
RedirectURI = "redirect_uri"
RefreshToken = "refresh_token"
Resource = "resource"
ResponseMode = "response_mode"
UILocales = "ui_locales"
)
-6
View File
@@ -1,6 +0,0 @@
package openid
const (
ClientAssertionTypeJwtBearer = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"
RefreshTokenValue = "refresh_token"
)
-17
View File
@@ -1,17 +0,0 @@
package openid
// TokenResponse is the struct representing the HTTP response from OpenID Connect providers returning a token in
// JSON form.
type TokenResponse struct {
AccessToken string `json:"access_token"`
ExpiresIn int64 `json:"expires_in"`
RefreshToken string `json:"refresh_token"`
TokenType string `json:"token_type"`
}
// TokenErrorResponse is the struct representing the HTTP error response returned from OpenID Connect providers
// in JSON form.
type TokenErrorResponse struct {
Error string `json:"error"`
ErrorDescription string `json:"error_description"`
}
+2 -2
View File
@@ -43,9 +43,9 @@ func ExternalID(r *http.Request, cfg openidconfig.Provider, idToken *openid.IDTo
func getSessionStateFrom(r *http.Request) (string, error) {
params := r.URL.Query()
sessionState := params.Get(openid.SessionState)
sessionState := params.Get("session_state")
if len(sessionState) == 0 {
return "", fmt.Errorf("missing required '%s' in params", openid.SessionState)
return "", fmt.Errorf("missing required 'session_state' in params")
}
return sessionState, nil
}