fix: dont (re)generate client secret with auth type none (#564)

internal/project/model/oidc_config.go

@@ -1,8 +1,15 @@
 package model
 
 import (
+	"fmt"
+	"strings"
+
+	"github.com/caos/logging"
+
 	"github.com/caos/zitadel/internal/crypto"
+	"github.com/caos/zitadel/internal/errors"
 	es_models "github.com/caos/zitadel/internal/eventstore/models"
+	"github.com/caos/zitadel/internal/id"
 )
 
 type OIDCConfig struct {
@@ -62,6 +69,34 @@ func (c *OIDCConfig) IsValid() bool {
 	return true
 }
 
+//ClientID random_number@projectname (eg. 495894098234@zitadel)
+func (c *OIDCConfig) GenerateNewClientID(idGenerator id.Generator, project *Project) error {
+	rndID, err := idGenerator.Next()
+	if err != nil {
+		return err
+	}
+
+	c.ClientID = fmt.Sprintf("%v@%v", rndID, strings.ReplaceAll(strings.ToLower(project.Name), " ", "_"))
+	return nil
+}
+
+func (c *OIDCConfig) GenerateClientSecretIfNeeded(generator crypto.Generator) (string, error) {
+	if c.AuthMethodType == OIDCAuthMethodTypeNone {
+		return "", nil
+	}
+	return c.GenerateNewClientSecret(generator)
+}
+
+func (c *OIDCConfig) GenerateNewClientSecret(generator crypto.Generator) (string, error) {
+	cryptoValue, stringSecret, err := crypto.NewCode(generator)
+	if err != nil {
+		logging.Log("MODEL-UpnTI").OnError(err).Error("unable to create client secret")
+		return "", errors.ThrowInternal(err, "MODEL-gH2Wl", "Errors.Project.CouldNotGenerateClientSecret")
+	}
+	c.ClientSecret = cryptoValue
+	return stringSecret, nil
+}
+
 func (c *OIDCConfig) getRequiredGrantTypes() []OIDCGrantType {
 	grantTypes := make([]OIDCGrantType, 0)
 	implicit := false

internal/project/repository/eventsourcing/eventstore.go

@@ -511,19 +511,16 @@ func (es *ProjectEventstore) AddApplication(ctx context.Context, app *proj_model
 	app.AppID = id
 
 	var stringPw string
-	var cryptoPw *crypto.CryptoValue
 	if app.OIDCConfig != nil {
 		app.OIDCConfig.AppID = id
-		stringPw, cryptoPw, err = generateNewClientSecret(es.pwGenerator)
+		err := app.OIDCConfig.GenerateNewClientID(es.idGenerator, existing)
 		if err != nil {
 			return nil, err
 		}
-		app.OIDCConfig.ClientSecret = cryptoPw
+		stringPw, err = app.OIDCConfig.GenerateClientSecretIfNeeded(es.pwGenerator)
-		clientID, err := generateNewClientID(es.idGenerator, existing)
 		if err != nil {
 			return nil, err
 		}
-		app.OIDCConfig.ClientID = clientID
 	}
 	repoProject := model.ProjectFromModel(existing)
 	repoApp := model.AppFromModel(app)
@@ -756,14 +753,17 @@ func (es *ProjectEventstore) ChangeOIDCConfigSecret(ctx context.Context, project
 	if app.Type != proj_model.AppTypeOIDC {
 		return nil, caos_errs.ThrowPreconditionFailed(nil, "EVENT-dile4", "Errors.Project.AppIsNotOIDC")
 	}
+	if app.OIDCConfig.AuthMethodType == proj_model.OIDCAuthMethodTypeNone {
+		return nil, caos_errs.ThrowPreconditionFailed(nil, "EVENT-GDrg2", "Errors.Project.OIDCAuthMethodNoneSecret")
+	}
 	repoProject := model.ProjectFromModel(existing)
 
-	stringPw, crypto, err := generateNewClientSecret(es.pwGenerator)
+	stringPw, err := app.OIDCConfig.GenerateNewClientSecret(es.pwGenerator)
 	if err != nil {
 		return nil, err
 	}
 
-	projectAggregate := OIDCConfigSecretChangedAggregate(es.Eventstore.AggregateCreator(), repoProject, appID, crypto)
+	projectAggregate := OIDCConfigSecretChangedAggregate(es.Eventstore.AggregateCreator(), repoProject, appID, app.OIDCConfig.ClientSecret)
 	err = es_sdk.Push(ctx, es.PushAggregates, repoProject.AppendEvents, projectAggregate)
 	if err != nil {
 		return nil, err

internal/project/repository/eventsourcing/eventstore_mock_test.go

@@ -3,6 +3,8 @@ package eventsourcing
 import (
 	"encoding/json"
 
+	"github.com/golang/mock/gomock"
+
 	mock_cache "github.com/caos/zitadel/internal/cache/mock"
 	"github.com/caos/zitadel/internal/crypto"
 	"github.com/caos/zitadel/internal/eventstore/mock"
@@ -11,7 +13,6 @@ import (
 	proj_model "github.com/caos/zitadel/internal/project/model"
 	"github.com/caos/zitadel/internal/project/repository/eventsourcing/model"
 	repo_model "github.com/caos/zitadel/internal/project/repository/eventsourcing/model"
-	"github.com/golang/mock/gomock"
 )
 
 func GetMockedEventstore(ctrl *gomock.Controller, mockEs *mock.MockEventstore) *ProjectEventstore {
@@ -131,13 +132,14 @@ func GetMockManipulateProjectWithRole(ctrl *gomock.Controller) *ProjectEventstor
 	return GetMockedEventstore(ctrl, mockEs)
 }
 
-func GetMockManipulateProjectWithOIDCApp(ctrl *gomock.Controller) *ProjectEventstore {
+func GetMockManipulateProjectWithOIDCApp(ctrl *gomock.Controller, authMethod proj_model.OIDCAuthMethodType) *ProjectEventstore {
 	data, _ := json.Marshal(model.Project{Name: "Name"})
 	appData, _ := json.Marshal(model.Application{AppID: "AppID", Name: "Name"})
 	oidcData, _ := json.Marshal(model.OIDCConfig{
-		AppID:         "AppID",
+		AppID:          "AppID",
-		ResponseTypes: []int32{int32(proj_model.OIDCResponseTypeCode)},
+		ResponseTypes:  []int32{int32(proj_model.OIDCResponseTypeCode)},
-		GrantTypes:    []int32{int32(proj_model.OIDCGrantTypeAuthorizationCode)},
+		GrantTypes:     []int32{int32(proj_model.OIDCGrantTypeAuthorizationCode)},
+		AuthMethodType: int32(authMethod),
 	})
 	events := []*es_models.Event{
 		&es_models.Event{AggregateID: "AggregateID", Sequence: 1, Type: model.ProjectAdded, Data: data},

internal/project/repository/eventsourcing/eventstore_test.go

@@ -5,11 +5,12 @@ import (
 	"encoding/json"
 	"testing"
 
+	"github.com/golang/mock/gomock"
+
 	"github.com/caos/zitadel/internal/api/authz"
 	caos_errs "github.com/caos/zitadel/internal/errors"
 	es_models "github.com/caos/zitadel/internal/eventstore/models"
 	"github.com/caos/zitadel/internal/project/model"
-	"github.com/golang/mock/gomock"
 )
 
 func TestProjectByID(t *testing.T) {
@@ -1111,7 +1112,6 @@ func TestAddApplication(t *testing.T) {
 	}
 	type res struct {
 		result  *model.Application
-		wantErr bool
 		errFunc func(err error) bool
 	}
 	tests := []struct {
@@ -1143,6 +1143,32 @@ func TestAddApplication(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "add app (none), ok",
+			args: args{
+				es:  GetMockManipulateProjectWithPw(ctrl),
+				ctx: authz.NewMockContext("orgID", "userID"),
+				app: &model.Application{ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 1},
+					AppID: "AppID",
+					Name:  "Name",
+					OIDCConfig: &model.OIDCConfig{
+						ResponseTypes:  []model.OIDCResponseType{model.OIDCResponseTypeCode},
+						GrantTypes:     []model.OIDCGrantType{model.OIDCGrantTypeAuthorizationCode},
+						AuthMethodType: model.OIDCAuthMethodTypeNone,
+					},
+				},
+			},
+			res: res{
+				result: &model.Application{ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 1},
+					Name: "Name",
+					OIDCConfig: &model.OIDCConfig{
+						ResponseTypes:  []model.OIDCResponseType{model.OIDCResponseTypeCode},
+						GrantTypes:     []model.OIDCGrantType{model.OIDCGrantTypeAuthorizationCode},
+						AuthMethodType: model.OIDCAuthMethodTypeNone,
+					},
+				},
+			},
+		},
 		{
 			name: "invalid app",
 			args: args{
@@ -1151,7 +1177,6 @@ func TestAddApplication(t *testing.T) {
 				app: &model.Application{ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 1}},
 			},
 			res: res{
-				wantErr: true,
 				errFunc: caos_errs.IsPreconditionFailed,
 			},
 		},
@@ -1170,7 +1195,6 @@ func TestAddApplication(t *testing.T) {
 				},
 			},
 			res: res{
-				wantErr: true,
 				errFunc: caos_errs.IsNotFound,
 			},
 		},
@@ -1178,22 +1202,24 @@ func TestAddApplication(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			result, err := tt.args.es.AddApplication(tt.args.ctx, tt.args.app)
-
+			if tt.res.errFunc == nil && err != nil {
-			if !tt.res.wantErr && result.AppID == "" {
+				t.Errorf("no error expected got:%T %v", err, err)
+			}
+			if tt.res.errFunc != nil && !tt.res.errFunc(err) {
+				t.Errorf("wrong error got %T: %v", err, err)
+			}
+			if tt.res.result != nil && result.AppID == "" {
 				t.Errorf("result has no id")
 			}
-			if !tt.res.wantErr && result.OIDCConfig == nil && result.OIDCConfig.ClientSecretString == "" {
+			if tt.res.result != nil && (tt.res.result.OIDCConfig.AuthMethodType != model.OIDCAuthMethodTypeNone && result.OIDCConfig.ClientSecretString == "") {
 				t.Errorf("result has no client secret")
 			}
-			if !tt.res.wantErr && result.OIDCConfig == nil && result.OIDCConfig.ClientID == "" {
+			if tt.res.result != nil && result.OIDCConfig.ClientID == "" {
 				t.Errorf("result has no clientid")
 			}
-			if !tt.res.wantErr && result.Name != tt.res.result.Name {
+			if tt.res.result != nil && tt.res.result.Name != result.Name {
 				t.Errorf("got wrong result key: expected: %v, actual: %v ", tt.res.result.Name, result.Name)
 			}
-			if tt.res.wantErr && !tt.res.errFunc(err) {
-				t.Errorf("got wrong err: %v ", err)
-			}
 		})
 	}
 }
@@ -1218,7 +1244,7 @@ func TestChangeApp(t *testing.T) {
 		{
 			name: "change app, ok",
 			args: args{
-				es:  GetMockManipulateProjectWithOIDCApp(ctrl),
+				es:  GetMockManipulateProjectWithOIDCApp(ctrl, model.OIDCAuthMethodTypeBasic),
 				ctx: authz.NewMockContext("orgID", "userID"),
 				app: &model.Application{ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 1},
 					AppID: "AppID",
@@ -1328,7 +1354,7 @@ func TestRemoveApp(t *testing.T) {
 		{
 			name: "remove app, ok",
 			args: args{
-				es:  GetMockManipulateProjectWithOIDCApp(ctrl),
+				es:  GetMockManipulateProjectWithOIDCApp(ctrl, model.OIDCAuthMethodTypeBasic),
 				ctx: authz.NewMockContext("orgID", "userID"),
 				app: &model.Application{ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 1},
 					AppID: "AppID",
@@ -1410,7 +1436,7 @@ func TestDeactivateApp(t *testing.T) {
 		{
 			name: "deactivate, ok",
 			args: args{
-				es:  GetMockManipulateProjectWithOIDCApp(ctrl),
+				es:  GetMockManipulateProjectWithOIDCApp(ctrl, model.OIDCAuthMethodTypeBasic),
 				ctx: authz.NewMockContext("orgID", "userID"),
 				app: &model.Application{ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 1},
 					AppID: "AppID",
@@ -1520,7 +1546,7 @@ func TestReactivateApp(t *testing.T) {
 		{
 			name: "reactivate, ok",
 			args: args{
-				es:  GetMockManipulateProjectWithOIDCApp(ctrl),
+				es:  GetMockManipulateProjectWithOIDCApp(ctrl, model.OIDCAuthMethodTypeBasic),
 				ctx: authz.NewMockContext("orgID", "userID"),
 				app: &model.Application{ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 1},
 					AppID: "AppID",
@@ -1630,7 +1656,7 @@ func TestChangeOIDCConfig(t *testing.T) {
 		{
 			name: "change oidc config, ok",
 			args: args{
-				es:  GetMockManipulateProjectWithOIDCApp(ctrl),
+				es:  GetMockManipulateProjectWithOIDCApp(ctrl, model.OIDCAuthMethodTypeBasic),
 				ctx: authz.NewMockContext("orgID", "userID"),
 				config: &model.OIDCConfig{
 					ObjectRoot:    es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 0},
@@ -1756,7 +1782,7 @@ func TestChangeOIDCConfigSecret(t *testing.T) {
 		{
 			name: "change oidc config secret, ok",
 			args: args{
-				es:  GetMockManipulateProjectWithOIDCApp(ctrl),
+				es:  GetMockManipulateProjectWithOIDCApp(ctrl, model.OIDCAuthMethodTypeBasic),
 				ctx: authz.NewMockContext("orgID", "userID"),
 				config: &model.OIDCConfig{
 					ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 0},
@@ -1765,13 +1791,29 @@ func TestChangeOIDCConfigSecret(t *testing.T) {
 			},
 			res: res{
 				result: &model.OIDCConfig{
-					ObjectRoot:    es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 0},
+					ObjectRoot:     es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 0},
-					AppID:         "AppID",
+					AppID:          "AppID",
-					ResponseTypes: []model.OIDCResponseType{model.OIDCResponseTypeIDToken},
+					ResponseTypes:  []model.OIDCResponseType{model.OIDCResponseTypeCode},
-					GrantTypes:    []model.OIDCGrantType{model.OIDCGrantTypeImplicit},
+					GrantTypes:     []model.OIDCGrantType{model.OIDCGrantTypeAuthorizationCode},
+					AuthMethodType: model.OIDCAuthMethodTypeBasic,
 				},
 			},
 		},
+		{
+			name: "auth method none, error",
+			args: args{
+				es:  GetMockManipulateProjectWithOIDCApp(ctrl, model.OIDCAuthMethodTypeNone),
+				ctx: authz.NewMockContext("orgID", "userID"),
+				config: &model.OIDCConfig{
+					ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 0},
+					AppID:      "AppID",
+				},
+			},
+			res: res{
+				wantErr: true,
+				errFunc: caos_errs.IsPreconditionFailed,
+			},
+		},
 		{
 			name: "no appID",
 			args: args{
@@ -1844,7 +1886,7 @@ func TestChangeOIDCConfigSecret(t *testing.T) {
 				t.Errorf("got wrong result AppID: expected: %v, actual: %v ", tt.res.result.AppID, result.AppID)
 			}
 			if !tt.res.wantErr && result.ClientSecretString == "" {
-				t.Errorf("got wrong result should habe client secret")
+				t.Errorf("got wrong result must have client secret")
 			}
 			if tt.res.wantErr && !tt.res.errFunc(err) {
 				t.Errorf("got wrong err: %v ", err)

internal/project/repository/eventsourcing/oidc.go

@@ -1,30 +0,0 @@
-package eventsourcing
-
-import (
-	"fmt"
-	"github.com/caos/logging"
-	"github.com/caos/zitadel/internal/crypto"
-	"github.com/caos/zitadel/internal/errors"
-	"github.com/caos/zitadel/internal/id"
-	"github.com/caos/zitadel/internal/project/model"
-	"strings"
-)
-
-//ClientID random_number@projectname (eg. 495894098234@zitadel)
-func generateNewClientID(idGenerator id.Generator, project *model.Project) (string, error) {
-	rndID, err := idGenerator.Next()
-	if err != nil {
-		return "", err
-	}
-
-	return fmt.Sprintf("%v@%v", rndID, strings.ReplaceAll(strings.ToLower(project.Name), " ", "_")), nil
-}
-
-func generateNewClientSecret(pwGenerator crypto.Generator) (string, *crypto.CryptoValue, error) {
-	cryptoValue, stringSecret, err := crypto.NewCode(pwGenerator)
-	if err != nil {
-		logging.Log("APP-UpnTI").OnError(err).Error("unable to create client secret")
-		return "", nil, errors.ThrowInternal(err, "APP-gH2Wl", "Errors.Project.CouldNotGenerateClientSecret")
-	}
-	return stringSecret, cryptoValue, nil
-}

internal/static/i18n/de.yaml

@@ -93,6 +93,7 @@ Errors:
     AppNotExisting: Applikation exisitert nicht
     OIDCConfigInvalid: OIDC Konfiguration ist ungültig
     AppIsNotOIDC: Applikation ist nicht vom Typ OIDC
+    OIDCAuthMethodNoneSecret: OIDC Auth Method None benötigt kein Secret
     RequiredFieldsMissing: Benötigte Felder fehlen
     GrantNotFound: Grant konnte nicht gefunden werden
     GrantInvalid: Projekt Grant ist ungültig

internal/static/i18n/en.yaml

@@ -93,6 +93,7 @@ Errors:
     AppNotExisting: Application doesn't exist
     OIDCConfigInvalid: OIDC configuration is invalid
     AppIsNotOIDC: Application is not type oidc
+    OIDCAuthMethodNoneSecret: OIDC Auth Method None does not require a secret
     RequiredFieldsMissing: Some required fields are missing
     GrantNotFound: Grant not found
     GrantInvalid: Project grant is invalid