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
2024-12-12 02:54:20 +00:00 · 2024-02-12 09:56:55 +01:00 · 2024-02-12 09:56:55 +01:00 · 83ed1f37d8
commit 83ed1f37d8
parent f0ba2b955a
3 changed files with 267 additions and 14 deletions
--- 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
+}
--- 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,
--- 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
 		}
 	}