fix: set correct owner on project grants (#9089)

# Which Problems Are Solved

In versions previous to v2.66 it was possible to set a different
resource owner on project grants. This was introduced with the new
resource based API. The resource owner was possible to overwrite using
the x-zitadel-org header.

Because of this issue project grants got the wrong resource owner,
instead of the owner of the project it got the granted org which is
wrong because a resource owner of an aggregate is not allowed to change.

# How the Problems Are Solved

- The wrong owners of the events are set to the original owner of the
project.
- A new event is pushed to these aggregates `project.owner.corrected` 
- The projection updates the owners of the user grants if that event was
written

# Additional Changes

The eventstore push function (replaced in version 2.66) writes the
correct resource owner.

# Additional Context

closes https://github.com/zitadel/zitadel/issues/9072
This commit is contained in:
Silvan
2025-01-15 11:22:16 +01:00
committed by GitHub
parent b664ffe993
commit 1949d1546a
11 changed files with 308 additions and 1 deletions

111
cmd/setup/45.go Normal file
View File

@@ -0,0 +1,111 @@
package setup
import (
"context"
_ "embed"
"encoding/json"
"fmt"
"github.com/zitadel/logging"
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/eventstore"
"github.com/zitadel/zitadel/internal/query/projection"
"github.com/zitadel/zitadel/internal/repository/instance"
"github.com/zitadel/zitadel/internal/repository/owner"
"github.com/zitadel/zitadel/internal/repository/project"
)
var (
//go:embed 45.sql
correctProjectOwnerEvents string
)
type CorrectProjectOwners struct {
eventstore *eventstore.Eventstore
}
func (mig *CorrectProjectOwners) Execute(ctx context.Context, _ eventstore.Event) error {
instances, err := mig.eventstore.InstanceIDs(
ctx,
eventstore.NewSearchQueryBuilder(eventstore.ColumnsInstanceIDs).
OrderDesc().
AddQuery().
AggregateTypes("instance").
EventTypes(instance.InstanceAddedEventType).
Builder(),
)
if err != nil {
return err
}
ctx = authz.SetCtxData(ctx, authz.CtxData{UserID: "SETUP"})
for i, instance := range instances {
ctx = authz.WithInstanceID(ctx, instance)
logging.WithFields("instance_id", instance, "migration", mig.String(), "progress", fmt.Sprintf("%d/%d", i+1, len(instances))).Info("correct owners of projects")
didCorrect, err := mig.correctInstanceProjects(ctx, instance)
if err != nil {
return err
}
if !didCorrect {
continue
}
_, err = projection.ProjectGrantProjection.Trigger(ctx)
logging.OnError(err).Debug("failed triggering project grant projection to update owners")
}
return nil
}
func (mig *CorrectProjectOwners) correctInstanceProjects(ctx context.Context, instance string) (didCorrect bool, err error) {
var correctedOwners []eventstore.Command
tx, err := mig.eventstore.Client().BeginTx(ctx, nil)
if err != nil {
return false, err
}
defer func() {
if err != nil {
_ = tx.Rollback()
return
}
err = tx.Commit()
}()
rows, err := tx.QueryContext(ctx, correctProjectOwnerEvents, instance)
if err != nil {
return false, err
}
defer rows.Close()
for rows.Next() {
aggregate := &eventstore.Aggregate{
InstanceID: instance,
Type: project.AggregateType,
Version: project.AggregateVersion,
}
var payload json.RawMessage
err := rows.Scan(
&aggregate.ID,
&aggregate.ResourceOwner,
&payload,
)
if err != nil {
return false, err
}
previousOwners := make(map[uint32]string)
if err := json.Unmarshal(payload, &previousOwners); err != nil {
return false, err
}
correctedOwners = append(correctedOwners, owner.NewCorrected(ctx, aggregate, previousOwners))
}
if rows.Err() != nil {
return false, rows.Err()
}
_, err = mig.eventstore.PushWithClient(ctx, tx, correctedOwners...)
return len(correctedOwners) > 0, err
}
func (*CorrectProjectOwners) String() string {
return "43_correct_project_owners"
}

79
cmd/setup/45.sql Normal file
View File

@@ -0,0 +1,79 @@
WITH corrupt_streams AS (
select
e.instance_id
, e.aggregate_type
, e.aggregate_id
, min(e.sequence) as min_sequence
, count(distinct e.owner) as owner_count
from
eventstore.events2 e
where
e.instance_id = $1
and aggregate_type = 'project'
group by
e.instance_id
, e.aggregate_type
, e.aggregate_id
having
count(distinct e.owner) > 1
), correct_owners AS (
select
e.instance_id
, e.aggregate_type
, e.aggregate_id
, e.owner
from
eventstore.events2 e
join
corrupt_streams cs
on
e.instance_id = cs.instance_id
and e.aggregate_type = cs.aggregate_type
and e.aggregate_id = cs.aggregate_id
and e.sequence = cs.min_sequence
), wrong_events AS (
select
e.instance_id
, e.aggregate_type
, e.aggregate_id
, e.sequence
, e.owner wrong_owner
, co.owner correct_owner
from
eventstore.events2 e
join
correct_owners co
on
e.instance_id = co.instance_id
and e.aggregate_type = co.aggregate_type
and e.aggregate_id = co.aggregate_id
and e.owner <> co.owner
), updated_events AS (
UPDATE eventstore.events2 e
SET owner = we.correct_owner
FROM
wrong_events we
WHERE
e.instance_id = we.instance_id
and e.aggregate_type = we.aggregate_type
and e.aggregate_id = we.aggregate_id
and e.sequence = we.sequence
RETURNING
we.aggregate_id
, we.correct_owner
, we.sequence
, we.wrong_owner
)
SELECT
ue.aggregate_id
, ue.correct_owner
, jsonb_object_agg(
ue.sequence::TEXT --formant to string because crdb is not able to handle int
, ue.wrong_owner
) payload
FROM
updated_events ue
GROUP BY
ue.aggregate_id
, ue.correct_owner
;

View File

@@ -130,6 +130,7 @@ type Steps struct {
s42Apps7OIDCConfigsLoginVersion *Apps7OIDCConfigsLoginVersion
s43CreateFieldsDomainIndex *CreateFieldsDomainIndex
s44ReplaceCurrentSequencesIndex *ReplaceCurrentSequencesIndex
s45CorrectProjectOwners *CorrectProjectOwners
}
func MustNewSteps(v *viper.Viper) *Steps {

View File

@@ -173,6 +173,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string)
steps.s42Apps7OIDCConfigsLoginVersion = &Apps7OIDCConfigsLoginVersion{dbClient: esPusherDBClient}
steps.s43CreateFieldsDomainIndex = &CreateFieldsDomainIndex{dbClient: queryDBClient}
steps.s44ReplaceCurrentSequencesIndex = &ReplaceCurrentSequencesIndex{dbClient: esPusherDBClient}
steps.s45CorrectProjectOwners = &CorrectProjectOwners{eventstore: eventstoreClient}
err = projection.Create(ctx, projectionDBClient, eventstoreClient, config.Projections, nil, nil, nil)
logging.OnError(err).Fatal("unable to start projections")
@@ -227,6 +228,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string)
steps.s36FillV2Milestones,
steps.s38BackChannelLogoutNotificationStart,
steps.s44ReplaceCurrentSequencesIndex,
steps.s45CorrectProjectOwners,
} {
mustExecuteMigration(ctx, eventstoreClient, step, "migration failed")
}

