Merge commit from fork

* fix: require permission to create and update session

* fix: require permission to fail auth requests

* merge main and fix integration tests

* fix merge

* fix integration tests

* fix integration tests

* fix saml permission check
This commit is contained in:
Livio Spring
2025-07-15 07:38:00 -04:00
committed by GitHub
parent 91487a0b23
commit 4c942f3477
33 changed files with 681 additions and 334 deletions

View File

@@ -137,6 +137,11 @@ func (c *Commands) FailAuthRequest(ctx context.Context, id string, reason domain
if writeModel.AuthRequestState != domain.AuthRequestStateAdded {
return nil, nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-Sx202nt", "Errors.AuthRequest.AlreadyHandled")
}
if authz.GetCtxData(ctx).UserID != writeModel.LoginClient {
if err := c.checkPermission(ctx, domain.PermissionSessionLink, writeModel.ResourceOwner, ""); err != nil {
return nil, nil, err
}
}
err = c.pushAppendAndReduce(ctx, writeModel, authrequest.NewFailedEvent(
ctx,
&authrequest.NewAggregate(id, authz.GetInstance(ctx).InstanceID()).Aggregate,

View File

@@ -911,7 +911,8 @@ func TestCommands_LinkSessionToAuthRequest(t *testing.T) {
func TestCommands_FailAuthRequest(t *testing.T) {
mockCtx := authz.NewMockContext("instanceID", "orgID", "loginClient")
type fields struct {
eventstore func(*testing.T) *eventstore.Eventstore
eventstore func(*testing.T) *eventstore.Eventstore
checkPermission domain.PermissionCheck
}
type args struct {
ctx context.Context
@@ -945,6 +946,45 @@ func TestCommands_FailAuthRequest(t *testing.T) {
wantErr: zerrors.ThrowPreconditionFailed(nil, "COMMAND-Sx202nt", "Errors.AuthRequest.AlreadyHandled"),
},
},
{
"missing permission",
fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
authrequest.NewAddedEvent(mockCtx, &authrequest.NewAggregate("V2_id", "instanceID").Aggregate,
"login",
"clientID",
"redirectURI",
"state",
"nonce",
[]string{"openid"},
[]string{"audience"},
domain.OIDCResponseTypeCode,
domain.OIDCResponseModeQuery,
nil,
nil,
nil,
nil,
nil,
nil,
true,
"issuer",
),
),
),
),
checkPermission: newMockPermissionCheckNotAllowed(),
},
args{
ctx: mockCtx,
id: "V2_id",
reason: domain.OIDCErrorReasonLoginRequired,
},
res{
wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"),
},
},
{
"failed",
fields{
@@ -977,6 +1017,7 @@ func TestCommands_FailAuthRequest(t *testing.T) {
domain.OIDCErrorReasonLoginRequired),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args{
ctx: mockCtx,
@@ -1006,7 +1047,8 @@ func TestCommands_FailAuthRequest(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := &Commands{
eventstore: tt.fields.eventstore(t),
eventstore: tt.fields.eventstore(t),
checkPermission: tt.fields.checkPermission,
}
details, got, err := c.FailAuthRequest(tt.args.ctx, tt.args.id, tt.args.reason)
require.ErrorIs(t, err, tt.res.wantErr)

View File

@@ -136,6 +136,9 @@ func (c *Commands) CancelDeviceAuth(ctx context.Context, id string, reason domai
if !model.State.Exists() {
return nil, zerrors.ThrowNotFound(nil, "COMMAND-gee5A", "Errors.DeviceAuth.NotFound")
}
if err := c.checkPermission(ctx, domain.PermissionSessionLink, model.ResourceOwner, ""); err != nil {
return nil, err
}
pushedEvents, err := c.eventstore.Push(ctx, deviceauth.NewCanceledEvent(ctx, model.aggregate, reason))
if err != nil {
return nil, err

View File

@@ -578,7 +578,8 @@ func TestCommands_CancelDeviceAuth(t *testing.T) {
pushErr := errors.New("pushErr")
type fields struct {
eventstore func(*testing.T) *eventstore.Eventstore
eventstore func(*testing.T) *eventstore.Eventstore
checkPermission domain.PermissionCheck
}
type args struct {
ctx context.Context
@@ -602,6 +603,26 @@ func TestCommands_CancelDeviceAuth(t *testing.T) {
args: args{ctx, "123", domain.DeviceAuthCanceledDenied},
wantErr: zerrors.ThrowNotFound(nil, "COMMAND-gee5A", "Errors.DeviceAuth.NotFound"),
},
{
name: "missing permission, error",
fields: fields{
eventstore: expectEventstore(
expectFilter(eventFromEventPusherWithInstanceID(
"instance1",
deviceauth.NewAddedEvent(
ctx,
deviceauth.NewAggregate("123", "instance1"),
"client_id", "123", "456", now,
[]string{"a", "b", "c"},
[]string{"projectID", "clientID"}, true,
),
)),
),
checkPermission: newMockPermissionCheckNotAllowed(),
},
args: args{ctx, "123", domain.DeviceAuthCanceledDenied},
wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"),
},
{
name: "push error",
fields: fields{
@@ -623,6 +644,7 @@ func TestCommands_CancelDeviceAuth(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{ctx, "123", domain.DeviceAuthCanceledDenied},
wantErr: pushErr,
@@ -648,6 +670,7 @@ func TestCommands_CancelDeviceAuth(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{ctx, "123", domain.DeviceAuthCanceledDenied},
wantDetails: &domain.ObjectDetails{
@@ -675,6 +698,7 @@ func TestCommands_CancelDeviceAuth(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{ctx, "123", domain.DeviceAuthCanceledExpired},
wantDetails: &domain.ObjectDetails{
@@ -685,7 +709,8 @@ func TestCommands_CancelDeviceAuth(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := &Commands{
eventstore: tt.fields.eventstore(t),
eventstore: tt.fields.eventstore(t),
checkPermission: tt.fields.checkPermission,
}
gotDetails, err := c.CancelDeviceAuth(tt.args.ctx, tt.args.id, tt.args.reason)
require.ErrorIs(t, err, tt.wantErr)

View File

@@ -119,6 +119,9 @@ func (c *Commands) FailSAMLRequest(ctx context.Context, id string, reason domain
if writeModel.SAMLRequestState != domain.SAMLRequestStateAdded {
return nil, nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-32lGj1Fhjt", "Errors.SAMLRequest.AlreadyHandled")
}
if err := c.checkPermission(ctx, domain.PermissionSessionLink, writeModel.ResourceOwner, ""); err != nil {
return nil, nil, err
}
err = c.pushAppendAndReduce(ctx, writeModel, samlrequest.NewFailedEvent(
ctx,
&samlrequest.NewAggregate(id, authz.GetInstance(ctx).InstanceID()).Aggregate,

View File

@@ -786,7 +786,8 @@ func TestCommands_LinkSessionToSAMLRequest(t *testing.T) {
func TestCommands_FailSAMLRequest(t *testing.T) {
mockCtx := authz.NewMockContext("instanceID", "orgID", "loginClient")
type fields struct {
eventstore func(t *testing.T) *eventstore.Eventstore
eventstore func(t *testing.T) *eventstore.Eventstore
checkPermission domain.PermissionCheck
}
type args struct {
ctx context.Context
@@ -820,7 +821,40 @@ func TestCommands_FailSAMLRequest(t *testing.T) {
res{
wantErr: zerrors.ThrowPreconditionFailed(nil, "COMMAND-32lGj1Fhjt", "Errors.SAMLRequest.AlreadyHandled"),
},
}, {
},
{
"missing permission",
fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
samlrequest.NewAddedEvent(mockCtx, &samlrequest.NewAggregate("V2_id", "instanceID").Aggregate,
"login",
"application",
"acs",
"relaystate",
"request",
"binding",
"issuer",
"destination",
"responseissuer",
),
),
),
),
checkPermission: newMockPermissionCheckNotAllowed(),
},
args{
ctx: mockCtx,
id: "V2_id",
reason: domain.SAMLErrorReasonAuthNFailed,
description: "desc",
},
res{
wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"),
},
},
{
"already failed",
fields{
eventstore: expectEventstore(
@@ -843,6 +877,7 @@ func TestCommands_FailSAMLRequest(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args{
ctx: mockCtx,
@@ -879,6 +914,7 @@ func TestCommands_FailSAMLRequest(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args{
ctx: mockCtx,
@@ -908,7 +944,8 @@ func TestCommands_FailSAMLRequest(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := &Commands{
eventstore: tt.fields.eventstore(t),
eventstore: tt.fields.eventstore(t),
checkPermission: tt.fields.checkPermission,
}
details, got, err := c.FailSAMLRequest(tt.args.ctx, tt.args.id, tt.args.reason)
require.ErrorIs(t, err, tt.res.wantErr)

View File

@@ -285,7 +285,13 @@ func (s *SessionCommands) commands(ctx context.Context) (string, []eventstore.Co
return token, s.eventCommands, nil
}
func (c *Commands) CreateSession(ctx context.Context, cmds []SessionCommand, metadata map[string][]byte, userAgent *domain.UserAgent, lifetime time.Duration) (set *SessionChanged, err error) {
func (c *Commands) CreateSession(
ctx context.Context,
cmds []SessionCommand,
metadata map[string][]byte,
userAgent *domain.UserAgent,
lifetime time.Duration,
) (set *SessionChanged, err error) {
sessionID, err := c.idGenerator.Next()
if err != nil {
return nil, err
@@ -295,17 +301,29 @@ func (c *Commands) CreateSession(ctx context.Context, cmds []SessionCommand, met
if err != nil {
return nil, err
}
if err = c.checkSessionWritePermission(ctx, sessionWriteModel, ""); err != nil {
return nil, err
}
cmd := c.NewSessionCommands(cmds, sessionWriteModel)
cmd.Start(ctx, userAgent)
return c.updateSession(ctx, cmd, metadata, lifetime)
}
func (c *Commands) UpdateSession(ctx context.Context, sessionID string, cmds []SessionCommand, metadata map[string][]byte, lifetime time.Duration) (set *SessionChanged, err error) {
func (c *Commands) UpdateSession(
ctx context.Context,
sessionID, sessionToken string,
cmds []SessionCommand,
metadata map[string][]byte,
lifetime time.Duration,
) (set *SessionChanged, err error) {
sessionWriteModel := NewSessionWriteModel(sessionID, authz.GetInstance(ctx).InstanceID())
err = c.eventstore.FilterToQueryReducer(ctx, sessionWriteModel)
if err != nil {
return nil, err
}
if err = c.checkSessionWritePermission(ctx, sessionWriteModel, sessionToken); err != nil {
return nil, err
}
cmd := c.NewSessionCommands(cmds, sessionWriteModel)
return c.updateSession(ctx, cmd, metadata, lifetime)
}
@@ -380,6 +398,21 @@ func (c *Commands) updateSession(ctx context.Context, checks *SessionCommands, m
return changed, nil
}
// checkSessionWritePermission will check that the provided sessionToken is correct or
// if empty, check that the caller is granted the "session.write" permission on the resource owner of the authenticated user.
// In case the user is not set and the userResourceOwner is not set (also the case for the session creation),
// it will check permission on the instance.
func (c *Commands) checkSessionWritePermission(ctx context.Context, model *SessionWriteModel, sessionToken string) error {
if sessionToken != "" {
return c.sessionTokenVerifier(ctx, sessionToken, model.AggregateID, model.TokenID)
}
userResourceOwner, err := c.sessionUserResourceOwner(ctx, model)
if err != nil {
return err
}
return c.checkPermission(ctx, domain.PermissionSessionWrite, userResourceOwner, model.UserID)
}
// checkSessionTerminationPermission will check that the provided sessionToken is correct or
// if empty, check that the caller is either terminating the own session or
// is granted the "session.delete" permission on the resource owner of the authenticated user.

View File

@@ -145,8 +145,9 @@ func TestSessionCommands_getHumanWriteModel(t *testing.T) {
func TestCommands_CreateSession(t *testing.T) {
type fields struct {
idGenerator id.Generator
tokenCreator func(sessionID string) (string, string, error)
idGenerator id.Generator
tokenCreator func(sessionID string) (string, string, error)
checkPermission domain.PermissionCheck
}
type args struct {
ctx context.Context
@@ -194,6 +195,22 @@ func TestCommands_CreateSession(t *testing.T) {
err: zerrors.ThrowInternal(nil, "id", "filter failed"),
},
},
{
"missing permission",
fields{
idGenerator: mock.NewIDGeneratorExpectIDs(t, "sessionID"),
checkPermission: newMockPermissionCheckNotAllowed(),
},
args{
ctx: context.Background(),
},
[]expect{
expectFilter(),
},
res{
err: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"),
},
},
{
"negative lifetime",
fields{
@@ -203,6 +220,7 @@ func TestCommands_CreateSession(t *testing.T) {
"token",
nil
},
checkPermission: newMockPermissionCheckAllowed(),
},
args{
ctx: authz.NewMockContext("instance1", "", ""),
@@ -230,6 +248,7 @@ func TestCommands_CreateSession(t *testing.T) {
"token",
nil
},
checkPermission: newMockPermissionCheckAllowed(),
},
args{
ctx: authz.NewMockContext("instance1", "", ""),
@@ -275,6 +294,7 @@ func TestCommands_CreateSession(t *testing.T) {
eventstore: expectEventstore(tt.expect...)(t),
idGenerator: tt.fields.idGenerator,
sessionTokenCreator: tt.fields.tokenCreator,
checkPermission: tt.fields.checkPermission,
}
got, err := c.CreateSession(tt.args.ctx, tt.args.checks, tt.args.metadata, tt.args.userAgent, tt.args.lifetime)
require.ErrorIs(t, err, tt.res.err)
@@ -285,15 +305,17 @@ func TestCommands_CreateSession(t *testing.T) {
func TestCommands_UpdateSession(t *testing.T) {
type fields struct {
eventstore func(*testing.T) *eventstore.Eventstore
tokenVerifier func(ctx context.Context, sessionToken, sessionID, tokenID string) (err error)
eventstore func(*testing.T) *eventstore.Eventstore
tokenVerifier func(ctx context.Context, sessionToken, sessionID, tokenID string) (err error)
checkPermission domain.PermissionCheck
}
type args struct {
ctx context.Context
sessionID string
checks []SessionCommand
metadata map[string][]byte
lifetime time.Duration
ctx context.Context
sessionID string
sessionToken string
checks []SessionCommand
metadata map[string][]byte
lifetime time.Duration
}
type res struct {
want *SessionChanged
@@ -319,6 +341,67 @@ func TestCommands_UpdateSession(t *testing.T) {
err: zerrors.ThrowInternal(nil, "id", "filter failed"),
},
},
{
"invalid session token",
fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
session.NewAddedEvent(context.Background(),
&session.NewAggregate("sessionID", "instance1").Aggregate,
&domain.UserAgent{
FingerprintID: gu.Ptr("fp1"),
IP: net.ParseIP("1.2.3.4"),
Description: gu.Ptr("firefox"),
Header: http.Header{"foo": []string{"bar"}},
},
)),
eventFromEventPusher(
session.NewTokenSetEvent(context.Background(), &session.NewAggregate("sessionID", "instance1").Aggregate,
"tokenID")),
),
),
tokenVerifier: newMockTokenVerifierInvalid(),
},
args{
ctx: context.Background(),
sessionID: "sessionID",
sessionToken: "invalid",
},
res{
err: zerrors.ThrowPermissionDenied(nil, "COMMAND-sGr42", "Errors.Session.Token.Invalid"),
},
},
{
"no token, no permission",
fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
session.NewAddedEvent(context.Background(),
&session.NewAggregate("sessionID", "instance1").Aggregate,
&domain.UserAgent{
FingerprintID: gu.Ptr("fp1"),
IP: net.ParseIP("1.2.3.4"),
Description: gu.Ptr("firefox"),
Header: http.Header{"foo": []string{"bar"}},
},
)),
eventFromEventPusher(
session.NewTokenSetEvent(context.Background(), &session.NewAggregate("sessionID", "instance1").Aggregate,
"tokenID")),
),
),
checkPermission: newMockPermissionCheckNotAllowed(),
},
args{
ctx: context.Background(),
sessionID: "sessionID",
},
res{
err: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"),
},
},
{
"no change",
fields{
@@ -344,8 +427,9 @@ func TestCommands_UpdateSession(t *testing.T) {
},
},
args{
ctx: context.Background(),
sessionID: "sessionID",
ctx: context.Background(),
sessionID: "sessionID",
sessionToken: "token",
},
res{
want: &SessionChanged{
@@ -364,8 +448,9 @@ func TestCommands_UpdateSession(t *testing.T) {
c := &Commands{
eventstore: tt.fields.eventstore(t),
sessionTokenVerifier: tt.fields.tokenVerifier,
checkPermission: tt.fields.checkPermission,
}
got, err := c.UpdateSession(tt.args.ctx, tt.args.sessionID, tt.args.checks, tt.args.metadata, tt.args.lifetime)
got, err := c.UpdateSession(tt.args.ctx, tt.args.sessionID, tt.args.sessionToken, tt.args.checks, tt.args.metadata, tt.args.lifetime)
require.ErrorIs(t, err, tt.res.err)
assert.Equal(t, tt.res.want, got)
})