IAM: Refactor user session management to middleware (#3139)

auth/api/iam/api.go

@@ -27,6 +27,7 @@ import (
 	"errors"
 	"fmt"
 	"github.com/nuts-foundation/nuts-node/http/cache"
+	"github.com/nuts-foundation/nuts-node/http/user"
 	"html/template"
 	"net/http"
 	"net/url"
@@ -71,17 +72,6 @@ const accessTokenValidity = 15 * time.Minute
 
 const oid4vciSessionValidity = 15 * time.Minute
 
-// userSessionCookieName is the name of the cookie used to store the user session.
-// It uses the __Host prefix, that instructs the user agent to treat it as a secure cookie:
-// - Must be set with the Secure attribute
-// - Must be set from an HTTPS uri
-// - Must not contain a Domain attribute
-// - Must contain a Path attribute
-// Also see:
-// - https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/06-Session_Management_Testing/02-Testing_for_Cookies_Attributes
-// - https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies
-const userSessionCookieName = "__Host-SID"
-
 // cacheControlMaxAgeURLs holds API endpoints that should have a max-age cache control header set.
 var cacheControlMaxAgeURLs = []string{
 	"/.well-known/did.json",
@@ -159,6 +149,29 @@ func (r Wrapper) Routes(router core.EchoRouter) {
 	}, audit.Middleware(apiModuleName))
 	router.Use(cache.MaxAge(5*time.Minute, cacheControlMaxAgeURLs...).Handle)
 	router.Use(cache.NoCache(cacheControlNoCacheURLs...).Handle)
+	router.Use(user.SessionMiddleware{
+		Skipper: func(c echo.Context) bool {
+			// The following URLs require a user session:
+			paths := []string{
+				"/oauth2/:did/user",
+				"/oauth2/:did/authorize",
+				"/oauth2/:did/callback",
+			}
+			for _, path := range paths {
+				if c.Path() == path {
+					return false
+				}
+			}
+			return true
+		},
+		TimeOut: time.Hour,
+		Store:   r.storageEngine.GetSessionDatabase().GetStore(time.Hour, "user", "session"),
+		CookiePath: func(tenantDID did.DID) string {
+			baseURL, _ := createOAuth2BaseURL(tenantDID)
+			// error only happens on invalid did:web DID, which can't happen here
+			return baseURL.Path
+		},
+	}.Handle)
 }
 
 func (r Wrapper) strictMiddleware(ctx echo.Context, request interface{}, operationID string, f StrictHandlerFunc) (interface{}, error) {
@@ -178,7 +191,6 @@ func middleware(ctx echo.Context, operationID string) {
 			HtmlPageTemplate: assets.ErrorTemplate,
 		})
 	}
-
 }
 
 // ResolveStatusCode maps errors returned by this API to specific HTTP status codes.

auth/api/iam/api_test.go

@@ -24,6 +24,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"github.com/nuts-foundation/nuts-node/http/user"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -396,23 +397,15 @@ func TestWrapper_HandleAuthorizeRequest(t *testing.T) {
 			RedirectURI:  "https://example.com/iam/holder/cb",
 			ResponseType: "code",
 		})
