diff --git a/internal/project/model/oidc_config.go b/internal/project/model/oidc_config.go index cc1f4b85e3..05d2b31ae4 100644 --- a/internal/project/model/oidc_config.go +++ b/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 diff --git a/internal/project/repository/eventsourcing/eventstore.go b/internal/project/repository/eventsourcing/eventstore.go index d38d95e7f7..3fc1f56e6d 100644 --- a/internal/project/repository/eventsourcing/eventstore.go +++ b/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 - clientID, err := generateNewClientID(es.idGenerator, existing) + stringPw, err = app.OIDCConfig.GenerateClientSecretIfNeeded(es.pwGenerator) 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 diff --git a/internal/project/repository/eventsourcing/eventstore_mock_test.go b/internal/project/repository/eventsourcing/eventstore_mock_test.go index 3df206a77d..df0f1aaa5a 100644 --- a/internal/project/repository/eventsourcing/eventstore_mock_test.go +++ b/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", - ResponseTypes: []int32{int32(proj_model.OIDCResponseTypeCode)}, - GrantTypes: []int32{int32(proj_model.OIDCGrantTypeAuthorizationCode)}, + AppID: "AppID", + ResponseTypes: []int32{int32(proj_model.OIDCResponseTypeCode)}, + 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}, diff --git a/internal/project/repository/eventsourcing/eventstore_test.go b/internal/project/repository/eventsourcing/eventstore_test.go index ca1f6d4a23..b13443cdb9 100644 --- a/internal/project/repository/eventsourcing/eventstore_test.go +++ b/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.wantErr && result.AppID == "" { + if tt.res.errFunc == nil && err != nil { + 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}, - AppID: "AppID", - ResponseTypes: []model.OIDCResponseType{model.OIDCResponseTypeIDToken}, - GrantTypes: []model.OIDCGrantType{model.OIDCGrantTypeImplicit}, + ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 0}, + AppID: "AppID", + ResponseTypes: []model.OIDCResponseType{model.OIDCResponseTypeCode}, + 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) diff --git a/internal/project/repository/eventsourcing/oidc.go b/internal/project/repository/eventsourcing/oidc.go deleted file mode 100644 index 93905c5b76..0000000000 --- a/internal/project/repository/eventsourcing/oidc.go +++ /dev/null @@ -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 -} diff --git a/internal/static/i18n/de.yaml b/internal/static/i18n/de.yaml index 301d11158b..75475fdc50 100644 --- a/internal/static/i18n/de.yaml +++ b/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 diff --git a/internal/static/i18n/en.yaml b/internal/static/i18n/en.yaml index 9b188df22a..551a26db89 100644 --- a/internal/static/i18n/en.yaml +++ b/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