Skip to content

Commit

Permalink
Refactor webhook method configuration and CLI commands (#1082)
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
chacha912 authored Nov 27, 2024
1 parent e863b62 commit e6cfb15
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 15 deletions.
2 changes: 1 addition & 1 deletion api/types/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions api/types/project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
51 changes: 51 additions & 0 deletions cmd/yorkie/project/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]",
Expand Down Expand Up @@ -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
Expand All @@ -90,6 +128,7 @@ func newUpdateCommand() *cobra.Command {
updatableProjectFields := &types.UpdatableProjectFields{
Name: &newName,
AuthWebhookURL: &newAuthWebhookURL,
AuthWebhookMethods: &newAuthWebhookMethods,
ClientDeactivateThreshold: &newClientDeactivateThreshold,
}

Expand Down Expand Up @@ -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",
Expand Down
44 changes: 33 additions & 11 deletions test/integration/auth_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit e6cfb15

Please sign in to comment.