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

IAM: Refactor user session management to middleware #3139

Merged
merged 3 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 24 additions & 11 deletions auth/api/iam/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/nuts-foundation/nuts-node/http/user"
"html/template"
"net/http"
"net/url"
Expand Down Expand Up @@ -73,17 +74,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"

//go:embed assets
var assetsFS embed.FS

Expand Down Expand Up @@ -143,6 +133,29 @@ func (r Wrapper) Routes(router core.EchoRouter) {
return next(c)
}
}, audit.Middleware(apiModuleName))
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) {
Expand Down
18 changes: 7 additions & 11 deletions auth/api/iam/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/nuts-foundation/nuts-node/http/user"
"net/http"
"net/http/httptest"
"net/url"
Expand Down Expand Up @@ -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(),
})

Expand Down Expand Up @@ -710,8 +703,11 @@ 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()
router.EXPECT().Use(gomock.AssignableToTypeOf(user.SessionMiddleware{}.Handle))

(&Wrapper{}).Routes(router)
(&Wrapper{
storageEngine: storage.NewTestStorageEngine(t),
}).Routes(router)
}

func TestWrapper_middleware(t *testing.T) {
Expand Down
5 changes: 2 additions & 3 deletions auth/api/iam/openid4vp.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/nuts-foundation/nuts-node/http/user"
"net/http"
"net/url"
"slices"
Expand Down Expand Up @@ -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"}
}
Expand Down
25 changes: 2 additions & 23 deletions auth/api/iam/openid4vp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package iam
import (
"context"
"encoding/json"
"github.com/nuts-foundation/nuts-node/http/user"
"net/http"
"net/url"
"strings"
Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand Down
39 changes: 0 additions & 39 deletions auth/api/iam/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
19 changes: 0 additions & 19 deletions auth/api/iam/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 0 additions & 9 deletions auth/api/iam/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading