From 83ed1f37d889ce3588093c81ba04a15d65175b5c Mon Sep 17 00:00:00 2001 From: Miguel Cabrerizo <30386061+doncicuto@users.noreply.github.com> Date: Mon, 12 Feb 2024 09:56:55 +0100 Subject: [PATCH] fix: trim whitespaces in redirect, post and origins uris set through console (#7334) * fix: trim whitespaces in redirect, postlogout and origins uris set through console * fix: add @livio-a review suggestions --- internal/command/project_application_oidc.go | 27 +- .../command/project_application_oidc_test.go | 252 +++++++++++++++++- internal/domain/application_oidc.go | 2 +- 3 files changed, 267 insertions(+), 14 deletions(-) diff --git a/internal/command/project_application_oidc.go b/internal/command/project_application_oidc.go index 32989e0a58..0ece1bf364 100644 --- a/internal/command/project_application_oidc.go +++ b/internal/command/project_application_oidc.go @@ -56,7 +56,7 @@ func (c *Commands) AddOIDCAppCommand(app *addOIDCApp, clientSecretAlg crypto.Has } for _, origin := range app.AdditionalOrigins { - if !http_util.IsOrigin(origin) { + if !http_util.IsOrigin(strings.TrimSpace(origin)) { return nil, zerrors.ThrowInvalidArgument(nil, "V2-DqWPX", "Errors.Invalid.Argument") } } @@ -98,19 +98,19 @@ func (c *Commands) AddOIDCAppCommand(app *addOIDCApp, clientSecretAlg crypto.Has app.ID, app.ClientID, app.ClientSecret, - app.RedirectUris, + trimStringSliceWhiteSpaces(app.RedirectUris), app.ResponseTypes, app.GrantTypes, app.ApplicationType, app.AuthMethodType, - app.PostLogoutRedirectUris, + trimStringSliceWhiteSpaces(app.PostLogoutRedirectUris), app.DevMode, app.AccessTokenType, app.AccessTokenRoleAssertion, app.IDTokenRoleAssertion, app.IDTokenUserinfoAssertion, app.ClockSkew, - app.AdditionalOrigins, + trimStringSliceWhiteSpaces(app.AdditionalOrigins), app.SkipSuccessPageForNativeApp, ), }, nil @@ -182,19 +182,19 @@ func (c *Commands) addOIDCApplicationWithID(ctx context.Context, oidcApp *domain oidcApp.AppID, oidcApp.ClientID, oidcApp.ClientSecret, - oidcApp.RedirectUris, + trimStringSliceWhiteSpaces(oidcApp.RedirectUris), oidcApp.ResponseTypes, oidcApp.GrantTypes, oidcApp.ApplicationType, oidcApp.AuthMethodType, - oidcApp.PostLogoutRedirectUris, + trimStringSliceWhiteSpaces(oidcApp.PostLogoutRedirectUris), oidcApp.DevMode, oidcApp.AccessTokenType, oidcApp.AccessTokenRoleAssertion, oidcApp.IDTokenRoleAssertion, oidcApp.IDTokenUserinfoAssertion, oidcApp.ClockSkew, - oidcApp.AdditionalOrigins, + trimStringSliceWhiteSpaces(oidcApp.AdditionalOrigins), oidcApp.SkipNativeAppSuccessPage, )) @@ -233,8 +233,8 @@ func (c *Commands) ChangeOIDCApplication(ctx context.Context, oidc *domain.OIDCA ctx, projectAgg, oidc.AppID, - oidc.RedirectUris, - oidc.PostLogoutRedirectUris, + trimStringSliceWhiteSpaces(oidc.RedirectUris), + trimStringSliceWhiteSpaces(oidc.PostLogoutRedirectUris), oidc.ResponseTypes, oidc.GrantTypes, oidc.ApplicationType, @@ -246,7 +246,7 @@ func (c *Commands) ChangeOIDCApplication(ctx context.Context, oidc *domain.OIDCA oidc.IDTokenRoleAssertion, oidc.IDTokenUserinfoAssertion, oidc.ClockSkew, - oidc.AdditionalOrigins, + trimStringSliceWhiteSpaces(oidc.AdditionalOrigins), oidc.SkipNativeAppSuccessPage, ) if err != nil { @@ -359,3 +359,10 @@ func getOIDCAppWriteModel(ctx context.Context, filter preparation.FilterToQueryR err = appWriteModel.Reduce() return appWriteModel, err } + +func trimStringSliceWhiteSpaces(slice []string) []string { + for i, s := range slice { + slice[i] = strings.TrimSpace(s) + } + return slice +} diff --git a/internal/command/project_application_oidc_test.go b/internal/command/project_application_oidc_test.go index 8efdd8c256..55a3f70515 100644 --- a/internal/command/project_application_oidc_test.go +++ b/internal/command/project_application_oidc_test.go @@ -82,7 +82,7 @@ func TestAddOIDCApp(t *testing.T) { }, }, { - name: "project not exists", + name: "project doesn't exist", fields: fields{}, args: args{ app: &addOIDCApp{ @@ -108,6 +108,73 @@ func TestAddOIDCApp(t *testing.T) { CreateErr: zerrors.ThrowNotFound(nil, "PROJE-6swVG", ""), }, }, + { + name: "correct, using uris with whitespaces", + fields: fields{ + idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "clientID"), + }, + args: args{ + app: &addOIDCApp{ + AddApp: AddApp{ + Aggregate: *agg, + ID: "id", + Name: "name", + }, + RedirectUris: []string{" https://test.ch "}, + PostLogoutRedirectUris: []string{" https://test.ch/logout "}, + GrantTypes: []domain.OIDCGrantType{domain.OIDCGrantTypeAuthorizationCode}, + ResponseTypes: []domain.OIDCResponseType{domain.OIDCResponseTypeCode}, + Version: domain.OIDCVersionV1, + AdditionalOrigins: []string{" https://sub.test.ch "}, + ApplicationType: domain.OIDCApplicationTypeWeb, + AuthMethodType: domain.OIDCAuthMethodTypeNone, + AccessTokenType: domain.OIDCTokenTypeBearer, + }, + filter: NewMultiFilter(). + Append(func(ctx context.Context, queryFactory *eventstore.SearchQueryBuilder) ([]eventstore.Event, error) { + return []eventstore.Event{ + project.NewProjectAddedEvent( + ctx, + &agg.Aggregate, + "project", + false, + false, + false, + domain.PrivateLabelingSettingUnspecified, + ), + }, nil + }). + Filter(), + }, + want: Want{ + Commands: []eventstore.Command{ + project.NewApplicationAddedEvent(ctx, &agg.Aggregate, + "id", + "name", + ), + project.NewOIDCConfigAddedEvent(ctx, &agg.Aggregate, + domain.OIDCVersionV1, + "id", + "clientID@project", + nil, + []string{"https://test.ch"}, + []domain.OIDCResponseType{domain.OIDCResponseTypeCode}, + []domain.OIDCGrantType{domain.OIDCGrantTypeAuthorizationCode}, + domain.OIDCApplicationTypeWeb, + domain.OIDCAuthMethodTypeNone, + []string{"https://test.ch/logout"}, + false, + domain.OIDCTokenTypeBearer, + false, + false, + false, + 0, + []string{"https://sub.test.ch"}, + false, + ), + }, + }, + }, { name: "correct", fields: fields{ @@ -279,6 +346,111 @@ func TestCommandSide_AddOIDCApplication(t *testing.T) { err: zerrors.IsErrorInvalidArgument, }, }, + { + name: "create oidc app basic using whitespaces in uris, ok", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + project.NewProjectAddedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, + "project", true, true, true, + domain.PrivateLabelingSettingUnspecified), + ), + ), + expectPush( + project.NewApplicationAddedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, + "app1", + "app", + ), + project.NewOIDCConfigAddedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, + domain.OIDCVersionV1, + "app1", + "client1@project", + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("a"), + }, + []string{"https://test.ch"}, + []domain.OIDCResponseType{domain.OIDCResponseTypeCode}, + []domain.OIDCGrantType{domain.OIDCGrantTypeAuthorizationCode}, + domain.OIDCApplicationTypeWeb, + domain.OIDCAuthMethodTypePost, + []string{"https://test.ch/logout"}, + true, + domain.OIDCTokenTypeBearer, + true, + true, + true, + time.Second*1, + []string{"https://sub.test.ch"}, + true, + ), + ), + ), + idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "app1", "client1"), + }, + args: args{ + ctx: context.Background(), + oidcApp: &domain.OIDCApp{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "project1", + }, + AppName: "app", + AuthMethodType: domain.OIDCAuthMethodTypePost, + OIDCVersion: domain.OIDCVersionV1, + RedirectUris: []string{" https://test.ch "}, + ResponseTypes: []domain.OIDCResponseType{domain.OIDCResponseTypeCode}, + GrantTypes: []domain.OIDCGrantType{domain.OIDCGrantTypeAuthorizationCode}, + ApplicationType: domain.OIDCApplicationTypeWeb, + PostLogoutRedirectUris: []string{" https://test.ch/logout "}, + DevMode: true, + AccessTokenType: domain.OIDCTokenTypeBearer, + AccessTokenRoleAssertion: true, + IDTokenRoleAssertion: true, + IDTokenUserinfoAssertion: true, + ClockSkew: time.Second * 1, + AdditionalOrigins: []string{" https://sub.test.ch "}, + SkipNativeAppSuccessPage: true, + }, + resourceOwner: "org1", + secretGenerator: GetMockSecretGenerator(t), + }, + res: res{ + want: &domain.OIDCApp{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "project1", + ResourceOwner: "org1", + }, + AppID: "app1", + AppName: "app", + ClientID: "client1@project", + ClientSecretString: "a", + AuthMethodType: domain.OIDCAuthMethodTypePost, + OIDCVersion: domain.OIDCVersionV1, + RedirectUris: []string{"https://test.ch"}, + ResponseTypes: []domain.OIDCResponseType{domain.OIDCResponseTypeCode}, + GrantTypes: []domain.OIDCGrantType{domain.OIDCGrantTypeAuthorizationCode}, + ApplicationType: domain.OIDCApplicationTypeWeb, + PostLogoutRedirectUris: []string{"https://test.ch/logout"}, + DevMode: true, + AccessTokenType: domain.OIDCTokenTypeBearer, + AccessTokenRoleAssertion: true, + IDTokenRoleAssertion: true, + IDTokenUserinfoAssertion: true, + ClockSkew: time.Second * 1, + AdditionalOrigins: []string{"https://sub.test.ch"}, + SkipNativeAppSuccessPage: true, + State: domain.AppStateActive, + Compliance: &domain.Compliance{}, + }, + }, + }, { name: "create oidc app basic, ok", fields: fields{ @@ -592,6 +764,80 @@ func TestCommandSide_ChangeOIDCApplication(t *testing.T) { err: zerrors.IsPreconditionFailed, }, }, + { + name: "no changes whitespaces are ignored, precondition error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + project.NewApplicationAddedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, + "app1", + "app", + ), + ), + eventFromEventPusher( + project.NewOIDCConfigAddedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, + domain.OIDCVersionV1, + "app1", + "client1@project", + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("a"), + }, + []string{"https://test.ch"}, + []domain.OIDCResponseType{domain.OIDCResponseTypeCode}, + []domain.OIDCGrantType{domain.OIDCGrantTypeAuthorizationCode}, + domain.OIDCApplicationTypeWeb, + domain.OIDCAuthMethodTypePost, + []string{"https://test.ch/logout"}, + true, + domain.OIDCTokenTypeBearer, + true, + true, + true, + time.Second*1, + []string{"https://sub.test.ch"}, + true, + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + oidcApp: &domain.OIDCApp{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "project1", + }, + AppID: "app1", + AppName: "app", + AuthMethodType: domain.OIDCAuthMethodTypePost, + OIDCVersion: domain.OIDCVersionV1, + RedirectUris: []string{"https://test.ch "}, + ResponseTypes: []domain.OIDCResponseType{domain.OIDCResponseTypeCode}, + GrantTypes: []domain.OIDCGrantType{domain.OIDCGrantTypeAuthorizationCode}, + ApplicationType: domain.OIDCApplicationTypeWeb, + PostLogoutRedirectUris: []string{" https://test.ch/logout"}, + DevMode: true, + AccessTokenType: domain.OIDCTokenTypeBearer, + AccessTokenRoleAssertion: true, + IDTokenRoleAssertion: true, + IDTokenUserinfoAssertion: true, + ClockSkew: time.Second * 1, + AdditionalOrigins: []string{" https://sub.test.ch "}, + SkipNativeAppSuccessPage: true, + }, + resourceOwner: "org1", + }, + res: res{ + err: zerrors.IsPreconditionFailed, + }, + }, { name: "change oidc app, ok", fields: fields{ @@ -652,11 +898,11 @@ func TestCommandSide_ChangeOIDCApplication(t *testing.T) { AppName: "app", AuthMethodType: domain.OIDCAuthMethodTypePost, OIDCVersion: domain.OIDCVersionV1, - RedirectUris: []string{"https://test-change.ch"}, + RedirectUris: []string{" https://test-change.ch "}, ResponseTypes: []domain.OIDCResponseType{domain.OIDCResponseTypeCode}, GrantTypes: []domain.OIDCGrantType{domain.OIDCGrantTypeAuthorizationCode}, ApplicationType: domain.OIDCApplicationTypeWeb, - PostLogoutRedirectUris: []string{"https://test-change.ch/logout"}, + PostLogoutRedirectUris: []string{" https://test-change.ch/logout "}, DevMode: true, AccessTokenType: domain.OIDCTokenTypeJWT, AccessTokenRoleAssertion: false, diff --git a/internal/domain/application_oidc.go b/internal/domain/application_oidc.go index ac2918478c..a5a8c0b2ba 100644 --- a/internal/domain/application_oidc.go +++ b/internal/domain/application_oidc.go @@ -141,7 +141,7 @@ func (a *OIDCApp) IsValid() bool { func (a *OIDCApp) OriginsValid() bool { for _, origin := range a.AdditionalOrigins { - if !http_util.IsOrigin(origin) { + if !http_util.IsOrigin(strings.TrimSpace(origin)) { return false } }