fix: add permission check for saml request query (#9520)

This commit is contained in:
Stefan Benz 2025-03-12 21:53:16 +01:00 committed by GitHub
parent d527a1c824
commit 5eb3a543e8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 47 additions and 10 deletions

View File

@ -12,6 +12,7 @@ import (
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/api/call"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/eventstore/handler/v2"
"github.com/zitadel/zitadel/internal/query/projection"
"github.com/zitadel/zitadel/internal/telemetry/tracing"
@ -28,9 +29,9 @@ type SamlRequest struct {
Binding string
}
func (a *SamlRequest) checkLoginClient(ctx context.Context) error {
func (a *SamlRequest) checkLoginClient(ctx context.Context, permissionCheck domain.PermissionCheck) error {
if uid := authz.GetCtxData(ctx).UserID; uid != a.LoginClient {
return zerrors.ThrowPermissionDenied(nil, "OIDCv2-aL0ag", "Errors.SamlRequest.WrongLoginClient")
return permissionCheck(ctx, domain.PermissionSessionRead, authz.GetInstance(ctx).InstanceID(), "")
}
return nil
}
@ -72,7 +73,7 @@ func (q *Queries) SamlRequestByID(ctx context.Context, shouldTriggerBulk bool, i
}
if checkLoginClient {
if err = dst.checkLoginClient(ctx); err != nil {
if err = dst.checkLoginClient(ctx, q.checkPermission); err != nil {
return nil, err
}
}

View File

@ -1,6 +1,7 @@
package query
import (
"context"
"database/sql"
"database/sql/driver"
_ "embed"
@ -13,6 +14,7 @@ import (
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/database"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/query/projection"
"github.com/zitadel/zitadel/internal/zerrors"
)
@ -41,6 +43,7 @@ func TestQueries_SamlRequestByID(t *testing.T) {
name string
args args
expect sqlExpectation
permissionCheck domain.PermissionCheck
want *SamlRequest
wantErr error
}{
@ -89,7 +92,7 @@ func TestQueries_SamlRequestByID(t *testing.T) {
wantErr: zerrors.ThrowInternal(sql.ErrConnDone, "QUERY-Ou8ue", "Errors.Internal"),
},
{
name: "wrong login client",
name: "wrong login client/ not permitted",
args: args{
shouldTriggerBulk: false,
id: "123",
@ -104,13 +107,46 @@ func TestQueries_SamlRequestByID(t *testing.T) {
"relayState",
"binding",
}, "123", "instanceID"),
wantErr: zerrors.ThrowPermissionDeniedf(nil, "OIDCv2-aL0ag", "Errors.SamlRequest.WrongLoginClient"),
permissionCheck: func(ctx context.Context, permission, orgID, resourceID string) (err error) {
return zerrors.ThrowPermissionDenied(nil, "id", "not permitted")
},
wantErr: zerrors.ThrowPermissionDenied(nil, "id", "not permitted"),
},
{
name: "wrong login client / permitted",
args: args{
shouldTriggerBulk: false,
id: "123",
checkLoginClient: true,
},
expect: mockQuery(expQuery, cols, []driver.Value{
"id",
testNow,
"otherLoginClient",
"issuer",
"acs",
"relayState",
"binding",
}, "123", "instanceID"),
permissionCheck: func(ctx context.Context, permission, orgID, resourceID string) (err error) {
return nil
},
want: &SamlRequest{
ID: "id",
CreationDate: testNow,
LoginClient: "otherLoginClient",
Issuer: "issuer",
ACS: "acs",
RelayState: "relayState",
Binding: "binding",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
execMock(t, tt.expect, func(db *sql.DB) {
q := &Queries{
checkPermission: tt.permissionCheck,
client: &database.DB{
DB: db,
Database: &prepareDB{},