From a1c2cb3f46fb49b25c8b2d427f146da851d937ee Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Sat, 22 Oct 2022 18:31:12 +0000 Subject: [PATCH 1/7] Link mentioned user in markdown only if they are visible to viewer We need to make sure a user can't confirm the existence of a user with private visibility Signed-off-by: Yarden Shoham --- services/markup/processorhelper.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/services/markup/processorhelper.go b/services/markup/processorhelper.go index 2b1cac2a5b886..fc26a3b13c03b 100644 --- a/services/markup/processorhelper.go +++ b/services/markup/processorhelper.go @@ -8,22 +8,24 @@ import ( "context" "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/log" + module_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/markup" ) func ProcessorHelper() *markup.ProcessorHelper { return &markup.ProcessorHelper{ IsUsernameMentionable: func(ctx context.Context, username string) bool { - // TODO: cast ctx to modules/context.Context and use IsUserVisibleToViewer - - // Only link if the user actually exists - userExists, err := user.IsUserExist(ctx, 0, username) + mentionedUser, err := user.GetUserByName(ctx, username) if err != nil { - log.Error("Failed to validate user in mention %q exists, assuming it does", username) - userExists = true + return false + } + + moduleCtx, ok := ctx.(*module_context.Context) + if !ok { + return false } - return userExists + + return user.IsUserVisibleToViewer(moduleCtx, mentionedUser, moduleCtx.Doer) }, } } From 08bdc8b9d55839b2b715db0ad5467c98d390a897 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Sat, 22 Oct 2022 18:53:26 +0000 Subject: [PATCH 2/7] Update test Signed-off-by: Yarden Shoham --- services/markup/processorhelper_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/markup/processorhelper_test.go b/services/markup/processorhelper_test.go index 386465bc9189c..25b7e1a18f705 100644 --- a/services/markup/processorhelper_test.go +++ b/services/markup/processorhelper_test.go @@ -5,16 +5,16 @@ package markup import ( - "context" "testing" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/context" "github.com/stretchr/testify/assert" ) func TestProcessorHelper(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - assert.True(t, ProcessorHelper().IsUsernameMentionable(context.Background(), "user10")) - assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), "no-such-user")) + assert.True(t, ProcessorHelper().IsUsernameMentionable(&context.Context{}, "user10")) + assert.False(t, ProcessorHelper().IsUsernameMentionable(&context.Context{}, "no-such-user")) } From ef2e5c680b02a0a53920320dcc77253916927389 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Sat, 22 Oct 2022 20:03:36 +0000 Subject: [PATCH 3/7] Revert "Update test" This reverts commit 08bdc8b9d55839b2b715db0ad5467c98d390a897. --- services/markup/processorhelper_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/markup/processorhelper_test.go b/services/markup/processorhelper_test.go index 25b7e1a18f705..386465bc9189c 100644 --- a/services/markup/processorhelper_test.go +++ b/services/markup/processorhelper_test.go @@ -5,16 +5,16 @@ package markup import ( + "context" "testing" "code.gitea.io/gitea/models/unittest" - "code.gitea.io/gitea/modules/context" "github.com/stretchr/testify/assert" ) func TestProcessorHelper(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - assert.True(t, ProcessorHelper().IsUsernameMentionable(&context.Context{}, "user10")) - assert.False(t, ProcessorHelper().IsUsernameMentionable(&context.Context{}, "no-such-user")) + assert.True(t, ProcessorHelper().IsUsernameMentionable(context.Background(), "user10")) + assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), "no-such-user")) } From eafd3804fc4c35dfa0cf22222895dc17e58f99aa Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Sat, 22 Oct 2022 20:06:15 +0000 Subject: [PATCH 4/7] Handle bad context Signed-off-by: Yarden Shoham --- services/markup/processorhelper.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/markup/processorhelper.go b/services/markup/processorhelper.go index fc26a3b13c03b..d8fdb30c1f744 100644 --- a/services/markup/processorhelper.go +++ b/services/markup/processorhelper.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/user" module_context "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" ) @@ -22,7 +23,8 @@ func ProcessorHelper() *markup.ProcessorHelper { moduleCtx, ok := ctx.(*module_context.Context) if !ok { - return false + log.Error("couldn't cast context, assuming user should be visible based on their visibility") + return mentionedUser.Visibility.IsPublic() } return user.IsUserVisibleToViewer(moduleCtx, mentionedUser, moduleCtx.Doer) From d5687d2c185149f1e2ba0066a5e433fdabf4721e Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Sat, 22 Oct 2022 20:09:40 +0000 Subject: [PATCH 5/7] Ease log level Signed-off-by: Yarden Shoham --- services/markup/processorhelper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/markup/processorhelper.go b/services/markup/processorhelper.go index d8fdb30c1f744..904130337273f 100644 --- a/services/markup/processorhelper.go +++ b/services/markup/processorhelper.go @@ -23,7 +23,7 @@ func ProcessorHelper() *markup.ProcessorHelper { moduleCtx, ok := ctx.(*module_context.Context) if !ok { - log.Error("couldn't cast context, assuming user should be visible based on their visibility") + log.Warn("couldn't cast context, assuming user should be visible based on their visibility") return mentionedUser.Visibility.IsPublic() } From 7cb1417a185fd946623d4cd06564ad890e189e75 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 23 Oct 2022 11:55:26 +0800 Subject: [PATCH 6/7] fix tests --- services/markup/processorhelper.go | 10 +++---- services/markup/processorhelper_test.go | 37 +++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/services/markup/processorhelper.go b/services/markup/processorhelper.go index 904130337273f..5042884e5ef9a 100644 --- a/services/markup/processorhelper.go +++ b/services/markup/processorhelper.go @@ -8,8 +8,7 @@ import ( "context" "code.gitea.io/gitea/models/user" - module_context "code.gitea.io/gitea/modules/context" - "code.gitea.io/gitea/modules/log" + gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/markup" ) @@ -21,13 +20,14 @@ func ProcessorHelper() *markup.ProcessorHelper { return false } - moduleCtx, ok := ctx.(*module_context.Context) + giteaCtx, ok := ctx.(*gitea_context.Context) if !ok { - log.Warn("couldn't cast context, assuming user should be visible based on their visibility") + // when using general context, use user's visibility to check return mentionedUser.Visibility.IsPublic() } - return user.IsUserVisibleToViewer(moduleCtx, mentionedUser, moduleCtx.Doer) + // when using gitea context (web context), use user's visibility and user's permission to check + return user.IsUserVisibleToViewer(giteaCtx, mentionedUser, giteaCtx.Doer) }, } } diff --git a/services/markup/processorhelper_test.go b/services/markup/processorhelper_test.go index 386465bc9189c..9fad14b6ad2c9 100644 --- a/services/markup/processorhelper_test.go +++ b/services/markup/processorhelper_test.go @@ -6,15 +6,48 @@ package markup import ( "context" + "net/http" "testing" + "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/models/user" + gitea_context "code.gitea.io/gitea/modules/context" "github.com/stretchr/testify/assert" ) func TestProcessorHelper(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - assert.True(t, ProcessorHelper().IsUsernameMentionable(context.Background(), "user10")) - assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), "no-such-user")) + + userPublic := "user1" + userPrivate := "user31" + userLimited := "user33" + userNoSuch := "no-such-user" + + unittest.AssertCount(t, &user.User{Name: userPublic}, 1) + unittest.AssertCount(t, &user.User{Name: userPrivate}, 1) + unittest.AssertCount(t, &user.User{Name: userLimited}, 1) + unittest.AssertCount(t, &user.User{Name: userNoSuch}, 0) + + // when using general context, use user's visibility to check + assert.True(t, ProcessorHelper().IsUsernameMentionable(context.Background(), userPublic)) + assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), userLimited)) + assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), userPrivate)) + assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), userNoSuch)) + + // when using web context, use user's visibility to check + var err error + giteaCtx := &gitea_context.Context{} + giteaCtx.Req, err = http.NewRequest("GET", "/", nil) + assert.NoError(t, err) + + giteaCtx.Doer = nil + assert.True(t, ProcessorHelper().IsUsernameMentionable(giteaCtx, userPublic)) + assert.False(t, ProcessorHelper().IsUsernameMentionable(giteaCtx, userPrivate)) + + giteaCtx.Doer, err = user.GetUserByName(db.DefaultContext, userPrivate) + assert.NoError(t, err) + assert.True(t, ProcessorHelper().IsUsernameMentionable(giteaCtx, userPublic)) + assert.True(t, ProcessorHelper().IsUsernameMentionable(giteaCtx, userPrivate)) } From 11de8be9d6e315a23f983770f0bf69fc943ee18e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 23 Oct 2022 12:09:02 +0800 Subject: [PATCH 7/7] Update services/markup/processorhelper_test.go --- services/markup/processorhelper_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/markup/processorhelper_test.go b/services/markup/processorhelper_test.go index 9fad14b6ad2c9..f7eab3d958b0b 100644 --- a/services/markup/processorhelper_test.go +++ b/services/markup/processorhelper_test.go @@ -36,7 +36,7 @@ func TestProcessorHelper(t *testing.T) { assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), userPrivate)) assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), userNoSuch)) - // when using web context, use user's visibility to check + // when using web context, use user.IsUserVisibleToViewer to check var err error giteaCtx := &gitea_context.Context{} giteaCtx.Req, err = http.NewRequest("GET", "/", nil)