View File

@@ -0,0 +1,26 @@
---
title: Technical Advisory 10014
---
## Date
Versions: >= v2.67.3, v2.66 >= v2.66.6
Date: 2025-01-17
## Description
Prior to version [v2.66.0](https://github.com/zitadel/zitadel/releases/tag/v2.66.0), some project grants were incorrectly created under the granted organization instead of the project owner's organization. To find these grants, users had to set the `x-zitadel-orgid` header to the granted organization ID when using the [`ListAllProjectGrants`](/apis/resources/mgmt/management-service-add-project-grant) gRPC method.
Zitadel [v2.66.0](https://github.com/zitadel/zitadel/releases/tag/v2.66.0) corrected this behavior for new grants. However, existing grants were not automatically updated. Version v2.66.6 corrects the owner of these existing grants.
## Impact
After the release of v2.66.6, if your application uses the [`ListAllProjectGrants`](/apis/resources/mgmt/management-service-add-project-grant) method with the `x-zitadel-orgid` header set to the granted organization ID, you will not retrieve any results.
## Mitigation
To ensure your application continues to function correctly after the release of v2.66.6, implement the following changes:
1. **Conditional Header:** Only set the `x-zitadel-orgid` header to the project owner's organization ID if the user executing the [`ListAllProjectGrants`](/apis/resources/mgmt/management-service-add-project-grant) method belongs to a different organization than the project.
2. **Use `grantedOrgIdQuery`:** Utilize the `grantedOrgIdQuery` parameter to filter grants for the specific granted organization.

View File

@@ -214,6 +214,18 @@ We understand that these advisories may include breaking changes, and we aim to
<td>-</td>
<td>2024-12-09</td>
</tr>
<tr>
<td>
<a href="./advisory/a10014">A-10014</a>
</td>
<td>Correction of project grant owner</td>
<td>Breaking Behavior Change</td>
<td>
Correct project grant owners, ensuring they are correctly associated with the projects organization.
</td>
<td>-</td>
<td>2025-01-10</td>
</tr>
</table>
## Subscribe to our Mailing List

View File

@@ -601,6 +601,12 @@ func NewCond(name string, value interface{}) Condition {
}
}
func NewUnequalCond(name string, value any) Condition {
return func(param string) (string, []any) {
return name + " <> " + param, []any{value}
}
}
func NewNamespacedCondition(name string, value interface{}) NamespacedCondition {
return func(namespace string) Condition {
return NewCond(namespace+"."+name, value)

View File

@@ -125,7 +125,18 @@ func scanToSequence(rows *sql.Rows, sequences []*latestSequence) error {
return nil
}
sequence.sequence = currentSequence
if sequence.aggregate.ResourceOwner == "" {
if resourceOwner != "" && sequence.aggregate.ResourceOwner != "" && sequence.aggregate.ResourceOwner != resourceOwner {
logging.WithFields(
"current_sequence", sequence.sequence,
"instance_id", sequence.aggregate.InstanceID,
"agg_type", sequence.aggregate.Type,
"agg_id", sequence.aggregate.ID,
"current_owner", resourceOwner,
"provided_owner", sequence.aggregate.ResourceOwner,
).Info("would have set wrong resource owner")
}
// set resource owner from previous events
if resourceOwner != "" {
sequence.aggregate.ResourceOwner = resourceOwner
}

View File

@@ -93,6 +93,10 @@ func (p *projectGrantProjection) Reducers() []handler.AggregateReducer {
Event: project.ProjectRemovedType,
Reduce: p.reduceProjectRemoved,
},
{
Event: project.ProjectOwnerCorrected,
Reduce: p.reduceOwnerCorrected,
},
},
},
{
@@ -269,3 +273,17 @@ func (p *projectGrantProjection) reduceOwnerRemoved(event eventstore.Event) (*ha
),
), nil
}
func (p *projectGrantProjection) reduceOwnerCorrected(event eventstore.Event) (*handler.Statement, error) {
return handler.NewUpdateStatement(
event,
[]handler.Column{
handler.NewCol(ProjectGrantColumnResourceOwner, event.Aggregate().ResourceOwner),
},
[]handler.Condition{
handler.NewCond(ProjectGrantColumnInstanceID, event.Aggregate().InstanceID),
handler.NewCond(ProjectGrantColumnProjectID, event.Aggregate().ID),
handler.NewUnequalCond(ProjectGrantColumnResourceOwner, event.Aggregate().ResourceOwner),
},
), nil
}

View File

@@ -0,0 +1,40 @@
package owner
import (
"context"
"github.com/zitadel/zitadel/internal/eventstore"
)
const OwnerCorrectedType = ".owner.corrected"
type Corrected struct {
eventstore.BaseEvent `json:"-"`
PreviousOwners map[uint32]string `json:"previousOwners,omitempty"`
}
var _ eventstore.Command = (*Corrected)(nil)
func (e *Corrected) Payload() interface{} {
return e
}
func (e *Corrected) UniqueConstraints() []*eventstore.UniqueConstraint {
return nil
}
func NewCorrected(
ctx context.Context,
aggregate *eventstore.Aggregate,
previousOwners map[uint32]string,
) *Corrected {
return &Corrected{
BaseEvent: *eventstore.NewBaseEventForPush(
ctx,
aggregate,
eventstore.EventType(aggregate.Type+OwnerCorrectedType),
),
PreviousOwners: previousOwners,
}
}

View File

@@ -16,6 +16,7 @@ const (
ProjectDeactivatedType = projectEventTypePrefix + "deactivated"
ProjectReactivatedType = projectEventTypePrefix + "reactivated"
ProjectRemovedType = projectEventTypePrefix + "removed"
ProjectOwnerCorrected = projectEventTypePrefix + "owner.corrected"
ProjectSearchType = "project"
ProjectObjectRevision = uint8(1)