[management] Return permission-denied error when setup key is expired or invalid (#8086)

* return permission-denied error when setup key is expired or invalid

Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io>

* fix tests, don't return permission-denied on internal errors

Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io>

* return not found error when key fetch failed

Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io>

* normalise errors returned from each call-site of GetSetupKeyBySecret

Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io>

* return PermissionDenied after isValid() check failure

Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io>

---------

Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io>
This commit is contained in:
dmitri-netbird
2026-10-07 10:21:24 +02:00
committed by GitHub
parent e2a2d379c5
commit ae7ad5e0c8
3 changed files with 10 additions and 6 deletions
+4 -4
View File
@@ -710,11 +710,11 @@ func (am *DefaultAccountManager) handleUserAddedPeer(ctx context.Context, accoun
func (am *DefaultAccountManager) handleSetupKeyAddedPeer(ctx context.Context, encodedHashedKey string, peer *nbpeer.Peer, opEvent *activity.Event, config *peerAddAuthConfig) error {
sk, err := am.Store.GetSetupKeyBySecret(ctx, store.LockingStrengthNone, encodedHashedKey)
if err != nil {
return status.Errorf(status.NotFound, "couldn't add peer: setup key is invalid")
return err
}
if !sk.IsValid() {
return status.Errorf(status.NotFound, "couldn't add peer: setup key is invalid")
return status.Errorf(status.PermissionDenied, "couldn't add peer: setup key is invalid")
}
if !sk.AllowExtraDNSLabels && len(peer.ExtraDNSLabels) > 0 {
@@ -915,12 +915,12 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, accountID, setupKe
case addedBySetupKey:
sk, err := transaction.GetSetupKeyBySecret(ctx, store.LockingStrengthUpdate, encodedHashedKey)
if err != nil {
return fmt.Errorf("failed to get setup key: %w", err)
return err
}
// we validate at the end to not block the setup key for too long
if !sk.IsValid() {
return status.Errorf(status.PreconditionFailed, "couldn't add peer: setup key is invalid")
return status.Errorf(status.PermissionDenied, "couldn't add peer: setup key is invalid")
}
err = transaction.IncrementSetupKeyUsage(ctx, peerAddConfig.SetupKeyID)
+5 -1
View File
@@ -1495,7 +1495,7 @@ func Test_RegisterPeerBySetupKey(t *testing.T) {
name: "Absent setup key",
existingSetupKeyID: "AAAAAAAA-38F5-4553-B31E-DD66C696CEBB",
expectAddPeerError: true,
errorType: status.NotFound,
errorType: status.PermissionDenied,
expectedErrorMsgSubstring: "couldn't add peer: setup key is invalid",
},
}
@@ -2642,6 +2642,8 @@ func TestHandleSetupKeyAddedPeer(t *testing.T) {
err = manager.handleSetupKeyAddedPeer(context.Background(), encodedHashedKey, peer, opEvent, config)
require.Error(t, err)
require.IsType(t, &status.Error{}, err)
require.Equal(t, err.(*status.Error).ErrorType, status.PermissionDenied)
assert.Contains(t, err.Error(), "setup key is invalid")
})
@@ -2662,6 +2664,8 @@ func TestHandleSetupKeyAddedPeer(t *testing.T) {
err = manager.handleSetupKeyAddedPeer(context.Background(), encodedHashedKey, peer, opEvent, config)
require.Error(t, err)
require.IsType(t, &status.Error{}, err)
require.Equal(t, err.(*status.Error).ErrorType, status.PermissionDenied)
assert.Contains(t, err.Error(), "setup key is invalid")
})
@@ -127,7 +127,7 @@ func (s *SqlStore) GetSetupKeyBySecret(ctx context.Context, lockStrength Locking
if result.Error != nil {
if errors.Is(result.Error, gorm.ErrRecordNotFound) {
return nil, status.Errorf(status.PreconditionFailed, "setup key not found")
return nil, status.Errorf(status.PermissionDenied, "couldn't add peer: setup key is invalid")
}
log.WithContext(ctx).Errorf("failed to get setup key by secret from store: %v", result.Error)
return nil, status.Errorf(status.Internal, "failed to get setup key by secret from store")