feat: Request aware cookie handling (#753)
* pkg/http: Add `secureCookieFunc` field to CookieHandler. Signed-off-by: Mark Laing <mark.laing@canonical.com> * pkg/http: Add `IsRequestAware` method CookieHandler. Signed-off-by: Mark Laing <mark.laing@canonical.com> * pkg/http: Use `secureCookieFunc` when checking a cookie (if set). Signed-off-by: Mark Laing <mark.laing@canonical.com> * pkg/http: Error on `SetCookie` if cookie handler is request aware. Signed-off-by: Mark Laing <mark.laing@canonical.com> * pkg/http: Add method to set request aware cookies. Signed-off-by: Mark Laing <mark.laing@canonical.com> * pkg/http: Add function to create a new request aware cookie handler. Signed-off-by: Mark Laing <mark.laing@canonical.com> * pkg/client/rp: Update `trySetStateCookie` function signature. Use `SetRequestAwareCookie` if the cookie handle is request aware. This function signature can be updated because it is not exported. Signed-off-by: Mark Laing <mark.laing@canonical.com> * pkg/client/rp: Add `GenerateAndStoreCodeChallengeWithRequest` function. It's not possible to add a `http.Request` argument to `GenerateAndStoreCodeChallenge` as this would be a breaking change. Instead, add a new function that accepts a request argument and call `SetRequestAwareCookie` here. Signed-off-by: Mark Laing <mark.laing@canonical.com> * pkg/client/rp: Update PKCE logic to pass request if required by cookie handler. Signed-off-by: Mark Laing <mark.laing@canonical.com> * pkg/http: Don't set MaxAge if cookie handler is request aware. The securecookie field can be nil. Expect the caller to set max age on the securecookie returned by the secureCookieFunc. Signed-off-by: Mark Laing <mark.laing@canonical.com> * pkg/client: Add integration tests for request aware cookie handling. Adds a new type `cookieSpec` which is accepted as an argument to `RunAuthorizationCodeFlow`. `TestRelyingPartySession` now runs with `wrapServer` true/false and with two cookie handlers, one static and one request aware. The request aware handler extracts encryption keys from a secret using a salt from a "login_id" cookie. Signed-off-by: Mark Laing <mark.laing@canonical.com> --------- Signed-off-by: Mark Laing <mark.laing@canonical.com>
This commit is contained in:
Mark Laing
GitHub
parent
21e830e275
commit
c0d0ba9b0f
3 changed files with 179 additions and 22 deletions
|
|
@ -3,6 +3,9 @@ package client_test
|
|||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/hmac"
|
||||
cryptoRand "crypto/rand"
|
||||
"crypto/sha512"
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
|
|
@ -18,6 +21,8 @@ import (
|
|||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/gorilla/securecookie"
|
||||
"github.com/jeremija/gosubmit"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
|
@ -42,6 +47,15 @@ var Logger = slog.New(
|
|||
|
||||
var CTX context.Context
|
||||
|
||||
type cookieSpec struct {
|
||||
cookieHandler *httphelper.CookieHandler
|
||||
extraCookies []*http.Cookie
|
||||
}
|
||||
|
||||
var defaultCookieSpec = cookieSpec{
|
||||
cookieHandler: httphelper.NewCookieHandler([]byte("test1234test1234"), []byte("test1234test1234"), httphelper.WithUnsecure()),
|
||||
}
|
||||
|
||||
func TestMain(m *testing.M) {
|
||||
os.Exit(func() int {
|
||||
ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGINT)
|
||||
|
|
@ -53,14 +67,65 @@ func TestMain(m *testing.M) {
|
|||
}
|
||||
|
||||
func TestRelyingPartySession(t *testing.T) {
|
||||
for _, wrapServer := range []bool{false, true} {
|
||||
t.Run(fmt.Sprint("wrapServer ", wrapServer), func(t *testing.T) {
|
||||
testRelyingPartySession(t, wrapServer)
|
||||
})
|
||||
secret := make([]byte, 64)
|
||||
_, _ = cryptoRand.Read(secret)
|
||||
hashFunc := sha512.New
|
||||
requestAwareCookieHandler := httphelper.NewRequestAwareCookieHandler(func(r *http.Request) (*securecookie.SecureCookie, error) {
|
||||
// Expect a login ID cookie to be set.
|
||||
loginIDCookie, err := r.Cookie("login_id")
|
||||
require.NoError(t, err)
|
||||
|
||||
// Login ID cookie will contain a UUID.
|
||||
loginID, err := uuid.Parse(loginIDCookie.Value)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Use a HMAC hash of the secret and login ID (salt).
|
||||
h := hmac.New(hashFunc, secret)
|
||||
_, err = h.Write(loginID[:])
|
||||
require.NoError(t, err)
|
||||
|
||||
// Write a separator for the hash key.
|
||||
_, err = h.Write([]byte("INTEGRITY"))
|
||||
require.NoError(t, err)
|
||||
hash := h.Sum(nil) // 64 bytes
|
||||
|
||||
// Write a separator for the block key.
|
||||
_, err = h.Write([]byte("ENCRYPTION"))
|
||||
require.NoError(t, err)
|
||||
block := h.Sum(nil)[:32] // Sum is 64 bytes but we only want 32 for AES-256.
|
||||
|
||||
return securecookie.New(hash, block), nil
|
||||
}, httphelper.WithUnsecure())
|
||||
|
||||
loginID := uuid.New()
|
||||
loginIDCookie := &http.Cookie{
|
||||
Name: "login_id",
|
||||
Value: loginID.String(),
|
||||
Secure: false,
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
Path: "/",
|
||||
}
|
||||
|
||||
cookieCases := []cookieSpec{
|
||||
defaultCookieSpec,
|
||||
{
|
||||
cookieHandler: requestAwareCookieHandler,
|
||||
extraCookies: []*http.Cookie{
|
||||
loginIDCookie,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, cookieCase := range cookieCases {
|
||||
for _, wrapServer := range []bool{false, true} {
|
||||
t.Run(fmt.Sprint("wrapServer ", wrapServer, " requestAwareCookieHandler ", cookieCase.cookieHandler.IsRequestAware()), func(t *testing.T) {
|
||||
testRelyingPartySession(t, wrapServer, cookieCase)
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func testRelyingPartySession(t *testing.T, wrapServer bool) {
|
||||
func testRelyingPartySession(t *testing.T, wrapServer bool, cookieSpec cookieSpec) {
|
||||
t.Log("------- start example OP ------")
|
||||
targetURL := "http://local-site"
|
||||
exampleStorage := storage.NewStorage(storage.NewUserStore(targetURL))
|
||||
|
|
@ -74,7 +139,7 @@ func testRelyingPartySession(t *testing.T, wrapServer bool) {
|
|||
clientID := t.Name() + "-" + strconv.FormatInt(seed.Int63(), 25)
|
||||
|
||||
t.Log("------- run authorization code flow ------")
|
||||
provider, tokens := RunAuthorizationCodeFlow(t, opServer, clientID, "secret")
|
||||
provider, tokens := RunAuthorizationCodeFlow(t, opServer, clientID, "secret", cookieSpec)
|
||||
|
||||
t.Log("------- refresh tokens ------")
|
||||
|
||||
|
|
@ -220,7 +285,7 @@ func testResourceServerTokenExchange(t *testing.T, wrapServer bool) {
|
|||
clientSecret := "secret"
|
||||
|
||||
t.Log("------- run authorization code flow ------")
|
||||
provider, tokens := RunAuthorizationCodeFlow(t, opServer, clientID, clientSecret)
|
||||
provider, tokens := RunAuthorizationCodeFlow(t, opServer, clientID, clientSecret, defaultCookieSpec)
|
||||
|
||||
resourceServer, err := rs.NewResourceServerClientCredentials(CTX, opServer.URL, clientID, clientSecret)
|
||||
require.NoError(t, err, "new resource server")
|
||||
|
|
@ -275,7 +340,7 @@ func testResourceServerTokenExchange(t *testing.T, wrapServer bool) {
|
|||
require.Nil(t, tokenExchangeResponse, "token exchange response")
|
||||
}
|
||||
|
||||
func RunAuthorizationCodeFlow(t *testing.T, opServer *httptest.Server, clientID, clientSecret string) (provider rp.RelyingParty, tokens *oidc.Tokens[*oidc.IDTokenClaims]) {
|
||||
func RunAuthorizationCodeFlow(t *testing.T, opServer *httptest.Server, clientID, clientSecret string, cookieSpec cookieSpec) (provider rp.RelyingParty, tokens *oidc.Tokens[*oidc.IDTokenClaims]) {
|
||||
targetURL := "http://local-site"
|
||||
localURL, err := url.Parse(targetURL + "/login?requestID=1234")
|
||||
require.NoError(t, err, "local url")
|
||||
|
|
@ -294,8 +359,6 @@ func RunAuthorizationCodeFlow(t *testing.T, opServer *httptest.Server, clientID,
|
|||
}
|
||||
|
||||
t.Log("------- create RP ------")
|
||||
key := []byte("test1234test1234")
|
||||
cookieHandler := httphelper.NewCookieHandler(key, key, httphelper.WithUnsecure())
|
||||
provider, err = rp.NewRelyingPartyOIDC(
|
||||
CTX,
|
||||
opServer.URL,
|
||||
|
|
@ -303,7 +366,7 @@ func RunAuthorizationCodeFlow(t *testing.T, opServer *httptest.Server, clientID,
|
|||
clientSecret,
|
||||
targetURL,
|
||||
[]string{"openid", "email", "profile", "offline_access"},
|
||||
rp.WithPKCE(cookieHandler),
|
||||
rp.WithPKCE(cookieSpec.cookieHandler),
|
||||
rp.WithAuthStyle(oauth2.AuthStyleInHeader),
|
||||
rp.WithVerifierOpts(
|
||||
rp.WithIssuedAtOffset(5*time.Second),
|
||||
|
|
@ -317,6 +380,11 @@ func RunAuthorizationCodeFlow(t *testing.T, opServer *httptest.Server, clientID,
|
|||
state := "state-" + strconv.FormatInt(seed.Int63(), 25)
|
||||
capturedW := httptest.NewRecorder()
|
||||
get := httptest.NewRequest("GET", localURL.String(), nil)
|
||||
for _, cookie := range cookieSpec.extraCookies {
|
||||
get.AddCookie(cookie)
|
||||
http.SetCookie(capturedW, cookie)
|
||||
}
|
||||
|
||||
rp.AuthURLHandler(func() string { return state }, provider,
|
||||
rp.WithPromptURLParam("Hello, World!", "Goodbye, World!"),
|
||||
rp.WithURLParam("custom", "param"),
|
||||
|
|
|
|||
|
|
@ -410,12 +410,19 @@ func AuthURLHandler(stateFn func() string, rp RelyingParty, urlParam ...URLParam
|
|||
}
|
||||
|
||||
state := stateFn()
|
||||
if err := trySetStateCookie(w, state, rp); err != nil {
|
||||
if err := trySetStateCookie(r, w, state, rp); err != nil {
|
||||
unauthorizedError(w, r, "failed to create state cookie: "+err.Error(), state, rp)
|
||||
return
|
||||
}
|
||||
if rp.IsPKCE() {
|
||||
codeChallenge, err := GenerateAndStoreCodeChallenge(w, rp)
|
||||
var codeChallenge string
|
||||
var err error
|
||||
if rp.CookieHandler().IsRequestAware() {
|
||||
codeChallenge, err = GenerateAndStoreCodeChallengeWithRequest(r, w, rp)
|
||||
} else {
|
||||
codeChallenge, err = GenerateAndStoreCodeChallenge(w, rp)
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
unauthorizedError(w, r, "failed to create code challenge: "+err.Error(), state, rp)
|
||||
return
|
||||
|
|
@ -436,6 +443,15 @@ func GenerateAndStoreCodeChallenge(w http.ResponseWriter, rp RelyingParty) (stri
|
|||
return oidc.NewSHACodeChallenge(codeVerifier), nil
|
||||
}
|
||||
|
||||
// GenerateAndStoreCodeChallenge generates a PKCE code challenge and stores its verifier into a secure cookie
|
||||
func GenerateAndStoreCodeChallengeWithRequest(r *http.Request, w http.ResponseWriter, rp RelyingParty) (string, error) {
|
||||
codeVerifier := base64.RawURLEncoding.EncodeToString([]byte(uuid.New().String()))
|
||||
if err := rp.CookieHandler().SetRequestAwareCookie(r, w, pkceCode, codeVerifier); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return oidc.NewSHACodeChallenge(codeVerifier), nil
|
||||
}
|
||||
|
||||
// ErrMissingIDToken is returned when an id_token was expected,
|
||||
// but not received in the token response.
|
||||
var ErrMissingIDToken = errors.New("id_token missing")
|
||||
|
|
@ -607,9 +623,16 @@ func Userinfo[U SubjectGetter](ctx context.Context, token, tokenType, subject st
|
|||
return userinfo, nil
|
||||
}
|
||||
|
||||
func trySetStateCookie(w http.ResponseWriter, state string, rp RelyingParty) error {
|
||||
func trySetStateCookie(r *http.Request, w http.ResponseWriter, state string, rp RelyingParty) error {
|
||||
if rp.CookieHandler() != nil {
|
||||
if err := rp.CookieHandler().SetCookie(w, stateParam, state); err != nil {
|
||||
var err error
|
||||
if rp.CookieHandler().IsRequestAware() {
|
||||
err = rp.CookieHandler().SetRequestAwareCookie(r, w, stateParam, state)
|
||||
} else {
|
||||
err = rp.CookieHandler().SetCookie(w, stateParam, state)
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,12 +8,13 @@ import (
|
|||
)
|
||||
|
||||
type CookieHandler struct {
|
||||
securecookie *securecookie.SecureCookie
|
||||
secureOnly bool
|
||||
sameSite http.SameSite
|
||||
maxAge int
|
||||
domain string
|
||||
path string
|
||||
securecookie *securecookie.SecureCookie
|
||||
secureCookieFunc func(r *http.Request) (*securecookie.SecureCookie, error)
|
||||
secureOnly bool
|
||||
sameSite http.SameSite
|
||||
maxAge int
|
||||
domain string
|
||||
path string
|
||||
}
|
||||
|
||||
func NewCookieHandler(hashKey, encryptKey []byte, opts ...CookieHandlerOpt) *CookieHandler {
|
||||
|
|
@ -30,6 +31,21 @@ func NewCookieHandler(hashKey, encryptKey []byte, opts ...CookieHandlerOpt) *Coo
|
|||
return c
|
||||
}
|
||||
|
||||
func NewRequestAwareCookieHandler(secureCookieFunc func(r *http.Request) (*securecookie.SecureCookie, error), opts ...CookieHandlerOpt) *CookieHandler {
|
||||
c := &CookieHandler{
|
||||
secureCookieFunc: secureCookieFunc,
|
||||
secureOnly: true,
|
||||
sameSite: http.SameSiteLaxMode,
|
||||
path: "/",
|
||||
}
|
||||
|
||||
for _, opt := range opts {
|
||||
opt(c)
|
||||
}
|
||||
|
||||
return c
|
||||
}
|
||||
|
||||
type CookieHandlerOpt func(*CookieHandler)
|
||||
|
||||
func WithUnsecure() CookieHandlerOpt {
|
||||
|
|
@ -47,6 +63,10 @@ func WithSameSite(sameSite http.SameSite) CookieHandlerOpt {
|
|||
func WithMaxAge(maxAge int) CookieHandlerOpt {
|
||||
return func(c *CookieHandler) {
|
||||
c.maxAge = maxAge
|
||||
if c.IsRequestAware() {
|
||||
return
|
||||
}
|
||||
|
||||
c.securecookie.MaxAge(maxAge)
|
||||
}
|
||||
}
|
||||
|
|
@ -68,8 +88,17 @@ func (c *CookieHandler) CheckCookie(r *http.Request, name string) (string, error
|
|||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
secureCookie := c.securecookie
|
||||
if c.IsRequestAware() {
|
||||
secureCookie, err = c.secureCookieFunc(r)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
}
|
||||
|
||||
var value string
|
||||
if err := c.securecookie.Decode(name, cookie.Value, &value); err != nil {
|
||||
if err := secureCookie.Decode(name, cookie.Value, &value); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return value, nil
|
||||
|
|
@ -87,6 +116,10 @@ func (c *CookieHandler) CheckQueryCookie(r *http.Request, name string) (string,
|
|||
}
|
||||
|
||||
func (c *CookieHandler) SetCookie(w http.ResponseWriter, name, value string) error {
|
||||
if c.IsRequestAware() {
|
||||
return errors.New("Cookie handler is request aware")
|
||||
}
|
||||
|
||||
encoded, err := c.securecookie.Encode(name, value)
|
||||
if err != nil {
|
||||
return err
|
||||
|
|
@ -104,6 +137,35 @@ func (c *CookieHandler) SetCookie(w http.ResponseWriter, name, value string) err
|
|||
return nil
|
||||
}
|
||||
|
||||
func (c *CookieHandler) SetRequestAwareCookie(r *http.Request, w http.ResponseWriter, name string, value string) error {
|
||||
if !c.IsRequestAware() {
|
||||
return errors.New("Cookie handler is not request aware")
|
||||
}
|
||||
|
||||
secureCookie, err := c.secureCookieFunc(r)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
encoded, err := secureCookie.Encode(name, value)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
http.SetCookie(w, &http.Cookie{
|
||||
Name: name,
|
||||
Value: encoded,
|
||||
Domain: c.domain,
|
||||
Path: c.path,
|
||||
MaxAge: c.maxAge,
|
||||
HttpOnly: true,
|
||||
Secure: c.secureOnly,
|
||||
SameSite: c.sameSite,
|
||||
})
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (c *CookieHandler) DeleteCookie(w http.ResponseWriter, name string) {
|
||||
http.SetCookie(w, &http.Cookie{
|
||||
Name: name,
|
||||
|
|
@ -116,3 +178,7 @@ func (c *CookieHandler) DeleteCookie(w http.ResponseWriter, name string) {
|
|||
SameSite: c.sameSite,
|
||||
})
|
||||
}
|
||||
|
||||
func (c *CookieHandler) IsRequestAware() bool {
|
||||
return c.secureCookieFunc != nil
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue