diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index 9b4592ccf10..bfb375277c6 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -521,19 +521,6 @@ components: SetupKeyRequest: type: object properties: - name: - description: Setup Key name - type: string - example: Default key - type: - description: Setup key type, one-off for single time usage and reusable - type: string - example: reusable - expires_in: - description: Expiration time in seconds, 0 will mean the key never expires - type: integer - minimum: 0 - example: 86400 revoked: description: Setup key revocation status type: boolean @@ -544,21 +531,9 @@ components: items: type: string example: "ch8i4ug6lnn4g9hqv7m0" - usage_limit: - description: A number of times this key can be used. The value of 0 indicates the unlimited usage. - type: integer - example: 0 - ephemeral: - description: Indicate that the peer will be ephemeral or not - type: boolean - example: true required: - - name - - type - - expires_in - revoked - auto_groups - - usage_limit CreateSetupKeyRequest: type: object properties: diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go index c1ef1ba2122..f219c4574f4 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -1098,23 +1098,8 @@ type SetupKeyRequest struct { // AutoGroups List of group IDs to auto-assign to peers registered with this key AutoGroups []string `json:"auto_groups"` - // Ephemeral Indicate that the peer will be ephemeral or not - Ephemeral *bool `json:"ephemeral,omitempty"` - - // ExpiresIn Expiration time in seconds, 0 will mean the key never expires - ExpiresIn int `json:"expires_in"` - - // Name Setup Key name - Name string `json:"name"` - // Revoked Setup key revocation status Revoked bool `json:"revoked"` - - // Type Setup key type, one-off for single time usage and reusable - Type string `json:"type"` - - // UsageLimit A number of times this key can be used. The value of 0 indicates the unlimited usage. - UsageLimit int `json:"usage_limit"` } // User defines model for User. diff --git a/management/server/http/setupkeys_handler.go b/management/server/http/setupkeys_handler.go index 31859f59bf0..9ba5977bb25 100644 --- a/management/server/http/setupkeys_handler.go +++ b/management/server/http/setupkeys_handler.go @@ -137,11 +137,6 @@ func (h *SetupKeysHandler) UpdateSetupKey(w http.ResponseWriter, r *http.Request return } - if req.Name == "" { - util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "setup key name field is invalid: %s", req.Name), w) - return - } - if req.AutoGroups == nil { util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "setup key AutoGroups field is invalid"), w) return @@ -150,7 +145,6 @@ func (h *SetupKeysHandler) UpdateSetupKey(w http.ResponseWriter, r *http.Request newKey := &server.SetupKey{} newKey.AutoGroups = req.AutoGroups newKey.Revoked = req.Revoked - newKey.Name = req.Name newKey.Id = keyID newKey, err = h.accountManager.SaveSetupKey(r.Context(), accountID, newKey, userID) diff --git a/management/server/setupkey.go b/management/server/setupkey.go index 554c66ba4fc..960532bf9f1 100644 --- a/management/server/setupkey.go +++ b/management/server/setupkey.go @@ -12,9 +12,10 @@ import ( "unicode/utf8" "github.com/google/uuid" + log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/status" - log "github.com/sirupsen/logrus" ) const ( @@ -276,7 +277,7 @@ func (am *DefaultAccountManager) CreateSetupKey(ctx context.Context, accountID s // SaveSetupKey saves the provided SetupKey to the database overriding the existing one. // Due to the unique nature of a SetupKey certain properties must not be overwritten // (e.g. the key itself, creation date, ID, etc). -// These properties are overwritten: Name, AutoGroups, Revoked. The rest is copied from the existing key. +// These properties are overwritten: AutoGroups, Revoked (only from false to true), and the UpdatedAt. The rest is copied from the existing key. func (am *DefaultAccountManager) SaveSetupKey(ctx context.Context, accountID string, keyToSave *SetupKey, userID string) (*SetupKey, error) { if keyToSave == nil { return nil, status.Errorf(status.InvalidArgument, "provided setup key to update is nil") @@ -312,9 +313,12 @@ func (am *DefaultAccountManager) SaveSetupKey(ctx context.Context, accountID str return err } - // only auto groups, revoked status, and name can be updated for now + if oldKey.Revoked && !keyToSave.Revoked { + return status.Errorf(status.InvalidArgument, "can't un-revoke a revoked setup key") + } + + // only auto groups, revoked status (from false to true) can be updated newKey = oldKey.Copy() - newKey.Name = keyToSave.Name newKey.AutoGroups = keyToSave.AutoGroups newKey.Revoked = keyToSave.Revoked newKey.UpdatedAt = time.Now().UTC() diff --git a/management/server/setupkey_test.go b/management/server/setupkey_test.go index 2ed8aef95c6..94ed022fa81 100644 --- a/management/server/setupkey_test.go +++ b/management/server/setupkey_test.go @@ -56,11 +56,9 @@ func TestDefaultAccountManager_SaveSetupKey(t *testing.T) { } autoGroups := []string{"group_1", "group_2"} - newKeyName := "my-new-test-key" revoked := true newKey, err := manager.SaveSetupKey(context.Background(), account.Id, &SetupKey{ Id: key.Id, - Name: newKeyName, Revoked: revoked, AutoGroups: autoGroups, }, userID) @@ -68,7 +66,7 @@ func TestDefaultAccountManager_SaveSetupKey(t *testing.T) { t.Fatal(err) } - assertKey(t, newKey, newKeyName, revoked, "reusable", 0, key.CreatedAt, key.ExpiresAt, + assertKey(t, newKey, keyName, revoked, "reusable", 0, key.CreatedAt, key.ExpiresAt, key.Id, time.Now().UTC(), autoGroups, true) // check the corresponding events that should have been generated @@ -76,7 +74,7 @@ func TestDefaultAccountManager_SaveSetupKey(t *testing.T) { assert.NotNil(t, ev) assert.Equal(t, account.Id, ev.AccountID) - assert.Equal(t, newKeyName, ev.Meta["name"]) + assert.Equal(t, keyName, ev.Meta["name"]) assert.Equal(t, fmt.Sprint(key.Type), fmt.Sprint(ev.Meta["type"])) assert.NotEmpty(t, ev.Meta["key"]) assert.Equal(t, userID, ev.InitiatorID) @@ -89,7 +87,6 @@ func TestDefaultAccountManager_SaveSetupKey(t *testing.T) { autoGroups = append(autoGroups, groupAll.ID) _, err = manager.SaveSetupKey(context.Background(), account.Id, &SetupKey{ Id: key.Id, - Name: newKeyName, Revoked: revoked, AutoGroups: autoGroups, }, userID) @@ -449,3 +446,31 @@ func TestSetupKeyAccountPeersUpdate(t *testing.T) { } }) } + +func TestDefaultAccountManager_CreateSetupKey_ShouldNotAllowToUpdateRevokedKey(t *testing.T) { + manager, err := createManager(t) + if err != nil { + t.Fatal(err) + } + + userID := "testingUser" + account, err := manager.GetOrCreateAccountByUser(context.Background(), userID, "") + if err != nil { + t.Fatal(err) + } + + key, err := manager.CreateSetupKey(context.Background(), account.Id, "testName", SetupKeyReusable, time.Hour, nil, SetupKeyUnlimitedUsage, userID, false) + assert.NoError(t, err) + + // revoke the key + updateKey := key.Copy() + updateKey.Revoked = true + _, err = manager.SaveSetupKey(context.Background(), account.Id, updateKey, userID) + assert.NoError(t, err) + + // re-activate revoked key + updateKey.Revoked = false + _, err = manager.SaveSetupKey(context.Background(), account.Id, updateKey, userID) + assert.Error(t, err, "should not allow to update revoked key") + +}