fix: consider oidc session events for authN milestones (#8089)

# Which Problems Are Solved After migrating the access token events in #7822, milestones based on authentication, resp. theses events would not be reached. # How the Problems Are Solved Additionally use the `oidc_session.Added` event to check for `milestone.AuthenticationSucceededOnInstance` and `milestone.AuthenticationSucceededOnApplication`. # Additional Changes None. # Additional Context - relates to #7822 - noticed internally (cherry picked from commit b6c10c4c83)
2024-12-14 03:54:21 +00:00 · 2024-06-12 06:49:14 +02:00 · 2024-06-12 06:49:14 +02:00 · 663484e1fb
commit 663484e1fb
parent eb8f61d1c1
12 changed files with 213 additions and 33 deletions
--- a/internal/api/grpc/admin/restrictions_integration_allow_public_org_registrations_test.go
+++ b/internal/api/grpc/admin/restrictions_integration_allow_public_org_registrations_test.go
@ -22,7 +22,7 @@ import (
 func TestServer_Restrictions_DisallowPublicOrgRegistration(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 	defer cancel()
-	domain, _, iamOwnerCtx := Tester.UseIsolatedInstance(t, ctx, SystemCTX)
+	domain, _, _, iamOwnerCtx := Tester.UseIsolatedInstance(t, ctx, SystemCTX)
 	regOrgUrl, err := url.Parse("http://" + domain + ":8080/ui/login/register/org")
 	require.NoError(t, err)
 	// The CSRF cookie must be sent with every request.
--- a/internal/api/grpc/admin/restrictions_integration_allowed_languages_test.go
+++ b/internal/api/grpc/admin/restrictions_integration_allowed_languages_test.go
@ -33,7 +33,7 @@ func TestServer_Restrictions_AllowedLanguages(t *testing.T) {
 		unsupportedLanguage       = language.Afrikaans
 	)

-	domain, _, iamOwnerCtx := Tester.UseIsolatedInstance(t, ctx, SystemCTX)
+	domain, _, _, iamOwnerCtx := Tester.UseIsolatedInstance(t, ctx, SystemCTX)
 	t.Run("assumed defaults are correct", func(tt *testing.T) {
 		tt.Run("languages are not restricted by default", func(ttt *testing.T) {
 			restrictions, err := Tester.Client.Admin.GetRestrictions(iamOwnerCtx, &admin.GetRestrictionsRequest{})
--- a/internal/api/grpc/system/instance_integration_test.go
+++ b/internal/api/grpc/system/instance_integration_test.go
@ -14,7 +14,7 @@ import (
 )

 func TestServer_ListInstances(t *testing.T) {
-	domain, instanceID, _ := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
+	domain, instanceID, _, _ := Tester.UseIsolatedInstance(t, CTX, SystemCTX)

 	tests := []struct {
 		name    string
--- a/internal/api/grpc/system/limits_integration_auditlogretention_test.go
+++ b/internal/api/grpc/system/limits_integration_auditlogretention_test.go
@ -21,7 +21,7 @@ import (
 )

 func TestServer_Limits_AuditLogRetention(t *testing.T) {
-	_, instanceID, iamOwnerCtx := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
+	_, instanceID, _, iamOwnerCtx := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
 	userID, projectID, appID, projectGrantID := seedObjects(iamOwnerCtx, t)
 	beforeTime := time.Now()
 	farPast := timestamppb.New(beforeTime.Add(-10 * time.Hour).UTC())
--- a/internal/api/grpc/system/limits_integration_block_test.go
+++ b/internal/api/grpc/system/limits_integration_block_test.go
@ -27,7 +27,7 @@ import (
 )

 func TestServer_Limits_Block(t *testing.T) {
-	domain, instanceID, iamOwnerCtx := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
+	domain, instanceID, _, iamOwnerCtx := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
 	tests := []*test{
 		publicAPIBlockingTest(domain),
 		{
--- a/internal/api/grpc/system/quotas_enabled/quota_integration_test.go
+++ b/internal/api/grpc/system/quotas_enabled/quota_integration_test.go
@ -66,7 +66,7 @@ func TestServer_QuotaNotification_Limit(t *testing.T) {
 }

 func TestServer_QuotaNotification_NoLimit(t *testing.T) {
-	_, instanceID, IAMOwnerCTX := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
+	_, instanceID, _, IAMOwnerCTX := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
 	amount := 10
 	percent := 50
 	percentAmount := amount * percent / 100
@ -148,7 +148,7 @@ func awaitNotification(t *testing.T, bodies chan []byte, unit quota.Unit, percen
 }

 func TestServer_AddAndRemoveQuota(t *testing.T) {
-	_, instanceID, _ := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
+	_, instanceID, _, _ := Tester.UseIsolatedInstance(t, CTX, SystemCTX)

 	got, err := Tester.Client.System.SetQuota(SystemCTX, &system.SetQuotaRequest{
 		InstanceId:    instanceID,
--- a/internal/integration/client.go
+++ b/internal/integration/client.go
@ -76,7 +76,7 @@ func newClient(cc *grpc.ClientConn) Client {
 	}
 }

-func (t *Tester) UseIsolatedInstance(tt *testing.T, iamOwnerCtx, systemCtx context.Context) (primaryDomain, instanceId string, authenticatedIamOwnerCtx context.Context) {
+func (t *Tester) UseIsolatedInstance(tt *testing.T, iamOwnerCtx, systemCtx context.Context) (primaryDomain, instanceId, adminID string, authenticatedIamOwnerCtx context.Context) {
 	primaryDomain = RandString(5) + ".integration.localhost"
 	instance, err := t.Client.System.CreateInstance(systemCtx, &system.CreateInstanceRequest{
 		InstanceName: "testinstance",
@ -89,20 +89,23 @@ func (t *Tester) UseIsolatedInstance(tt *testing.T, iamOwnerCtx, systemCtx conte
 			},
 		},
 	})
-	if err != nil {
-		panic(err)
-	}
+	require.NoError(tt, err)
 	t.createClientConn(iamOwnerCtx, fmt.Sprintf("%s:%d", primaryDomain, t.Config.Port))
 	instanceId = instance.GetInstanceId()
+	owner, err := t.Queries.GetUserByLoginName(authz.WithInstanceID(iamOwnerCtx, instanceId), true, "owner@"+primaryDomain)
+	require.NoError(tt, err)
 	t.Users.Set(instanceId, IAMOwner, &User{
+		User:  owner,
 		Token: instance.GetPat(),
 	})
 	newCtx := t.WithInstanceAuthorization(iamOwnerCtx, IAMOwner, instanceId)
+	var adminUser *mgmt.ImportHumanUserResponse
 	// the following serves two purposes:
 	// 1. it ensures that the instance is ready to be used
 	// 2. it enables a normal login with the default admin user credentials
 	require.EventuallyWithT(tt, func(collectT *assert.CollectT) {
-		_, importErr := t.Client.Mgmt.ImportHumanUser(newCtx, &mgmt.ImportHumanUserRequest{
+		var importErr error
+		adminUser, importErr = t.Client.Mgmt.ImportHumanUser(newCtx, &mgmt.ImportHumanUserRequest{
 			UserName: "zitadel-admin@zitadel.localhost",
 			Email: &mgmt.ImportHumanUserRequest_Email{
 				Email:           "zitadel-admin@zitadel.localhost",
@ -117,7 +120,7 @@ func (t *Tester) UseIsolatedInstance(tt *testing.T, iamOwnerCtx, systemCtx conte
 		})
 		assert.NoError(collectT, importErr)
 	}, 2*time.Minute, 100*time.Millisecond, "instance not ready")
-	return primaryDomain, instanceId, newCtx
+	return primaryDomain, instanceId, adminUser.GetUserId(), newCtx
 }

 func (s *Tester) CreateHumanUser(ctx context.Context) *user.AddHumanUserResponse {
--- a/internal/integration/oidc.go
+++ b/internal/integration/oidc.go
@ -151,7 +151,10 @@ func (s *Tester) CreateAPIClientBasic(ctx context.Context, projectID string) (*m
 const CodeVerifier = "codeVerifier"

 func (s *Tester) CreateOIDCAuthRequest(ctx context.Context, clientID, loginClient, redirectURI string, scope ...string) (authRequestID string, err error) {
-	provider, err := s.CreateRelyingParty(ctx, clientID, redirectURI, scope...)
+	return s.CreateOIDCAuthRequestWithDomain(ctx, s.Config.ExternalDomain, clientID, loginClient, redirectURI, scope...)
+}
+func (s *Tester) CreateOIDCAuthRequestWithDomain(ctx context.Context, domain, clientID, loginClient, redirectURI string, scope ...string) (authRequestID string, err error) {
+	provider, err := s.CreateRelyingPartyForDomain(ctx, domain, clientID, redirectURI, scope...)
 	if err != nil {
 		return "", err
 	}
@ -212,11 +215,15 @@ func (s *Tester) OIDCIssuer() string {
 }

 func (s *Tester) CreateRelyingParty(ctx context.Context, clientID, redirectURI string, scope ...string) (rp.RelyingParty, error) {
+	return s.CreateRelyingPartyForDomain(ctx, s.Config.ExternalDomain, clientID, redirectURI, scope...)
+}
+
+func (s *Tester) CreateRelyingPartyForDomain(ctx context.Context, domain, clientID, redirectURI string, scope ...string) (rp.RelyingParty, error) {
 	if len(scope) == 0 {
 		scope = []string{oidc.ScopeOpenID}
 	}
 	loginClient := &http.Client{Transport: &loginRoundTripper{http.DefaultTransport}}
-	return rp.NewRelyingPartyOIDC(ctx, s.OIDCIssuer(), clientID, "", redirectURI, scope, rp.WithHTTPClient(loginClient))
+	return rp.NewRelyingPartyOIDC(ctx, http_util.BuildHTTP(domain, s.Config.Port, s.Config.ExternalSecure), clientID, "", redirectURI, scope, rp.WithHTTPClient(loginClient))
 }

 type loginRoundTripper struct {
--- a/internal/notification/handlers/quota_notifier_test.go
+++ b/internal/notification/handlers/quota_notifier_test.go
@ -21,7 +21,7 @@ import (
 )

 func TestServer_QuotaNotification_Limit(t *testing.T) {
-	_, instanceID, iamOwnerCtx := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
+	_, instanceID, _, iamOwnerCtx := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
 	amount := 10
 	percent := 50
 	percentAmount := amount * percent / 100
@ -67,7 +67,7 @@ func TestServer_QuotaNotification_Limit(t *testing.T) {
 }

 func TestServer_QuotaNotification_NoLimit(t *testing.T) {
-	_, instanceID, iamOwnerCtx := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
+	_, instanceID, _, iamOwnerCtx := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
 	amount := 10
 	percent := 50
 	percentAmount := amount * percent / 100
--- a/internal/notification/handlers/telemetry_pusher_integration_test.go
+++ b/internal/notification/handlers/telemetry_pusher_integration_test.go
@ -4,38 +4,126 @@ package handlers_test

 import (
 	"bytes"
+	"context"
 	"encoding/json"
+	"net/url"
 	"testing"
 	"time"

+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/zitadel/oidc/v3/pkg/client/rp"
+	"github.com/zitadel/oidc/v3/pkg/oidc"
+
+	"github.com/zitadel/zitadel/internal/integration"
+	"github.com/zitadel/zitadel/pkg/grpc/app"
 	"github.com/zitadel/zitadel/pkg/grpc/management"
+	"github.com/zitadel/zitadel/pkg/grpc/object"
+	oidc_v2 "github.com/zitadel/zitadel/pkg/grpc/oidc/v2beta"
+	"github.com/zitadel/zitadel/pkg/grpc/project"
 	"github.com/zitadel/zitadel/pkg/grpc/system"
 )

 func TestServer_TelemetryPushMilestones(t *testing.T) {
-	primaryDomain, instanceID, iamOwnerCtx := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
+	primaryDomain, instanceID, adminID, iamOwnerCtx := Tester.UseIsolatedInstance(t, CTX, SystemCTX)
 	t.Log("testing against instance with primary domain", primaryDomain)
 	awaitMilestone(t, Tester.MilestoneChan, primaryDomain, "InstanceCreated")
-	project, err := Tester.Client.Mgmt.AddProject(iamOwnerCtx, &management.AddProjectRequest{Name: "integration"})
-	if err != nil {
-		t.Fatal(err)
-	}
+
+	projectAdded, err := Tester.Client.Mgmt.AddProject(iamOwnerCtx, &management.AddProjectRequest{Name: "integration"})
+	require.NoError(t, err)
 	awaitMilestone(t, Tester.MilestoneChan, primaryDomain, "ProjectCreated")
-	if _, err = Tester.Client.Mgmt.AddOIDCApp(iamOwnerCtx, &management.AddOIDCAppRequest{
-		ProjectId: project.GetId(),
+
+	redirectURI := "http://localhost:8888"
+	application, err := Tester.Client.Mgmt.AddOIDCApp(iamOwnerCtx, &management.AddOIDCAppRequest{
+		ProjectId:       projectAdded.GetId(),
 		Name:            "integration",
-	}); err != nil {
-		t.Fatal(err)
-	}
+		RedirectUris:    []string{redirectURI},
+		ResponseTypes:   []app.OIDCResponseType{app.OIDCResponseType_OIDC_RESPONSE_TYPE_CODE},
+		GrantTypes:      []app.OIDCGrantType{app.OIDCGrantType_OIDC_GRANT_TYPE_AUTHORIZATION_CODE},
+		AppType:         app.OIDCAppType_OIDC_APP_TYPE_WEB,
+		AuthMethodType:  app.OIDCAuthMethodType_OIDC_AUTH_METHOD_TYPE_NONE,
+		DevMode:         true,
+		AccessTokenType: app.OIDCTokenType_OIDC_TOKEN_TYPE_JWT,
+	})
+	require.NoError(t, err)
 	awaitMilestone(t, Tester.MilestoneChan, primaryDomain, "ApplicationCreated")
-	// TODO: trigger and await milestone AuthenticationSucceededOnInstance
-	// TODO: trigger and await milestone AuthenticationSucceededOnApplication
-	if _, err = Tester.Client.System.RemoveInstance(SystemCTX, &system.RemoveInstanceRequest{InstanceId: instanceID}); err != nil {
-		t.Fatal(err)
-	}
+
+	// create the session to be used for the authN of the clients
+	sessionID, sessionToken, _, _ := Tester.CreatePasswordSession(t, iamOwnerCtx, adminID, "Password1!")
+
+	console := consoleOIDCConfig(iamOwnerCtx, t)
+	loginToClient(iamOwnerCtx, t, primaryDomain, console.GetClientId(), instanceID, console.GetRedirectUris()[0], sessionID, sessionToken)
+	awaitMilestone(t, Tester.MilestoneChan, primaryDomain, "AuthenticationSucceededOnInstance")
+
+	// make sure the client has been projected
+	require.EventuallyWithT(t, func(collectT *assert.CollectT) {
+		_, err := Tester.Client.Mgmt.GetAppByID(iamOwnerCtx, &management.GetAppByIDRequest{
+			ProjectId: projectAdded.GetId(),
+			AppId:     application.GetAppId(),
+		})
+		assert.NoError(collectT, err)
+	}, 1*time.Minute, 100*time.Millisecond, "app not found")
+	loginToClient(iamOwnerCtx, t, primaryDomain, application.GetClientId(), instanceID, redirectURI, sessionID, sessionToken)
+	awaitMilestone(t, Tester.MilestoneChan, primaryDomain, "AuthenticationSucceededOnApplication")
+
+	_, err = Tester.Client.System.RemoveInstance(SystemCTX, &system.RemoveInstanceRequest{InstanceId: instanceID})
+	require.NoError(t, err)
 	awaitMilestone(t, Tester.MilestoneChan, primaryDomain, "InstanceDeleted")
 }

+func loginToClient(iamOwnerCtx context.Context, t *testing.T, primaryDomain, clientID, instanceID, redirectURI, sessionID, sessionToken string) {
+	authRequestID, err := Tester.CreateOIDCAuthRequestWithDomain(iamOwnerCtx, primaryDomain, clientID, Tester.Users.Get(instanceID, integration.IAMOwner).ID, redirectURI, "openid")
+	require.NoError(t, err)
+	callback, err := Tester.Client.OIDCv2.CreateCallback(iamOwnerCtx, &oidc_v2.CreateCallbackRequest{
+		AuthRequestId: authRequestID,
+		CallbackKind: &oidc_v2.CreateCallbackRequest_Session{Session: &oidc_v2.Session{
+			SessionId:    sessionID,
+			SessionToken: sessionToken,
+		}},
+	})
+	require.NoError(t, err)
+	provider, err := Tester.CreateRelyingPartyForDomain(iamOwnerCtx, primaryDomain, clientID, redirectURI)
+	require.NoError(t, err)
+	callbackURL, err := url.Parse(callback.GetCallbackUrl())
+	require.NoError(t, err)
+	code := callbackURL.Query().Get("code")
+	_, err = rp.CodeExchange[*oidc.IDTokenClaims](iamOwnerCtx, code, provider, rp.WithCodeVerifier(integration.CodeVerifier))
+	require.NoError(t, err)
+}
+
+func consoleOIDCConfig(iamOwnerCtx context.Context, t *testing.T) *app.OIDCConfig {
+	projects, err := Tester.Client.Mgmt.ListProjects(iamOwnerCtx, &management.ListProjectsRequest{
+		Queries: []*project.ProjectQuery{
+			{
+				Query: &project.ProjectQuery_NameQuery{
+					NameQuery: &project.ProjectNameQuery{
+						Name:   "ZITADEL",
+						Method: object.TextQueryMethod_TEXT_QUERY_METHOD_EQUALS,
+					},
+				},
+			},
+		},
+	})
+	require.NoError(t, err)
+	require.Len(t, projects.GetResult(), 1)
+	apps, err := Tester.Client.Mgmt.ListApps(iamOwnerCtx, &management.ListAppsRequest{
+		ProjectId: projects.GetResult()[0].GetId(),
+		Queries: []*app.AppQuery{
+			{
+				Query: &app.AppQuery_NameQuery{
+					NameQuery: &app.AppNameQuery{
+						Name:   "Console",
+						Method: object.TextQueryMethod_TEXT_QUERY_METHOD_EQUALS,
+					},
+				},
+			},
+		},
+	})
+	require.NoError(t, err)
+	require.Len(t, apps.GetResult(), 1)
+	return apps.GetResult()[0].GetOidcConfig()
+}
+
 func awaitMilestone(t *testing.T, bodies chan []byte, primaryDomain, expectMilestoneType string) {
 	for {
 		select {
--- a/internal/query/projection/milestones.go
+++ b/internal/query/projection/milestones.go
@ -10,6 +10,7 @@ import (
 	"github.com/zitadel/zitadel/internal/eventstore/handler/v2"
 	"github.com/zitadel/zitadel/internal/repository/instance"
 	"github.com/zitadel/zitadel/internal/repository/milestone"
+	"github.com/zitadel/zitadel/internal/repository/oidcsession"
 	"github.com/zitadel/zitadel/internal/repository/project"
 	"github.com/zitadel/zitadel/internal/repository/user"
 )
@ -104,6 +105,15 @@ func (p *milestoneProjection) Reducers() []handler.AggregateReducer {
 				},
 			},
 		},
+		{
+			Aggregate: oidcsession.AggregateType,
+			EventReducers: []handler.EventReducer{
+				{
+					Event:  oidcsession.AddedType,
+					Reduce: p.reduceOIDCSessionAdded,
+				},
+			},
+		},
 		{
 			Aggregate: milestone.AggregateType,
 			EventReducers: []handler.EventReducer{
@ -217,6 +227,40 @@ func (p *milestoneProjection) reduceUserTokenAdded(event eventstore.Event) (*han
 	return handler.NewMultiStatement(e, statements...), nil
 }

+func (p *milestoneProjection) reduceOIDCSessionAdded(event eventstore.Event) (*handler.Statement, error) {
+	e, err := assertEvent[*oidcsession.AddedEvent](event)
+	if err != nil {
+		return nil, err
+	}
+	statements := []func(eventstore.Event) handler.Exec{
+		handler.AddUpdateStatement(
+			[]handler.Column{
+				handler.NewCol(MilestoneColumnReachedDate, event.CreatedAt()),
+			},
+			[]handler.Condition{
+				handler.NewCond(MilestoneColumnInstanceID, event.Aggregate().InstanceID),
+				handler.NewCond(MilestoneColumnType, milestone.AuthenticationSucceededOnInstance),
+				handler.NewIsNullCond(MilestoneColumnReachedDate),
+			},
+		),
+	}
+	// We ignore authentications without app, for example JWT profile or PAT
+	if e.ClientID != "" {
+		statements = append(statements, handler.AddUpdateStatement(
+			[]handler.Column{
+				handler.NewCol(MilestoneColumnReachedDate, event.CreatedAt()),
+			},
+			[]handler.Condition{
+				handler.NewCond(MilestoneColumnInstanceID, event.Aggregate().InstanceID),
+				handler.NewCond(MilestoneColumnType, milestone.AuthenticationSucceededOnApplication),
+				handler.Not(handler.NewTextArrayContainsCond(MilestoneColumnIgnoreClientIDs, e.ClientID)),
+				handler.NewIsNullCond(MilestoneColumnReachedDate),
+			},
+		))
+	}
+	return handler.NewMultiStatement(e, statements...), nil
+}
+
 func (p *milestoneProjection) reduceInstanceRemoved(event eventstore.Event) (*handler.Statement, error) {
 	if _, err := assertEvent[*instance.InstanceRemovedEvent](event); err != nil {
 		return nil, err
--- a/internal/query/projection/milestones_test.go
+++ b/internal/query/projection/milestones_test.go
@ -9,6 +9,7 @@ import (
 	"github.com/zitadel/zitadel/internal/eventstore/handler/v2"
 	"github.com/zitadel/zitadel/internal/repository/instance"
 	"github.com/zitadel/zitadel/internal/repository/milestone"
+	"github.com/zitadel/zitadel/internal/repository/oidcsession"
 	"github.com/zitadel/zitadel/internal/repository/project"
 	"github.com/zitadel/zitadel/internal/repository/user"
 	"github.com/zitadel/zitadel/internal/zerrors"
@ -294,6 +295,43 @@ func TestMilestonesProjection_reduces(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "reduceOIDCSessionAdded",
+			args: args{
+				event: getEvent(timedTestEvent(
+					oidcsession.AddedType,
+					oidcsession.AggregateType,
+					[]byte(`{"clientID": "client-id"}`),
+					now,
+				), eventstore.GenericEventMapper[oidcsession.AddedEvent]),
+			},
+			reduce: (&milestoneProjection{}).reduceOIDCSessionAdded,
+			want: wantReduce{
+				aggregateType: eventstore.AggregateType("oidc_session"),
+				sequence:      15,
+				executer: &testExecuter{
+					executions: []execution{
+						{
+							expectedStmt: "UPDATE projections.milestones SET reached_date = $1 WHERE (instance_id = $2) AND (type = $3) AND (reached_date IS NULL)",
+							expectedArgs: []interface{}{
+								now,
+								"instance-id",
+								milestone.AuthenticationSucceededOnInstance,
+							},
+						},
+						{
+							expectedStmt: "UPDATE projections.milestones SET reached_date = $1 WHERE (instance_id = $2) AND (type = $3) AND (NOT (ignore_client_ids @> $4)) AND (reached_date IS NULL)",
+							expectedArgs: []interface{}{
+								now,
+								"instance-id",
+								milestone.AuthenticationSucceededOnApplication,
+								database.TextArray[string]{"client-id"},
+							},
+						},
+					},
+				},
+			},
+		},
 		{
 			name: "reduceInstanceRemoved",
 			args: args{