-		_ = ctx.client.userSessionStore().Put("session-id", UserSession{
-			TenantDID: holderDID,
-			Wallet: UserWallet{
-				DID: holderDID,
-			},
-		})
+		callCtx, _ := user.CreateTestSession(requestContext(nil), holderDID)
 		clientMetadata := oauth.OAuthClientMetadata{VPFormats: oauth.DefaultOpenIDSupportedFormats()}
 		ctx.iamClient.EXPECT().ClientMetadata(gomock.Any(), "https://example.com/.well-known/authorization-server/iam/verifier").Return(&clientMetadata, nil)
 		pdEndpoint := "https://example.com/oauth2/did:web:example.com:iam:verifier/presentation_definition?scope=test"
 		ctx.iamClient.EXPECT().PresentationDefinition(gomock.Any(), pdEndpoint).Return(&pe.PresentationDefinition{}, nil)
 		ctx.wallet.EXPECT().BuildSubmission(gomock.Any(), holderDID, pe.PresentationDefinition{}, clientMetadata.VPFormats, gomock.Any()).Return(&vc.VerifiablePresentation{}, &pe.PresentationSubmission{}, nil)
 		ctx.iamClient.EXPECT().PostAuthorizationResponse(gomock.Any(), vc.VerifiablePresentation{}, pe.PresentationSubmission{}, "https://example.com/oauth2/did:web:example.com:iam:verifier/response", "state").Return("https://example.com/iam/holder/redirect", nil)
 
