From e6cfb154c6ef0017acc6b47010471ac9b4c90e71 Mon Sep 17 00:00:00 2001 From: Yourim Cha <81357083+chacha912@users.noreply.github.com> Date: Wed, 27 Nov 2024 18:59:18 +0900 Subject: [PATCH] Refactor webhook method configuration and CLI commands (#1082) Previously, webhooks were triggered for all methods when none were specified. Now, webhooks only trigger for explicitly configured methods, providing more intuitive behavior and better user control. Add CLI commands to manage webhook methods with support for: - Adding/removing single methods - Adding/removing multiple methods - Supporting ALL methods configuration CLI command examples: ```sh # Add a single method $ yorkie project update project-name --auth-webhook-method-add PushPull # Remove a method $ yorkie project update project-name --auth-webhook-method-rm PushPull # Add ALL methods $ yorkie project update project-name --auth-webhook-method-add ALL # Remove ALL methods $ yorkie project update project-name --auth-webhook-method-rm ALL # Add multiple methods $ yorkie project update project-name \ --auth-webhook-method-add ActivateClient \ --auth-webhook-method-add PushPull \ --auth-webhook-method-add WatchDocuments ``` --- api/types/project.go | 2 +- api/types/project_test.go | 6 ++-- cmd/yorkie/project/update.go | 51 +++++++++++++++++++++++++++ test/integration/auth_webhook_test.go | 44 +++++++++++++++++------ 4 files changed, 88 insertions(+), 15 deletions(-) diff --git a/api/types/project.go b/api/types/project.go index 3ed8dd909..7095cb830 100644 --- a/api/types/project.go +++ b/api/types/project.go @@ -62,7 +62,7 @@ func (p *Project) RequireAuth(method Method) bool { } if len(p.AuthWebhookMethods) == 0 { - return true + return false } for _, m := range p.AuthWebhookMethods { diff --git a/api/types/project_test.go b/api/types/project_test.go index 2b75bc1f9..cca5923b9 100644 --- a/api/types/project_test.go +++ b/api/types/project_test.go @@ -35,13 +35,13 @@ func TestProjectInfo(t *testing.T) { assert.True(t, info.RequireAuth(types.ActivateClient)) assert.False(t, info.RequireAuth(types.DetachDocument)) - // 2. Allow all + // 2. No method specified info2 := &types.Project{ AuthWebhookURL: validWebhookURL, AuthWebhookMethods: []string{}, } - assert.True(t, info2.RequireAuth(types.ActivateClient)) - assert.True(t, info2.RequireAuth(types.DetachDocument)) + assert.False(t, info2.RequireAuth(types.ActivateClient)) + assert.False(t, info2.RequireAuth(types.DetachDocument)) // 3. Empty webhook URL info3 := &types.Project{ diff --git a/cmd/yorkie/project/update.go b/cmd/yorkie/project/update.go index 9684e3900..6f0aee096 100644 --- a/cmd/yorkie/project/update.go +++ b/cmd/yorkie/project/update.go @@ -35,10 +35,23 @@ import ( var ( flagAuthWebhookURL string + flagAuthWebhookMethodsAdd []string + flagAuthWebhookMethodsRm []string flagName string flagClientDeactivateThreshold string ) +var allAuthWebhookMethods = []string{ + string(types.ActivateClient), + string(types.DeactivateClient), + string(types.AttachDocument), + string(types.DetachDocument), + string(types.RemoveDocument), + string(types.PushPull), + string(types.WatchDocuments), + string(types.Broadcast), +} + func newUpdateCommand() *cobra.Command { return &cobra.Command{ Use: "update [name]", @@ -82,6 +95,31 @@ func newUpdateCommand() *cobra.Command { newAuthWebhookURL = flagAuthWebhookURL } + methods := make(map[string]struct{}) + for _, m := range project.AuthWebhookMethods { + methods[m] = struct{}{} + } + for _, m := range flagAuthWebhookMethodsRm { + if m == "ALL" { + methods = make(map[string]struct{}) + } else { + delete(methods, m) + } + } + for _, m := range flagAuthWebhookMethodsAdd { + if m == "ALL" { + for _, m := range allAuthWebhookMethods { + methods[m] = struct{}{} + } + } else { + methods[m] = struct{}{} + } + } + newAuthWebhookMethods := make([]string, 0, len(methods)) + for m := range methods { + newAuthWebhookMethods = append(newAuthWebhookMethods, m) + } + newClientDeactivateThreshold := project.ClientDeactivateThreshold if flagClientDeactivateThreshold != "" { newClientDeactivateThreshold = flagClientDeactivateThreshold @@ -90,6 +128,7 @@ func newUpdateCommand() *cobra.Command { updatableProjectFields := &types.UpdatableProjectFields{ Name: &newName, AuthWebhookURL: &newAuthWebhookURL, + AuthWebhookMethods: &newAuthWebhookMethods, ClientDeactivateThreshold: &newClientDeactivateThreshold, } @@ -154,6 +193,18 @@ func init() { "", "authorization-webhook update url", ) + cmd.Flags().StringArrayVar( + &flagAuthWebhookMethodsAdd, + "auth-webhook-method-add", + []string{}, + "authorization-webhook methods to add ('ALL' for all methods)", + ) + cmd.Flags().StringArrayVar( + &flagAuthWebhookMethodsRm, + "auth-webhook-method-rm", + []string{}, + "authorization-webhook methods to remove ('ALL' for all methods)", + ) cmd.Flags().StringVar( &flagClientDeactivateThreshold, "client-deactivate-threshold", diff --git a/test/integration/auth_webhook_test.go b/test/integration/auth_webhook_test.go index 49382876c..05c7ddf28 100644 --- a/test/integration/auth_webhook_test.go +++ b/test/integration/auth_webhook_test.go @@ -41,6 +41,17 @@ import ( "github.com/yorkie-team/yorkie/test/helper" ) +var allWebhookMethods = &[]string{ + string(types.ActivateClient), + string(types.DeactivateClient), + string(types.AttachDocument), + string(types.DetachDocument), + string(types.RemoveDocument), + string(types.PushPull), + string(types.WatchDocuments), + string(types.Broadcast), +} + func newAuthServer(t *testing.T) (*httptest.Server, string) { token := xid.New().String() @@ -110,7 +121,8 @@ func TestProjectAuthWebhook(t *testing.T) { ctx, project.ID.String(), &types.UpdatableProjectFields{ - AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookMethods: allWebhookMethods, }, ) assert.NoError(t, err) @@ -140,7 +152,8 @@ func TestProjectAuthWebhook(t *testing.T) { ctx, project.ID.String(), &types.UpdatableProjectFields{ - AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookMethods: allWebhookMethods, }, ) assert.NoError(t, err) @@ -179,7 +192,8 @@ func TestProjectAuthWebhook(t *testing.T) { ctx, project.ID.String(), &types.UpdatableProjectFields{ - AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookMethods: allWebhookMethods, }, ) assert.NoError(t, err) @@ -278,7 +292,8 @@ func TestAuthWebhookErrorHandling(t *testing.T) { ctx, project.ID.String(), &types.UpdatableProjectFields{ - AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookMethods: allWebhookMethods, }, ) assert.NoError(t, err) @@ -318,7 +333,8 @@ func TestAuthWebhookErrorHandling(t *testing.T) { ctx, project.ID.String(), &types.UpdatableProjectFields{ - AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookMethods: allWebhookMethods, }, ) assert.NoError(t, err) @@ -346,7 +362,8 @@ func TestAuthWebhookErrorHandling(t *testing.T) { ctx, project.ID.String(), &types.UpdatableProjectFields{ - AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookMethods: allWebhookMethods, }, ) assert.NoError(t, err) @@ -375,7 +392,8 @@ func TestAuthWebhookErrorHandling(t *testing.T) { ctx, project.ID.String(), &types.UpdatableProjectFields{ - AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookMethods: allWebhookMethods, }, ) assert.NoError(t, err) @@ -434,7 +452,8 @@ func TestAuthWebhookCache(t *testing.T) { ctx, project.ID.String(), &types.UpdatableProjectFields{ - AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookMethods: allWebhookMethods, }, ) assert.NoError(t, err) @@ -511,7 +530,8 @@ func TestAuthWebhookCache(t *testing.T) { ctx, project.ID.String(), &types.UpdatableProjectFields{ - AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookMethods: allWebhookMethods, }, ) assert.NoError(t, err) @@ -574,7 +594,8 @@ func TestAuthWebhookCache(t *testing.T) { ctx, project.ID.String(), &types.UpdatableProjectFields{ - AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookMethods: allWebhookMethods, }, ) assert.NoError(t, err) @@ -622,7 +643,8 @@ func TestAuthWebhookNewToken(t *testing.T) { ctx, project.ID.String(), &types.UpdatableProjectFields{ - AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookURL: &project.AuthWebhookURL, + AuthWebhookMethods: allWebhookMethods, }, ) assert.NoError(t, err)