-		res, err := ctx.client.HandleAuthorizeRequest(requestContext(map[string]interface{}{}, func(request *http.Request) {
-			request.Header = make(http.Header)
-			request.AddCookie(createUserSessionCookie("session-id", "/"))
-		}), HandleAuthorizeRequestRequestObject{
+		res, err := ctx.client.HandleAuthorizeRequest(callCtx, HandleAuthorizeRequestRequestObject{
 			Did: holderDID.String(),
 		})
 
@@ -713,7 +706,9 @@ func TestWrapper_Routes(t *testing.T) {
 		router.EXPECT().GET(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
 		router.EXPECT().POST(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
 
-		(&Wrapper{}).Routes(router)
+		(&Wrapper{
+			storageEngine: storage.NewTestStorageEngine(t),
+		}).Routes(router)
 	})
 	t.Run("cache middleware URLs match registered paths", func(t *testing.T) {
 		ctrl := gomock.NewController(t)
@@ -729,7 +724,9 @@ func TestWrapper_Routes(t *testing.T) {
 			return nil
 		}).AnyTimes()
 		router.EXPECT().Use(gomock.Any()).AnyTimes()
-		(&Wrapper{}).Routes(router)
+		(&Wrapper{
+			storageEngine: storage.NewTestStorageEngine(t),
+		}).Routes(router)
 
 		// Check that all cache-control max-age paths are actual paths
 		for _, path := range cacheControlMaxAgeURLs {

auth/api/iam/openid4vp.go

@@ -23,6 +23,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"github.com/nuts-foundation/nuts-node/http/user"
 	"net/http"
 	"net/url"
 	"slices"
@@ -285,9 +286,7 @@ func (r Wrapper) handleAuthorizeRequestFromVerifier(ctx context.Context, tenantD
 		return r.sendAndHandleDirectPostError(ctx, oauth.OAuth2Error{Code: oauth.InvalidRequest, Description: "missing nonce parameter"}, responseURI, state)
 	}
 
-	// TODO: Create session if it does not exist (use client state to get original Authorization Code request)?
-	//       Although it would be quite weird (maybe it expired).
-	userSession, err := r.loadUserSession(ctx.Value(httpRequestContextKey{}).(*http.Request), tenantDID, nil)
+	userSession, err := user.GetSession(ctx)
 	if userSession == nil {
 		return nil, oauth.OAuth2Error{Code: oauth.InvalidRequest, InternalError: err, Description: "no user session found"}
 	}

auth/api/iam/openid4vp_test.go

@@ -21,6 +21,7 @@ package iam
 import (
 	"context"
 	"encoding/json"
+	"github.com/nuts-foundation/nuts-node/http/user"
 	"net/http"
 	"net/url"
 	"strings"
@@ -190,25 +191,12 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) {
 		RedirectURI: "https://example.com/iam/holder/cb",
 		VerifierDID: &verifierDID,
 	}
-	userSession := UserSession{
-		TenantDID: holderDID,
-		Wallet: UserWallet{
-			DID: did.MustParseDID("did:jwk:123"),
-		},
-	}
-
-	httpRequest := &http.Request{
-		Header: http.Header{},
-	}
-	const userSessionID = "session_id"
-	httpRequest.AddCookie(createUserSessionCookie(userSessionID, "/"))
-	httpRequestCtx := context.WithValue(context.Background(), httpRequestContextKey{}, httpRequest)
+	httpRequestCtx, _ := user.CreateTestSession(context.Background(), holderDID)
 	t.Run("invalid client_id", func(t *testing.T) {
 		ctx := newTestClient(t)
 		params := defaultParams()
 		params[oauth.ClientIDParam] = "did:nuts:1"
 		expectPostError(t, ctx, oauth.InvalidRequest, "invalid client_id parameter (only did:web is supported)", responseURI, "state")
-		_ = ctx.client.userSessionStore().Put(userSessionID, userSession)
 
 		_, err := ctx.client.handleAuthorizeRequestFromVerifier(httpRequestCtx, holderDID, params, pe.WalletOwnerOrganization)
 
@@ -229,7 +217,6 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) {
 		params := defaultParams()
 		ctx.iamClient.EXPECT().ClientMetadata(gomock.Any(), "https://example.com/.well-known/authorization-server/iam/verifier").Return(nil, assert.AnError)
 		expectPostError(t, ctx, oauth.ServerError, "failed to get client metadata (verifier)", responseURI, "state")
-		_ = ctx.client.userSessionStore().Put(userSessionID, userSession)
 
 		_, err := ctx.client.handleAuthorizeRequestFromVerifier(httpRequestCtx, holderDID, params, pe.WalletOwnerOrganization)
 
@@ -249,7 +236,6 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) {
 		ctx := newTestClient(t)
 		params := defaultParams()
 		params[oauth.ClientMetadataParam] = "not empty"
-		_ = ctx.client.userSessionStore().Put(userSessionID, userSession)
 		expectPostError(t, ctx, oauth.InvalidRequest, "client_metadata and client_metadata_uri are mutually exclusive", responseURI, "state")
 
 		_, err := ctx.client.handleAuthorizeRequestFromVerifier(httpRequestCtx, holderDID, params, pe.WalletOwnerOrganization)
@@ -261,7 +247,6 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) {
 		params := defaultParams()
 		delete(params, oauth.ClientMetadataURIParam)
 		params[oauth.ClientMetadataParam] = "{invalid"
-		_ = ctx.client.userSessionStore().Put(userSessionID, userSession)
 		expectPostError(t, ctx, oauth.InvalidRequest, "invalid client_metadata", responseURI, "state")
 
 		_, err := ctx.client.handleAuthorizeRequestFromVerifier(httpRequestCtx, holderDID, params, pe.WalletOwnerOrganization)
@@ -273,7 +258,6 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) {
 		params := defaultParams()
 		ctx.iamClient.EXPECT().ClientMetadata(gomock.Any(), "https://example.com/.well-known/authorization-server/iam/verifier").Return(nil, assert.AnError)
 		expectPostError(t, ctx, oauth.ServerError, "failed to get client metadata (verifier)", responseURI, "state")
-		_ = ctx.client.userSessionStore().Put(userSessionID, userSession)
 
 		_, err := ctx.client.handleAuthorizeRequestFromVerifier(httpRequestCtx, holderDID, params, pe.WalletOwnerOrganization)
 
@@ -311,7 +295,6 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) {
 		ctx.iamClient.EXPECT().ClientMetadata(gomock.Any(), "https://example.com/.well-known/authorization-server/iam/verifier").Return(&clientMetadata, nil)
 		params := defaultParams()
 		params[oauth.PresentationDefParam] = "not empty"
-		_ = ctx.client.userSessionStore().Put(userSessionID, userSession)
 		expectPostError(t, ctx, oauth.InvalidRequest, "presentation_definition and presentation_definition_uri are mutually exclusive", responseURI, "state")
 
 		_, err := ctx.client.handleAuthorizeRequestFromVerifier(httpRequestCtx, holderDID, params, pe.WalletOwnerOrganization)
@@ -324,7 +307,6 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) {
 		delete(params, oauth.PresentationDefUriParam)
 		params[oauth.PresentationDefParam] = "{invalid"
 		ctx.iamClient.EXPECT().ClientMetadata(gomock.Any(), "https://example.com/.well-known/authorization-server/iam/verifier").Return(&clientMetadata, nil)
-		_ = ctx.client.userSessionStore().Put(userSessionID, userSession)
 		expectPostError(t, ctx, oauth.InvalidRequest, "invalid presentation_definition", responseURI, "state")
 
 		_, err := ctx.client.handleAuthorizeRequestFromVerifier(httpRequestCtx, holderDID, params, pe.WalletOwnerOrganization)
@@ -338,7 +320,6 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) {
 		ctx.iamClient.EXPECT().ClientMetadata(gomock.Any(), "https://example.com/.well-known/authorization-server/iam/verifier").Return(&clientMetadata, nil)
 		ctx.iamClient.EXPECT().PresentationDefinition(gomock.Any(), pdEndpoint).Return(nil, assert.AnError)
 		expectPostError(t, ctx, oauth.InvalidPresentationDefinitionURI, "failed to retrieve presentation definition on https://example.com/iam/verifier/presentation_definition?scope=test", responseURI, "state")
-		_ = ctx.client.userSessionStore().Put(userSessionID, userSession)
 
 		_, err := ctx.client.handleAuthorizeRequestFromVerifier(httpRequestCtx, holderDID, params, pe.WalletOwnerOrganization)
 
@@ -352,7 +333,6 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) {
 		ctx.iamClient.EXPECT().PresentationDefinition(gomock.Any(), pdEndpoint).Return(&pe.PresentationDefinition{}, nil)
 		ctx.wallet.EXPECT().BuildSubmission(gomock.Any(), holderDID, pe.PresentationDefinition{}, clientMetadata.VPFormats, gomock.Any()).Return(nil, nil, assert.AnError)
 		expectPostError(t, ctx, oauth.ServerError, assert.AnError.Error(), responseURI, "state")
-		_ = ctx.client.userSessionStore().Put(userSessionID, userSession)
 
 		_, err := ctx.client.handleAuthorizeRequestFromVerifier(httpRequestCtx, holderDID, params, pe.WalletOwnerOrganization)
 
@@ -366,7 +346,6 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) {
 		ctx.iamClient.EXPECT().PresentationDefinition(gomock.Any(), pdEndpoint).Return(&pe.PresentationDefinition{}, nil)
 		ctx.wallet.EXPECT().BuildSubmission(gomock.Any(), holderDID, pe.PresentationDefinition{}, clientMetadata.VPFormats, gomock.Any()).Return(nil, nil, holder.ErrNoCredentials)
 		expectPostError(t, ctx, oauth.InvalidRequest, "no credentials available (PD ID: , wallet: did:web:example.com:iam:holder)", responseURI, "state")
-		_ = ctx.client.userSessionStore().Put(userSessionID, userSession)
 
 		_, err := ctx.client.handleAuthorizeRequestFromVerifier(httpRequestCtx, holderDID, params, pe.WalletOwnerOrganization)
 

auth/api/iam/session.go

@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"net/url"
 
-	"github.com/lestrrat-go/jwx/v2/jwk"
 	"github.com/nuts-foundation/go-did/did"
 	"github.com/nuts-foundation/go-did/vc"
 	"github.com/nuts-foundation/nuts-node/http"
@@ -138,44 +137,6 @@ func (v *PEXConsumer) credentialMap() (map[string]vc.VerifiableCredential, error
 	return credentialMap, nil
 }
 
-// UserSession is a session-bound Verifiable Credential wallet.
-type UserSession struct {
-	// TenantDID is the requesting DID when the user session was created, typically the employer's (of the user) DID.
-	// A session needs to be scoped to the tenant DID, since the session gives access to the tenant's wallet,
-	// and the user session might contain session-bound credentials (e.g. EmployeeCredential) that were issued by the tenant.
-	TenantDID did.DID `json:"tenantDID"`
-	// PreAuthorizedUser is the user that is pre-authorized by the client application.
-	// It is stored to later assert that subsequent RequestUserAccessToken() calls that (accidentally or intentionally)
-	// re-use the browser session, are indeed for the same client application user.
-	PreAuthorizedUser *UserDetails `json:"preauthorized_user"`
-	Wallet            UserWallet   `json:"wallet"`
-}
-
-// UserWallet is a session-bound Verifiable Credential wallet.
-// It's an in-memory wallet which contains the user's private key in plain text.
-// This is OK, since the associated credentials are intended for protocol compatibility (OpenID4VP with a low-assurance EmployeeCredential),
-// when an actual user wallet is involved, this wallet isn't used.
-type UserWallet struct {
-	Credentials []vc.VerifiableCredential
-	// JWK is an in-memory key pair associated with the user's wallet in JWK form.
-	JWK []byte
-	// DID is the did:jwk DID of the user's wallet.
-	DID did.DID
-}
-
-// Key returns the JWK as jwk.Key
-func (w UserWallet) Key() (jwk.Key, error) {
-	set, err := jwk.Parse(w.JWK)
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse JWK: %w", err)
-	}
-	result, available := set.Key(0)
-	if !available {
-		return nil, errors.New("expected exactly 1 key in the JWK set")
-	}
-	return result, nil
-}
-
 // ServerState is a convenience type for extracting different types of data from the session.
 type ServerState struct {
 	CredentialMap          map[string]vc.VerifiableCredential

auth/api/iam/session_test.go

@@ -19,32 +19,13 @@
 package iam
 
 import (
-	"crypto/ecdsa"
-	"crypto/elliptic"
-	"crypto/rand"
 	"encoding/json"
-	"github.com/lestrrat-go/jwx/v2/jwk"
 	"github.com/nuts-foundation/nuts-node/vcr/pe"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"testing"
 )
 
-func TestUserWallet_Key(t *testing.T) {
-	t.Run("ok", func(t *testing.T) {
-		pk, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
-		keyAsJWK, err := jwk.FromRaw(pk)
-		require.NoError(t, err)
-		jwkAsJSON, _ := json.Marshal(keyAsJWK)
-		wallet := UserWallet{
-			JWK: jwkAsJSON,
-		}
-		key, err := wallet.Key()
-		require.NoError(t, err)
-		assert.Equal(t, keyAsJWK, key)
-	})
-}
-
 func TestOpenID4VPVerifier_next(t *testing.T) {
 	userPresentationDefinition := PresentationDefinition{
 		Id: "user",

auth/api/iam/types.go

@@ -19,8 +19,6 @@
 package iam
 
 import (
-	"net/http"
-
 	"github.com/nuts-foundation/go-did/did"
 	"github.com/nuts-foundation/go-did/vc"
 	"github.com/nuts-foundation/nuts-node/auth/oauth"
@@ -66,13 +64,6 @@ type WalletOwnerType = pe.WalletOwnerType
 // RequiredPresentationDefinitions is an alias
 type RequiredPresentationDefinitions = pe.WalletOwnerMapping
 
-// CookieReader is an interface for reading cookies from an HTTP request.
-// It is implemented by echo.Context and http.Request.
-type CookieReader interface {
-	// Cookie returns the named cookie provided in the request.
-	Cookie(name string) (*http.Cookie, error)
-}
-
 const (
 	// responseModeQuery returns the answer to the authorization request append as query parameters to the provided redirect_uri
 	responseModeQuery = "query" // default if no response_mode is specified