From 830dee177128fa374c0138e9f3838e1f0a2bd7e0 Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Thu, 28 Sep 2023 08:54:49 +0200 Subject: [PATCH 01/14] Expose store metrics with milliseconds bucketing (#1179) As the current upper 10000 microseconds(10ms) bucket may be too low for `management.store.persistence.duration` metric --- management/server/telemetry/store_metrics.go | 37 ++++++++++++++------ 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/management/server/telemetry/store_metrics.go b/management/server/telemetry/store_metrics.go index 704ef65d47f..98c13f12a99 100644 --- a/management/server/telemetry/store_metrics.go +++ b/management/server/telemetry/store_metrics.go @@ -11,37 +11,54 @@ import ( // StoreMetrics represents all metrics related to the FileStore type StoreMetrics struct { - globalLockAcquisitionDuration syncint64.Histogram - persistenceDuration syncint64.Histogram - ctx context.Context + globalLockAcquisitionDurationMicro syncint64.Histogram + globalLockAcquisitionDurationMs syncint64.Histogram + persistenceDurationMicro syncint64.Histogram + persistenceDurationMs syncint64.Histogram + ctx context.Context } // NewStoreMetrics creates an instance of StoreMetrics func NewStoreMetrics(ctx context.Context, meter metric.Meter) (*StoreMetrics, error) { - globalLockAcquisitionDuration, err := meter.SyncInt64().Histogram("management.store.global.lock.acquisition.duration.micro", + globalLockAcquisitionDurationMicro, err := meter.SyncInt64().Histogram("management.store.global.lock.acquisition.duration.micro", instrument.WithUnit("microseconds")) if err != nil { return nil, err } - persistenceDuration, err := meter.SyncInt64().Histogram("management.store.persistence.duration.micro", + + globalLockAcquisitionDurationMs, err := meter.SyncInt64().Histogram("management.store.global.lock.acquisition.duration.ms") + if err != nil { + return nil, err + } + + persistenceDurationMicro, err := meter.SyncInt64().Histogram("management.store.persistence.duration.micro", instrument.WithUnit("microseconds")) if err != nil { return nil, err } + persistenceDurationMs, err := meter.SyncInt64().Histogram("management.store.persistence.duration.ms") + if err != nil { + return nil, err + } + return &StoreMetrics{ - globalLockAcquisitionDuration: globalLockAcquisitionDuration, - persistenceDuration: persistenceDuration, - ctx: ctx, + globalLockAcquisitionDurationMicro: globalLockAcquisitionDurationMicro, + globalLockAcquisitionDurationMs: globalLockAcquisitionDurationMs, + persistenceDurationMicro: persistenceDurationMicro, + persistenceDurationMs: persistenceDurationMs, + ctx: ctx, }, nil } // CountGlobalLockAcquisitionDuration counts the duration of the global lock acquisition func (metrics *StoreMetrics) CountGlobalLockAcquisitionDuration(duration time.Duration) { - metrics.globalLockAcquisitionDuration.Record(metrics.ctx, duration.Microseconds()) + metrics.globalLockAcquisitionDurationMicro.Record(metrics.ctx, duration.Microseconds()) + metrics.globalLockAcquisitionDurationMs.Record(metrics.ctx, duration.Milliseconds()) } // CountPersistenceDuration counts the duration of a store persistence operation func (metrics *StoreMetrics) CountPersistenceDuration(duration time.Duration) { - metrics.persistenceDuration.Record(metrics.ctx, duration.Microseconds()) + metrics.persistenceDurationMicro.Record(metrics.ctx, duration.Microseconds()) + metrics.persistenceDurationMs.Record(metrics.ctx, duration.Milliseconds()) } From 1956ca169e2d1bf752524655c0b6cc1ae72fba06 Mon Sep 17 00:00:00 2001 From: Bethuel Mmbaga Date: Thu, 28 Sep 2023 15:02:37 +0300 Subject: [PATCH 02/14] Resolve client authentication issue in daemon mode (#1181) --- client/android/login.go | 2 +- client/cmd/login.go | 15 +- client/cmd/up.go | 15 +- client/internal/auth/oauth.go | 7 +- client/internal/auth/util.go | 6 - client/proto/daemon.pb.go | 263 ++++++++++++++++++---------------- client/proto/daemon.proto | 1 + client/server/server.go | 2 +- 8 files changed, 164 insertions(+), 147 deletions(-) diff --git a/client/android/login.go b/client/android/login.go index 8d2636c9aab..afd61055f07 100644 --- a/client/android/login.go +++ b/client/android/login.go @@ -193,7 +193,7 @@ func (a *Auth) login(urlOpener URLOpener) error { } func (a *Auth) foregroundGetTokenInfo(urlOpener URLOpener) (*auth.TokenInfo, error) { - oAuthFlow, err := auth.NewOAuthFlow(a.ctx, a.config) + oAuthFlow, err := auth.NewOAuthFlow(a.ctx, a.config, false) if err != nil { return nil, err } diff --git a/client/cmd/login.go b/client/cmd/login.go index 5433db522c2..2ddab46f35c 100644 --- a/client/cmd/login.go +++ b/client/cmd/login.go @@ -3,6 +3,7 @@ package cmd import ( "context" "fmt" + "os" "strings" "time" @@ -80,9 +81,10 @@ var loginCmd = &cobra.Command{ client := proto.NewDaemonServiceClient(conn) loginRequest := proto.LoginRequest{ - SetupKey: setupKey, - PreSharedKey: preSharedKey, - ManagementUrl: managementURL, + SetupKey: setupKey, + PreSharedKey: preSharedKey, + ManagementUrl: managementURL, + IsLinuxDesktopClient: isLinuxRunningDesktop(), } var loginErr error @@ -163,7 +165,7 @@ func foregroundLogin(ctx context.Context, cmd *cobra.Command, config *internal.C } func foregroundGetTokenInfo(ctx context.Context, cmd *cobra.Command, config *internal.Config) (*auth.TokenInfo, error) { - oAuthFlow, err := auth.NewOAuthFlow(ctx, config) + oAuthFlow, err := auth.NewOAuthFlow(ctx, config, isLinuxRunningDesktop()) if err != nil { return nil, err } @@ -202,3 +204,8 @@ func openURL(cmd *cobra.Command, verificationURIComplete, userCode string) { "https://docs.netbird.io/how-to/register-machines-using-setup-keys") } } + +// isLinuxRunningDesktop checks if a Linux OS is running desktop environment +func isLinuxRunningDesktop() bool { + return os.Getenv("DESKTOP_SESSION") != "" || os.Getenv("XDG_CURRENT_DESKTOP") != "" +} diff --git a/client/cmd/up.go b/client/cmd/up.go index a275e88dbf8..8d682c46b2e 100644 --- a/client/cmd/up.go +++ b/client/cmd/up.go @@ -141,13 +141,14 @@ func runInDaemonMode(ctx context.Context, cmd *cobra.Command) error { } loginRequest := proto.LoginRequest{ - SetupKey: setupKey, - PreSharedKey: preSharedKey, - ManagementUrl: managementURL, - AdminURL: adminURL, - NatExternalIPs: natExternalIPs, - CleanNATExternalIPs: natExternalIPs != nil && len(natExternalIPs) == 0, - CustomDNSAddress: customDNSAddressConverted, + SetupKey: setupKey, + PreSharedKey: preSharedKey, + ManagementUrl: managementURL, + AdminURL: adminURL, + NatExternalIPs: natExternalIPs, + CleanNATExternalIPs: natExternalIPs != nil && len(natExternalIPs) == 0, + CustomDNSAddress: customDNSAddressConverted, + IsLinuxDesktopClient: isLinuxRunningDesktop(), } var loginErr error diff --git a/client/internal/auth/oauth.go b/client/internal/auth/oauth.go index 8731e4f0b27..82adf91b9ec 100644 --- a/client/internal/auth/oauth.go +++ b/client/internal/auth/oauth.go @@ -6,6 +6,7 @@ import ( "net/http" "runtime" + log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" gstatus "google.golang.org/grpc/status" @@ -63,14 +64,16 @@ func (t TokenInfo) GetTokenToUse() string { // and if that also fails, the authentication process is deemed unsuccessful // // On Linux distros without desktop environment support, it only tries to initialize the Device Code Flow -func NewOAuthFlow(ctx context.Context, config *internal.Config) (OAuthFlow, error) { - if runtime.GOOS == "linux" && !isLinuxRunningDesktop() { +func NewOAuthFlow(ctx context.Context, config *internal.Config, isLinuxDesktopClient bool) (OAuthFlow, error) { + if runtime.GOOS == "linux" && !isLinuxDesktopClient { return authenticateWithDeviceCodeFlow(ctx, config) } pkceFlow, err := authenticateWithPKCEFlow(ctx, config) if err != nil { // fallback to device code flow + log.Debugf("failed to initialize pkce authentication with error: %v\n", err) + log.Debug("falling back to device code flow") return authenticateWithDeviceCodeFlow(ctx, config) } return pkceFlow, nil diff --git a/client/internal/auth/util.go b/client/internal/auth/util.go index e61e0f1756a..33a0e6e35ad 100644 --- a/client/internal/auth/util.go +++ b/client/internal/auth/util.go @@ -7,7 +7,6 @@ import ( "encoding/json" "fmt" "io" - "os" "reflect" "strings" ) @@ -61,8 +60,3 @@ func isValidAccessToken(token string, audience string) error { return fmt.Errorf("invalid JWT token audience field") } - -// isLinuxRunningDesktop checks if a Linux OS is running desktop environment -func isLinuxRunningDesktop() bool { - return os.Getenv("DESKTOP_SESSION") != "" || os.Getenv("XDG_CURRENT_DESKTOP") != "" -} diff --git a/client/proto/daemon.pb.go b/client/proto/daemon.pb.go index 13b55fc17e8..4dc98942019 100644 --- a/client/proto/daemon.pb.go +++ b/client/proto/daemon.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: // protoc-gen-go v1.26.0 -// protoc v3.21.9 +// protoc v4.23.4 // source: daemon.proto package proto @@ -40,8 +40,9 @@ type LoginRequest struct { // cleanNATExternalIPs clean map list of external IPs. // This is needed because the generated code // omits initialized empty slices due to omitempty tags - CleanNATExternalIPs bool `protobuf:"varint,6,opt,name=cleanNATExternalIPs,proto3" json:"cleanNATExternalIPs,omitempty"` - CustomDNSAddress []byte `protobuf:"bytes,7,opt,name=customDNSAddress,proto3" json:"customDNSAddress,omitempty"` + CleanNATExternalIPs bool `protobuf:"varint,6,opt,name=cleanNATExternalIPs,proto3" json:"cleanNATExternalIPs,omitempty"` + CustomDNSAddress []byte `protobuf:"bytes,7,opt,name=customDNSAddress,proto3" json:"customDNSAddress,omitempty"` + IsLinuxDesktopClient bool `protobuf:"varint,8,opt,name=isLinuxDesktopClient,proto3" json:"isLinuxDesktopClient,omitempty"` } func (x *LoginRequest) Reset() { @@ -125,6 +126,13 @@ func (x *LoginRequest) GetCustomDNSAddress() []byte { return nil } +func (x *LoginRequest) GetIsLinuxDesktopClient() bool { + if x != nil { + return x.IsLinuxDesktopClient + } + return false +} + type LoginResponse struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -1043,7 +1051,7 @@ var file_daemon_proto_rawDesc = []byte{ 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2f, 0x64, 0x65, 0x73, 0x63, 0x72, 0x69, 0x70, 0x74, 0x6f, 0x72, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x1a, 0x1f, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2f, 0x74, 0x69, 0x6d, 0x65, 0x73, 0x74, - 0x61, 0x6d, 0x70, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0x96, 0x02, 0x0a, 0x0c, 0x4c, 0x6f, + 0x61, 0x6d, 0x70, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xca, 0x02, 0x0a, 0x0c, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x1a, 0x0a, 0x08, 0x73, 0x65, 0x74, 0x75, 0x70, 0x4b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x73, 0x65, 0x74, 0x75, 0x70, 0x4b, 0x65, 0x79, 0x12, 0x22, 0x0a, 0x0c, 0x70, 0x72, 0x65, 0x53, 0x68, 0x61, @@ -1061,128 +1069,131 @@ var file_daemon_proto_rawDesc = []byte{ 0x6e, 0x61, 0x6c, 0x49, 0x50, 0x73, 0x12, 0x2a, 0x0a, 0x10, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x44, 0x4e, 0x53, 0x41, 0x64, 0x64, 0x72, 0x65, 0x73, 0x73, 0x18, 0x07, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x10, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x44, 0x4e, 0x53, 0x41, 0x64, 0x64, 0x72, 0x65, - 0x73, 0x73, 0x22, 0xb5, 0x01, 0x0a, 0x0d, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, 0x65, 0x73, 0x70, - 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x24, 0x0a, 0x0d, 0x6e, 0x65, 0x65, 0x64, 0x73, 0x53, 0x53, 0x4f, - 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x18, 0x01, 0x20, 0x01, 0x28, 0x08, 0x52, 0x0d, 0x6e, 0x65, 0x65, - 0x64, 0x73, 0x53, 0x53, 0x4f, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x12, 0x1a, 0x0a, 0x08, 0x75, 0x73, - 0x65, 0x72, 0x43, 0x6f, 0x64, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x75, 0x73, - 0x65, 0x72, 0x43, 0x6f, 0x64, 0x65, 0x12, 0x28, 0x0a, 0x0f, 0x76, 0x65, 0x72, 0x69, 0x66, 0x69, - 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x55, 0x52, 0x49, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, - 0x0f, 0x76, 0x65, 0x72, 0x69, 0x66, 0x69, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x55, 0x52, 0x49, - 0x12, 0x38, 0x0a, 0x17, 0x76, 0x65, 0x72, 0x69, 0x66, 0x69, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, - 0x55, 0x52, 0x49, 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x18, 0x04, 0x20, 0x01, 0x28, - 0x09, 0x52, 0x17, 0x76, 0x65, 0x72, 0x69, 0x66, 0x69, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x55, - 0x52, 0x49, 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x22, 0x31, 0x0a, 0x13, 0x57, 0x61, - 0x69, 0x74, 0x53, 0x53, 0x4f, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, - 0x74, 0x12, 0x1a, 0x0a, 0x08, 0x75, 0x73, 0x65, 0x72, 0x43, 0x6f, 0x64, 0x65, 0x18, 0x01, 0x20, - 0x01, 0x28, 0x09, 0x52, 0x08, 0x75, 0x73, 0x65, 0x72, 0x43, 0x6f, 0x64, 0x65, 0x22, 0x16, 0x0a, - 0x14, 0x57, 0x61, 0x69, 0x74, 0x53, 0x53, 0x4f, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, 0x65, 0x73, - 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x0b, 0x0a, 0x09, 0x55, 0x70, 0x52, 0x65, 0x71, 0x75, 0x65, - 0x73, 0x74, 0x22, 0x0c, 0x0a, 0x0a, 0x55, 0x70, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, - 0x22, 0x3d, 0x0a, 0x0d, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, - 0x74, 0x12, 0x2c, 0x0a, 0x11, 0x67, 0x65, 0x74, 0x46, 0x75, 0x6c, 0x6c, 0x50, 0x65, 0x65, 0x72, - 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x18, 0x01, 0x20, 0x01, 0x28, 0x08, 0x52, 0x11, 0x67, 0x65, - 0x74, 0x46, 0x75, 0x6c, 0x6c, 0x50, 0x65, 0x65, 0x72, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x22, - 0x82, 0x01, 0x0a, 0x0e, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, - 0x73, 0x65, 0x12, 0x16, 0x0a, 0x06, 0x73, 0x74, 0x61, 0x74, 0x75, 0x73, 0x18, 0x01, 0x20, 0x01, - 0x28, 0x09, 0x52, 0x06, 0x73, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x32, 0x0a, 0x0a, 0x66, 0x75, - 0x6c, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x12, - 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x46, 0x75, 0x6c, 0x6c, 0x53, 0x74, 0x61, 0x74, - 0x75, 0x73, 0x52, 0x0a, 0x66, 0x75, 0x6c, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x24, - 0x0a, 0x0d, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x56, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x18, - 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0d, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x56, 0x65, 0x72, - 0x73, 0x69, 0x6f, 0x6e, 0x22, 0x0d, 0x0a, 0x0b, 0x44, 0x6f, 0x77, 0x6e, 0x52, 0x65, 0x71, 0x75, - 0x65, 0x73, 0x74, 0x22, 0x0e, 0x0a, 0x0c, 0x44, 0x6f, 0x77, 0x6e, 0x52, 0x65, 0x73, 0x70, 0x6f, - 0x6e, 0x73, 0x65, 0x22, 0x12, 0x0a, 0x10, 0x47, 0x65, 0x74, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, - 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x22, 0xb3, 0x01, 0x0a, 0x11, 0x47, 0x65, 0x74, 0x43, - 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x24, 0x0a, - 0x0d, 0x6d, 0x61, 0x6e, 0x61, 0x67, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x55, 0x72, 0x6c, 0x18, 0x01, - 0x20, 0x01, 0x28, 0x09, 0x52, 0x0d, 0x6d, 0x61, 0x6e, 0x61, 0x67, 0x65, 0x6d, 0x65, 0x6e, 0x74, - 0x55, 0x72, 0x6c, 0x12, 0x1e, 0x0a, 0x0a, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x46, 0x69, 0x6c, - 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0a, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x46, - 0x69, 0x6c, 0x65, 0x12, 0x18, 0x0a, 0x07, 0x6c, 0x6f, 0x67, 0x46, 0x69, 0x6c, 0x65, 0x18, 0x03, - 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x6c, 0x6f, 0x67, 0x46, 0x69, 0x6c, 0x65, 0x12, 0x22, 0x0a, - 0x0c, 0x70, 0x72, 0x65, 0x53, 0x68, 0x61, 0x72, 0x65, 0x64, 0x4b, 0x65, 0x79, 0x18, 0x04, 0x20, - 0x01, 0x28, 0x09, 0x52, 0x0c, 0x70, 0x72, 0x65, 0x53, 0x68, 0x61, 0x72, 0x65, 0x64, 0x4b, 0x65, - 0x79, 0x12, 0x1a, 0x0a, 0x08, 0x61, 0x64, 0x6d, 0x69, 0x6e, 0x55, 0x52, 0x4c, 0x18, 0x05, 0x20, - 0x01, 0x28, 0x09, 0x52, 0x08, 0x61, 0x64, 0x6d, 0x69, 0x6e, 0x55, 0x52, 0x4c, 0x22, 0xcf, 0x02, - 0x0a, 0x09, 0x50, 0x65, 0x65, 0x72, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x0e, 0x0a, 0x02, 0x49, - 0x50, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, 0x49, 0x50, 0x12, 0x16, 0x0a, 0x06, 0x70, - 0x75, 0x62, 0x4b, 0x65, 0x79, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x06, 0x70, 0x75, 0x62, - 0x4b, 0x65, 0x79, 0x12, 0x1e, 0x0a, 0x0a, 0x63, 0x6f, 0x6e, 0x6e, 0x53, 0x74, 0x61, 0x74, 0x75, - 0x73, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0a, 0x63, 0x6f, 0x6e, 0x6e, 0x53, 0x74, 0x61, - 0x74, 0x75, 0x73, 0x12, 0x46, 0x0a, 0x10, 0x63, 0x6f, 0x6e, 0x6e, 0x53, 0x74, 0x61, 0x74, 0x75, - 0x73, 0x55, 0x70, 0x64, 0x61, 0x74, 0x65, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1a, 0x2e, - 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, - 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x52, 0x10, 0x63, 0x6f, 0x6e, 0x6e, 0x53, - 0x74, 0x61, 0x74, 0x75, 0x73, 0x55, 0x70, 0x64, 0x61, 0x74, 0x65, 0x12, 0x18, 0x0a, 0x07, 0x72, - 0x65, 0x6c, 0x61, 0x79, 0x65, 0x64, 0x18, 0x05, 0x20, 0x01, 0x28, 0x08, 0x52, 0x07, 0x72, 0x65, - 0x6c, 0x61, 0x79, 0x65, 0x64, 0x12, 0x16, 0x0a, 0x06, 0x64, 0x69, 0x72, 0x65, 0x63, 0x74, 0x18, - 0x06, 0x20, 0x01, 0x28, 0x08, 0x52, 0x06, 0x64, 0x69, 0x72, 0x65, 0x63, 0x74, 0x12, 0x34, 0x0a, - 0x15, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x49, 0x63, 0x65, 0x43, 0x61, 0x6e, 0x64, 0x69, 0x64, 0x61, - 0x74, 0x65, 0x54, 0x79, 0x70, 0x65, 0x18, 0x07, 0x20, 0x01, 0x28, 0x09, 0x52, 0x15, 0x6c, 0x6f, - 0x63, 0x61, 0x6c, 0x49, 0x63, 0x65, 0x43, 0x61, 0x6e, 0x64, 0x69, 0x64, 0x61, 0x74, 0x65, 0x54, - 0x79, 0x70, 0x65, 0x12, 0x36, 0x0a, 0x16, 0x72, 0x65, 0x6d, 0x6f, 0x74, 0x65, 0x49, 0x63, 0x65, - 0x43, 0x61, 0x6e, 0x64, 0x69, 0x64, 0x61, 0x74, 0x65, 0x54, 0x79, 0x70, 0x65, 0x18, 0x08, 0x20, - 0x01, 0x28, 0x09, 0x52, 0x16, 0x72, 0x65, 0x6d, 0x6f, 0x74, 0x65, 0x49, 0x63, 0x65, 0x43, 0x61, - 0x6e, 0x64, 0x69, 0x64, 0x61, 0x74, 0x65, 0x54, 0x79, 0x70, 0x65, 0x12, 0x12, 0x0a, 0x04, 0x66, - 0x71, 0x64, 0x6e, 0x18, 0x09, 0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x66, 0x71, 0x64, 0x6e, 0x22, - 0x76, 0x0a, 0x0e, 0x4c, 0x6f, 0x63, 0x61, 0x6c, 0x50, 0x65, 0x65, 0x72, 0x53, 0x74, 0x61, 0x74, - 0x65, 0x12, 0x0e, 0x0a, 0x02, 0x49, 0x50, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, 0x49, - 0x50, 0x12, 0x16, 0x0a, 0x06, 0x70, 0x75, 0x62, 0x4b, 0x65, 0x79, 0x18, 0x02, 0x20, 0x01, 0x28, - 0x09, 0x52, 0x06, 0x70, 0x75, 0x62, 0x4b, 0x65, 0x79, 0x12, 0x28, 0x0a, 0x0f, 0x6b, 0x65, 0x72, - 0x6e, 0x65, 0x6c, 0x49, 0x6e, 0x74, 0x65, 0x72, 0x66, 0x61, 0x63, 0x65, 0x18, 0x03, 0x20, 0x01, - 0x28, 0x08, 0x52, 0x0f, 0x6b, 0x65, 0x72, 0x6e, 0x65, 0x6c, 0x49, 0x6e, 0x74, 0x65, 0x72, 0x66, - 0x61, 0x63, 0x65, 0x12, 0x12, 0x0a, 0x04, 0x66, 0x71, 0x64, 0x6e, 0x18, 0x04, 0x20, 0x01, 0x28, - 0x09, 0x52, 0x04, 0x66, 0x71, 0x64, 0x6e, 0x22, 0x3d, 0x0a, 0x0b, 0x53, 0x69, 0x67, 0x6e, 0x61, - 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x10, 0x0a, 0x03, 0x55, 0x52, 0x4c, 0x18, 0x01, 0x20, - 0x01, 0x28, 0x09, 0x52, 0x03, 0x55, 0x52, 0x4c, 0x12, 0x1c, 0x0a, 0x09, 0x63, 0x6f, 0x6e, 0x6e, - 0x65, 0x63, 0x74, 0x65, 0x64, 0x18, 0x02, 0x20, 0x01, 0x28, 0x08, 0x52, 0x09, 0x63, 0x6f, 0x6e, - 0x6e, 0x65, 0x63, 0x74, 0x65, 0x64, 0x22, 0x41, 0x0a, 0x0f, 0x4d, 0x61, 0x6e, 0x61, 0x67, 0x65, - 0x6d, 0x65, 0x6e, 0x74, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x10, 0x0a, 0x03, 0x55, 0x52, 0x4c, - 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x55, 0x52, 0x4c, 0x12, 0x1c, 0x0a, 0x09, 0x63, - 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x65, 0x64, 0x18, 0x02, 0x20, 0x01, 0x28, 0x08, 0x52, 0x09, - 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x65, 0x64, 0x22, 0xef, 0x01, 0x0a, 0x0a, 0x46, 0x75, - 0x6c, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x41, 0x0a, 0x0f, 0x6d, 0x61, 0x6e, 0x61, - 0x67, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x53, 0x74, 0x61, 0x74, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, - 0x0b, 0x32, 0x17, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x4d, 0x61, 0x6e, 0x61, 0x67, - 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x53, 0x74, 0x61, 0x74, 0x65, 0x52, 0x0f, 0x6d, 0x61, 0x6e, 0x61, - 0x67, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x35, 0x0a, 0x0b, 0x73, - 0x69, 0x67, 0x6e, 0x61, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, - 0x32, 0x13, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x53, 0x69, 0x67, 0x6e, 0x61, 0x6c, - 0x53, 0x74, 0x61, 0x74, 0x65, 0x52, 0x0b, 0x73, 0x69, 0x67, 0x6e, 0x61, 0x6c, 0x53, 0x74, 0x61, - 0x74, 0x65, 0x12, 0x3e, 0x0a, 0x0e, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x50, 0x65, 0x65, 0x72, 0x53, - 0x74, 0x61, 0x74, 0x65, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x16, 0x2e, 0x64, 0x61, 0x65, - 0x6d, 0x6f, 0x6e, 0x2e, 0x4c, 0x6f, 0x63, 0x61, 0x6c, 0x50, 0x65, 0x65, 0x72, 0x53, 0x74, 0x61, - 0x74, 0x65, 0x52, 0x0e, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x50, 0x65, 0x65, 0x72, 0x53, 0x74, 0x61, - 0x74, 0x65, 0x12, 0x27, 0x0a, 0x05, 0x70, 0x65, 0x65, 0x72, 0x73, 0x18, 0x04, 0x20, 0x03, 0x28, - 0x0b, 0x32, 0x11, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x50, 0x65, 0x65, 0x72, 0x53, - 0x74, 0x61, 0x74, 0x65, 0x52, 0x05, 0x70, 0x65, 0x65, 0x72, 0x73, 0x32, 0xf7, 0x02, 0x0a, 0x0d, - 0x44, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x12, 0x36, 0x0a, - 0x05, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x12, 0x14, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, - 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x15, 0x2e, 0x64, - 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, 0x65, 0x73, 0x70, 0x6f, - 0x6e, 0x73, 0x65, 0x22, 0x00, 0x12, 0x4b, 0x0a, 0x0c, 0x57, 0x61, 0x69, 0x74, 0x53, 0x53, 0x4f, - 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x12, 0x1b, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x57, - 0x61, 0x69, 0x74, 0x53, 0x53, 0x4f, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, 0x65, 0x71, 0x75, 0x65, - 0x73, 0x74, 0x1a, 0x1c, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x57, 0x61, 0x69, 0x74, - 0x53, 0x53, 0x4f, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, - 0x22, 0x00, 0x12, 0x2d, 0x0a, 0x02, 0x55, 0x70, 0x12, 0x11, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, - 0x6e, 0x2e, 0x55, 0x70, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x12, 0x2e, 0x64, 0x61, - 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x55, 0x70, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, - 0x00, 0x12, 0x39, 0x0a, 0x06, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x15, 0x2e, 0x64, 0x61, - 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x52, 0x65, 0x71, 0x75, 0x65, - 0x73, 0x74, 0x1a, 0x16, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x53, 0x74, 0x61, 0x74, - 0x75, 0x73, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x12, 0x33, 0x0a, 0x04, - 0x44, 0x6f, 0x77, 0x6e, 0x12, 0x13, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x44, 0x6f, - 0x77, 0x6e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x14, 0x2e, 0x64, 0x61, 0x65, 0x6d, - 0x6f, 0x6e, 0x2e, 0x44, 0x6f, 0x77, 0x6e, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, - 0x00, 0x12, 0x42, 0x0a, 0x09, 0x47, 0x65, 0x74, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x12, 0x18, - 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x47, 0x65, 0x74, 0x43, 0x6f, 0x6e, 0x66, 0x69, - 0x67, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x19, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, - 0x6e, 0x2e, 0x47, 0x65, 0x74, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x52, 0x65, 0x73, 0x70, 0x6f, - 0x6e, 0x73, 0x65, 0x22, 0x00, 0x42, 0x08, 0x5a, 0x06, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, - 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x73, 0x73, 0x12, 0x32, 0x0a, 0x14, 0x69, 0x73, 0x4c, 0x69, 0x6e, 0x75, 0x78, 0x44, 0x65, 0x73, + 0x6b, 0x74, 0x6f, 0x70, 0x43, 0x6c, 0x69, 0x65, 0x6e, 0x74, 0x18, 0x08, 0x20, 0x01, 0x28, 0x08, + 0x52, 0x14, 0x69, 0x73, 0x4c, 0x69, 0x6e, 0x75, 0x78, 0x44, 0x65, 0x73, 0x6b, 0x74, 0x6f, 0x70, + 0x43, 0x6c, 0x69, 0x65, 0x6e, 0x74, 0x22, 0xb5, 0x01, 0x0a, 0x0d, 0x4c, 0x6f, 0x67, 0x69, 0x6e, + 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x24, 0x0a, 0x0d, 0x6e, 0x65, 0x65, 0x64, + 0x73, 0x53, 0x53, 0x4f, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x18, 0x01, 0x20, 0x01, 0x28, 0x08, 0x52, + 0x0d, 0x6e, 0x65, 0x65, 0x64, 0x73, 0x53, 0x53, 0x4f, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x12, 0x1a, + 0x0a, 0x08, 0x75, 0x73, 0x65, 0x72, 0x43, 0x6f, 0x64, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, + 0x52, 0x08, 0x75, 0x73, 0x65, 0x72, 0x43, 0x6f, 0x64, 0x65, 0x12, 0x28, 0x0a, 0x0f, 0x76, 0x65, + 0x72, 0x69, 0x66, 0x69, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x55, 0x52, 0x49, 0x18, 0x03, 0x20, + 0x01, 0x28, 0x09, 0x52, 0x0f, 0x76, 0x65, 0x72, 0x69, 0x66, 0x69, 0x63, 0x61, 0x74, 0x69, 0x6f, + 0x6e, 0x55, 0x52, 0x49, 0x12, 0x38, 0x0a, 0x17, 0x76, 0x65, 0x72, 0x69, 0x66, 0x69, 0x63, 0x61, + 0x74, 0x69, 0x6f, 0x6e, 0x55, 0x52, 0x49, 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x18, + 0x04, 0x20, 0x01, 0x28, 0x09, 0x52, 0x17, 0x76, 0x65, 0x72, 0x69, 0x66, 0x69, 0x63, 0x61, 0x74, + 0x69, 0x6f, 0x6e, 0x55, 0x52, 0x49, 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x22, 0x31, + 0x0a, 0x13, 0x57, 0x61, 0x69, 0x74, 0x53, 0x53, 0x4f, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, 0x65, + 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x1a, 0x0a, 0x08, 0x75, 0x73, 0x65, 0x72, 0x43, 0x6f, 0x64, + 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x75, 0x73, 0x65, 0x72, 0x43, 0x6f, 0x64, + 0x65, 0x22, 0x16, 0x0a, 0x14, 0x57, 0x61, 0x69, 0x74, 0x53, 0x53, 0x4f, 0x4c, 0x6f, 0x67, 0x69, + 0x6e, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x0b, 0x0a, 0x09, 0x55, 0x70, 0x52, + 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x22, 0x0c, 0x0a, 0x0a, 0x55, 0x70, 0x52, 0x65, 0x73, 0x70, + 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x3d, 0x0a, 0x0d, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x52, 0x65, + 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x2c, 0x0a, 0x11, 0x67, 0x65, 0x74, 0x46, 0x75, 0x6c, 0x6c, + 0x50, 0x65, 0x65, 0x72, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x18, 0x01, 0x20, 0x01, 0x28, 0x08, + 0x52, 0x11, 0x67, 0x65, 0x74, 0x46, 0x75, 0x6c, 0x6c, 0x50, 0x65, 0x65, 0x72, 0x53, 0x74, 0x61, + 0x74, 0x75, 0x73, 0x22, 0x82, 0x01, 0x0a, 0x0e, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x52, 0x65, + 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x16, 0x0a, 0x06, 0x73, 0x74, 0x61, 0x74, 0x75, 0x73, + 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x06, 0x73, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x32, + 0x0a, 0x0a, 0x66, 0x75, 0x6c, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x18, 0x02, 0x20, 0x01, + 0x28, 0x0b, 0x32, 0x12, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x46, 0x75, 0x6c, 0x6c, + 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x52, 0x0a, 0x66, 0x75, 0x6c, 0x6c, 0x53, 0x74, 0x61, 0x74, + 0x75, 0x73, 0x12, 0x24, 0x0a, 0x0d, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x56, 0x65, 0x72, 0x73, + 0x69, 0x6f, 0x6e, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0d, 0x64, 0x61, 0x65, 0x6d, 0x6f, + 0x6e, 0x56, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x22, 0x0d, 0x0a, 0x0b, 0x44, 0x6f, 0x77, 0x6e, + 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x22, 0x0e, 0x0a, 0x0c, 0x44, 0x6f, 0x77, 0x6e, 0x52, + 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x12, 0x0a, 0x10, 0x47, 0x65, 0x74, 0x43, 0x6f, + 0x6e, 0x66, 0x69, 0x67, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x22, 0xb3, 0x01, 0x0a, 0x11, + 0x47, 0x65, 0x74, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, + 0x65, 0x12, 0x24, 0x0a, 0x0d, 0x6d, 0x61, 0x6e, 0x61, 0x67, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x55, + 0x72, 0x6c, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0d, 0x6d, 0x61, 0x6e, 0x61, 0x67, 0x65, + 0x6d, 0x65, 0x6e, 0x74, 0x55, 0x72, 0x6c, 0x12, 0x1e, 0x0a, 0x0a, 0x63, 0x6f, 0x6e, 0x66, 0x69, + 0x67, 0x46, 0x69, 0x6c, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0a, 0x63, 0x6f, 0x6e, + 0x66, 0x69, 0x67, 0x46, 0x69, 0x6c, 0x65, 0x12, 0x18, 0x0a, 0x07, 0x6c, 0x6f, 0x67, 0x46, 0x69, + 0x6c, 0x65, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x6c, 0x6f, 0x67, 0x46, 0x69, 0x6c, + 0x65, 0x12, 0x22, 0x0a, 0x0c, 0x70, 0x72, 0x65, 0x53, 0x68, 0x61, 0x72, 0x65, 0x64, 0x4b, 0x65, + 0x79, 0x18, 0x04, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0c, 0x70, 0x72, 0x65, 0x53, 0x68, 0x61, 0x72, + 0x65, 0x64, 0x4b, 0x65, 0x79, 0x12, 0x1a, 0x0a, 0x08, 0x61, 0x64, 0x6d, 0x69, 0x6e, 0x55, 0x52, + 0x4c, 0x18, 0x05, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x61, 0x64, 0x6d, 0x69, 0x6e, 0x55, 0x52, + 0x4c, 0x22, 0xcf, 0x02, 0x0a, 0x09, 0x50, 0x65, 0x65, 0x72, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, + 0x0e, 0x0a, 0x02, 0x49, 0x50, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, 0x49, 0x50, 0x12, + 0x16, 0x0a, 0x06, 0x70, 0x75, 0x62, 0x4b, 0x65, 0x79, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, + 0x06, 0x70, 0x75, 0x62, 0x4b, 0x65, 0x79, 0x12, 0x1e, 0x0a, 0x0a, 0x63, 0x6f, 0x6e, 0x6e, 0x53, + 0x74, 0x61, 0x74, 0x75, 0x73, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0a, 0x63, 0x6f, 0x6e, + 0x6e, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x46, 0x0a, 0x10, 0x63, 0x6f, 0x6e, 0x6e, 0x53, + 0x74, 0x61, 0x74, 0x75, 0x73, 0x55, 0x70, 0x64, 0x61, 0x74, 0x65, 0x18, 0x04, 0x20, 0x01, 0x28, + 0x0b, 0x32, 0x1a, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, + 0x62, 0x75, 0x66, 0x2e, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x52, 0x10, 0x63, + 0x6f, 0x6e, 0x6e, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x55, 0x70, 0x64, 0x61, 0x74, 0x65, 0x12, + 0x18, 0x0a, 0x07, 0x72, 0x65, 0x6c, 0x61, 0x79, 0x65, 0x64, 0x18, 0x05, 0x20, 0x01, 0x28, 0x08, + 0x52, 0x07, 0x72, 0x65, 0x6c, 0x61, 0x79, 0x65, 0x64, 0x12, 0x16, 0x0a, 0x06, 0x64, 0x69, 0x72, + 0x65, 0x63, 0x74, 0x18, 0x06, 0x20, 0x01, 0x28, 0x08, 0x52, 0x06, 0x64, 0x69, 0x72, 0x65, 0x63, + 0x74, 0x12, 0x34, 0x0a, 0x15, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x49, 0x63, 0x65, 0x43, 0x61, 0x6e, + 0x64, 0x69, 0x64, 0x61, 0x74, 0x65, 0x54, 0x79, 0x70, 0x65, 0x18, 0x07, 0x20, 0x01, 0x28, 0x09, + 0x52, 0x15, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x49, 0x63, 0x65, 0x43, 0x61, 0x6e, 0x64, 0x69, 0x64, + 0x61, 0x74, 0x65, 0x54, 0x79, 0x70, 0x65, 0x12, 0x36, 0x0a, 0x16, 0x72, 0x65, 0x6d, 0x6f, 0x74, + 0x65, 0x49, 0x63, 0x65, 0x43, 0x61, 0x6e, 0x64, 0x69, 0x64, 0x61, 0x74, 0x65, 0x54, 0x79, 0x70, + 0x65, 0x18, 0x08, 0x20, 0x01, 0x28, 0x09, 0x52, 0x16, 0x72, 0x65, 0x6d, 0x6f, 0x74, 0x65, 0x49, + 0x63, 0x65, 0x43, 0x61, 0x6e, 0x64, 0x69, 0x64, 0x61, 0x74, 0x65, 0x54, 0x79, 0x70, 0x65, 0x12, + 0x12, 0x0a, 0x04, 0x66, 0x71, 0x64, 0x6e, 0x18, 0x09, 0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x66, + 0x71, 0x64, 0x6e, 0x22, 0x76, 0x0a, 0x0e, 0x4c, 0x6f, 0x63, 0x61, 0x6c, 0x50, 0x65, 0x65, 0x72, + 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x0e, 0x0a, 0x02, 0x49, 0x50, 0x18, 0x01, 0x20, 0x01, 0x28, + 0x09, 0x52, 0x02, 0x49, 0x50, 0x12, 0x16, 0x0a, 0x06, 0x70, 0x75, 0x62, 0x4b, 0x65, 0x79, 0x18, + 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x06, 0x70, 0x75, 0x62, 0x4b, 0x65, 0x79, 0x12, 0x28, 0x0a, + 0x0f, 0x6b, 0x65, 0x72, 0x6e, 0x65, 0x6c, 0x49, 0x6e, 0x74, 0x65, 0x72, 0x66, 0x61, 0x63, 0x65, + 0x18, 0x03, 0x20, 0x01, 0x28, 0x08, 0x52, 0x0f, 0x6b, 0x65, 0x72, 0x6e, 0x65, 0x6c, 0x49, 0x6e, + 0x74, 0x65, 0x72, 0x66, 0x61, 0x63, 0x65, 0x12, 0x12, 0x0a, 0x04, 0x66, 0x71, 0x64, 0x6e, 0x18, + 0x04, 0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x66, 0x71, 0x64, 0x6e, 0x22, 0x3d, 0x0a, 0x0b, 0x53, + 0x69, 0x67, 0x6e, 0x61, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x10, 0x0a, 0x03, 0x55, 0x52, + 0x4c, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x55, 0x52, 0x4c, 0x12, 0x1c, 0x0a, 0x09, + 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x65, 0x64, 0x18, 0x02, 0x20, 0x01, 0x28, 0x08, 0x52, + 0x09, 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x65, 0x64, 0x22, 0x41, 0x0a, 0x0f, 0x4d, 0x61, + 0x6e, 0x61, 0x67, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x10, 0x0a, + 0x03, 0x55, 0x52, 0x4c, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x55, 0x52, 0x4c, 0x12, + 0x1c, 0x0a, 0x09, 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x65, 0x64, 0x18, 0x02, 0x20, 0x01, + 0x28, 0x08, 0x52, 0x09, 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x65, 0x64, 0x22, 0xef, 0x01, + 0x0a, 0x0a, 0x46, 0x75, 0x6c, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x41, 0x0a, 0x0f, + 0x6d, 0x61, 0x6e, 0x61, 0x67, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x53, 0x74, 0x61, 0x74, 0x65, 0x18, + 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x17, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x4d, + 0x61, 0x6e, 0x61, 0x67, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x53, 0x74, 0x61, 0x74, 0x65, 0x52, 0x0f, + 0x6d, 0x61, 0x6e, 0x61, 0x67, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, + 0x35, 0x0a, 0x0b, 0x73, 0x69, 0x67, 0x6e, 0x61, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x18, 0x02, + 0x20, 0x01, 0x28, 0x0b, 0x32, 0x13, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x53, 0x69, + 0x67, 0x6e, 0x61, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x52, 0x0b, 0x73, 0x69, 0x67, 0x6e, 0x61, + 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x3e, 0x0a, 0x0e, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x50, + 0x65, 0x65, 0x72, 0x53, 0x74, 0x61, 0x74, 0x65, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x16, + 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x4c, 0x6f, 0x63, 0x61, 0x6c, 0x50, 0x65, 0x65, + 0x72, 0x53, 0x74, 0x61, 0x74, 0x65, 0x52, 0x0e, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x50, 0x65, 0x65, + 0x72, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x27, 0x0a, 0x05, 0x70, 0x65, 0x65, 0x72, 0x73, 0x18, + 0x04, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x11, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x50, + 0x65, 0x65, 0x72, 0x53, 0x74, 0x61, 0x74, 0x65, 0x52, 0x05, 0x70, 0x65, 0x65, 0x72, 0x73, 0x32, + 0xf7, 0x02, 0x0a, 0x0d, 0x44, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, + 0x65, 0x12, 0x36, 0x0a, 0x05, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x12, 0x14, 0x2e, 0x64, 0x61, 0x65, + 0x6d, 0x6f, 0x6e, 0x2e, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, + 0x1a, 0x15, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, + 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x12, 0x4b, 0x0a, 0x0c, 0x57, 0x61, 0x69, + 0x74, 0x53, 0x53, 0x4f, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x12, 0x1b, 0x2e, 0x64, 0x61, 0x65, 0x6d, + 0x6f, 0x6e, 0x2e, 0x57, 0x61, 0x69, 0x74, 0x53, 0x53, 0x4f, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, + 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x1c, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, + 0x57, 0x61, 0x69, 0x74, 0x53, 0x53, 0x4f, 0x4c, 0x6f, 0x67, 0x69, 0x6e, 0x52, 0x65, 0x73, 0x70, + 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x12, 0x2d, 0x0a, 0x02, 0x55, 0x70, 0x12, 0x11, 0x2e, 0x64, + 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x55, 0x70, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, + 0x12, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x55, 0x70, 0x52, 0x65, 0x73, 0x70, 0x6f, + 0x6e, 0x73, 0x65, 0x22, 0x00, 0x12, 0x39, 0x0a, 0x06, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, + 0x15, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x52, + 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x16, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, + 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, + 0x12, 0x33, 0x0a, 0x04, 0x44, 0x6f, 0x77, 0x6e, 0x12, 0x13, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, + 0x6e, 0x2e, 0x44, 0x6f, 0x77, 0x6e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x14, 0x2e, + 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x44, 0x6f, 0x77, 0x6e, 0x52, 0x65, 0x73, 0x70, 0x6f, + 0x6e, 0x73, 0x65, 0x22, 0x00, 0x12, 0x42, 0x0a, 0x09, 0x47, 0x65, 0x74, 0x43, 0x6f, 0x6e, 0x66, + 0x69, 0x67, 0x12, 0x18, 0x2e, 0x64, 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x47, 0x65, 0x74, 0x43, + 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x19, 0x2e, 0x64, + 0x61, 0x65, 0x6d, 0x6f, 0x6e, 0x2e, 0x47, 0x65, 0x74, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x52, + 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x42, 0x08, 0x5a, 0x06, 0x2f, 0x70, 0x72, + 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/client/proto/daemon.proto b/client/proto/daemon.proto index 53d921d767c..8bed1ec9dc3 100644 --- a/client/proto/daemon.proto +++ b/client/proto/daemon.proto @@ -51,6 +51,7 @@ message LoginRequest { bytes customDNSAddress = 7; + bool isLinuxDesktopClient = 8; } message LoginResponse { diff --git a/client/server/server.go b/client/server/server.go index 6748f62ab6f..faac2227355 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -208,7 +208,7 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro state.Set(internal.StatusConnecting) if msg.SetupKey == "" { - oAuthFlow, err := auth.NewOAuthFlow(ctx, config) + oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.IsLinuxDesktopClient) if err != nil { state.Set(internal.StatusLoginFailed) return nil, err From 8118d60ffb252686a3643f58ca276573cf50977d Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Thu, 28 Sep 2023 14:32:36 +0200 Subject: [PATCH 03/14] Add peer groups support for network routes (#1150) This commit enhances the functionality of the network routes endpoint by introducing a new parameter called `peers_group`. This addition allows users to associate network routes with specific peer groups, simplifying the management and distribution of routes within a network. --- management/server/account.go | 96 +++- management/server/account_test.go | 5 +- management/server/group.go | 4 +- management/server/http/api/openapi.yml | 12 +- management/server/http/api/types.gen.go | 16 +- management/server/http/routes_handler.go | 69 ++- management/server/http/routes_handler_test.go | 134 +++++- management/server/mock_server/account_mock.go | 18 +- management/server/route.go | 117 ++++- management/server/route_test.go | 449 +++++++++++++++++- route/route.go | 10 +- 11 files changed, 829 insertions(+), 101 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 65a284b5f9b..b4f93916666 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -83,14 +83,14 @@ type AccountManager interface { DeleteGroup(accountId, userId, groupID string) error ListGroups(accountId string) ([]*Group, error) GroupAddPeer(accountId, groupID, peerID string) error - GroupDeletePeer(accountId, groupID, peerKey string) error + GroupDeletePeer(accountId, groupID, peerID string) error GroupListPeers(accountId, groupID string) ([]*Peer, error) GetPolicy(accountID, policyID, userID string) (*Policy, error) SavePolicy(accountID, userID string, policy *Policy) error DeletePolicy(accountID, policyID, userID string) error ListPolicies(accountID, userID string) ([]*Policy, error) GetRoute(accountID, routeID, userID string) (*route.Route, error) - CreateRoute(accountID string, prefix, peerID, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) + CreateRoute(accountID, prefix, peerID string, peerGroupIDs []string, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) SaveRoute(accountID, userID string, route *route.Route) error DeleteRoute(accountID, routeID, userID string) error ListRoutes(accountID, userID string) ([]*route.Route, error) @@ -253,22 +253,39 @@ func (a *Account) filterRoutesByGroups(routes []*route.Route, groupListMap looku func (a *Account) getEnabledAndDisabledRoutesByPeer(peerID string) ([]*route.Route, []*route.Route) { var enabledRoutes []*route.Route var disabledRoutes []*route.Route + + takeRoute := func(r *route.Route, id string) { + peer := a.GetPeer(peerID) + if peer == nil { + log.Errorf("route %s has peer %s that doesn't exist under account %s", r.ID, peerID, a.Id) + return + } + + if r.Enabled { + enabledRoutes = append(enabledRoutes, r) + return + } + disabledRoutes = append(disabledRoutes, r) + } + for _, r := range a.Routes { - if r.Peer == peerID { - // We need to set Peer.Key instead of Peer.ID because this object will be sent to agents as part of a network map. - // Ideally we should have a separate field for that, but fine for now. - peer := a.GetPeer(peerID) - if peer == nil { - log.Errorf("route %s has peer %s that doesn't exist under account %s", r.ID, peerID, a.Id) - continue - } - raut := r.Copy() - raut.Peer = peer.Key - if r.Enabled { - enabledRoutes = append(enabledRoutes, raut) - continue + if len(r.PeerGroups) != 0 { + for _, groupID := range r.PeerGroups { + group := a.GetGroup(groupID) + if group == nil { + log.Errorf("route %s has peers group %s that doesn't exist under account %s", r.ID, groupID, a.Id) + continue + } + for _, id := range group.Peers { + if id == peerID { + takeRoute(r, id) + break + } + } } - disabledRoutes = append(disabledRoutes, raut) + } + if r.Peer == peerID { + takeRoute(r, peerID) } } return enabledRoutes, disabledRoutes @@ -316,8 +333,51 @@ func (a *Account) GetPeerNetworkMap(peerID, dnsDomain string) *NetworkMap { } peersToConnect = append(peersToConnect, p) } - // Please mind, that the returned route.Route objects will contain Peer.Key instead of Peer.ID. - routesUpdate := a.getRoutesToSync(peerID, peersToConnect) + + routes := a.getRoutesToSync(peerID, peersToConnect) + + takePeer := func(id string) (*Peer, bool) { + peer := a.GetPeer(id) + if peer == nil || peer.Meta.GoOS != "linux" { + return nil, false + } + return peer, true + } + + // We need to set Peer.Key instead of Peer.ID because this object will be sent to agents as part of a network map. + // Ideally we should have a separate field for that, but fine for now. + var routesUpdate []*route.Route + seenPeers := make(map[string]bool) + for _, r := range routes { + if r.Peer != "" { + peer, valid := takePeer(r.Peer) + if !valid { + continue + } + rCopy := r.Copy() + rCopy.Peer = peer.Key // client expects the key + routesUpdate = append(routesUpdate, rCopy) + continue + } + for _, groupID := range r.PeerGroups { + if group := a.GetGroup(groupID); group != nil { + for _, peerId := range group.Peers { + peer, valid := takePeer(peerId) + if !valid { + continue + } + + if _, ok := seenPeers[peer.ID]; !ok { + rCopy := r.Copy() + rCopy.ID = r.ID + ":" + peer.ID // we have to provide unit route id when distribute network map + rCopy.Peer = peer.Key // client expects the key + routesUpdate = append(routesUpdate, rCopy) + } + seenPeers[peer.ID] = true + } + } + } + } dnsManagementStatus := a.getPeerDNSManagementStatus(peerID) dnsUpdate := nbdns.Config{ diff --git a/management/server/account_test.go b/management/server/account_test.go index 204e9894759..8c8a1b00192 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -1385,8 +1385,9 @@ func TestAccount_Copy(t *testing.T) { }, Routes: map[string]*route.Route{ "route1": { - ID: "route1", - Groups: []string{"group1"}, + ID: "route1", + PeerGroups: []string{}, + Groups: []string{"group1"}, }, }, NameServerGroups: map[string]*nbdns.NameServerGroup{ diff --git a/management/server/group.go b/management/server/group.go index 697fe5d70db..0aebc645403 100644 --- a/management/server/group.go +++ b/management/server/group.go @@ -285,7 +285,7 @@ func (am *DefaultAccountManager) GroupAddPeer(accountID, groupID, peerID string) } // GroupDeletePeer removes peer from the group -func (am *DefaultAccountManager) GroupDeletePeer(accountID, groupID, peerKey string) error { +func (am *DefaultAccountManager) GroupDeletePeer(accountID, groupID, peerID string) error { unlock := am.Store.AcquireAccountLock(accountID) defer unlock() @@ -301,7 +301,7 @@ func (am *DefaultAccountManager) GroupDeletePeer(accountID, groupID, peerKey str account.Network.IncSerial() for i, itemID := range group.Peers { - if itemID == peerKey { + if itemID == peerID { group.Peers = append(group.Peers[:i], group.Peers[i+1:]...) if err := am.Store.SaveAccount(account); err != nil { return err diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index db2561481e3..cfd14958f3c 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -745,9 +745,15 @@ components: type: boolean example: true peer: - description: Peer Identifier associated with route + description: Peer Identifier associated with route. This property can not be set together with `peer_groups` type: string example: chacbco6lnnbn6cg5s91 + peer_groups: + description: Peers Group Identifier associated with route. This property can not be set together with `peer` + type: array + items: + type: string + example: chacbco6lnnbn6cg5s91 network: description: Network range in CIDR format type: string @@ -773,7 +779,9 @@ components: - description - network_id - enabled - - peer + # Only one property has to be set + #- peer + #- peer_groups - network - metric - masquerade diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go index dec73630dab..fd3eedde30c 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -1,6 +1,6 @@ // Package api provides primitives to interact with the openapi HTTP API. // -// Code generated by github.com/deepmap/oapi-codegen version v1.11.1-0.20220912230023-4a1477f6a8ba DO NOT EDIT. +// Code generated by github.com/deepmap/oapi-codegen version v1.15.0 DO NOT EDIT. package api import ( @@ -599,8 +599,11 @@ type Route struct { // NetworkType Network type indicating if it is IPv4 or IPv6 NetworkType string `json:"network_type"` - // Peer Peer Identifier associated with route - Peer string `json:"peer"` + // Peer Peer Identifier associated with route. This property can not be set together with `peer_groups` + Peer *string `json:"peer,omitempty"` + + // PeerGroups Peers Group Identifier associated with route. This property can not be set together with `peer` + PeerGroups *[]string `json:"peer_groups,omitempty"` } // RouteRequest defines model for RouteRequest. @@ -626,8 +629,11 @@ type RouteRequest struct { // NetworkId Route network identifier, to group HA routes NetworkId string `json:"network_id"` - // Peer Peer Identifier associated with route - Peer string `json:"peer"` + // Peer Peer Identifier associated with route. This property can not be set together with `peer_groups` + Peer *string `json:"peer,omitempty"` + + // PeerGroups Peers Group Identifier associated with route. This property can not be set together with `peer` + PeerGroups *[]string `json:"peer_groups,omitempty"` } // Rule defines model for Rule. diff --git a/management/server/http/routes_handler.go b/management/server/http/routes_handler.go index a6dfa9c74df..348bdbfd688 100644 --- a/management/server/http/routes_handler.go +++ b/management/server/http/routes_handler.go @@ -82,7 +82,33 @@ func (h *RoutesHandler) CreateRoute(w http.ResponseWriter, r *http.Request) { return } - newRoute, err := h.accountManager.CreateRoute(account.Id, newPrefix.String(), req.Peer, req.Description, req.NetworkId, req.Masquerade, req.Metric, req.Groups, req.Enabled, user.Id) + peerId := "" + if req.Peer != nil { + peerId = *req.Peer + } + + peerGroupIds := []string{} + if req.PeerGroups != nil { + peerGroupIds = *req.PeerGroups + } + + if (peerId != "" && len(peerGroupIds) > 0) || (peerId == "" && len(peerGroupIds) == 0) { + util.WriteError(status.Errorf(status.InvalidArgument, "only one peer or peer_groups should be provided"), w) + return + } + + // do not allow non Linux peers + if peer := account.GetPeer(peerId); peer != nil { + if peer.Meta.GoOS != "linux" { + util.WriteError(status.Errorf(status.InvalidArgument, "non-linux peers are non supported as network routes"), w) + return + } + } + + newRoute, err := h.accountManager.CreateRoute( + account.Id, newPrefix.String(), peerId, peerGroupIds, + req.Description, req.NetworkId, req.Masquerade, req.Metric, req.Groups, req.Enabled, user.Id, + ) if err != nil { util.WriteError(err, w) return @@ -135,19 +161,49 @@ func (h *RoutesHandler) UpdateRoute(w http.ResponseWriter, r *http.Request) { return } + if req.Peer != nil && req.PeerGroups != nil { + util.WriteError(status.Errorf(status.InvalidArgument, "only peer or peers_group should be provided"), w) + return + } + + if req.Peer == nil && req.PeerGroups == nil { + util.WriteError(status.Errorf(status.InvalidArgument, "either peer or peers_group should be provided"), w) + return + } + + peerID := "" + if req.Peer != nil { + peerID = *req.Peer + } + + // do not allow non Linux peers + if peer := account.GetPeer(peerID); peer != nil { + if peer.Meta.GoOS != "linux" { + util.WriteError(status.Errorf(status.InvalidArgument, "non-linux peers are non supported as network routes"), w) + return + } + } + newRoute := &route.Route{ ID: routeID, Network: newPrefix, NetID: req.NetworkId, NetworkType: prefixType, Masquerade: req.Masquerade, - Peer: req.Peer, Metric: req.Metric, Description: req.Description, Enabled: req.Enabled, Groups: req.Groups, } + if req.Peer != nil { + newRoute.Peer = peerID + } + + if req.PeerGroups != nil { + newRoute.PeerGroups = *req.PeerGroups + } + err = h.accountManager.SaveRoute(account.Id, user.Id, newRoute) if err != nil { util.WriteError(err, w) @@ -208,16 +264,21 @@ func (h *RoutesHandler) GetRoute(w http.ResponseWriter, r *http.Request) { } func toRouteResponse(serverRoute *route.Route) *api.Route { - return &api.Route{ + route := &api.Route{ Id: serverRoute.ID, Description: serverRoute.Description, NetworkId: serverRoute.NetID, Enabled: serverRoute.Enabled, - Peer: serverRoute.Peer, + Peer: &serverRoute.Peer, Network: serverRoute.Network.String(), NetworkType: serverRoute.NetworkType.String(), Masquerade: serverRoute.Masquerade, Metric: serverRoute.Metric, Groups: serverRoute.Groups, } + + if len(serverRoute.PeerGroups) > 0 { + route.PeerGroups = &serverRoute.PeerGroups + } + return route } diff --git a/management/server/http/routes_handler_test.go b/management/server/http/routes_handler_test.go index 3f2b7b91094..0f797c3b311 100644 --- a/management/server/http/routes_handler_test.go +++ b/management/server/http/routes_handler_test.go @@ -23,16 +23,23 @@ import ( ) const ( - existingRouteID = "existingRouteID" - notFoundRouteID = "notFoundRouteID" - existingPeerIP = "100.64.0.100" - existingPeerID = "peer-id" - notFoundPeerID = "nonExistingPeer" - existingPeerKey = "existingPeerKey" - testAccountID = "test_id" - existingGroupID = "testGroup" + existingRouteID = "existingRouteID" + existingRouteID2 = "existingRouteID2" // for peer_groups test + notFoundRouteID = "notFoundRouteID" + existingPeerIP1 = "100.64.0.100" + existingPeerIP2 = "100.64.0.101" + notFoundPeerID = "nonExistingPeer" + existingPeerKey = "existingPeerKey" + nonLinuxExistingPeerKey = "darwinExistingPeerKey" + testAccountID = "test_id" + existingGroupID = "testGroup" + notFoundGroupID = "nonExistingGroup" ) +var emptyString = "" +var existingPeerID = "peer-id" +var nonLinuxExistingPeerID = "darwin-peer-id" + var baseExistingRoute = &route.Route{ ID: existingRouteID, Description: "base route", @@ -51,8 +58,19 @@ var testingAccount = &server.Account{ Peers: map[string]*server.Peer{ existingPeerID: { Key: existingPeerKey, - IP: netip.MustParseAddr(existingPeerIP).AsSlice(), + IP: netip.MustParseAddr(existingPeerIP1).AsSlice(), ID: existingPeerID, + Meta: server.PeerSystemMeta{ + GoOS: "linux", + }, + }, + nonLinuxExistingPeerID: { + Key: nonLinuxExistingPeerID, + IP: netip.MustParseAddr(existingPeerIP2).AsSlice(), + ID: nonLinuxExistingPeerID, + Meta: server.PeerSystemMeta{ + GoOS: "darwin", + }, }, }, Users: map[string]*server.User{ @@ -67,17 +85,26 @@ func initRoutesTestData() *RoutesHandler { if routeID == existingRouteID { return baseExistingRoute, nil } + if routeID == existingRouteID2 { + route := baseExistingRoute.Copy() + route.PeerGroups = []string{existingGroupID} + return route, nil + } return nil, status.Errorf(status.NotFound, "route with ID %s not found", routeID) }, - CreateRouteFunc: func(accountID string, network, peerID, description, netID string, masquerade bool, metric int, groups []string, enabled bool, _ string) (*route.Route, error) { + CreateRouteFunc: func(accountID, network, peerID string, peerGroups []string, description, netID string, masquerade bool, metric int, groups []string, enabled bool, _ string) (*route.Route, error) { if peerID == notFoundPeerID { return nil, status.Errorf(status.InvalidArgument, "peer with ID %s not found", peerID) } + if len(peerGroups) > 0 && peerGroups[0] == notFoundGroupID { + return nil, status.Errorf(status.InvalidArgument, "peer groups with ID %s not found", peerGroups[0]) + } networkType, p, _ := route.ParseNetwork(network) return &route.Route{ ID: existingRouteID, NetID: netID, Peer: peerID, + PeerGroups: peerGroups, Network: p, NetworkType: networkType, Description: description, @@ -124,6 +151,9 @@ func initRoutesTestData() *RoutesHandler { } func TestRoutesHandlers(t *testing.T) { + baseExistingRouteWithPeerGroups := baseExistingRoute.Copy() + baseExistingRouteWithPeerGroups.PeerGroups = []string{existingGroupID} + tt := []struct { name string expectedStatus int @@ -147,6 +177,14 @@ func TestRoutesHandlers(t *testing.T) { requestPath: "/api/routes/" + notFoundRouteID, expectedStatus: http.StatusNotFound, }, + { + name: "Get Existing Route with Peer Groups", + requestType: http.MethodGet, + requestPath: "/api/routes/" + existingRouteID2, + expectedStatus: http.StatusOK, + expectedBody: true, + expectedRoute: toRouteResponse(baseExistingRouteWithPeerGroups), + }, { name: "Delete Existing Route", requestType: http.MethodDelete, @@ -173,13 +211,21 @@ func TestRoutesHandlers(t *testing.T) { Description: "Post", NetworkId: "awesomeNet", Network: "192.168.0.0/16", - Peer: existingPeerID, + Peer: &existingPeerID, NetworkType: route.IPv4NetworkString, Masquerade: false, Enabled: false, Groups: []string{existingGroupID}, }, }, + { + name: "POST Non Linux Peer", + requestType: http.MethodPost, + requestPath: "/api/routes", + requestBody: bytes.NewBufferString(fmt.Sprintf("{\"Description\":\"Post\",\"Network\":\"192.168.0.0/16\",\"network_id\":\"awesomeNet\",\"Peer\":\"%s\",\"groups\":[\"%s\"]}", nonLinuxExistingPeerID, existingGroupID)), + expectedStatus: http.StatusUnprocessableEntity, + expectedBody: false, + }, { name: "POST Not Found Peer", requestType: http.MethodPost, @@ -204,6 +250,24 @@ func TestRoutesHandlers(t *testing.T) { expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, + { + name: "POST UnprocessableEntity when both peer and peer_groups are provided", + requestType: http.MethodPost, + requestPath: "/api/routes", + requestBody: bytes.NewBuffer( + []byte(fmt.Sprintf("{\"Description\":\"Post\",\"Network\":\"192.168.0.0/16\",\"network_id\":\"awesomeNet\",\"peer\":\"%s\",\"peer_groups\":[\"%s\"],\"groups\":[\"%s\"]}", existingPeerID, existingGroupID, existingGroupID))), + expectedStatus: http.StatusUnprocessableEntity, + expectedBody: false, + }, + { + name: "POST UnprocessableEntity when no peer and peer_groups are provided", + requestType: http.MethodPost, + requestPath: "/api/routes", + requestBody: bytes.NewBuffer( + []byte(fmt.Sprintf("{\"Description\":\"Post\",\"Network\":\"192.168.0.0/16\",\"network_id\":\"awesomeNet\",\"groups\":[\"%s\"]}", existingPeerID))), + expectedStatus: http.StatusUnprocessableEntity, + expectedBody: false, + }, { name: "PUT OK", requestType: http.MethodPut, @@ -216,7 +280,27 @@ func TestRoutesHandlers(t *testing.T) { Description: "Post", NetworkId: "awesomeNet", Network: "192.168.0.0/16", - Peer: existingPeerID, + Peer: &existingPeerID, + NetworkType: route.IPv4NetworkString, + Masquerade: false, + Enabled: false, + Groups: []string{existingGroupID}, + }, + }, + { + name: "PUT OK when peer_groups provided", + requestType: http.MethodPut, + requestPath: "/api/routes/" + existingRouteID, + requestBody: bytes.NewBufferString(fmt.Sprintf("{\"Description\":\"Post\",\"Network\":\"192.168.0.0/16\",\"network_id\":\"awesomeNet\",\"peer_groups\":[\"%s\"],\"groups\":[\"%s\"]}", existingGroupID, existingGroupID)), + expectedStatus: http.StatusOK, + expectedBody: true, + expectedRoute: &api.Route{ + Id: existingRouteID, + Description: "Post", + NetworkId: "awesomeNet", + Network: "192.168.0.0/16", + Peer: &emptyString, + PeerGroups: &[]string{existingGroupID}, NetworkType: route.IPv4NetworkString, Masquerade: false, Enabled: false, @@ -239,6 +323,14 @@ func TestRoutesHandlers(t *testing.T) { expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, + { + name: "PUT Non Linux Peer", + requestType: http.MethodPut, + requestPath: "/api/routes/" + existingRouteID, + requestBody: bytes.NewBufferString(fmt.Sprintf("{\"Description\":\"Post\",\"Network\":\"192.168.0.0/16\",\"network_id\":\"awesomeNet\",\"Peer\":\"%s\",\"groups\":[\"%s\"]}", nonLinuxExistingPeerID, existingGroupID)), + expectedStatus: http.StatusUnprocessableEntity, + expectedBody: false, + }, { name: "PUT Invalid Network Identifier", requestType: http.MethodPut, @@ -255,6 +347,24 @@ func TestRoutesHandlers(t *testing.T) { expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, + { + name: "PUT UnprocessableEntity when both peer and peer_groups are provided", + requestType: http.MethodPut, + requestPath: "/api/routes/" + existingRouteID, + requestBody: bytes.NewBuffer( + []byte(fmt.Sprintf("{\"Description\":\"Post\",\"Network\":\"192.168.0.0/16\",\"network_id\":\"awesomeNet\",\"peer\":\"%s\",\"peer_groups\":[\"%s\"],\"groups\":[\"%s\"]}", existingPeerID, existingGroupID, existingGroupID))), + expectedStatus: http.StatusUnprocessableEntity, + expectedBody: false, + }, + { + name: "PUT UnprocessableEntity when no peer and peer_groups are provided", + requestType: http.MethodPut, + requestPath: "/api/routes/" + existingRouteID, + requestBody: bytes.NewBuffer( + []byte(fmt.Sprintf("{\"Description\":\"Post\",\"Network\":\"192.168.0.0/16\",\"network_id\":\"awesomeNet\",\"groups\":[\"%s\"]}", existingPeerID))), + expectedStatus: http.StatusUnprocessableEntity, + expectedBody: false, + }, } p := initRoutesTestData() diff --git a/management/server/mock_server/account_mock.go b/management/server/mock_server/account_mock.go index 24bf9f3c9ed..ed1130e09ea 100644 --- a/management/server/mock_server/account_mock.go +++ b/management/server/mock_server/account_mock.go @@ -33,8 +33,8 @@ type MockAccountManager struct { SaveGroupFunc func(accountID, userID string, group *server.Group) error DeleteGroupFunc func(accountID, userId, groupID string) error ListGroupsFunc func(accountID string) ([]*server.Group, error) - GroupAddPeerFunc func(accountID, groupID, peerKey string) error - GroupDeletePeerFunc func(accountID, groupID, peerKey string) error + GroupAddPeerFunc func(accountID, groupID, peerID string) error + GroupDeletePeerFunc func(accountID, groupID, peerID string) error GroupListPeersFunc func(accountID, groupID string) ([]*server.Peer, error) GetRuleFunc func(accountID, ruleID, userID string) (*server.Rule, error) SaveRuleFunc func(accountID, userID string, rule *server.Rule) error @@ -50,7 +50,7 @@ type MockAccountManager struct { UpdatePeerMetaFunc func(peerID string, meta server.PeerSystemMeta) error UpdatePeerSSHKeyFunc func(peerID string, sshKey string) error UpdatePeerFunc func(accountID, userID string, peer *server.Peer) (*server.Peer, error) - CreateRouteFunc func(accountID string, prefix, peer, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) + CreateRouteFunc func(accountID, prefix, peer string, peerGroups []string, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) GetRouteFunc func(accountID, routeID, userID string) (*route.Route, error) SaveRouteFunc func(accountID, userID string, route *route.Route) error DeleteRouteFunc func(accountID, routeID, userID string) error @@ -281,17 +281,17 @@ func (am *MockAccountManager) ListGroups(accountID string) ([]*server.Group, err } // GroupAddPeer mock implementation of GroupAddPeer from server.AccountManager interface -func (am *MockAccountManager) GroupAddPeer(accountID, groupID, peerKey string) error { +func (am *MockAccountManager) GroupAddPeer(accountID, groupID, peerID string) error { if am.GroupAddPeerFunc != nil { - return am.GroupAddPeerFunc(accountID, groupID, peerKey) + return am.GroupAddPeerFunc(accountID, groupID, peerID) } return status.Errorf(codes.Unimplemented, "method GroupAddPeer is not implemented") } // GroupDeletePeer mock implementation of GroupDeletePeer from server.AccountManager interface -func (am *MockAccountManager) GroupDeletePeer(accountID, groupID, peerKey string) error { +func (am *MockAccountManager) GroupDeletePeer(accountID, groupID, peerID string) error { if am.GroupDeletePeerFunc != nil { - return am.GroupDeletePeerFunc(accountID, groupID, peerKey) + return am.GroupDeletePeerFunc(accountID, groupID, peerID) } return status.Errorf(codes.Unimplemented, "method GroupDeletePeer is not implemented") } @@ -401,9 +401,9 @@ func (am *MockAccountManager) UpdatePeer(accountID, userID string, peer *server. } // CreateRoute mock implementation of CreateRoute from server.AccountManager interface -func (am *MockAccountManager) CreateRoute(accountID string, network, peerID, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) { +func (am *MockAccountManager) CreateRoute(accountID, network, peerID string, peerGroups []string, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) { if am.CreateRouteFunc != nil { - return am.CreateRouteFunc(accountID, network, peerID, description, netID, masquerade, metric, groups, enabled, userID) + return am.CreateRouteFunc(accountID, network, peerID, peerGroups, description, netID, masquerade, metric, groups, enabled, userID) } return nil, status.Errorf(codes.Unimplemented, "method CreateRoute is not implemented") } diff --git a/management/server/route.go b/management/server/route.go index b232c2bb6d0..cdd21420350 100644 --- a/management/server/route.go +++ b/management/server/route.go @@ -39,30 +39,82 @@ func (am *DefaultAccountManager) GetRoute(accountID, routeID, userID string) (*r return nil, status.Errorf(status.NotFound, "route with ID %s not found", routeID) } -// checkPrefixPeerExists checks the combination of prefix and peer id, if it exists returns an error, otherwise returns nil -func (am *DefaultAccountManager) checkPrefixPeerExists(accountID, peerID string, prefix netip.Prefix) error { +// checkRoutePrefixExistsForPeers checks if a route with a given prefix exists for a single peer or multiple peer groups. +func (am *DefaultAccountManager) checkRoutePrefixExistsForPeers(account *Account, peerID, routeID string, peerGroupIDs []string, prefix netip.Prefix) error { + // routes can have both peer and peer_groups + routesWithPrefix := account.GetRoutesByPrefix(prefix) + + // lets remember all the peers and the peer groups from routesWithPrefix + seenPeers := make(map[string]bool) + seenPeerGroups := make(map[string]bool) + + for _, prefixRoute := range routesWithPrefix { + // we skip route(s) with the same network ID as we want to allow updating of the existing route + // when create a new route routeID is newly generated so nothing will be skipped + if routeID == prefixRoute.ID { + continue + } - if peerID == "" { - return nil + if prefixRoute.Peer != "" { + seenPeers[prefixRoute.ID] = true + } + for _, groupID := range prefixRoute.PeerGroups { + seenPeerGroups[groupID] = true + + group := account.GetGroup(groupID) + if group == nil { + return status.Errorf( + status.InvalidArgument, "failed to add route with prefix %s - peer group %s doesn't exist", + prefix.String(), groupID) + } + + for _, pID := range group.Peers { + seenPeers[pID] = true + } + } } - account, err := am.Store.GetAccount(accountID) - if err != nil { - return err + if peerID != "" { + // check that peerID exists and is not in any route as single peer or part of the group + peer := account.GetPeer(peerID) + if peer == nil { + return status.Errorf(status.InvalidArgument, "peer with ID %s not found", peerID) + } + if _, ok := seenPeers[peerID]; ok { + return status.Errorf(status.AlreadyExists, + "failed to add route with prefix %s - peer %s already has this route", prefix.String(), peerID) + } } - routesWithPrefix := account.GetRoutesByPrefix(prefix) + // check that peerGroupIDs are not in any route peerGroups list + for _, groupID := range peerGroupIDs { + group := account.GetGroup(groupID) // we validated the group existent before entering this function, o need to check again. - for _, prefixRoute := range routesWithPrefix { - if prefixRoute.Peer == peerID { - return status.Errorf(status.AlreadyExists, "failed to add route with prefix %s - peer already has this route", prefix.String()) + if _, ok := seenPeerGroups[groupID]; ok { + return status.Errorf( + status.AlreadyExists, "failed to add route with prefix %s - peer group %s already has this route", + prefix.String(), group.Name) + } + + // check that the peers from peerGroupIDs groups are not the same peers we saw in routesWithPrefix + for _, id := range group.Peers { + if _, ok := seenPeers[id]; ok { + peer := account.GetPeer(peerID) + if peer == nil { + return status.Errorf(status.InvalidArgument, "peer with ID %s not found", peerID) + } + return status.Errorf(status.AlreadyExists, + "failed to add route with prefix %s - peer %s from the group %s already has this route", + prefix.String(), peer.Name, group.Name) + } } } + return nil } // CreateRoute creates and saves a new route -func (am *DefaultAccountManager) CreateRoute(accountID string, network, peerID, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) { +func (am *DefaultAccountManager) CreateRoute(accountID, network, peerID string, peerGroupIDs []string, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) { unlock := am.Store.AcquireAccountLock(accountID) defer unlock() @@ -71,19 +123,29 @@ func (am *DefaultAccountManager) CreateRoute(accountID string, network, peerID, return nil, err } - if peerID != "" { - peer := account.GetPeer(peerID) - if peer == nil { - return nil, status.Errorf(status.InvalidArgument, "peer with ID %s not found", peerID) - } + if peerID != "" && len(peerGroupIDs) != 0 { + return nil, status.Errorf( + status.InvalidArgument, + "peer with ID %s and peers group %s should not be provided at the same time", + peerID, peerGroupIDs) } var newRoute route.Route + newRoute.ID = xid.New().String() + prefixType, newPrefix, err := route.ParseNetwork(network) if err != nil { return nil, status.Errorf(status.InvalidArgument, "failed to parse IP %s", network) } - err = am.checkPrefixPeerExists(accountID, peerID, newPrefix) + + if len(peerGroupIDs) > 0 { + err = validateGroups(peerGroupIDs, account.Groups) + if err != nil { + return nil, err + } + } + + err = am.checkRoutePrefixExistsForPeers(account, peerID, newRoute.ID, peerGroupIDs, newPrefix) if err != nil { return nil, err } @@ -102,7 +164,7 @@ func (am *DefaultAccountManager) CreateRoute(accountID string, network, peerID, } newRoute.Peer = peerID - newRoute.ID = xid.New().String() + newRoute.PeerGroups = peerGroupIDs newRoute.Network = newPrefix newRoute.NetworkType = prefixType newRoute.Description = description @@ -160,13 +222,22 @@ func (am *DefaultAccountManager) SaveRoute(accountID, userID string, routeToSave return err } - if routeToSave.Peer != "" { - peer := account.GetPeer(routeToSave.Peer) - if peer == nil { - return status.Errorf(status.InvalidArgument, "peer with ID %s not found", routeToSave.Peer) + if routeToSave.Peer != "" && len(routeToSave.PeerGroups) != 0 { + return status.Errorf(status.InvalidArgument, "peer with ID and peer groups should not be provided at the same time") + } + + if len(routeToSave.PeerGroups) > 0 { + err = validateGroups(routeToSave.PeerGroups, account.Groups) + if err != nil { + return err } } + err = am.checkRoutePrefixExistsForPeers(account, routeToSave.Peer, routeToSave.ID, routeToSave.Copy().PeerGroups, routeToSave.Network) + if err != nil { + return err + } + err = validateGroups(routeToSave.Groups, account.Groups) if err != nil { return err diff --git a/management/server/route_test.go b/management/server/route_test.go index 81ce21a3fe8..32f15843bfd 100644 --- a/management/server/route_test.go +++ b/management/server/route_test.go @@ -14,24 +14,37 @@ import ( const ( peer1Key = "BhRPtynAAYRDy08+q4HTMsos8fs4plTP4NOSh7C1ry8=" peer2Key = "/yF0+vCfv+mRR5k0dca0TrGdO/oiNeAI58gToZm5NyI=" + peer3Key = "ayF0+vCfv+mRR5k0dca0TrGdO/oiNeAI58gToZm5NaF=" + peer4Key = "ayF0+vCfv+mRR5k0dca0TrGdO/oiNeAI58gToZm5acc=" + peer5Key = "ayF0+vCfv+mRR5k0dca0TrGdO/oiNeAI58gToZm5a55=" peer1ID = "peer-1-id" peer2ID = "peer-2-id" + peer3ID = "peer-3-id" + peer4ID = "peer-4-id" + peer5ID = "peer-5-id" routeGroup1 = "routeGroup1" routeGroup2 = "routeGroup2" + routeGroup3 = "routeGroup3" // for existing route + routeGroup4 = "routeGroup4" // for existing route + routeGroupHA1 = "routeGroupHA1" + routeGroupHA2 = "routeGroupHA2" routeInvalidGroup1 = "routeInvalidGroup1" userID = "testingUser" + existingNetwork = "10.10.10.0/24" + existingRouteID = "random-id" ) func TestCreateRoute(t *testing.T) { type input struct { - network string - netID string - peerKey string - description string - masquerade bool - metric int - enabled bool - groups []string + network string + netID string + peerKey string + peerGroupIDs []string + description string + masquerade bool + metric int + enabled bool + groups []string } testCases := []struct { @@ -67,6 +80,48 @@ func TestCreateRoute(t *testing.T) { Groups: []string{routeGroup1}, }, }, + { + name: "Happy Path Peer Groups", + inputArgs: input{ + network: "192.168.0.0/16", + netID: "happy", + peerGroupIDs: []string{routeGroupHA1, routeGroupHA2}, + description: "super", + masquerade: false, + metric: 9999, + enabled: true, + groups: []string{routeGroup1, routeGroup2}, + }, + errFunc: require.NoError, + shouldCreate: true, + expectedRoute: &route.Route{ + Network: netip.MustParsePrefix("192.168.0.0/16"), + NetworkType: route.IPv4Network, + NetID: "happy", + PeerGroups: []string{routeGroupHA1, routeGroupHA2}, + Description: "super", + Masquerade: false, + Metric: 9999, + Enabled: true, + Groups: []string{routeGroup1, routeGroup2}, + }, + }, + { + name: "Both peer and peer_groups Provided Should Fail", + inputArgs: input{ + network: "192.168.0.0/16", + netID: "happy", + peerKey: peer1ID, + peerGroupIDs: []string{routeGroupHA1}, + description: "super", + masquerade: false, + metric: 9999, + enabled: true, + groups: []string{routeGroup1}, + }, + errFunc: require.Error, + shouldCreate: false, + }, { name: "Bad Prefix Should Fail", inputArgs: input{ @@ -97,6 +152,36 @@ func TestCreateRoute(t *testing.T) { errFunc: require.Error, shouldCreate: false, }, + { + name: "Bad Peer already has this route", + inputArgs: input{ + network: existingNetwork, + netID: "bad", + peerKey: peer5ID, + description: "super", + masquerade: false, + metric: 9999, + enabled: true, + groups: []string{routeGroup1}, + }, + errFunc: require.Error, + shouldCreate: false, + }, + { + name: "Bad Peers Group already has this route", + inputArgs: input{ + network: existingNetwork, + netID: "bad", + peerGroupIDs: []string{routeGroup1, routeGroup3}, + description: "super", + masquerade: false, + metric: 9999, + enabled: true, + groups: []string{routeGroup1}, + }, + errFunc: require.Error, + shouldCreate: false, + }, { name: "Empty Peer Should Create", inputArgs: input{ @@ -238,13 +323,14 @@ func TestCreateRoute(t *testing.T) { account, err := initTestRouteAccount(t, am) if err != nil { - t.Error("failed to init testing account") + t.Errorf("failed to init testing account: %s", err) } outRoute, err := am.CreateRoute( account.Id, testCase.inputArgs.network, testCase.inputArgs.peerKey, + testCase.inputArgs.peerGroupIDs, testCase.inputArgs.description, testCase.inputArgs.netID, testCase.inputArgs.masquerade, @@ -272,6 +358,7 @@ func TestCreateRoute(t *testing.T) { func TestSaveRoute(t *testing.T) { validPeer := peer2ID + validUsedPeer := peer5ID invalidPeer := "nonExisting" validPrefix := netip.MustParsePrefix("192.168.0.0/24") invalidPrefix, _ := netip.ParsePrefix("192.168.0.0/34") @@ -279,11 +366,14 @@ func TestSaveRoute(t *testing.T) { invalidMetric := 99999 validNetID := "12345678901234567890qw" invalidNetID := "12345678901234567890qwertyuiopqwertyuiop1" + validGroupHA1 := routeGroupHA1 + validGroupHA2 := routeGroupHA2 testCases := []struct { name string existingRoute *route.Route newPeer *string + newPeerGroups []string newMetric *int newPrefix *netip.Prefix newGroups []string @@ -325,6 +415,55 @@ func TestSaveRoute(t *testing.T) { Groups: []string{routeGroup2}, }, }, + { + name: "Happy Path Peer Groups", + existingRoute: &route.Route{ + ID: "testingRoute", + Network: netip.MustParsePrefix("192.168.0.0/16"), + NetID: validNetID, + NetworkType: route.IPv4Network, + Description: "super", + Masquerade: false, + Metric: 9999, + Enabled: true, + Groups: []string{routeGroup1}, + }, + newPeerGroups: []string{validGroupHA1, validGroupHA2}, + newMetric: &validMetric, + newPrefix: &validPrefix, + newGroups: []string{routeGroup2}, + errFunc: require.NoError, + shouldCreate: true, + expectedRoute: &route.Route{ + ID: "testingRoute", + Network: validPrefix, + NetID: validNetID, + NetworkType: route.IPv4Network, + PeerGroups: []string{validGroupHA1, validGroupHA2}, + Description: "super", + Masquerade: false, + Metric: validMetric, + Enabled: true, + Groups: []string{routeGroup2}, + }, + }, + { + name: "Both peer and peers_roup Provided Should Fail", + existingRoute: &route.Route{ + ID: "testingRoute", + Network: netip.MustParsePrefix("192.168.0.0/16"), + NetID: validNetID, + NetworkType: route.IPv4Network, + Description: "super", + Masquerade: false, + Metric: 9999, + Enabled: true, + Groups: []string{routeGroup1}, + }, + newPeer: &validPeer, + newPeerGroups: []string{validGroupHA1}, + errFunc: require.Error, + }, { name: "Bad Prefix Should Fail", existingRoute: &route.Route{ @@ -461,6 +600,71 @@ func TestSaveRoute(t *testing.T) { newGroups: []string{routeInvalidGroup1}, errFunc: require.Error, }, + { + name: "Allow to modify existing route with new peer", + existingRoute: &route.Route{ + ID: "testingRoute", + Network: netip.MustParsePrefix(existingNetwork), + NetID: validNetID, + NetworkType: route.IPv4Network, + Peer: peer1ID, + Description: "super", + Masquerade: false, + Metric: 9999, + Enabled: true, + Groups: []string{routeGroup1}, + }, + newPeer: &validPeer, + errFunc: require.NoError, + shouldCreate: true, + expectedRoute: &route.Route{ + ID: "testingRoute", + Network: netip.MustParsePrefix(existingNetwork), + NetID: validNetID, + NetworkType: route.IPv4Network, + Peer: validPeer, + PeerGroups: []string{}, + Description: "super", + Masquerade: false, + Metric: 9999, + Enabled: true, + Groups: []string{routeGroup1}, + }, + }, + { + name: "Do not allow to modify existing route with a peer from another route", + existingRoute: &route.Route{ + ID: "testingRoute", + Network: netip.MustParsePrefix(existingNetwork), + NetID: validNetID, + NetworkType: route.IPv4Network, + Peer: peer1ID, + Description: "super", + Masquerade: false, + Metric: 9999, + Enabled: true, + Groups: []string{routeGroup1}, + }, + newPeer: &validUsedPeer, + errFunc: require.Error, + }, + { + name: "Do not allow to modify existing route with a peers group from another route", + existingRoute: &route.Route{ + ID: "testingRoute", + Network: netip.MustParsePrefix(existingNetwork), + NetID: validNetID, + NetworkType: route.IPv4Network, + PeerGroups: []string{routeGroup3}, + Description: "super", + Masquerade: false, + Metric: 9999, + Enabled: true, + Groups: []string{routeGroup1}, + }, + newPeerGroups: []string{routeGroup4}, + errFunc: require.Error, + }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { @@ -488,6 +692,9 @@ func TestSaveRoute(t *testing.T) { if testCase.newPeer != nil { routeToSave.Peer = *testCase.newPeer } + if len(testCase.newPeerGroups) != 0 { + routeToSave.PeerGroups = testCase.newPeerGroups + } if testCase.newMetric != nil { routeToSave.Metric = *testCase.newMetric } @@ -569,6 +776,96 @@ func TestDeleteRoute(t *testing.T) { } } +func TestGetNetworkMap_RouteSyncPeerGroups(t *testing.T) { + baseRoute := &route.Route{ + Network: netip.MustParsePrefix("192.168.0.0/16"), + NetID: "superNet", + NetworkType: route.IPv4Network, + PeerGroups: []string{routeGroupHA1, routeGroupHA2}, + Description: "ha route", + Masquerade: false, + Metric: 9999, + Enabled: true, + Groups: []string{routeGroup1, routeGroup2}, + } + + am, err := createRouterManager(t) + if err != nil { + t.Error("failed to create account manager") + } + + account, err := initTestRouteAccount(t, am) + if err != nil { + t.Error("failed to init testing account") + } + + newAccountRoutes, err := am.GetNetworkMap(peer1ID) + require.NoError(t, err) + require.Len(t, newAccountRoutes.Routes, 0, "new accounts should have no routes") + + newRoute, err := am.CreateRoute( + account.Id, baseRoute.Network.String(), baseRoute.Peer, baseRoute.PeerGroups, baseRoute.Description, + baseRoute.NetID, baseRoute.Masquerade, baseRoute.Metric, baseRoute.Groups, baseRoute.Enabled, userID) + require.NoError(t, err) + require.Equal(t, newRoute.Enabled, true) + + peer1Routes, err := am.GetNetworkMap(peer1ID) + require.NoError(t, err) + require.Len(t, peer1Routes.Routes, 3, "HA route should have more than 1 routes") + + peer2Routes, err := am.GetNetworkMap(peer2ID) + require.NoError(t, err) + require.Len(t, peer2Routes.Routes, 3, "HA route should have more than 1 routes") + + peer4Routes, err := am.GetNetworkMap(peer4ID) + require.NoError(t, err) + require.Len(t, peer4Routes.Routes, 3, "HA route should have more than 1 routes") + + groups, err := am.ListGroups(account.Id) + require.NoError(t, err) + var groupHA1, groupHA2 *Group + for _, group := range groups { + switch group.Name { + case routeGroupHA1: + groupHA1 = group + case routeGroupHA2: + groupHA2 = group + } + } + + err = am.GroupDeletePeer(account.Id, groupHA1.ID, peer2ID) + require.NoError(t, err) + + peer2RoutesAfterDelete, err := am.GetNetworkMap(peer2ID) + require.NoError(t, err) + require.Len(t, peer2RoutesAfterDelete.Routes, 2, "after peer deletion group should have only 2 route") + + err = am.GroupDeletePeer(account.Id, groupHA2.ID, peer4ID) + require.NoError(t, err) + + peer2RoutesAfterDelete, err = am.GetNetworkMap(peer2ID) + require.NoError(t, err) + require.Len(t, peer2RoutesAfterDelete.Routes, 1, "after peer deletion group should have only 1 route") + + err = am.GroupAddPeer(account.Id, groupHA2.ID, peer4ID) + require.NoError(t, err) + + peer1RoutesAfterAdd, err := am.GetNetworkMap(peer1ID) + require.NoError(t, err) + require.Len(t, peer1RoutesAfterAdd.Routes, 2, "HA route should have more than 1 route") + + peer2RoutesAfterAdd, err := am.GetNetworkMap(peer2ID) + require.NoError(t, err) + require.Len(t, peer2RoutesAfterAdd.Routes, 2, "HA route should have more than 1 route") + + err = am.DeleteRoute(account.Id, newRoute.ID, userID) + require.NoError(t, err) + + peer1DeletedRoute, err := am.GetNetworkMap(peer1ID) + require.NoError(t, err) + require.Len(t, peer1DeletedRoute.Routes, 0, "we should receive one route for peer1") +} + func TestGetNetworkMap_RouteSync(t *testing.T) { // no routes for peer in different groups // no routes when route is deleted @@ -599,7 +896,7 @@ func TestGetNetworkMap_RouteSync(t *testing.T) { require.NoError(t, err) require.Len(t, newAccountRoutes.Routes, 0, "new accounts should have no routes") - createdRoute, err := am.CreateRoute(account.Id, baseRoute.Network.String(), peer1ID, + createdRoute, err := am.CreateRoute(account.Id, baseRoute.Network.String(), peer1ID, []string{}, baseRoute.Description, baseRoute.NetID, baseRoute.Masquerade, baseRoute.Metric, baseRoute.Groups, false, userID) require.NoError(t, err) @@ -695,6 +992,8 @@ func createRouterStore(t *testing.T) (Store, error) { } func initTestRouteAccount(t *testing.T, am *DefaultAccountManager) (*Account, error) { + t.Helper() + accountID := "testingAcc" domain := "example.com" @@ -754,6 +1053,81 @@ func initTestRouteAccount(t *testing.T, am *DefaultAccountManager) (*Account, er } account.Peers[peer2.ID] = peer2 + ips = account.getTakenIPs() + peer3IP, err := AllocatePeerIP(account.Network.Net, ips) + if err != nil { + return nil, err + } + + peer3 := &Peer{ + IP: peer3IP, + ID: peer3ID, + Key: peer3Key, + Name: "test-host3@netbird.io", + UserID: userID, + Meta: PeerSystemMeta{ + Hostname: "test-host3@netbird.io", + GoOS: "darwin", + Kernel: "Darwin", + Core: "13.4.1", + Platform: "arm64", + OS: "darwin", + WtVersion: "development", + UIVersion: "development", + }, + } + account.Peers[peer3.ID] = peer3 + + ips = account.getTakenIPs() + peer4IP, err := AllocatePeerIP(account.Network.Net, ips) + if err != nil { + return nil, err + } + + peer4 := &Peer{ + IP: peer4IP, + ID: peer4ID, + Key: peer4Key, + Name: "test-host4@netbird.io", + UserID: userID, + Meta: PeerSystemMeta{ + Hostname: "test-host4@netbird.io", + GoOS: "linux", + Kernel: "Linux", + Core: "21.04", + Platform: "x86_64", + OS: "Ubuntu", + WtVersion: "development", + UIVersion: "development", + }, + } + account.Peers[peer4.ID] = peer4 + + ips = account.getTakenIPs() + peer5IP, err := AllocatePeerIP(account.Network.Net, ips) + if err != nil { + return nil, err + } + + peer5 := &Peer{ + IP: peer5IP, + ID: peer5ID, + Key: peer5Key, + Name: "test-host4@netbird.io", + UserID: userID, + Meta: PeerSystemMeta{ + Hostname: "test-host4@netbird.io", + GoOS: "linux", + Kernel: "Linux", + Core: "21.04", + Platform: "x86_64", + OS: "Ubuntu", + WtVersion: "development", + UIVersion: "development", + }, + } + account.Peers[peer5.ID] = peer5 + err = am.Store.SaveAccount(account) if err != nil { return nil, err @@ -770,24 +1144,57 @@ func initTestRouteAccount(t *testing.T, am *DefaultAccountManager) (*Account, er if err != nil { return nil, err } - - newGroup := &Group{ - ID: routeGroup1, - Name: routeGroup1, - Peers: []string{peer1.ID}, + err = am.GroupAddPeer(accountID, groupAll.ID, peer3ID) + if err != nil { + return nil, err } - err = am.SaveGroup(accountID, userID, newGroup) + err = am.GroupAddPeer(accountID, groupAll.ID, peer4ID) if err != nil { return nil, err } - newGroup = &Group{ - ID: routeGroup2, - Name: routeGroup2, - Peers: []string{peer2.ID}, + newGroup := []*Group{ + { + ID: routeGroup1, + Name: routeGroup1, + Peers: []string{peer1.ID}, + }, + { + ID: routeGroup2, + Name: routeGroup2, + Peers: []string{peer2.ID}, + }, + { + ID: routeGroup3, + Name: routeGroup3, + Peers: []string{peer5.ID}, + }, + { + ID: routeGroup4, + Name: routeGroup4, + Peers: []string{peer5.ID}, + }, + { + ID: routeGroupHA1, + Name: routeGroupHA1, + Peers: []string{peer1.ID, peer2.ID, peer3.ID}, // we have one non Linux peer, see peer3 + }, + { + ID: routeGroupHA2, + Name: routeGroupHA2, + Peers: []string{peer1.ID, peer4.ID}, + }, + } + + for _, group := range newGroup { + err = am.SaveGroup(accountID, userID, group) + if err != nil { + return nil, err + } } - err = am.SaveGroup(accountID, userID, newGroup) + _, err = am.CreateRoute(account.Id, existingNetwork, "", []string{routeGroup3, routeGroup4}, + "", existingRouteID, false, 1000, []string{groupAll.ID}, true, userID) if err != nil { return nil, err } diff --git a/route/route.go b/route/route.go index 5c45e2cf58a..eb7bcba2f32 100644 --- a/route/route.go +++ b/route/route.go @@ -70,6 +70,7 @@ type Route struct { NetID string Description string Peer string + PeerGroups []string NetworkType NetworkType Masquerade bool Metric int @@ -79,7 +80,7 @@ type Route struct { // EventMeta returns activity event meta related to the route func (r *Route) EventMeta() map[string]any { - return map[string]any{"name": r.NetID, "network_range": r.Network.String(), "peer_id": r.Peer} + return map[string]any{"name": r.NetID, "network_range": r.Network.String(), "peer_id": r.Peer, "peer_groups": r.PeerGroups} } // Copy copies a route object @@ -91,12 +92,14 @@ func (r *Route) Copy() *Route { Network: r.Network, NetworkType: r.NetworkType, Peer: r.Peer, + PeerGroups: make([]string, len(r.PeerGroups)), Metric: r.Metric, Masquerade: r.Masquerade, Enabled: r.Enabled, Groups: make([]string, len(r.Groups)), } copy(route.Groups, r.Groups) + copy(route.PeerGroups, r.PeerGroups) return route } @@ -111,7 +114,8 @@ func (r *Route) IsEqual(other *Route) bool { other.Metric == r.Metric && other.Masquerade == r.Masquerade && other.Enabled == r.Enabled && - compareGroupsList(r.Groups, other.Groups) + compareList(r.Groups, other.Groups) && + compareList(r.PeerGroups, other.PeerGroups) } // ParseNetwork Parses a network prefix string and returns a netip.Prefix object and if is invalid, IPv4 or IPv6 @@ -134,7 +138,7 @@ func ParseNetwork(networkString string) (NetworkType, netip.Prefix, error) { return IPv4Network, masked, nil } -func compareGroupsList(list, other []string) bool { +func compareList(list, other []string) bool { if len(list) != len(other) { return false } From 0c470e7838506e394677bd2a42e5a35a80ed8eb9 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Thu, 28 Sep 2023 21:53:28 +0200 Subject: [PATCH 04/14] Update delete method for user API (#1160) --- management/server/http/api/openapi.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index cfd14958f3c..658d389f668 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -1152,8 +1152,8 @@ paths: '500': "$ref": "#/components/responses/internal_error" delete: - summary: Block a User - description: This method blocks a user from accessing the system, but leaves the IDP user intact. + summary: Delete a User + description: This method removes a user from accessing the system. For this leaves the IDP user intact unless the `--user-delete-from-idp` is passed to management startup. tags: [ Users ] security: - BearerAuth: [ ] From 8c5c6815e0d29511d789a4a608922d4d2bc943f2 Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Thu, 28 Sep 2023 23:51:47 +0200 Subject: [PATCH 05/14] Reimplement isValidAccessToken without reflect (#1183) The use of reflection should generally be minimized in Go code because it can make the code less readable, less type-safe, and potentially slower. In this particular case we can simply rely on type switch. --- client/internal/auth/util.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/client/internal/auth/util.go b/client/internal/auth/util.go index 33a0e6e35ad..31c81d7019c 100644 --- a/client/internal/auth/util.go +++ b/client/internal/auth/util.go @@ -7,7 +7,6 @@ import ( "encoding/json" "fmt" "io" - "reflect" "strings" ) @@ -44,15 +43,14 @@ func isValidAccessToken(token string, audience string) error { } // Audience claim of JWT can be a string or an array of strings - typ := reflect.TypeOf(claims.Audience) - switch typ.Kind() { - case reflect.String: - if claims.Audience == audience { + switch aud := claims.Audience.(type) { + case string: + if aud == audience { return nil } - case reflect.Slice: - for _, aud := range claims.Audience.([]interface{}) { - if audience == aud { + case []interface{}: + for _, audItem := range aud { + if audStr, ok := audItem.(string); ok && audStr == audience { return nil } } From c81b83b3468034d2d757ee889cc693acc6ea3b7a Mon Sep 17 00:00:00 2001 From: Bethuel Mmbaga Date: Fri, 29 Sep 2023 00:58:04 +0300 Subject: [PATCH 06/14] Enhance compatibility of install.sh for systems without sudo (#1176) This commit modifies the install.sh script to improve compatibility with systems lacking the sudo command. A conditional check is added at the beginning of the script to see if the sudo command exists. If it does, operations in the script that previously required sudo would proceed as normal, using the sudo command. If the system does not have sudo, the shell would execute these operations without it. This change enhances the usability of this script in restricted environments where sudo is not installed or available to users. --- release_files/install.sh | 67 +++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/release_files/install.sh b/release_files/install.sh index 2ba0495fd5c..c553cc28a45 100755 --- a/release_files/install.sh +++ b/release_files/install.sh @@ -1,4 +1,3 @@ -#!/bin/sh # This code is based on the netbird-installer contribution by physk on GitHub. # Source: https://github.com/physk/netbird-installer set -e @@ -17,6 +16,12 @@ OS_TYPE="" ARCH="$(uname -m)" PACKAGE_MANAGER="bin" INSTALL_DIR="" +SUDO="" + + +if command -v sudo > /dev/null && [ "$(id -u)" -ne 0 ]; then + SUDO="sudo" +fi get_latest_release() { if [ -n "$GITHUB_TOKEN" ]; then @@ -65,18 +70,18 @@ download_release_binary() { unzip -q -o "$BINARY_NAME" mv "netbird_ui_${OS_TYPE}_${ARCH}" "$INSTALL_DIR" else - sudo mkdir -p "$INSTALL_DIR" + ${SUDO} mkdir -p "$INSTALL_DIR" tar -xzvf "$BINARY_NAME" - sudo mv "${1%_"${BINARY_BASE_NAME}"}" "$INSTALL_DIR/" + ${SUDO} mv "${1%_"${BINARY_BASE_NAME}"}" "$INSTALL_DIR/" fi } add_apt_repo() { - sudo apt-get update - sudo apt-get install ca-certificates curl gnupg -y + ${SUDO} apt-get update + ${SUDO} apt-get install ca-certificates curl gnupg -y # Remove old keys and repo source files - sudo rm -f \ + ${SUDO} rm -f \ /etc/apt/sources.list.d/netbird.list \ /etc/apt/sources.list.d/wiretrustee.list \ /etc/apt/trusted.gpg.d/wiretrustee.gpg \ @@ -84,16 +89,16 @@ add_apt_repo() { /usr/share/keyrings/wiretrustee-archive-keyring.gpg curl -sSL https://pkgs.netbird.io/debian/public.key \ - | sudo gpg --dearmor -o /usr/share/keyrings/netbird-archive-keyring.gpg + | ${SUDO} gpg --dearmor -o /usr/share/keyrings/netbird-archive-keyring.gpg echo 'deb [signed-by=/usr/share/keyrings/netbird-archive-keyring.gpg] https://pkgs.netbird.io/debian stable main' \ - | sudo tee /etc/apt/sources.list.d/netbird.list + | ${SUDO} tee /etc/apt/sources.list.d/netbird.list - sudo apt-get update + ${SUDO} apt-get update } add_rpm_repo() { -cat <<-EOF | sudo tee /etc/yum.repos.d/netbird.repo +cat <<-EOF | ${SUDO} tee /etc/yum.repos.d/netbird.repo [NetBird] name=NetBird baseurl=https://pkgs.netbird.io/yum/ @@ -112,7 +117,7 @@ add_aur_repo() { for PKG in $INSTALL_PKGS; do if ! pacman -Q "$PKG" > /dev/null 2>&1; then # Install missing package(s) - sudo pacman -S "$PKG" --noconfirm + ${SUDO} pacman -S "$PKG" --noconfirm # Add installed package for clean up later REMOVE_PKGS="$REMOVE_PKGS $PKG" @@ -129,7 +134,7 @@ add_aur_repo() { fi # Clean up the installed packages - sudo pacman -Rs "$REMOVE_PKGS" --noconfirm + ${SUDO} pacman -Rs "$REMOVE_PKGS" --noconfirm } install_native_binaries() { @@ -194,31 +199,31 @@ install_netbird() { case "$PACKAGE_MANAGER" in apt) add_apt_repo - sudo apt-get install netbird -y + ${SUDO} apt-get install netbird -y if ! $SKIP_UI_APP; then - sudo apt-get install netbird-ui -y + ${SUDO} apt-get install netbird-ui -y fi ;; yum) add_rpm_repo - sudo yum -y install netbird + ${SUDO} yum -y install netbird if ! $SKIP_UI_APP; then - sudo yum -y install netbird-ui + ${SUDO} yum -y install netbird-ui fi ;; dnf) add_rpm_repo - sudo dnf -y install dnf-plugin-config-manager - sudo dnf config-manager --add-repo /etc/yum.repos.d/netbird.repo - sudo dnf -y install netbird + ${SUDO} dnf -y install dnf-plugin-config-manager + ${SUDO} dnf config-manager --add-repo /etc/yum.repos.d/netbird.repo + ${SUDO} dnf -y install netbird if ! $SKIP_UI_APP; then - sudo dnf -y install netbird-ui + ${SUDO} dnf -y install netbird-ui fi ;; pacman) - sudo pacman -Syy + ${SUDO} pacman -Syy add_aur_repo ;; brew) @@ -251,7 +256,7 @@ install_netbird() { echo "Build and apply new configuration:" echo "" - echo "sudo nixos-rebuild switch" + echo "${SUDO} nixos-rebuild switch" exit 0 fi @@ -260,14 +265,14 @@ install_netbird() { esac # Add package manager to config - sudo mkdir -p "$CONFIG_FOLDER" - echo "package_manager=$PACKAGE_MANAGER" | sudo tee "$CONFIG_FILE" > /dev/null + ${SUDO} mkdir -p "$CONFIG_FOLDER" + echo "package_manager=$PACKAGE_MANAGER" | ${SUDO} tee "$CONFIG_FILE" > /dev/null # Load and start netbird service - if ! sudo netbird service install 2>&1; then + if ! ${SUDO} netbird service install 2>&1; then echo "NetBird service has already been loaded" fi - if ! sudo netbird service start 2>&1; then + if ! ${SUDO} netbird service start 2>&1; then echo "NetBird service has already been started" fi @@ -282,7 +287,7 @@ version_greater_equal() { } is_bin_package_manager() { - if sudo test -f "$1" && sudo grep -q "package_manager=bin" "$1" ; then + if ${SUDO} test -f "$1" && ${SUDO} grep -q "package_manager=bin" "$1" ; then return 0 else return 1 @@ -305,12 +310,12 @@ update_netbird() { echo "" echo "Initiating NetBird update. This will stop the netbird service and restart it after the update" - sudo netbird service stop - sudo netbird service uninstall + ${SUDO} netbird service stop + ${SUDO} netbird service uninstall install_native_binaries - sudo netbird service install - sudo netbird service start + ${SUDO} netbird service install + ${SUDO} netbird service start fi else echo "NetBird installation was done using a package manager. Please use your system's package manager to update" From 6ad3894a517ee20d1e996143072db5525125e8a7 Mon Sep 17 00:00:00 2001 From: Misha Bragin Date: Fri, 29 Sep 2023 17:37:04 +0200 Subject: [PATCH 07/14] Fix peer login expiration event duplication (#1185) --- management/server/user.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/management/server/user.go b/management/server/user.go index 23c6f228d34..9466a801b9a 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -836,6 +836,9 @@ func (am *DefaultAccountManager) GetUsersFromAccount(accountID, userID string) ( func (am *DefaultAccountManager) expireAndUpdatePeers(account *Account, peers []*Peer) error { var peerIDs []string for _, peer := range peers { + if peer.Status.LoginExpired { + continue + } peerIDs = append(peerIDs, peer.ID) peer.MarkLoginExpired(true) account.UpdatePeer(peer) From b23011fbe80342ab3a7ec8ab8610073aa0a9168e Mon Sep 17 00:00:00 2001 From: Misha Bragin Date: Sun, 1 Oct 2023 19:51:39 +0200 Subject: [PATCH 08/14] Delete user peers when deleting a user (#1186) --- management/server/account.go | 2 +- management/server/account_test.go | 4 +- management/server/ephemeral.go | 2 +- management/server/ephemeral_test.go | 4 +- management/server/http/peers_handler.go | 2 +- management/server/mock_server/account_mock.go | 6 +- management/server/peer.go | 98 ++++++++++++------- management/server/user.go | 10 +- 8 files changed, 79 insertions(+), 49 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index b4f93916666..d186beb66fd 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -66,7 +66,7 @@ type AccountManager interface { GetPeerByKey(peerKey string) (*Peer, error) GetPeers(accountID, userID string) ([]*Peer, error) MarkPeerConnected(peerKey string, connected bool) error - DeletePeer(accountID, peerID, userID string) (*Peer, error) + DeletePeer(accountID, peerID, userID string) error GetPeerByIP(accountId string, peerIP string) (*Peer, error) UpdatePeer(accountID, userID string, peer *Peer) (*Peer, error) GetNetworkMap(peerID string) (*NetworkMap, error) diff --git a/management/server/account_test.go b/management/server/account_test.go index 8c8a1b00192..5f711df804f 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -1062,7 +1062,7 @@ func TestAccountManager_NetworkUpdates(t *testing.T) { } }() - if _, err := manager.DeletePeer(account.Id, peer3.ID, userID); err != nil { + if err := manager.DeletePeer(account.Id, peer3.ID, userID); err != nil { t.Errorf("delete peer: %v", err) return } @@ -1129,7 +1129,7 @@ func TestAccountManager_DeletePeer(t *testing.T) { return } - _, err = manager.DeletePeer(account.Id, peerKey, userID) + err = manager.DeletePeer(account.Id, peerKey, userID) if err != nil { return } diff --git a/management/server/ephemeral.go b/management/server/ephemeral.go index a7b4239837c..0e76e58acc9 100644 --- a/management/server/ephemeral.go +++ b/management/server/ephemeral.go @@ -162,7 +162,7 @@ func (e *EphemeralManager) cleanup() { for id, p := range deletePeers { log.Debugf("delete ephemeral peer: %s", id) - _, err := e.accountManager.DeletePeer(p.account.Id, id, activity.SystemInitiator) + err := e.accountManager.DeletePeer(p.account.Id, id, activity.SystemInitiator) if err != nil { log.Tracef("failed to delete ephemeral peer: %s", err) } diff --git a/management/server/ephemeral_test.go b/management/server/ephemeral_test.go index a763f4cef6d..d271e5fcafe 100644 --- a/management/server/ephemeral_test.go +++ b/management/server/ephemeral_test.go @@ -29,9 +29,9 @@ type MocAccountManager struct { store *MockStore } -func (a MocAccountManager) DeletePeer(accountID, peerID, userID string) (*Peer, error) { +func (a MocAccountManager) DeletePeer(accountID, peerID, userID string) error { delete(a.store.account.Peers, peerID) - return nil, nil //nolint:nilnil + return nil //nolint:nil } func TestNewManager(t *testing.T) { diff --git a/management/server/http/peers_handler.go b/management/server/http/peers_handler.go index 100549aadf9..adf4a972102 100644 --- a/management/server/http/peers_handler.go +++ b/management/server/http/peers_handler.go @@ -61,7 +61,7 @@ func (h *PeersHandler) updatePeer(account *server.Account, user *server.User, pe } func (h *PeersHandler) deletePeer(accountID, userID string, peerID string, w http.ResponseWriter) { - _, err := h.accountManager.DeletePeer(accountID, peerID, userID) + err := h.accountManager.DeletePeer(accountID, peerID, userID) if err != nil { util.WriteError(err, w) return diff --git a/management/server/mock_server/account_mock.go b/management/server/mock_server/account_mock.go index ed1130e09ea..889bbb2211a 100644 --- a/management/server/mock_server/account_mock.go +++ b/management/server/mock_server/account_mock.go @@ -24,7 +24,7 @@ type MockAccountManager struct { GetPeerByKeyFunc func(peerKey string) (*server.Peer, error) GetPeersFunc func(accountID, userID string) ([]*server.Peer, error) MarkPeerConnectedFunc func(peerKey string, connected bool) error - DeletePeerFunc func(accountID, peerKey, userID string) (*server.Peer, error) + DeletePeerFunc func(accountID, peerKey, userID string) error GetPeerByIPFunc func(accountId string, peerIP string) (*server.Peer, error) GetNetworkMapFunc func(peerKey string) (*server.NetworkMap, error) GetPeerNetworkFunc func(peerKey string) (*server.Network, error) @@ -90,11 +90,11 @@ func (am *MockAccountManager) GetUsersFromAccount(accountID string, userID strin } // DeletePeer mock implementation of DeletePeer from server.AccountManager interface -func (am *MockAccountManager) DeletePeer(accountID, peerID, userID string) (*server.Peer, error) { +func (am *MockAccountManager) DeletePeer(accountID, peerID, userID string) error { if am.DeletePeerFunc != nil { return am.DeletePeerFunc(accountID, peerID, userID) } - return nil, status.Errorf(codes.Unimplemented, "method DeletePeer is not implemented") + return status.Errorf(codes.Unimplemented, "method DeletePeer is not implemented") } // GetOrCreateAccountByUser mock implementation of GetOrCreateAccountByUser from server.AccountManager interface diff --git a/management/server/peer.go b/management/server/peer.go index f9631719f2a..31ebd7cd5bf 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -372,55 +372,79 @@ func (am *DefaultAccountManager) UpdatePeer(accountID, userID string, update *Pe return peer, nil } -// DeletePeer removes peer from the account by its IP -func (am *DefaultAccountManager) DeletePeer(accountID, peerID, userID string) (*Peer, error) { - unlock := am.Store.AcquireAccountLock(accountID) - defer unlock() +// deletePeers will delete all specified peers and send updates to the remote peers. Don't call without acquiring account lock +func (am *DefaultAccountManager) deletePeers(account *Account, peerIDs []string, userID string) error { - account, err := am.Store.GetAccount(accountID) - if err != nil { - return nil, err - } + var peerError error + for _, peerID := range peerIDs { - peer := account.GetPeer(peerID) - if peer == nil { - return nil, status.Errorf(status.NotFound, "peer %s not found", peerID) - } + peer := account.GetPeer(peerID) + if peer == nil { + peerError = status.Errorf(status.NotFound, "peer %s not found", peerID) + goto save + } - account.DeletePeer(peerID) + account.DeletePeer(peerID) + + peerError = am.peersUpdateManager.SendUpdate(peer.ID, + &UpdateMessage{ + Update: &proto.SyncResponse{ + // fill those field for backward compatibility + RemotePeers: []*proto.RemotePeerConfig{}, + RemotePeersIsEmpty: true, + // new field + NetworkMap: &proto.NetworkMap{ + Serial: account.Network.CurrentSerial(), + RemotePeers: []*proto.RemotePeerConfig{}, + RemotePeersIsEmpty: true, + FirewallRules: []*proto.FirewallRule{}, + FirewallRulesIsEmpty: true, + }, + }, + }) + if peerError != nil { + goto save + } - err = am.Store.SaveAccount(account) + am.peersUpdateManager.CloseChannel(peerID) + am.storeEvent(userID, peer.ID, account.Id, activity.PeerRemovedByUser, peer.EventMeta(am.GetDNSDomain())) + } + +save: + err := am.Store.SaveAccount(account) if err != nil { - return nil, err + if peerError != nil { + log.Errorf("account save error: %s", err) + return peerError + } else { + return err + } } - err = am.peersUpdateManager.SendUpdate(peer.ID, - &UpdateMessage{ - Update: &proto.SyncResponse{ - // fill those field for backward compatibility - RemotePeers: []*proto.RemotePeerConfig{}, - RemotePeersIsEmpty: true, - // new field - NetworkMap: &proto.NetworkMap{ - Serial: account.Network.CurrentSerial(), - RemotePeers: []*proto.RemotePeerConfig{}, - RemotePeersIsEmpty: true, - FirewallRules: []*proto.FirewallRule{}, - FirewallRulesIsEmpty: true, - }, - }, - }) + err = am.updateAccountPeers(account) if err != nil { - return nil, err + if peerError != nil { + log.Errorf("update account peers error: %s", err) + return peerError + } else { + return err + } } - if err := am.updateAccountPeers(account); err != nil { - return nil, err + return peerError +} + +// DeletePeer removes peer from the account by its IP +func (am *DefaultAccountManager) DeletePeer(accountID, peerID, userID string) error { + unlock := am.Store.AcquireAccountLock(accountID) + defer unlock() + + account, err := am.Store.GetAccount(accountID) + if err != nil { + return err } - am.peersUpdateManager.CloseChannel(peerID) - am.storeEvent(userID, peer.ID, account.Id, activity.PeerRemovedByUser, peer.EventMeta(am.GetDNSDomain())) - return peer, nil + return am.deletePeers(account, []string{peerID}, userID) } // GetPeerByIP returns peer by its IP diff --git a/management/server/user.go b/management/server/user.go index 9466a801b9a..a20a8b2b0b6 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -338,8 +338,13 @@ func (am *DefaultAccountManager) DeleteUser(accountID, initiatorUserID string, t return status.Errorf(status.Internal, "failed to find user peers") } - if err := am.expireAndUpdatePeers(account, peers); err != nil { - log.Errorf("failed update deleted peers expiration: %s", err) + peerIDs := make([]string, 0, len(peers)) + for _, peer := range peers { + peerIDs = append(peerIDs, peer.ID) + } + + err = am.deletePeers(account, peerIDs, initiatorUserID) + if err != nil { return err } @@ -370,6 +375,7 @@ func (am *DefaultAccountManager) DeleteUser(accountID, initiatorUserID string, t delete(account.Users, targetUserID) + // todo should be unnecessary because we save account in the am.deletePeers err = am.Store.SaveAccount(account) if err != nil { return err From 22f69d7852174ffbd091db98f6a8087a92929b28 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Mon, 2 Oct 2023 12:10:12 +0200 Subject: [PATCH 09/14] Add routing groups metrics (#1187) add routing groups metrics and tests for the property generation --- management/server/metrics/selfhosted.go | 7 + management/server/metrics/selfhosted_test.go | 239 +++++++++++++++++++ 2 files changed, 246 insertions(+) create mode 100644 management/server/metrics/selfhosted_test.go diff --git a/management/server/metrics/selfhosted.go b/management/server/metrics/selfhosted.go index 696df5f3cc1..3b3db0baa87 100644 --- a/management/server/metrics/selfhosted.go +++ b/management/server/metrics/selfhosted.go @@ -176,6 +176,7 @@ func (w *Worker) generateProperties() properties { rulesDirection map[string]int groups int routes int + routesWithRGGroups int nameservers int uiClient int version string @@ -201,6 +202,11 @@ func (w *Worker) generateProperties() properties { groups = groups + len(account.Groups) routes = routes + len(account.Routes) + for _, route := range account.Routes { + if len(route.PeerGroups) > 0 { + routesWithRGGroups++ + } + } nameservers = nameservers + len(account.NameServerGroups) for _, policy := range account.Policies { @@ -282,6 +288,7 @@ func (w *Worker) generateProperties() properties { metricsProperties["rules"] = rules metricsProperties["groups"] = groups metricsProperties["routes"] = routes + metricsProperties["routes_with_routing_groups"] = routesWithRGGroups metricsProperties["nameservers"] = nameservers metricsProperties["version"] = version metricsProperties["min_active_peer_version"] = minActivePeerVersion diff --git a/management/server/metrics/selfhosted_test.go b/management/server/metrics/selfhosted_test.go new file mode 100644 index 00000000000..c61613fd26f --- /dev/null +++ b/management/server/metrics/selfhosted_test.go @@ -0,0 +1,239 @@ +package metrics + +import ( + "testing" + + nbdns "github.com/netbirdio/netbird/dns" + "github.com/netbirdio/netbird/management/server" + "github.com/netbirdio/netbird/route" +) + +type mockDatasource struct{} + +// GetAllConnectedPeers returns a map of connected peer IDs for use in tests with predefined information +func (mockDatasource) GetAllConnectedPeers() map[string]struct{} { + return map[string]struct{}{ + "1": {}, + } +} + +// GetAllAccounts returns a list of *server.Account for use in tests with predefined information +func (mockDatasource) GetAllAccounts() []*server.Account { + return []*server.Account{ + { + Id: "1", + Settings: &server.Settings{PeerLoginExpirationEnabled: true}, + SetupKeys: map[string]*server.SetupKey{ + "1": { + Id: "1", + Ephemeral: true, + UsedTimes: 1, + }, + }, + Groups: map[string]*server.Group{ + "1": {}, + "2": {}, + }, + NameServerGroups: map[string]*nbdns.NameServerGroup{ + "1": {}, + }, + Peers: map[string]*server.Peer{ + "1": { + ID: "1", + UserID: "test", + SSHEnabled: true, + Meta: server.PeerSystemMeta{GoOS: "linux", WtVersion: "0.0.1"}, + }, + }, + Policies: []*server.Policy{ + { + Rules: []*server.PolicyRule{ + { + Bidirectional: true, + Protocol: server.PolicyRuleProtocolTCP, + }, + }, + }, + { + Rules: []*server.PolicyRule{ + { + Bidirectional: false, + Protocol: server.PolicyRuleProtocolTCP, + }, + }, + }, + }, + Routes: map[string]*route.Route{ + "1": { + ID: "1", + PeerGroups: make([]string, 1), + }, + }, + Users: map[string]*server.User{ + "1": { + IsServiceUser: true, + PATs: map[string]*server.PersonalAccessToken{ + "1": {}, + }, + }, + "2": { + IsServiceUser: false, + PATs: map[string]*server.PersonalAccessToken{ + "1": {}, + }, + }, + }, + }, + { + Id: "2", + Settings: &server.Settings{PeerLoginExpirationEnabled: true}, + SetupKeys: map[string]*server.SetupKey{ + "1": { + Id: "1", + Ephemeral: true, + UsedTimes: 1, + }, + }, + Groups: map[string]*server.Group{ + "1": {}, + "2": {}, + }, + NameServerGroups: map[string]*nbdns.NameServerGroup{ + "1": {}, + }, + Peers: map[string]*server.Peer{ + "1": { + ID: "1", + UserID: "test", + SSHEnabled: true, + Meta: server.PeerSystemMeta{GoOS: "linux", WtVersion: "0.0.1"}, + }, + }, + Policies: []*server.Policy{ + { + Rules: []*server.PolicyRule{ + { + Bidirectional: true, + Protocol: server.PolicyRuleProtocolTCP, + }, + }, + }, + { + Rules: []*server.PolicyRule{ + { + Bidirectional: false, + Protocol: server.PolicyRuleProtocolTCP, + }, + }, + }, + }, + Routes: map[string]*route.Route{ + "1": { + ID: "1", + PeerGroups: make([]string, 1), + }, + }, + Users: map[string]*server.User{ + "1": { + IsServiceUser: true, + PATs: map[string]*server.PersonalAccessToken{ + "1": {}, + }, + }, + "2": { + IsServiceUser: false, + PATs: map[string]*server.PersonalAccessToken{ + "1": {}, + }, + }, + }, + }, + } +} + +// TestGenerateProperties tests and validate the properties generation by using the mockDatasource for the Worker.generateProperties +func TestGenerateProperties(t *testing.T) { + ds := mockDatasource{} + worker := Worker{ + dataSource: ds, + connManager: ds, + } + + properties := worker.generateProperties() + + if properties["accounts"] != 2 { + t.Errorf("expected 2 accounts, got %d", properties["accounts"]) + } + if properties["peers"] != 2 { + t.Errorf("expected 2 peers, got %d", properties["peers"]) + } + if properties["routes"] != 2 { + t.Errorf("expected 2 routes, got %d", properties["routes"]) + } + if properties["rules"] != 4 { + t.Errorf("expected 4 rules, got %d", properties["rules"]) + } + if properties["users"] != 2 { + t.Errorf("expected 1 users, got %d", properties["users"]) + } + if properties["setup_keys_usage"] != 2 { + t.Errorf("expected 1 setup_keys_usage, got %d", properties["setup_keys_usage"]) + } + if properties["pats"] != 4 { + t.Errorf("expected 4 personal_access_tokens, got %d", properties["pats"]) + } + if properties["peers_ssh_enabled"] != 2 { + t.Errorf("expected 2 peers_ssh_enabled, got %d", properties["peers_ssh_enabled"]) + } + if properties["routes_with_routing_groups"] != 2 { + t.Errorf("expected 2 routes_with_routing_groups, got %d", properties["routes_with_routing_groups"]) + } + if properties["rules_protocol_tcp"] != 4 { + t.Errorf("expected 4 rules_protocol_tcp, got %d", properties["rules_protocol_tcp"]) + } + if properties["rules_direction_oneway"] != 2 { + t.Errorf("expected 2 rules_direction_oneway, got %d", properties["rules_direction_oneway"]) + } + + if properties["active_peers_last_day"] != 2 { + t.Errorf("expected 2 active_peers_last_day, got %d", properties["active_peers_last_day"]) + } + if properties["min_active_peer_version"] != "0.0.1" { + t.Errorf("expected 0.0.1 min_active_peer_version, got %s", properties["min_active_peer_version"]) + } + if properties["max_active_peer_version"] != "0.0.1" { + t.Errorf("expected 0.0.1 max_active_peer_version, got %s", properties["max_active_peer_version"]) + } + + if properties["peers_login_expiration_enabled"] != 2 { + t.Errorf("expected 2 peers_login_expiration_enabled, got %d", properties["peers_login_expiration_enabled"]) + } + + if properties["service_users"] != 2 { + t.Errorf("expected 2 service_users, got %d", properties["service_users"]) + } + + if properties["peer_os_linux"] != 2 { + t.Errorf("expected 2 peer_os_linux, got %d", properties["peer_os_linux"]) + } + + if properties["ephemeral_peers_setup_keys"] != 2 { + t.Errorf("expected 2 ephemeral_peers_setup_keys, got %d", properties["ephemeral_peers_setup_keys_usage"]) + } + + if properties["ephemeral_peers_setup_keys_usage"] != 2 { + t.Errorf("expected 2 ephemeral_peers_setup_keys_usage, got %d", properties["ephemeral_peers_setup_keys_usage"]) + } + + if properties["nameservers"] != 2 { + t.Errorf("expected 2 nameservers, got %d", properties["nameservers"]) + } + + if properties["groups"] != 4 { + t.Errorf("expected 4 groups, got %d", properties["groups"]) + } + + if properties["user_peers"] != 2 { + t.Errorf("expected 2 user_peers, got %d", properties["user_peers"]) + } +} From a952e7c72fda7c5e63b8762ad9dd2858c0df9176 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Mon, 2 Oct 2023 19:18:08 +0200 Subject: [PATCH 10/14] Prevent return extra userData (#1190) If there is a difference between local and cached data, we trigger a cache refresh; as we remove users from the local store and potentially from the remote IDP, we need to switch the source of truth to the local store to prevent unwanted endless cache for cases where the removal from the IDP fails or for cases where the userDeleteFromIDPEnabled got enabled after the first user deletion. --- management/server/account.go | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index d186beb66fd..c45821b1bce 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -637,8 +637,8 @@ func (a *Account) Copy() *Account { } routes := map[string]*route.Route{} - for id, route := range a.Routes { - routes[id] = route.Copy() + for id, r := range a.Routes { + routes[id] = r.Copy() } nsGroups := map[string]*nbdns.NameServerGroup{} @@ -1054,7 +1054,36 @@ func (am *DefaultAccountManager) addAccountIDToIDPAppMeta(userID string, account func (am *DefaultAccountManager) loadAccount(_ context.Context, accountID interface{}) ([]*idp.UserData, error) { log.Debugf("account %s not found in cache, reloading", accountID) - return am.idpManager.GetAccount(fmt.Sprintf("%v", accountID)) + accountIDString := fmt.Sprintf("%v", accountID) + + account, err := am.Store.GetAccount(accountIDString) + if err != nil { + return nil, err + } + + userData, err := am.idpManager.GetAccount(accountIDString) + if err != nil { + return nil, err + } + + dataMap := make(map[string]*idp.UserData, len(userData)) + for _, datum := range userData { + dataMap[datum.ID] = datum + } + + matchedUserData := make([]*idp.UserData, 0) + for _, user := range account.Users { + if user.IsServiceUser { + continue + } + datum, ok := dataMap[user.Id] + if !ok { + log.Warnf("user %s not found in IDP", user.Id) + continue + } + matchedUserData = append(matchedUserData, datum) + } + return matchedUserData, nil } func (am *DefaultAccountManager) lookupUserInCacheByEmail(email string, accountID string) (*idp.UserData, error) { From e26ec0b937fe11a905280a7fc250b5bea47f6bf4 Mon Sep 17 00:00:00 2001 From: Bethuel Mmbaga Date: Tue, 3 Oct 2023 17:40:28 +0300 Subject: [PATCH 11/14] Optimize Cache and IDP Management (#1147) This pull request modifies the IdP and cache manager(s) to prevent the sending of app metadata to the upstream IDP on self-hosted instances. As a result, the IdP will now load all users from the IdP without filtering based on accountID. We disable user invites as the administrator's own IDP system manages them. --- management/server/account.go | 21 ++ management/server/idp/authentik.go | 178 ++---------- management/server/idp/azure.go | 336 ++-------------------- management/server/idp/azure_test.go | 168 +---------- management/server/idp/google_workspace.go | 174 ++--------- management/server/idp/idp.go | 5 + management/server/idp/keycloak.go | 266 +++-------------- management/server/idp/keycloak_test.go | 114 -------- management/server/idp/okta.go | 172 ++--------- management/server/idp/okta_test.go | 44 +-- management/server/idp/zitadel.go | 248 ++-------------- management/server/idp/zitadel_test.go | 126 +------- 12 files changed, 182 insertions(+), 1670 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index c45821b1bce..b8bfd5770e7 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -988,6 +988,27 @@ func (am *DefaultAccountManager) warmupIDPCache() error { return err } + // If the Identity Provider does not support writing AppMetadata, + // in cases like this, we expect it to return all users in an "unset" field. + // We iterate over the users in the "unset" field, look up their AccountID in our store, and + // update their AppMetadata with the AccountID. + if unsetData, ok := userData[idp.UnsetAccountID]; ok { + for _, user := range unsetData { + accountID, err := am.Store.GetAccountByUser(user.ID) + if err == nil { + data := userData[accountID.Id] + if data == nil { + data = make([]*idp.UserData, 0, 1) + } + + user.AppMetadata.WTAccountID = accountID.Id + + userData[accountID.Id] = append(data, user) + } + } + } + delete(userData, idp.UnsetAccountID) + for accountID, users := range userData { err = am.cacheManager.Set(am.ctx, accountID, users, cacheStore.WithExpiration(cacheEntryExpiration())) if err != nil { diff --git a/management/server/idp/authentik.go b/management/server/idp/authentik.go index 102222d0d06..ca995b2996e 100644 --- a/management/server/idp/authentik.go +++ b/management/server/idp/authentik.go @@ -210,47 +210,7 @@ func (ac *AuthentikCredentials) Authenticate() (JWTToken, error) { } // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. -func (am *AuthentikManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - ctx, err := am.authenticationContext() - if err != nil { - return err - } - - userPk, err := strconv.ParseInt(userID, 10, 32) - if err != nil { - return err - } - - var pendingInvite bool - if appMetadata.WTPendingInvite != nil { - pendingInvite = *appMetadata.WTPendingInvite - } - - patchedUserReq := api.PatchedUserRequest{ - Attributes: map[string]interface{}{ - wtAccountID: appMetadata.WTAccountID, - wtPendingInvite: pendingInvite, - }, - } - _, resp, err := am.apiClient.CoreApi.CoreUsersPartialUpdate(ctx, int32(userPk)). - PatchedUserRequest(patchedUserReq). - Execute() - if err != nil { - return err - } - defer resp.Body.Close() - - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - - if resp.StatusCode != http.StatusOK { - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountRequestStatusError() - } - return fmt.Errorf("unable to update user %s, statusCode %d", userID, resp.StatusCode) - } - +func (am *AuthentikManager) UpdateUserAppMetadata(_ string, _ AppMetadata) error { return nil } @@ -283,7 +243,10 @@ func (am *AuthentikManager) GetUserDataByID(userID string, appMetadata AppMetada return nil, fmt.Errorf("unable to get user %s, statusCode %d", userID, resp.StatusCode) } - return parseAuthentikUser(*user) + userData := parseAuthentikUser(*user) + userData.AppMetadata = appMetadata + + return userData, nil } // GetAccount returns all the users for a given profile. @@ -293,8 +256,7 @@ func (am *AuthentikManager) GetAccount(accountID string) ([]*UserData, error) { return nil, err } - accountFilter := fmt.Sprintf("{%q:%q}", wtAccountID, accountID) - userList, resp, err := am.apiClient.CoreApi.CoreUsersList(ctx).Attributes(accountFilter).Execute() + userList, resp, err := am.apiClient.CoreApi.CoreUsersList(ctx).Execute() if err != nil { return nil, err } @@ -313,10 +275,9 @@ func (am *AuthentikManager) GetAccount(accountID string) ([]*UserData, error) { users := make([]*UserData, 0) for _, user := range userList.Results { - userData, err := parseAuthentikUser(user) - if err != nil { - return nil, err - } + userData := parseAuthentikUser(user) + userData.AppMetadata.WTAccountID = accountID + users = append(users, userData) } @@ -350,65 +311,16 @@ func (am *AuthentikManager) GetAllAccounts() (map[string][]*UserData, error) { indexedUsers := make(map[string][]*UserData) for _, user := range userList.Results { - userData, err := parseAuthentikUser(user) - if err != nil { - return nil, err - } - - accountID := userData.AppMetadata.WTAccountID - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } + userData := parseAuthentikUser(user) + indexedUsers[UnsetAccountID] = append(indexedUsers[UnsetAccountID], userData) } return indexedUsers, nil } // CreateUser creates a new user in authentik Idp and sends an invitation. -func (am *AuthentikManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { - ctx, err := am.authenticationContext() - if err != nil { - return nil, err - } - - groupID, err := am.getUserGroupByName("netbird") - if err != nil { - return nil, err - } - - defaultBoolValue := true - createUserRequest := api.UserRequest{ - Email: &email, - Name: name, - IsActive: &defaultBoolValue, - Groups: []string{groupID}, - Username: email, - Attributes: map[string]interface{}{ - wtAccountID: accountID, - wtPendingInvite: &defaultBoolValue, - }, - } - user, resp, err := am.apiClient.CoreApi.CoreUsersCreate(ctx).UserRequest(createUserRequest).Execute() - if err != nil { - return nil, err - } - defer resp.Body.Close() - - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountCreateUser() - } - - if resp.StatusCode != http.StatusCreated { - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountRequestStatusError() - } - return nil, fmt.Errorf("unable to create user, statusCode %d", resp.StatusCode) - } - - return parseAuthentikUser(*user) +func (am *AuthentikManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserByEmail searches users with a given email. @@ -438,11 +350,7 @@ func (am *AuthentikManager) GetUserByEmail(email string) ([]*UserData, error) { users := make([]*UserData, 0) for _, user := range userList.Results { - userData, err := parseAuthentikUser(user) - if err != nil { - return nil, err - } - users = append(users, userData) + users = append(users, parseAuthentikUser(user)) } return users, nil @@ -501,64 +409,10 @@ func (am *AuthentikManager) authenticationContext() (context.Context, error) { return context.WithValue(context.Background(), api.ContextAPIKeys, value), nil } -// getUserGroupByName retrieves the user group for assigning new users. -// If the group is not found, a new group with the specified name will be created. -func (am *AuthentikManager) getUserGroupByName(name string) (string, error) { - ctx, err := am.authenticationContext() - if err != nil { - return "", err - } - - groupList, resp, err := am.apiClient.CoreApi.CoreGroupsList(ctx).Name(name).Execute() - if err != nil { - return "", err - } - defer resp.Body.Close() - - if groupList != nil { - if len(groupList.Results) > 0 { - return groupList.Results[0].Pk, nil - } - } - - createGroupRequest := api.GroupRequest{Name: name} - group, resp, err := am.apiClient.CoreApi.CoreGroupsCreate(ctx).GroupRequest(createGroupRequest).Execute() - if err != nil { - return "", err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusCreated { - return "", fmt.Errorf("unable to create user group, statusCode: %d", resp.StatusCode) - } - - return group.Pk, nil -} - -func parseAuthentikUser(user api.User) (*UserData, error) { - var attributes struct { - AccountID string `json:"wt_account_id"` - PendingInvite bool `json:"wt_pending_invite"` - } - - helper := JsonParser{} - buf, err := helper.Marshal(user.Attributes) - if err != nil { - return nil, err - } - - err = helper.Unmarshal(buf, &attributes) - if err != nil { - return nil, err - } - +func parseAuthentikUser(user api.User) *UserData { return &UserData{ Email: *user.Email, Name: user.Name, ID: strconv.FormatInt(int64(user.Pk), 10), - AppMetadata: AppMetadata{ - WTAccountID: attributes.AccountID, - WTPendingInvite: &attributes.PendingInvite, - }, - }, nil + } } diff --git a/management/server/idp/azure.go b/management/server/idp/azure.go index 22e6825ae82..e4224c26d96 100644 --- a/management/server/idp/azure.go +++ b/management/server/idp/azure.go @@ -1,7 +1,6 @@ package idp import ( - "encoding/json" "fmt" "io" "net/http" @@ -11,19 +10,13 @@ import ( "time" "github.com/golang-jwt/jwt" - "github.com/netbirdio/netbird/management/server/telemetry" log "github.com/sirupsen/logrus" -) - -const ( - // azure extension properties template - wtAccountIDTpl = "extension_%s_wt_account_id" - wtPendingInviteTpl = "extension_%s_wt_pending_invite" - profileFields = "id,displayName,mail,userPrincipalName" - extensionFields = "id,name,targetObjects" + "github.com/netbirdio/netbird/management/server/telemetry" ) +const profileFields = "id,displayName,mail,userPrincipalName" + // AzureManager azure manager client instance. type AzureManager struct { ClientID string @@ -58,21 +51,6 @@ type AzureCredentials struct { // azureProfile represents an azure user profile. type azureProfile map[string]any -// passwordProfile represent authentication method for, -// newly created user profile. -type passwordProfile struct { - ForceChangePasswordNextSignIn bool `json:"forceChangePasswordNextSignIn"` - Password string `json:"password"` -} - -// azureExtension represent custom attribute, -// that can be added to user objects in Azure Active Directory (AD). -type azureExtension struct { - Name string `json:"name"` - DataType string `json:"dataType"` - TargetObjects []string `json:"targetObjects"` -} - // NewAzureManager creates a new instance of the AzureManager. func NewAzureManager(config AzureClientConfig, appMetrics telemetry.AppMetrics) (*AzureManager, error) { httpTransport := http.DefaultTransport.(*http.Transport).Clone() @@ -115,7 +93,7 @@ func NewAzureManager(config AzureClientConfig, appMetrics telemetry.AppMetrics) appMetrics: appMetrics, } - manager := &AzureManager{ + return &AzureManager{ ObjectID: config.ObjectID, ClientID: config.ClientID, GraphAPIEndpoint: config.GraphAPIEndpoint, @@ -123,14 +101,7 @@ func NewAzureManager(config AzureClientConfig, appMetrics telemetry.AppMetrics) credentials: credentials, helper: helper, appMetrics: appMetrics, - } - - err := manager.configureAppMetadata() - if err != nil { - return nil, err - } - - return manager, nil + }, nil } // jwtStillValid returns true if the token still valid and have enough time to be used and get a response from azure. @@ -236,44 +207,14 @@ func (ac *AzureCredentials) Authenticate() (JWTToken, error) { } // CreateUser creates a new user in azure AD Idp. -func (am *AzureManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { - payload, err := buildAzureCreateUserRequestPayload(email, name, accountID, am.ClientID) - if err != nil { - return nil, err - } - - body, err := am.post("users", payload) - if err != nil { - return nil, err - } - - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountCreateUser() - } - - var profile azureProfile - err = am.helper.Unmarshal(body, &profile) - if err != nil { - return nil, err - } - - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - profile[wtAccountIDField] = accountID - - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - profile[wtPendingInviteField] = true - - return profile.userData(am.ClientID), nil +func (am *AzureManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserDataByID requests user data from keycloak via ID. func (am *AzureManager) GetUserDataByID(userID string, appMetadata AppMetadata) (*UserData, error) { - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - selectFields := strings.Join([]string{profileFields, wtAccountIDField, wtPendingInviteField}, ",") - q := url.Values{} - q.Add("$select", selectFields) + q.Add("$select", profileFields) body, err := am.get("users/"+userID, q) if err != nil { @@ -290,18 +231,17 @@ func (am *AzureManager) GetUserDataByID(userID string, appMetadata AppMetadata) return nil, err } - return profile.userData(am.ClientID), nil + userData := profile.userData() + userData.AppMetadata = appMetadata + + return userData, nil } // GetUserByEmail searches users with a given email. // If no users have been found, this function returns an empty list. func (am *AzureManager) GetUserByEmail(email string) ([]*UserData, error) { - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - selectFields := strings.Join([]string{profileFields, wtAccountIDField, wtPendingInviteField}, ",") - q := url.Values{} - q.Add("$select", selectFields) + q.Add("$select", profileFields) body, err := am.get("users/"+email, q) if err != nil { @@ -319,20 +259,15 @@ func (am *AzureManager) GetUserByEmail(email string) ([]*UserData, error) { } users := make([]*UserData, 0) - users = append(users, profile.userData(am.ClientID)) + users = append(users, profile.userData()) return users, nil } // GetAccount returns all the users for a given profile. func (am *AzureManager) GetAccount(accountID string) ([]*UserData, error) { - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - selectFields := strings.Join([]string{profileFields, wtAccountIDField, wtPendingInviteField}, ",") - q := url.Values{} - q.Add("$select", selectFields) - q.Add("$filter", fmt.Sprintf("%s eq '%s'", wtAccountIDField, accountID)) + q.Add("$select", profileFields) body, err := am.get("users", q) if err != nil { @@ -351,7 +286,10 @@ func (am *AzureManager) GetAccount(accountID string) ([]*UserData, error) { users := make([]*UserData, 0) for _, profile := range profiles.Value { - users = append(users, profile.userData(am.ClientID)) + userData := profile.userData() + userData.AppMetadata.WTAccountID = accountID + + users = append(users, userData) } return users, nil @@ -360,12 +298,8 @@ func (am *AzureManager) GetAccount(accountID string) ([]*UserData, error) { // GetAllAccounts gets all registered accounts with corresponding user data. // It returns a list of users indexed by accountID. func (am *AzureManager) GetAllAccounts() (map[string][]*UserData, error) { - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - selectFields := strings.Join([]string{profileFields, wtAccountIDField, wtPendingInviteField}, ",") - q := url.Values{} - q.Add("$select", selectFields) + q.Add("$select", profileFields) body, err := am.get("users", q) if err != nil { @@ -384,67 +318,15 @@ func (am *AzureManager) GetAllAccounts() (map[string][]*UserData, error) { indexedUsers := make(map[string][]*UserData) for _, profile := range profiles.Value { - userData := profile.userData(am.ClientID) - - accountID := userData.AppMetadata.WTAccountID - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } - + userData := profile.userData() + indexedUsers[UnsetAccountID] = append(indexedUsers[UnsetAccountID], userData) } return indexedUsers, nil } // UpdateUserAppMetadata updates user app metadata based on userID. -func (am *AzureManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - jwtToken, err := am.credentials.Authenticate() - if err != nil { - return err - } - - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - - data, err := am.helper.Marshal(map[string]any{ - wtAccountIDField: appMetadata.WTAccountID, - wtPendingInviteField: appMetadata.WTPendingInvite, - }) - if err != nil { - return err - } - payload := strings.NewReader(string(data)) - - reqURL := fmt.Sprintf("%s/users/%s", am.GraphAPIEndpoint, userID) - req, err := http.NewRequest(http.MethodPatch, reqURL, payload) - if err != nil { - return err - } - req.Header.Add("authorization", "Bearer "+jwtToken.AccessToken) - req.Header.Add("content-type", "application/json") - - log.Debugf("updating idp metadata for user %s", userID) - - resp, err := am.httpClient.Do(req) - if err != nil { - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountRequestError() - } - return err - } - defer resp.Body.Close() - - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - - if resp.StatusCode != http.StatusNoContent { - return fmt.Errorf("unable to update the appMetadata, statusCode %d", resp.StatusCode) - } - +func (am *AzureManager) UpdateUserAppMetadata(_ string, _ AppMetadata) error { return nil } @@ -454,7 +336,7 @@ func (am *AzureManager) InviteUserByID(_ string) error { return fmt.Errorf("method InviteUserByID not implemented") } -// DeleteUser from Azure +// DeleteUser from Azure. func (am *AzureManager) DeleteUser(userID string) error { jwtToken, err := am.credentials.Authenticate() if err != nil { @@ -491,81 +373,6 @@ func (am *AzureManager) DeleteUser(userID string) error { return nil } -func (am *AzureManager) getUserExtensions() ([]azureExtension, error) { - q := url.Values{} - q.Add("$select", extensionFields) - - resource := fmt.Sprintf("applications/%s/extensionProperties", am.ObjectID) - body, err := am.get(resource, q) - if err != nil { - return nil, err - } - - var extensions struct{ Value []azureExtension } - err = am.helper.Unmarshal(body, &extensions) - if err != nil { - return nil, err - } - - return extensions.Value, nil -} - -func (am *AzureManager) createUserExtension(name string) (*azureExtension, error) { - extension := azureExtension{ - Name: name, - DataType: "string", - TargetObjects: []string{"User"}, - } - - payload, err := am.helper.Marshal(extension) - if err != nil { - return nil, err - } - - resource := fmt.Sprintf("applications/%s/extensionProperties", am.ObjectID) - body, err := am.post(resource, string(payload)) - if err != nil { - return nil, err - } - - var userExtension azureExtension - err = am.helper.Unmarshal(body, &userExtension) - if err != nil { - return nil, err - } - - return &userExtension, nil -} - -// configureAppMetadata sets up app metadata extensions if they do not exists. -func (am *AzureManager) configureAppMetadata() error { - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - - extensions, err := am.getUserExtensions() - if err != nil { - return err - } - - // If the wt_account_id extension does not already exist, create it. - if !hasExtension(extensions, wtAccountIDField) { - _, err = am.createUserExtension(wtAccountID) - if err != nil { - return err - } - } - - // If the wt_pending_invite extension does not already exist, create it. - if !hasExtension(extensions, wtPendingInviteField) { - _, err = am.createUserExtension(wtPendingInvite) - if err != nil { - return err - } - } - - return nil -} - // get perform Get requests. func (am *AzureManager) get(resource string, q url.Values) ([]byte, error) { jwtToken, err := am.credentials.Authenticate() @@ -602,44 +409,8 @@ func (am *AzureManager) get(resource string, q url.Values) ([]byte, error) { return io.ReadAll(resp.Body) } -// post perform Post requests. -func (am *AzureManager) post(resource string, body string) ([]byte, error) { - jwtToken, err := am.credentials.Authenticate() - if err != nil { - return nil, err - } - - reqURL := fmt.Sprintf("%s/%s", am.GraphAPIEndpoint, resource) - req, err := http.NewRequest(http.MethodPost, reqURL, strings.NewReader(body)) - if err != nil { - return nil, err - } - req.Header.Add("authorization", "Bearer "+jwtToken.AccessToken) - req.Header.Add("content-type", "application/json") - - resp, err := am.httpClient.Do(req) - if err != nil { - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountRequestError() - } - - return nil, err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusCreated { - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountRequestStatusError() - } - - return nil, fmt.Errorf("unable to post %s, statusCode %d", reqURL, resp.StatusCode) - } - - return io.ReadAll(resp.Body) -} - // userData construct user data from keycloak profile. -func (ap azureProfile) userData(clientID string) *UserData { +func (ap azureProfile) userData() *UserData { id, ok := ap["id"].(string) if !ok { id = "" @@ -655,66 +426,9 @@ func (ap azureProfile) userData(clientID string) *UserData { name = "" } - accountIDField := extensionName(wtAccountIDTpl, clientID) - accountID, ok := ap[accountIDField].(string) - if !ok { - accountID = "" - } - - pendingInviteField := extensionName(wtPendingInviteTpl, clientID) - pendingInvite, ok := ap[pendingInviteField].(bool) - if !ok { - pendingInvite = false - } - return &UserData{ Email: email, Name: name, ID: id, - AppMetadata: AppMetadata{ - WTAccountID: accountID, - WTPendingInvite: &pendingInvite, - }, - } -} - -func buildAzureCreateUserRequestPayload(email, name, accountID, clientID string) (string, error) { - wtAccountIDField := extensionName(wtAccountIDTpl, clientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, clientID) - - req := &azureProfile{ - "accountEnabled": true, - "displayName": name, - "mailNickName": strings.Join(strings.Split(name, " "), ""), - "userPrincipalName": email, - "passwordProfile": passwordProfile{ - ForceChangePasswordNextSignIn: true, - Password: GeneratePassword(8, 1, 1, 1), - }, - wtAccountIDField: accountID, - wtPendingInviteField: true, - } - - str, err := json.Marshal(req) - if err != nil { - return "", err - } - - return string(str), nil -} - -func extensionName(extensionTpl, clientID string) string { - clientID = strings.ReplaceAll(clientID, "-", "") - return fmt.Sprintf(extensionTpl, clientID) -} - -// hasExtension checks whether a given extension by name, -// exists in an list of extensions. -func hasExtension(extensions []azureExtension, name string) bool { - for _, ext := range extensions { - if ext.Name == name { - return true - } } - return false } diff --git a/management/server/idp/azure_test.go b/management/server/idp/azure_test.go index 9d845ffbeef..b4dc96b23cd 100644 --- a/management/server/idp/azure_test.go +++ b/management/server/idp/azure_test.go @@ -8,15 +8,6 @@ import ( "github.com/stretchr/testify/assert" ) -type mockAzureCredentials struct { - jwtToken JWTToken - err error -} - -func (mc *mockAzureCredentials) Authenticate() (JWTToken, error) { - return mc.jwtToken, mc.err -} - func TestAzureJwtStillValid(t *testing.T) { type jwtStillValidTest struct { name string @@ -124,206 +115,63 @@ func TestAzureAuthenticate(t *testing.T) { } } -func TestAzureUpdateUserAppMetadata(t *testing.T) { - type updateUserAppMetadataTest struct { - name string - inputReqBody string - expectedReqBody string - appMetadata AppMetadata - statusCode int - helper ManagerHelper - managerCreds ManagerCredentials - assertErrFunc assert.ErrorAssertionFunc - assertErrFuncMessage string - } - - appMetadata := AppMetadata{WTAccountID: "ok"} - - updateUserAppMetadataTestCase1 := updateUserAppMetadataTest{ - name: "Bad Authentication", - expectedReqBody: "", - appMetadata: appMetadata, - statusCode: 400, - helper: JsonParser{}, - managerCreds: &mockAzureCredentials{ - jwtToken: JWTToken{}, - err: fmt.Errorf("error"), - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase2 := updateUserAppMetadataTest{ - name: "Bad Status Code", - expectedReqBody: fmt.Sprintf("{\"extension__wt_account_id\":\"%s\",\"extension__wt_pending_invite\":null}", appMetadata.WTAccountID), - appMetadata: appMetadata, - statusCode: 400, - helper: JsonParser{}, - managerCreds: &mockAzureCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase3 := updateUserAppMetadataTest{ - name: "Bad Response Parsing", - statusCode: 400, - helper: &mockJsonParser{marshalErrorString: "error"}, - managerCreds: &mockAzureCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase4 := updateUserAppMetadataTest{ - name: "Good request", - expectedReqBody: fmt.Sprintf("{\"extension__wt_account_id\":\"%s\",\"extension__wt_pending_invite\":null}", appMetadata.WTAccountID), - appMetadata: appMetadata, - statusCode: 204, - helper: JsonParser{}, - managerCreds: &mockAzureCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.NoError, - assertErrFuncMessage: "shouldn't return error", - } - - invite := true - updateUserAppMetadataTestCase5 := updateUserAppMetadataTest{ - name: "Update Pending Invite", - expectedReqBody: fmt.Sprintf("{\"extension__wt_account_id\":\"%s\",\"extension__wt_pending_invite\":true}", appMetadata.WTAccountID), - appMetadata: AppMetadata{ - WTAccountID: "ok", - WTPendingInvite: &invite, - }, - statusCode: 204, - helper: JsonParser{}, - managerCreds: &mockAzureCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.NoError, - assertErrFuncMessage: "shouldn't return error", - } - - for _, testCase := range []updateUserAppMetadataTest{updateUserAppMetadataTestCase1, updateUserAppMetadataTestCase2, - updateUserAppMetadataTestCase3, updateUserAppMetadataTestCase4, updateUserAppMetadataTestCase5} { - t.Run(testCase.name, func(t *testing.T) { - reqClient := mockHTTPClient{ - resBody: testCase.inputReqBody, - code: testCase.statusCode, - } - - manager := &AzureManager{ - httpClient: &reqClient, - credentials: testCase.managerCreds, - helper: testCase.helper, - } - - err := manager.UpdateUserAppMetadata("1", testCase.appMetadata) - testCase.assertErrFunc(t, err, testCase.assertErrFuncMessage) - - assert.Equal(t, testCase.expectedReqBody, reqClient.reqBody, "request body should match") - }) - } -} - func TestAzureProfile(t *testing.T) { type azureProfileTest struct { name string - clientID string invite bool inputProfile azureProfile expectedUserData UserData } azureProfileTestCase1 := azureProfileTest{ - name: "Good Request", - clientID: "25d0b095-0484-40d2-9fd3-03f8f4abbb3c", - invite: false, + name: "Good Request", + invite: false, inputProfile: azureProfile{ "id": "test1", "displayName": "John Doe", "userPrincipalName": "test1@test.com", - "extension_25d0b095048440d29fd303f8f4abbb3c_wt_account_id": "1", - "extension_25d0b095048440d29fd303f8f4abbb3c_wt_pending_invite": false, }, expectedUserData: UserData{ Email: "test1@test.com", Name: "John Doe", ID: "test1", - AppMetadata: AppMetadata{ - WTAccountID: "1", - }, }, } azureProfileTestCase2 := azureProfileTest{ - name: "Missing User ID", - clientID: "25d0b095-0484-40d2-9fd3-03f8f4abbb3c", - invite: true, + name: "Missing User ID", + invite: true, inputProfile: azureProfile{ "displayName": "John Doe", "userPrincipalName": "test2@test.com", - "extension_25d0b095048440d29fd303f8f4abbb3c_wt_account_id": "1", - "extension_25d0b095048440d29fd303f8f4abbb3c_wt_pending_invite": true, }, expectedUserData: UserData{ Email: "test2@test.com", Name: "John Doe", - AppMetadata: AppMetadata{ - WTAccountID: "1", - }, }, } azureProfileTestCase3 := azureProfileTest{ - name: "Missing User Name", - clientID: "25d0b095-0484-40d2-9fd3-03f8f4abbb3c", - invite: false, + name: "Missing User Name", + invite: false, inputProfile: azureProfile{ "id": "test3", "userPrincipalName": "test3@test.com", - "extension_25d0b095048440d29fd303f8f4abbb3c_wt_account_id": "1", - "extension_25d0b095048440d29fd303f8f4abbb3c_wt_pending_invite": false, }, expectedUserData: UserData{ ID: "test3", Email: "test3@test.com", - AppMetadata: AppMetadata{ - WTAccountID: "1", - }, - }, - } - - azureProfileTestCase4 := azureProfileTest{ - name: "Missing Extension Fields", - clientID: "25d0b095-0484-40d2-9fd3-03f8f4abbb3c", - invite: false, - inputProfile: azureProfile{ - "id": "test4", - "displayName": "John Doe", - "userPrincipalName": "test4@test.com", - }, - expectedUserData: UserData{ - ID: "test4", - Name: "John Doe", - Email: "test4@test.com", - AppMetadata: AppMetadata{}, }, } - for _, testCase := range []azureProfileTest{azureProfileTestCase1, azureProfileTestCase2, azureProfileTestCase3, azureProfileTestCase4} { + for _, testCase := range []azureProfileTest{azureProfileTestCase1, azureProfileTestCase2, azureProfileTestCase3} { t.Run(testCase.name, func(t *testing.T) { testCase.expectedUserData.AppMetadata.WTPendingInvite = &testCase.invite - userData := testCase.inputProfile.userData(testCase.clientID) + userData := testCase.inputProfile.userData() assert.Equal(t, testCase.expectedUserData.ID, userData.ID, "User id should match") assert.Equal(t, testCase.expectedUserData.Email, userData.Email, "User email should match") assert.Equal(t, testCase.expectedUserData.Name, userData.Name, "User name should match") - assert.Equal(t, testCase.expectedUserData.AppMetadata.WTAccountID, userData.AppMetadata.WTAccountID, "Account id should match") - assert.Equal(t, testCase.expectedUserData.AppMetadata.WTPendingInvite, userData.AppMetadata.WTPendingInvite, "Pending invite should match") }) } } diff --git a/management/server/idp/google_workspace.go b/management/server/idp/google_workspace.go index 40854e59860..ed2de9a4225 100644 --- a/management/server/idp/google_workspace.go +++ b/management/server/idp/google_workspace.go @@ -5,15 +5,14 @@ import ( "encoding/base64" "fmt" "net/http" - "strings" "time" - "github.com/netbirdio/netbird/management/server/telemetry" log "github.com/sirupsen/logrus" "golang.org/x/oauth2/google" admin "google.golang.org/api/admin/directory/v1" - "google.golang.org/api/googleapi" "google.golang.org/api/option" + + "github.com/netbirdio/netbird/management/server/telemetry" ) // GoogleWorkspaceManager Google Workspace manager client instance. @@ -73,17 +72,13 @@ func NewGoogleWorkspaceManager(config GoogleWorkspaceClientConfig, appMetrics te } service, err := admin.NewService(context.Background(), - option.WithScopes(admin.AdminDirectoryUserScope, admin.AdminDirectoryUserschemaScope), + option.WithScopes(admin.AdminDirectoryUserReadonlyScope), option.WithCredentials(adminCredentials), ) if err != nil { return nil, err } - if err = configureAppMetadataSchema(service, config.CustomerID); err != nil { - return nil, err - } - return &GoogleWorkspaceManager{ usersService: service.Users, CustomerID: config.CustomerID, @@ -95,27 +90,7 @@ func NewGoogleWorkspaceManager(config GoogleWorkspaceClientConfig, appMetrics te } // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. -func (gm *GoogleWorkspaceManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - metadata, err := gm.helper.Marshal(appMetadata) - if err != nil { - return err - } - - user := &admin.User{ - CustomSchemas: map[string]googleapi.RawMessage{ - "app_metadata": metadata, - }, - } - - _, err = gm.usersService.Update(userID, user).Do() - if err != nil { - return err - } - - if gm.appMetrics != nil { - gm.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - +func (gm *GoogleWorkspaceManager) UpdateUserAppMetadata(_ string, _ AppMetadata) error { return nil } @@ -130,23 +105,23 @@ func (gm *GoogleWorkspaceManager) GetUserDataByID(userID string, appMetadata App gm.appMetrics.IDPMetrics().CountGetUserDataByID() } - return parseGoogleWorkspaceUser(user) + userData := parseGoogleWorkspaceUser(user) + userData.AppMetadata = appMetadata + + return userData, nil } // GetAccount returns all the users for a given profile. func (gm *GoogleWorkspaceManager) GetAccount(accountID string) ([]*UserData, error) { - query := fmt.Sprintf("app_metadata.wt_account_id=\"%s\"", accountID) - usersList, err := gm.usersService.List().Customer(gm.CustomerID).Query(query).Projection("full").Do() + usersList, err := gm.usersService.List().Customer(gm.CustomerID).Projection("full").Do() if err != nil { return nil, err } usersData := make([]*UserData, 0) for _, user := range usersList.Users { - userData, err := parseGoogleWorkspaceUser(user) - if err != nil { - return nil, err - } + userData := parseGoogleWorkspaceUser(user) + userData.AppMetadata.WTAccountID = accountID usersData = append(usersData, userData) } @@ -168,61 +143,16 @@ func (gm *GoogleWorkspaceManager) GetAllAccounts() (map[string][]*UserData, erro indexedUsers := make(map[string][]*UserData) for _, user := range usersList.Users { - userData, err := parseGoogleWorkspaceUser(user) - if err != nil { - return nil, err - } - - accountID := userData.AppMetadata.WTAccountID - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } + userData := parseGoogleWorkspaceUser(user) + indexedUsers[UnsetAccountID] = append(indexedUsers[UnsetAccountID], userData) } return indexedUsers, nil } // CreateUser creates a new user in Google Workspace and sends an invitation. -func (gm *GoogleWorkspaceManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { - invite := true - metadata := AppMetadata{ - WTAccountID: accountID, - WTPendingInvite: &invite, - } - - username := &admin.UserName{} - fields := strings.Fields(name) - if n := len(fields); n > 0 { - username.GivenName = strings.Join(fields[:n-1], " ") - username.FamilyName = fields[n-1] - } - - payload, err := gm.helper.Marshal(metadata) - if err != nil { - return nil, err - } - - user := &admin.User{ - Name: username, - PrimaryEmail: email, - CustomSchemas: map[string]googleapi.RawMessage{ - "app_metadata": payload, - }, - Password: GeneratePassword(8, 1, 1, 1), - } - user, err = gm.usersService.Insert(user).Do() - if err != nil { - return nil, err - } - - if gm.appMetrics != nil { - gm.appMetrics.IDPMetrics().CountCreateUser() - } - - return parseGoogleWorkspaceUser(user) +func (gm *GoogleWorkspaceManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserByEmail searches users with a given email. @@ -237,13 +167,8 @@ func (gm *GoogleWorkspaceManager) GetUserByEmail(email string) ([]*UserData, err gm.appMetrics.IDPMetrics().CountGetUserByEmail() } - userData, err := parseGoogleWorkspaceUser(user) - if err != nil { - return nil, err - } - users := make([]*UserData, 0) - users = append(users, userData) + users = append(users, parseGoogleWorkspaceUser(user)) return users, nil } @@ -281,8 +206,7 @@ func getGoogleCredentials(serviceAccountKey string) (*google.Credentials, error) creds, err := google.CredentialsFromJSON( context.Background(), decodeKey, - admin.AdminDirectoryUserschemaScope, - admin.AdminDirectoryUserScope, + admin.AdminDirectoryUserReadonlyScope, ) if err == nil { // No need to fallback to the default Google credentials path @@ -294,8 +218,7 @@ func getGoogleCredentials(serviceAccountKey string) (*google.Credentials, error) creds, err = google.FindDefaultCredentials( context.Background(), - admin.AdminDirectoryUserschemaScope, - admin.AdminDirectoryUserScope, + admin.AdminDirectoryUserReadonlyScope, ) if err != nil { return nil, err @@ -304,62 +227,11 @@ func getGoogleCredentials(serviceAccountKey string) (*google.Credentials, error) return creds, nil } -// configureAppMetadataSchema create a custom schema for managing app metadata fields in Google Workspace. -func configureAppMetadataSchema(service *admin.Service, customerID string) error { - schemaList, err := service.Schemas.List(customerID).Do() - if err != nil { - return err - } - - // checks if app_metadata schema is already created - for _, schema := range schemaList.Schemas { - if schema.SchemaName == "app_metadata" { - return nil - } - } - - // create new app_metadata schema - appMetadataSchema := &admin.Schema{ - SchemaName: "app_metadata", - Fields: []*admin.SchemaFieldSpec{ - { - FieldName: "wt_account_id", - FieldType: "STRING", - MultiValued: false, - }, - { - FieldName: "wt_pending_invite", - FieldType: "BOOL", - MultiValued: false, - }, - }, - } - _, err = service.Schemas.Insert(customerID, appMetadataSchema).Do() - if err != nil { - return err - } - - return nil -} - // parseGoogleWorkspaceUser parse google user to UserData. -func parseGoogleWorkspaceUser(user *admin.User) (*UserData, error) { - var appMetadata AppMetadata - - // Get app metadata from custom schemas - if user.CustomSchemas != nil { - rawMessage := user.CustomSchemas["app_metadata"] - helper := JsonParser{} - - if err := helper.Unmarshal(rawMessage, &appMetadata); err != nil { - return nil, err - } - } - +func parseGoogleWorkspaceUser(user *admin.User) *UserData { return &UserData{ - ID: user.Id, - Email: user.PrimaryEmail, - Name: user.Name.FullName, - AppMetadata: appMetadata, - }, nil + ID: user.Id, + Email: user.PrimaryEmail, + Name: user.Name.FullName, + } } diff --git a/management/server/idp/idp.go b/management/server/idp/idp.go index 1e2ac658b8e..3b6a633c89e 100644 --- a/management/server/idp/idp.go +++ b/management/server/idp/idp.go @@ -9,6 +9,11 @@ import ( "github.com/netbirdio/netbird/management/server/telemetry" ) +const ( + // UnsetAccountID is a special key to map users without an account ID + UnsetAccountID = "unset" +) + // Manager idp manager interface type Manager interface { UpdateUserAppMetadata(userId string, appMetadata AppMetadata) error diff --git a/management/server/idp/keycloak.go b/management/server/idp/keycloak.go index d65a78ae3f0..3a6f80d0359 100644 --- a/management/server/idp/keycloak.go +++ b/management/server/idp/keycloak.go @@ -1,12 +1,10 @@ package idp import ( - "encoding/json" "fmt" "io" "net/http" "net/url" - "path" "strconv" "strings" "sync" @@ -18,11 +16,6 @@ import ( "github.com/netbirdio/netbird/management/server/telemetry" ) -const ( - wtAccountID = "wt_account_id" - wtPendingInvite = "wt_pending_invite" -) - // KeycloakManager keycloak manager client instance. type KeycloakManager struct { adminEndpoint string @@ -51,28 +44,10 @@ type KeycloakCredentials struct { appMetrics telemetry.AppMetrics } -// keycloakUserCredential describe the authentication method for, -// newly created user profile. -type keycloakUserCredential struct { - Type string `json:"type"` - Value string `json:"value"` - Temporary bool `json:"temporary"` -} - // keycloakUserAttributes holds additional user data fields. type keycloakUserAttributes map[string][]string -// createUserRequest is a user create request. -type keycloakCreateUserRequest struct { - Email string `json:"email"` - Username string `json:"username"` - Enabled bool `json:"enabled"` - EmailVerified bool `json:"emailVerified"` - Credentials []keycloakUserCredential `json:"credentials"` - Attributes keycloakUserAttributes `json:"attributes"` -} - -// keycloakProfile represents an keycloak user profile response. +// keycloakProfile represents a keycloak user profile response. type keycloakProfile struct { ID string `json:"id"` CreatedTimestamp int64 `json:"createdTimestamp"` @@ -230,62 +205,8 @@ func (kc *KeycloakCredentials) Authenticate() (JWTToken, error) { } // CreateUser creates a new user in keycloak Idp and sends an invite. -func (km *KeycloakManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { - jwtToken, err := km.credentials.Authenticate() - if err != nil { - return nil, err - } - - invite := true - appMetadata := AppMetadata{ - WTAccountID: accountID, - WTPendingInvite: &invite, - } - - payloadString, err := buildKeycloakCreateUserRequestPayload(email, name, appMetadata) - if err != nil { - return nil, err - } - - reqURL := fmt.Sprintf("%s/users", km.adminEndpoint) - payload := strings.NewReader(payloadString) - - req, err := http.NewRequest(http.MethodPost, reqURL, payload) - if err != nil { - return nil, err - } - req.Header.Add("authorization", "Bearer "+jwtToken.AccessToken) - req.Header.Add("content-type", "application/json") - - if km.appMetrics != nil { - km.appMetrics.IDPMetrics().CountCreateUser() - } - - resp, err := km.httpClient.Do(req) - if err != nil { - if km.appMetrics != nil { - km.appMetrics.IDPMetrics().CountRequestError() - } - - return nil, err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusCreated { - if km.appMetrics != nil { - km.appMetrics.IDPMetrics().CountRequestStatusError() - } - - return nil, fmt.Errorf("unable to create user, statusCode %d", resp.StatusCode) - } - - locationHeader := resp.Header.Get("location") - userID, err := extractUserIDFromLocationHeader(locationHeader) - if err != nil { - return nil, err - } - - return km.GetUserDataByID(userID, appMetadata) +func (km *KeycloakManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserByEmail searches users with a given email. @@ -319,7 +240,7 @@ func (km *KeycloakManager) GetUserByEmail(email string) ([]*UserData, error) { } // GetUserDataByID requests user data from keycloak via ID. -func (km *KeycloakManager) GetUserDataByID(userID string, appMetadata AppMetadata) (*UserData, error) { +func (km *KeycloakManager) GetUserDataByID(userID string, _ AppMetadata) (*UserData, error) { body, err := km.get("users/"+userID, nil) if err != nil { return nil, err @@ -338,12 +259,9 @@ func (km *KeycloakManager) GetUserDataByID(userID string, appMetadata AppMetadat return profile.userData(), nil } -// GetAccount returns all the users for a given profile. +// GetAccount returns all the users for a given account profile. func (km *KeycloakManager) GetAccount(accountID string) ([]*UserData, error) { - q := url.Values{} - q.Add("q", wtAccountID+":"+accountID) - - body, err := km.get("users", q) + profiles, err := km.fetchAllUserProfiles() if err != nil { return nil, err } @@ -352,15 +270,12 @@ func (km *KeycloakManager) GetAccount(accountID string) ([]*UserData, error) { km.appMetrics.IDPMetrics().CountGetAccount() } - profiles := make([]keycloakProfile, 0) - err = km.helper.Unmarshal(body, &profiles) - if err != nil { - return nil, err - } - users := make([]*UserData, 0) for _, profile := range profiles { - users = append(users, profile.userData()) + userData := profile.userData() + userData.AppMetadata.WTAccountID = accountID + + users = append(users, userData) } return users, nil @@ -369,15 +284,7 @@ func (km *KeycloakManager) GetAccount(accountID string) ([]*UserData, error) { // GetAllAccounts gets all registered accounts with corresponding user data. // It returns a list of users indexed by accountID. func (km *KeycloakManager) GetAllAccounts() (map[string][]*UserData, error) { - totalUsers, err := km.totalUsersCount() - if err != nil { - return nil, err - } - - q := url.Values{} - q.Add("max", fmt.Sprint(*totalUsers)) - - body, err := km.get("users", q) + profiles, err := km.fetchAllUserProfiles() if err != nil { return nil, err } @@ -386,78 +293,17 @@ func (km *KeycloakManager) GetAllAccounts() (map[string][]*UserData, error) { km.appMetrics.IDPMetrics().CountGetAllAccounts() } - profiles := make([]keycloakProfile, 0) - err = km.helper.Unmarshal(body, &profiles) - if err != nil { - return nil, err - } - indexedUsers := make(map[string][]*UserData) for _, profile := range profiles { userData := profile.userData() - - accountID := userData.AppMetadata.WTAccountID - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } + indexedUsers[UnsetAccountID] = append(indexedUsers[UnsetAccountID], userData) } return indexedUsers, nil } // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. -func (km *KeycloakManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - jwtToken, err := km.credentials.Authenticate() - if err != nil { - return err - } - - attrs := keycloakUserAttributes{} - attrs.Set(wtAccountID, appMetadata.WTAccountID) - if appMetadata.WTPendingInvite != nil { - attrs.Set(wtPendingInvite, strconv.FormatBool(*appMetadata.WTPendingInvite)) - } else { - attrs.Set(wtPendingInvite, "false") - } - - reqURL := fmt.Sprintf("%s/users/%s", km.adminEndpoint, userID) - data, err := km.helper.Marshal(map[string]any{ - "attributes": attrs, - }) - if err != nil { - return err - } - payload := strings.NewReader(string(data)) - - req, err := http.NewRequest(http.MethodPut, reqURL, payload) - if err != nil { - return err - } - req.Header.Add("authorization", "Bearer "+jwtToken.AccessToken) - req.Header.Add("content-type", "application/json") - - log.Debugf("updating IdP metadata for user %s", userID) - - resp, err := km.httpClient.Do(req) - if err != nil { - if km.appMetrics != nil { - km.appMetrics.IDPMetrics().CountRequestError() - } - return err - } - defer resp.Body.Close() - - if km.appMetrics != nil { - km.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - - if resp.StatusCode != http.StatusNoContent { - return fmt.Errorf("unable to update the appMetadata, statusCode %d", resp.StatusCode) - } - +func (km *KeycloakManager) UpdateUserAppMetadata(_ string, _ AppMetadata) error { return nil } @@ -467,7 +313,7 @@ func (km *KeycloakManager) InviteUserByID(_ string) error { return fmt.Errorf("method InviteUserByID not implemented") } -// DeleteUser from Keycloack +// DeleteUser from Keycloak by user ID. func (km *KeycloakManager) DeleteUser(userID string) error { jwtToken, err := km.credentials.Authenticate() if err != nil { @@ -475,7 +321,6 @@ func (km *KeycloakManager) DeleteUser(userID string) error { } reqURL := fmt.Sprintf("%s/users/%s", km.adminEndpoint, url.QueryEscape(userID)) - req, err := http.NewRequest(http.MethodDelete, reqURL, nil) if err != nil { return err @@ -508,32 +353,27 @@ func (km *KeycloakManager) DeleteUser(userID string) error { return nil } -func buildKeycloakCreateUserRequestPayload(email string, name string, appMetadata AppMetadata) (string, error) { - attrs := keycloakUserAttributes{} - attrs.Set(wtAccountID, appMetadata.WTAccountID) - attrs.Set(wtPendingInvite, strconv.FormatBool(*appMetadata.WTPendingInvite)) - - req := &keycloakCreateUserRequest{ - Email: email, - Username: name, - Enabled: true, - EmailVerified: true, - Credentials: []keycloakUserCredential{ - { - Type: "password", - Value: GeneratePassword(8, 1, 1, 1), - Temporary: false, - }, - }, - Attributes: attrs, - } - - str, err := json.Marshal(req) +func (km *KeycloakManager) fetchAllUserProfiles() ([]keycloakProfile, error) { + totalUsers, err := km.totalUsersCount() + if err != nil { + return nil, err + } + + q := url.Values{} + q.Add("max", fmt.Sprint(*totalUsers)) + + body, err := km.get("users", q) if err != nil { - return "", err + return nil, err } - return string(str), nil + profiles := make([]keycloakProfile, 0) + err = km.helper.Unmarshal(body, &profiles) + if err != nil { + return nil, err + } + + return profiles, nil } // get perform Get requests. @@ -588,53 +428,11 @@ func (km *KeycloakManager) totalUsersCount() (*int, error) { return &count, nil } -// extractUserIDFromLocationHeader extracts the user ID from the location, -// header once the user is created successfully -func extractUserIDFromLocationHeader(locationHeader string) (string, error) { - userURL, err := url.Parse(locationHeader) - if err != nil { - return "", err - } - - return path.Base(userURL.Path), nil -} - // userData construct user data from keycloak profile. func (kp keycloakProfile) userData() *UserData { - accountID := kp.Attributes.Get(wtAccountID) - pendingInvite, err := strconv.ParseBool(kp.Attributes.Get(wtPendingInvite)) - if err != nil { - pendingInvite = false - } - return &UserData{ Email: kp.Email, Name: kp.Username, ID: kp.ID, - AppMetadata: AppMetadata{ - WTAccountID: accountID, - WTPendingInvite: &pendingInvite, - }, - } -} - -// Set sets the key to value. It replaces any existing -// values. -func (ka keycloakUserAttributes) Set(key, value string) { - ka[key] = []string{value} -} - -// Get returns the first value associated with the given key. -// If there are no values associated with the key, Get returns -// the empty string. -func (ka keycloakUserAttributes) Get(key string) string { - if ka == nil { - return "" - } - - values := ka[key] - if len(values) == 0 { - return "" } - return values[0] } diff --git a/management/server/idp/keycloak_test.go b/management/server/idp/keycloak_test.go index 0c33fc13754..9b6c1d3c63c 100644 --- a/management/server/idp/keycloak_test.go +++ b/management/server/idp/keycloak_test.go @@ -84,15 +84,6 @@ func TestNewKeycloakManager(t *testing.T) { } } -type mockKeycloakCredentials struct { - jwtToken JWTToken - err error -} - -func (mc *mockKeycloakCredentials) Authenticate() (JWTToken, error) { - return mc.jwtToken, mc.err -} - func TestKeycloakRequestJWTToken(t *testing.T) { type requestJWTTokenTest struct { @@ -316,108 +307,3 @@ func TestKeycloakAuthenticate(t *testing.T) { }) } } - -func TestKeycloakUpdateUserAppMetadata(t *testing.T) { - type updateUserAppMetadataTest struct { - name string - inputReqBody string - expectedReqBody string - appMetadata AppMetadata - statusCode int - helper ManagerHelper - managerCreds ManagerCredentials - assertErrFunc assert.ErrorAssertionFunc - assertErrFuncMessage string - } - - appMetadata := AppMetadata{WTAccountID: "ok"} - - updateUserAppMetadataTestCase1 := updateUserAppMetadataTest{ - name: "Bad Authentication", - expectedReqBody: "", - appMetadata: appMetadata, - statusCode: 400, - helper: JsonParser{}, - managerCreds: &mockKeycloakCredentials{ - jwtToken: JWTToken{}, - err: fmt.Errorf("error"), - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase2 := updateUserAppMetadataTest{ - name: "Bad Status Code", - expectedReqBody: fmt.Sprintf("{\"attributes\":{\"wt_account_id\":[\"%s\"],\"wt_pending_invite\":[\"false\"]}}", appMetadata.WTAccountID), - appMetadata: appMetadata, - statusCode: 400, - helper: JsonParser{}, - managerCreds: &mockKeycloakCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase3 := updateUserAppMetadataTest{ - name: "Bad Response Parsing", - statusCode: 400, - helper: &mockJsonParser{marshalErrorString: "error"}, - managerCreds: &mockKeycloakCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase4 := updateUserAppMetadataTest{ - name: "Good request", - expectedReqBody: fmt.Sprintf("{\"attributes\":{\"wt_account_id\":[\"%s\"],\"wt_pending_invite\":[\"false\"]}}", appMetadata.WTAccountID), - appMetadata: appMetadata, - statusCode: 204, - helper: JsonParser{}, - managerCreds: &mockKeycloakCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.NoError, - assertErrFuncMessage: "shouldn't return error", - } - - invite := true - updateUserAppMetadataTestCase5 := updateUserAppMetadataTest{ - name: "Update Pending Invite", - expectedReqBody: fmt.Sprintf("{\"attributes\":{\"wt_account_id\":[\"%s\"],\"wt_pending_invite\":[\"true\"]}}", appMetadata.WTAccountID), - appMetadata: AppMetadata{ - WTAccountID: "ok", - WTPendingInvite: &invite, - }, - statusCode: 204, - helper: JsonParser{}, - managerCreds: &mockKeycloakCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.NoError, - assertErrFuncMessage: "shouldn't return error", - } - - for _, testCase := range []updateUserAppMetadataTest{updateUserAppMetadataTestCase1, updateUserAppMetadataTestCase2, - updateUserAppMetadataTestCase3, updateUserAppMetadataTestCase4, updateUserAppMetadataTestCase5} { - t.Run(testCase.name, func(t *testing.T) { - reqClient := mockHTTPClient{ - resBody: testCase.inputReqBody, - code: testCase.statusCode, - } - - manager := &KeycloakManager{ - httpClient: &reqClient, - credentials: testCase.managerCreds, - helper: testCase.helper, - } - - err := manager.UpdateUserAppMetadata("1", testCase.appMetadata) - testCase.assertErrFunc(t, err, testCase.assertErrFuncMessage) - - assert.Equal(t, testCase.expectedReqBody, reqClient.reqBody, "request body should match") - }) - } -} diff --git a/management/server/idp/okta.go b/management/server/idp/okta.go index 0e93c494c54..3e7b9357ec6 100644 --- a/management/server/idp/okta.go +++ b/management/server/idp/okta.go @@ -8,9 +8,9 @@ import ( "strings" "time" - "github.com/netbirdio/netbird/management/server/telemetry" "github.com/okta/okta-sdk-golang/v2/okta" - "github.com/okta/okta-sdk-golang/v2/okta/query" + + "github.com/netbirdio/netbird/management/server/telemetry" ) // OktaManager okta manager client instance. @@ -76,11 +76,6 @@ func NewOktaManager(config OktaClientConfig, appMetrics telemetry.AppMetrics) (* return nil, err } - err = updateUserProfileSchema(client) - if err != nil { - return nil, err - } - credentials := &OktaCredentials{ clientConfig: config, httpClient: httpClient, @@ -103,49 +98,8 @@ func (oc *OktaCredentials) Authenticate() (JWTToken, error) { } // CreateUser creates a new user in okta Idp and sends an invitation. -func (om *OktaManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { - var ( - sendEmail = true - activate = true - userProfile = okta.UserProfile{ - "email": email, - "login": email, - wtAccountID: accountID, - wtPendingInvite: true, - } - ) - - fields := strings.Fields(name) - if n := len(fields); n > 0 { - userProfile["firstName"] = strings.Join(fields[:n-1], " ") - userProfile["lastName"] = fields[n-1] - } - - user, resp, err := om.client.User.CreateUser(context.Background(), - okta.CreateUserRequest{ - Profile: &userProfile, - }, - &query.Params{ - Activate: &activate, - SendEmail: &sendEmail, - }, - ) - if err != nil { - return nil, err - } - - if om.appMetrics != nil { - om.appMetrics.IDPMetrics().CountCreateUser() - } - - if resp.StatusCode != http.StatusOK { - if om.appMetrics != nil { - om.appMetrics.IDPMetrics().CountRequestStatusError() - } - return nil, fmt.Errorf("unable to create user, statusCode %d", resp.StatusCode) - } - - return parseOktaUser(user) +func (om *OktaManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserDataByID requests user data from keycloak via ID. @@ -166,7 +120,13 @@ func (om *OktaManager) GetUserDataByID(userID string, appMetadata AppMetadata) ( return nil, fmt.Errorf("unable to get user %s, statusCode %d", userID, resp.StatusCode) } - return parseOktaUser(user) + userData, err := parseOktaUser(user) + if err != nil { + return nil, err + } + userData.AppMetadata = appMetadata + + return userData, nil } // GetUserByEmail searches users with a given email. @@ -200,8 +160,7 @@ func (om *OktaManager) GetUserByEmail(email string) ([]*UserData, error) { // GetAccount returns all the users for a given profile. func (om *OktaManager) GetAccount(accountID string) ([]*UserData, error) { - search := fmt.Sprintf("profile.wt_account_id eq %q", accountID) - users, resp, err := om.client.User.ListUsers(context.Background(), &query.Params{Search: search}) + users, resp, err := om.client.User.ListUsers(context.Background(), nil) if err != nil { return nil, err } @@ -223,6 +182,7 @@ func (om *OktaManager) GetAccount(accountID string) ([]*UserData, error) { if err != nil { return nil, err } + userData.AppMetadata.WTAccountID = accountID list = append(list, userData) } @@ -256,13 +216,7 @@ func (om *OktaManager) GetAllAccounts() (map[string][]*UserData, error) { return nil, err } - accountID := userData.AppMetadata.WTAccountID - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } + indexedUsers[UnsetAccountID] = append(indexedUsers[UnsetAccountID], userData) } return indexedUsers, nil @@ -270,46 +224,6 @@ func (om *OktaManager) GetAllAccounts() (map[string][]*UserData, error) { // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. func (om *OktaManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - user, resp, err := om.client.User.GetUser(context.Background(), userID) - if err != nil { - return err - } - - if resp.StatusCode != http.StatusOK { - if om.appMetrics != nil { - om.appMetrics.IDPMetrics().CountRequestStatusError() - } - return fmt.Errorf("unable to update user, statusCode %d", resp.StatusCode) - } - - profile := *user.Profile - - if appMetadata.WTPendingInvite != nil { - profile[wtPendingInvite] = *appMetadata.WTPendingInvite - } - - if appMetadata.WTAccountID != "" { - profile[wtAccountID] = appMetadata.WTAccountID - } - - user.Profile = &profile - _, resp, err = om.client.User.UpdateUser(context.Background(), userID, *user, nil) - if err != nil { - fmt.Println(err.Error()) - return err - } - - if om.appMetrics != nil { - om.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - - if resp.StatusCode != http.StatusOK { - if om.appMetrics != nil { - om.appMetrics.IDPMetrics().CountRequestStatusError() - } - return fmt.Errorf("unable to update user, statusCode %d", resp.StatusCode) - } - return nil } @@ -341,60 +255,12 @@ func (om *OktaManager) DeleteUser(userID string) error { return nil } -// updateUserProfileSchema updates the Okta user schema to include custom fields, -// wt_account_id and wt_pending_invite. -func updateUserProfileSchema(client *okta.Client) error { - // Ensure Okta doesn't enforce user input for these fields, as they are solely used by Netbird - userPermissions := []*okta.UserSchemaAttributePermission{{Action: "HIDE", Principal: "SELF"}} - - _, resp, err := client.UserSchema.UpdateUserProfile( - context.Background(), - "default", - okta.UserSchema{ - Definitions: &okta.UserSchemaDefinitions{ - Custom: &okta.UserSchemaPublic{ - Id: "#custom", - Type: "object", - Properties: map[string]*okta.UserSchemaAttribute{ - wtAccountID: { - MaxLength: 100, - MinLength: 1, - Required: new(bool), - Scope: "NONE", - Title: "Wt Account Id", - Type: "string", - Permissions: userPermissions, - }, - wtPendingInvite: { - Required: new(bool), - Scope: "NONE", - Title: "Wt Pending Invite", - Type: "boolean", - Permissions: userPermissions, - }, - }, - }, - }, - }) - if err != nil { - return err - } - - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("unable to update user profile schema, statusCode %d", resp.StatusCode) - } - - return nil -} - // parseOktaUserToUserData parse okta user to UserData. func parseOktaUser(user *okta.User) (*UserData, error) { var oktaUser struct { - Email string `json:"email"` - FirstName string `json:"firstName"` - LastName string `json:"lastName"` - AccountID string `json:"wt_account_id"` - PendingInvite bool `json:"wt_pending_invite"` + Email string `json:"email"` + FirstName string `json:"firstName"` + LastName string `json:"lastName"` } if user == nil { @@ -418,9 +284,5 @@ func parseOktaUser(user *okta.User) (*UserData, error) { Email: oktaUser.Email, Name: strings.Join([]string{oktaUser.FirstName, oktaUser.LastName}, " "), ID: user.Id, - AppMetadata: AppMetadata{ - WTAccountID: oktaUser.AccountID, - WTPendingInvite: &oktaUser.PendingInvite, - }, }, nil } diff --git a/management/server/idp/okta_test.go b/management/server/idp/okta_test.go index 02c28b3aefb..20df246f82a 100644 --- a/management/server/idp/okta_test.go +++ b/management/server/idp/okta_test.go @@ -1,31 +1,28 @@ package idp import ( + "testing" + "github.com/okta/okta-sdk-golang/v2/okta" "github.com/stretchr/testify/assert" - "testing" ) func TestParseOktaUser(t *testing.T) { type parseOktaUserTest struct { name string - invite bool inputProfile *okta.User expectedUserData *UserData assertErrFunc assert.ErrorAssertionFunc } parseOktaTestCase1 := parseOktaUserTest{ - name: "Good Request", - invite: true, + name: "Good Request", inputProfile: &okta.User{ Id: "123", Profile: &okta.UserProfile{ - "email": "test@example.com", - "firstName": "John", - "lastName": "Doe", - "wt_account_id": "456", - "wt_pending_invite": true, + "email": "test@example.com", + "firstName": "John", + "lastName": "Doe", }, }, expectedUserData: &UserData{ @@ -41,36 +38,17 @@ func TestParseOktaUser(t *testing.T) { parseOktaTestCase2 := parseOktaUserTest{ name: "Invalid okta user", - invite: true, inputProfile: nil, expectedUserData: nil, assertErrFunc: assert.Error, } - parseOktaTestCase3 := parseOktaUserTest{ - name: "Invalid pending invite type", - invite: false, - inputProfile: &okta.User{ - Id: "123", - Profile: &okta.UserProfile{ - "email": "test@example.com", - "firstName": "John", - "lastName": "Doe", - "wt_account_id": "456", - "wt_pending_invite": "true", - }, - }, - expectedUserData: nil, - assertErrFunc: assert.Error, - } - - for _, testCase := range []parseOktaUserTest{parseOktaTestCase1, parseOktaTestCase2, parseOktaTestCase3} { + for _, testCase := range []parseOktaUserTest{parseOktaTestCase1, parseOktaTestCase2} { t.Run(testCase.name, func(t *testing.T) { userData, err := parseOktaUser(testCase.inputProfile) testCase.assertErrFunc(t, err, testCase.assertErrFunc) if err == nil { - testCase.expectedUserData.AppMetadata.WTPendingInvite = &testCase.invite assert.True(t, userDataEqual(testCase.expectedUserData, userData), "user data should match") } }) @@ -83,13 +61,5 @@ func userDataEqual(a, b *UserData) bool { if a.Email != b.Email || a.Name != b.Name || a.ID != b.ID { return false } - if a.AppMetadata.WTAccountID != b.AppMetadata.WTAccountID { - return false - } - - if a.AppMetadata.WTPendingInvite != nil && b.AppMetadata.WTPendingInvite != nil && - *a.AppMetadata.WTPendingInvite != *b.AppMetadata.WTPendingInvite { - return false - } return true } diff --git a/management/server/idp/zitadel.go b/management/server/idp/zitadel.go index e42b9a5064b..5325e51bebe 100644 --- a/management/server/idp/zitadel.go +++ b/management/server/idp/zitadel.go @@ -1,13 +1,10 @@ package idp import ( - "encoding/base64" - "encoding/json" "fmt" "io" "net/http" "net/url" - "strconv" "strings" "sync" "time" @@ -68,12 +65,6 @@ type zitadelUser struct { type zitadelAttributes map[string][]map[string]any -// zitadelMetadata holds additional user data. -type zitadelMetadata struct { - Key string `json:"key"` - Value string `json:"value"` -} - // zitadelProfile represents an zitadel user profile response. type zitadelProfile struct { ID string `json:"id"` @@ -82,7 +73,6 @@ type zitadelProfile struct { PreferredLoginName string `json:"preferredLoginName"` LoginNames []string `json:"loginNames"` Human *zitadelUser `json:"human"` - Metadata []zitadelMetadata } // NewZitadelManager creates a new instance of the ZitadelManager. @@ -235,42 +225,8 @@ func (zc *ZitadelCredentials) Authenticate() (JWTToken, error) { } // CreateUser creates a new user in zitadel Idp and sends an invite. -func (zm *ZitadelManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { - payload, err := buildZitadelCreateUserRequestPayload(email, name) - if err != nil { - return nil, err - } - - body, err := zm.post("users/human/_import", payload) - if err != nil { - return nil, err - } - - if zm.appMetrics != nil { - zm.appMetrics.IDPMetrics().CountCreateUser() - } - - var result struct { - UserID string `json:"userId"` - } - err = zm.helper.Unmarshal(body, &result) - if err != nil { - return nil, err - } - - invite := true - appMetadata := AppMetadata{ - WTAccountID: accountID, - WTPendingInvite: &invite, - } - - // Add metadata to new user - err = zm.UpdateUserAppMetadata(result.UserID, appMetadata) - if err != nil { - return nil, err - } - - return zm.GetUserDataByID(result.UserID, appMetadata) +func (zm *ZitadelManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserByEmail searches users with a given email. @@ -308,12 +264,6 @@ func (zm *ZitadelManager) GetUserByEmail(email string) ([]*UserData, error) { users := make([]*UserData, 0) for _, profile := range profiles.Result { - metadata, err := zm.getUserMetadata(profile.ID) - if err != nil { - return nil, err - } - profile.Metadata = metadata - users = append(users, profile.userData()) } @@ -337,18 +287,15 @@ func (zm *ZitadelManager) GetUserDataByID(userID string, appMetadata AppMetadata return nil, err } - metadata, err := zm.getUserMetadata(userID) - if err != nil { - return nil, err - } - profile.User.Metadata = metadata + userData := profile.User.userData() + userData.AppMetadata = appMetadata - return profile.User.userData(), nil + return userData, nil } // GetAccount returns all the users for a given profile. func (zm *ZitadelManager) GetAccount(accountID string) ([]*UserData, error) { - accounts, err := zm.GetAllAccounts() + body, err := zm.post("users/_search", "") if err != nil { return nil, err } @@ -357,7 +304,21 @@ func (zm *ZitadelManager) GetAccount(accountID string) ([]*UserData, error) { zm.appMetrics.IDPMetrics().CountGetAccount() } - return accounts[accountID], nil + var profiles struct{ Result []zitadelProfile } + err = zm.helper.Unmarshal(body, &profiles) + if err != nil { + return nil, err + } + + users := make([]*UserData, 0) + for _, profile := range profiles.Result { + userData := profile.userData() + userData.AppMetadata.WTAccountID = accountID + + users = append(users, userData) + } + + return users, nil } // GetAllAccounts gets all registered accounts with corresponding user data. @@ -380,22 +341,8 @@ func (zm *ZitadelManager) GetAllAccounts() (map[string][]*UserData, error) { indexedUsers := make(map[string][]*UserData) for _, profile := range profiles.Result { - // fetch user metadata - metadata, err := zm.getUserMetadata(profile.ID) - if err != nil { - return nil, err - } - profile.Metadata = metadata - userData := profile.userData() - accountID := userData.AppMetadata.WTAccountID - - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } + indexedUsers[UnsetAccountID] = append(indexedUsers[UnsetAccountID], userData) } return indexedUsers, nil @@ -403,42 +350,7 @@ func (zm *ZitadelManager) GetAllAccounts() (map[string][]*UserData, error) { // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. // Metadata values are base64 encoded. -func (zm *ZitadelManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - if appMetadata.WTPendingInvite == nil { - appMetadata.WTPendingInvite = new(bool) - } - pendingInviteBuf := strconv.AppendBool([]byte{}, *appMetadata.WTPendingInvite) - - wtAccountIDValue := base64.StdEncoding.EncodeToString([]byte(appMetadata.WTAccountID)) - wtPendingInviteValue := base64.StdEncoding.EncodeToString(pendingInviteBuf) - - metadata := zitadelAttributes{ - "metadata": { - { - "key": wtAccountID, - "value": wtAccountIDValue, - }, - { - "key": wtPendingInvite, - "value": wtPendingInviteValue, - }, - }, - } - payload, err := zm.helper.Marshal(metadata) - if err != nil { - return err - } - - resource := fmt.Sprintf("users/%s/metadata/_bulk", userID) - _, err = zm.post(resource, string(payload)) - if err != nil { - return err - } - - if zm.appMetrics != nil { - zm.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - +func (zm *ZitadelManager) UpdateUserAppMetadata(_ string, _ AppMetadata) error { return nil } @@ -460,24 +372,6 @@ func (zm *ZitadelManager) DeleteUser(userID string) error { } return nil - -} - -// getUserMetadata requests user metadata from zitadel via ID. -func (zm *ZitadelManager) getUserMetadata(userID string) ([]zitadelMetadata, error) { - resource := fmt.Sprintf("users/%s/metadata/_search", userID) - body, err := zm.post(resource, "") - if err != nil { - return nil, err - } - - var metadata struct{ Result []zitadelMetadata } - err = zm.helper.Unmarshal(body, &metadata) - if err != nil { - return nil, err - } - - return metadata.Result, nil } // post perform Post requests. @@ -517,38 +411,7 @@ func (zm *ZitadelManager) post(resource string, body string) ([]byte, error) { } // delete perform Delete requests. -func (zm *ZitadelManager) delete(resource string) error { - jwtToken, err := zm.credentials.Authenticate() - if err != nil { - return err - } - - reqURL := fmt.Sprintf("%s/%s", zm.managementEndpoint, resource) - req, err := http.NewRequest(http.MethodDelete, reqURL, nil) - if err != nil { - return err - } - req.Header.Add("authorization", "Bearer "+jwtToken.AccessToken) - req.Header.Add("content-type", "application/json") - - resp, err := zm.httpClient.Do(req) - if err != nil { - if zm.appMetrics != nil { - zm.appMetrics.IDPMetrics().CountRequestError() - } - - return err - } - defer resp.Body.Close() - - if resp.StatusCode != 200 { - if zm.appMetrics != nil { - zm.appMetrics.IDPMetrics().CountRequestStatusError() - } - - return fmt.Errorf("unable to delete %s, statusCode %d", reqURL, resp.StatusCode) - } - +func (zm *ZitadelManager) delete(_ string) error { return nil } @@ -588,38 +451,13 @@ func (zm *ZitadelManager) get(resource string, q url.Values) ([]byte, error) { return io.ReadAll(resp.Body) } -// value returns string represented by the base64 string value. -func (zm zitadelMetadata) value() string { - value, err := base64.StdEncoding.DecodeString(zm.Value) - if err != nil { - return "" - } - - return string(value) -} - // userData construct user data from zitadel profile. func (zp zitadelProfile) userData() *UserData { var ( - email string - name string - wtAccountIDValue string - wtPendingInviteValue bool + email string + name string ) - for _, metadata := range zp.Metadata { - if metadata.Key == wtAccountID { - wtAccountIDValue = metadata.value() - } - - if metadata.Key == wtPendingInvite { - value, err := strconv.ParseBool(metadata.value()) - if err == nil { - wtPendingInviteValue = value - } - } - } - // Obtain the email for the human account and the login name, // for the machine account. if zp.Human != nil { @@ -636,39 +474,5 @@ func (zp zitadelProfile) userData() *UserData { Email: email, Name: name, ID: zp.ID, - AppMetadata: AppMetadata{ - WTAccountID: wtAccountIDValue, - WTPendingInvite: &wtPendingInviteValue, - }, - } -} - -func buildZitadelCreateUserRequestPayload(email string, name string) (string, error) { - var firstName, lastName string - - words := strings.Fields(name) - if n := len(words); n > 0 { - firstName = strings.Join(words[:n-1], " ") - lastName = words[n-1] } - - req := &zitadelUser{ - UserName: name, - Profile: zitadelUserInfo{ - FirstName: strings.TrimSpace(firstName), - LastName: strings.TrimSpace(lastName), - DisplayName: name, - }, - Email: zitadelEmail{ - Email: email, - IsEmailVerified: false, - }, - } - - str, err := json.Marshal(req) - if err != nil { - return "", err - } - - return string(str), nil } diff --git a/management/server/idp/zitadel_test.go b/management/server/idp/zitadel_test.go index b558bba733d..9a771b36a7c 100644 --- a/management/server/idp/zitadel_test.go +++ b/management/server/idp/zitadel_test.go @@ -7,9 +7,10 @@ import ( "testing" "time" - "github.com/netbirdio/netbird/management/server/telemetry" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/server/telemetry" ) func TestNewZitadelManager(t *testing.T) { @@ -63,15 +64,6 @@ func TestNewZitadelManager(t *testing.T) { } } -type mockZitadelCredentials struct { - jwtToken JWTToken - err error -} - -func (mc *mockZitadelCredentials) Authenticate() (JWTToken, error) { - return mc.jwtToken, mc.err -} - func TestZitadelRequestJWTToken(t *testing.T) { type requestJWTTokenTest struct { @@ -296,98 +288,6 @@ func TestZitadelAuthenticate(t *testing.T) { } } -func TestZitadelUpdateUserAppMetadata(t *testing.T) { - type updateUserAppMetadataTest struct { - name string - inputReqBody string - expectedReqBody string - appMetadata AppMetadata - statusCode int - helper ManagerHelper - managerCreds ManagerCredentials - assertErrFunc assert.ErrorAssertionFunc - assertErrFuncMessage string - } - - appMetadata := AppMetadata{WTAccountID: "ok"} - - updateUserAppMetadataTestCase1 := updateUserAppMetadataTest{ - name: "Bad Authentication", - expectedReqBody: "", - appMetadata: appMetadata, - statusCode: 400, - helper: JsonParser{}, - managerCreds: &mockZitadelCredentials{ - jwtToken: JWTToken{}, - err: fmt.Errorf("error"), - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase2 := updateUserAppMetadataTest{ - name: "Bad Response Parsing", - statusCode: 400, - helper: &mockJsonParser{marshalErrorString: "error"}, - managerCreds: &mockZitadelCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase3 := updateUserAppMetadataTest{ - name: "Good request", - expectedReqBody: "{\"metadata\":[{\"key\":\"wt_account_id\",\"value\":\"b2s=\"},{\"key\":\"wt_pending_invite\",\"value\":\"ZmFsc2U=\"}]}", - appMetadata: appMetadata, - statusCode: 200, - helper: JsonParser{}, - managerCreds: &mockZitadelCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.NoError, - assertErrFuncMessage: "shouldn't return error", - } - - invite := true - updateUserAppMetadataTestCase4 := updateUserAppMetadataTest{ - name: "Update Pending Invite", - expectedReqBody: "{\"metadata\":[{\"key\":\"wt_account_id\",\"value\":\"b2s=\"},{\"key\":\"wt_pending_invite\",\"value\":\"dHJ1ZQ==\"}]}", - appMetadata: AppMetadata{ - WTAccountID: "ok", - WTPendingInvite: &invite, - }, - statusCode: 200, - helper: JsonParser{}, - managerCreds: &mockZitadelCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.NoError, - assertErrFuncMessage: "shouldn't return error", - } - - for _, testCase := range []updateUserAppMetadataTest{updateUserAppMetadataTestCase1, updateUserAppMetadataTestCase2, - updateUserAppMetadataTestCase3, updateUserAppMetadataTestCase4} { - t.Run(testCase.name, func(t *testing.T) { - reqClient := mockHTTPClient{ - resBody: testCase.inputReqBody, - code: testCase.statusCode, - } - - manager := &ZitadelManager{ - httpClient: &reqClient, - credentials: testCase.managerCreds, - helper: testCase.helper, - } - - err := manager.UpdateUserAppMetadata("1", testCase.appMetadata) - testCase.assertErrFunc(t, err, testCase.assertErrFuncMessage) - - assert.Equal(t, testCase.expectedReqBody, reqClient.reqBody, "request body should match") - }) - } -} - func TestZitadelProfile(t *testing.T) { type azureProfileTest struct { name string @@ -418,16 +318,6 @@ func TestZitadelProfile(t *testing.T) { IsEmailVerified: true, }, }, - Metadata: []zitadelMetadata{ - { - Key: "wt_account_id", - Value: "MQ==", - }, - { - Key: "wt_pending_invite", - Value: "ZmFsc2U=", - }, - }, }, expectedUserData: UserData{ ID: "test1", @@ -451,16 +341,6 @@ func TestZitadelProfile(t *testing.T) { "machine", }, Human: nil, - Metadata: []zitadelMetadata{ - { - Key: "wt_account_id", - Value: "MQ==", - }, - { - Key: "wt_pending_invite", - Value: "dHJ1ZQ==", - }, - }, }, expectedUserData: UserData{ ID: "test2", @@ -480,8 +360,6 @@ func TestZitadelProfile(t *testing.T) { assert.Equal(t, testCase.expectedUserData.ID, userData.ID, "User id should match") assert.Equal(t, testCase.expectedUserData.Email, userData.Email, "User email should match") assert.Equal(t, testCase.expectedUserData.Name, userData.Name, "User name should match") - assert.Equal(t, testCase.expectedUserData.AppMetadata.WTAccountID, userData.AppMetadata.WTAccountID, "Account id should match") - assert.Equal(t, testCase.expectedUserData.AppMetadata.WTPendingInvite, userData.AppMetadata.WTPendingInvite, "Pending invite should match") }) } } From 35bc493cc3c2fbaa6f7471a7d46578c9b9a2cbb2 Mon Sep 17 00:00:00 2001 From: Misha Bragin Date: Tue, 3 Oct 2023 16:46:58 +0200 Subject: [PATCH 12/14] Reorder peer deletion when deleteing a user (#1191) --- management/server/peer.go | 62 +++++++++------------ management/server/turncredentials.go | 6 +-- management/server/updatechannel.go | 7 ++- management/server/updatechannel_test.go | 15 ++---- management/server/user.go | 72 ++++++++++++++----------- 5 files changed, 72 insertions(+), 90 deletions(-) diff --git a/management/server/peer.go b/management/server/peer.go index 31ebd7cd5bf..4e9bbedfd58 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -375,18 +375,22 @@ func (am *DefaultAccountManager) UpdatePeer(accountID, userID string, update *Pe // deletePeers will delete all specified peers and send updates to the remote peers. Don't call without acquiring account lock func (am *DefaultAccountManager) deletePeers(account *Account, peerIDs []string, userID string) error { - var peerError error + // the first loop is needed to ensure all peers present under the account before modifying, otherwise + // we might have some inconsistencies + peers := make([]*Peer, 0, len(peerIDs)) for _, peerID := range peerIDs { peer := account.GetPeer(peerID) if peer == nil { - peerError = status.Errorf(status.NotFound, "peer %s not found", peerID) - goto save + return status.Errorf(status.NotFound, "peer %s not found", peerID) } + peers = append(peers, peer) + } - account.DeletePeer(peerID) - - peerError = am.peersUpdateManager.SendUpdate(peer.ID, + // the 2nd loop performs the actual modification + for _, peer := range peers { + account.DeletePeer(peer.ID) + am.peersUpdateManager.SendUpdate(peer.ID, &UpdateMessage{ Update: &proto.SyncResponse{ // fill those field for backward compatibility @@ -402,36 +406,11 @@ func (am *DefaultAccountManager) deletePeers(account *Account, peerIDs []string, }, }, }) - if peerError != nil { - goto save - } - - am.peersUpdateManager.CloseChannel(peerID) + am.peersUpdateManager.CloseChannel(peer.ID) am.storeEvent(userID, peer.ID, account.Id, activity.PeerRemovedByUser, peer.EventMeta(am.GetDNSDomain())) } -save: - err := am.Store.SaveAccount(account) - if err != nil { - if peerError != nil { - log.Errorf("account save error: %s", err) - return peerError - } else { - return err - } - } - - err = am.updateAccountPeers(account) - if err != nil { - if peerError != nil { - log.Errorf("update account peers error: %s", err) - return peerError - } else { - return err - } - } - - return peerError + return nil } // DeletePeer removes peer from the account by its IP @@ -444,7 +423,17 @@ func (am *DefaultAccountManager) DeletePeer(accountID, peerID, userID string) er return err } - return am.deletePeers(account, []string{peerID}, userID) + err = am.deletePeers(account, []string{peerID}, userID) + if err != nil { + return err + } + + err = am.Store.SaveAccount(account) + if err != nil { + return err + } + + return am.updateAccountPeers(account) } // GetPeerByIP returns peer by its IP @@ -943,10 +932,7 @@ func (am *DefaultAccountManager) updateAccountPeers(account *Account) error { } update := toSyncResponse(nil, peer, nil, remotePeerNetworkMap, am.GetDNSDomain()) - err = am.peersUpdateManager.SendUpdate(peer.ID, &UpdateMessage{Update: update}) - if err != nil { - return err - } + am.peersUpdateManager.SendUpdate(peer.ID, &UpdateMessage{Update: update}) } return nil diff --git a/management/server/turncredentials.go b/management/server/turncredentials.go index 1114aeeabac..aedcf2ee159 100644 --- a/management/server/turncredentials.go +++ b/management/server/turncredentials.go @@ -118,11 +118,7 @@ func (m *TimeBasedAuthSecretsManager) SetupRefresh(peerID string) { }, } log.Debugf("sending new TURN credentials to peer %s", peerID) - err := m.updateManager.SendUpdate(peerID, &UpdateMessage{Update: update}) - if err != nil { - log.Errorf("error while sending TURN update to peer %s %v", peerID, err) - // todo maybe continue trying? - } + m.updateManager.SendUpdate(peerID, &UpdateMessage{Update: update}) } } }() diff --git a/management/server/updatechannel.go b/management/server/updatechannel.go index 74438654735..5e6bcbb1cf0 100644 --- a/management/server/updatechannel.go +++ b/management/server/updatechannel.go @@ -29,7 +29,7 @@ func NewPeersUpdateManager() *PeersUpdateManager { } // SendUpdate sends update message to the peer's channel -func (p *PeersUpdateManager) SendUpdate(peerID string, update *UpdateMessage) error { +func (p *PeersUpdateManager) SendUpdate(peerID string, update *UpdateMessage) { p.channelsMux.Lock() defer p.channelsMux.Unlock() if channel, ok := p.peerChannels[peerID]; ok { @@ -39,10 +39,9 @@ func (p *PeersUpdateManager) SendUpdate(peerID string, update *UpdateMessage) er default: log.Warnf("channel for peer %s is %d full", peerID, len(channel)) } - return nil + } else { + log.Debugf("peer %s has no channel", peerID) } - log.Debugf("peer %s has no channel", peerID) - return nil } // CreateChannel creates a go channel for a given peer used to deliver updates relevant to the peer. diff --git a/management/server/updatechannel_test.go b/management/server/updatechannel_test.go index c37cd422870..6cfb4d52fc9 100644 --- a/management/server/updatechannel_test.go +++ b/management/server/updatechannel_test.go @@ -31,10 +31,7 @@ func TestSendUpdate(t *testing.T) { if _, ok := peersUpdater.peerChannels[peer]; !ok { t.Error("Error creating the channel") } - err := peersUpdater.SendUpdate(peer, update1) - if err != nil { - t.Error("Error sending update: ", err) - } + peersUpdater.SendUpdate(peer, update1) select { case <-peersUpdater.peerChannels[peer]: default: @@ -42,10 +39,7 @@ func TestSendUpdate(t *testing.T) { } for range [channelBufferSize]int{} { - err = peersUpdater.SendUpdate(peer, update1) - if err != nil { - t.Errorf("got an early error sending update: %v ", err) - } + peersUpdater.SendUpdate(peer, update1) } update2 := &UpdateMessage{Update: &proto.SyncResponse{ @@ -54,10 +48,7 @@ func TestSendUpdate(t *testing.T) { }, }} - err = peersUpdater.SendUpdate(peer, update2) - if err != nil { - t.Error("update shouldn't return an error when channel buffer is full") - } + peersUpdater.SendUpdate(peer, update2) timeout := time.After(5 * time.Second) for range [channelBufferSize]int{} { select { diff --git a/management/server/user.go b/management/server/user.go index a20a8b2b0b6..ed22bca709b 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -307,6 +307,12 @@ func (am *DefaultAccountManager) GetUser(claims jwtclaims.AuthorizationClaims) ( return user, nil } +func (am *DefaultAccountManager) deleteServiceUser(account *Account, initiatorUserID string, targetUser *User) { + meta := map[string]any{"name": targetUser.ServiceUserName} + am.storeEvent(initiatorUserID, targetUser.Id, account.Id, activity.ServiceUserDeleted, meta) + delete(account.Users, targetUser.Id) +} + // DeleteUser deletes a user from the given account. func (am *DefaultAccountManager) DeleteUser(accountID, initiatorUserID string, targetUserID string) error { if initiatorUserID == targetUserID { @@ -320,11 +326,6 @@ func (am *DefaultAccountManager) DeleteUser(accountID, initiatorUserID string, t return err } - targetUser := account.Users[targetUserID] - if targetUser == nil { - return status.Errorf(status.NotFound, "user not found") - } - executingUser := account.Users[initiatorUserID] if executingUser == nil { return status.Errorf(status.NotFound, "user not found") @@ -333,55 +334,64 @@ func (am *DefaultAccountManager) DeleteUser(accountID, initiatorUserID string, t return status.Errorf(status.PermissionDenied, "only admins can delete users") } - peers, err := account.FindUserPeers(targetUserID) - if err != nil { - return status.Errorf(status.Internal, "failed to find user peers") + targetUser := account.Users[targetUserID] + if targetUser == nil { + return status.Errorf(status.NotFound, "target user not found") } - peerIDs := make([]string, 0, len(peers)) - for _, peer := range peers { - peerIDs = append(peerIDs, peer.ID) + // handle service user first and exit, no need to fetch extra data from IDP, etc + if targetUser.IsServiceUser { + am.deleteServiceUser(account, initiatorUserID, targetUser) + return am.Store.SaveAccount(account) } - err = am.deletePeers(account, peerIDs, initiatorUserID) - if err != nil { - return err - } + return am.deleteRegularUser(account, initiatorUserID, targetUserID) +} +func (am *DefaultAccountManager) deleteRegularUser(account *Account, initiatorUserID, targetUserID string) error { tuEmail, tuName, err := am.getEmailAndNameOfTargetUser(account.Id, initiatorUserID, targetUserID) if err != nil { log.Errorf("failed to resolve email address: %s", err) return err } - var meta map[string]any - var eventAction activity.Activity - if targetUser.IsServiceUser { - meta = map[string]any{"name": targetUser.ServiceUserName} - eventAction = activity.ServiceUserDeleted - } else { - meta = map[string]any{"name": tuName, "email": tuEmail} - eventAction = activity.UserDeleted - } - am.storeEvent(initiatorUserID, targetUserID, accountID, eventAction, meta) - - if !targetUser.IsServiceUser && !isNil(am.idpManager) { - err := am.deleteUserFromIDP(targetUserID, accountID) + if !isNil(am.idpManager) { + err = am.deleteUserFromIDP(targetUserID, account.Id) if err != nil { log.Debugf("failed to delete user from IDP: %s", targetUserID) return err } } - delete(account.Users, targetUserID) + err = am.deleteUserPeers(initiatorUserID, targetUserID, account) + if err != nil { + return err + } - // todo should be unnecessary because we save account in the am.deletePeers + delete(account.Users, targetUserID) err = am.Store.SaveAccount(account) if err != nil { return err } - return nil + meta := map[string]any{"name": tuName, "email": tuEmail} + am.storeEvent(initiatorUserID, targetUserID, account.Id, activity.UserDeleted, meta) + + return am.updateAccountPeers(account) +} + +func (am *DefaultAccountManager) deleteUserPeers(initiatorUserID string, targetUserID string, account *Account) error { + peers, err := account.FindUserPeers(targetUserID) + if err != nil { + return status.Errorf(status.Internal, "failed to find user peers") + } + + peerIDs := make([]string, 0, len(peers)) + for _, peer := range peers { + peerIDs = append(peerIDs, peer.ID) + } + + return am.deletePeers(account, peerIDs, initiatorUserID) } // InviteUser resend invitations to users who haven't activated their accounts prior to the expiration period. From 26bbc33e7a17c113c98c333f638ce99bfab3bc57 Mon Sep 17 00:00:00 2001 From: Bethuel Mmbaga Date: Tue, 3 Oct 2023 20:33:42 +0300 Subject: [PATCH 13/14] Add jumpcloud IdP (#1124) added intergration with JumpCloud User API. Use the steps in setup.md for configuration. Additional changes: - Enhance compatibility for providers that lack audience support in the Authorization Code Flow and the Authorization - - Code Flow with Proof Key for Code Exchange (PKCE) using NETBIRD_DASH_AUTH_USE_AUDIENCE=falseenv - Verify tokens by utilizing the client ID when audience support is absent in providers --- client/internal/auth/pkce_flow.go | 8 +- client/internal/pkce_auth.go | 3 - go.mod | 1 + go.sum | 2 + infrastructure_files/base.setup.env | 13 +- infrastructure_files/configure.sh | 6 + infrastructure_files/docker-compose.yml.tmpl | 2 +- .../docker-compose.yml.tmpl.traefik | 2 +- infrastructure_files/management.json.tmpl | 2 +- infrastructure_files/setup.env.example | 3 + management/server/idp/idp.go | 6 +- management/server/idp/jumpcloud.go | 257 ++++++++++++++++++ management/server/idp/jumpcloud_test.go | 46 ++++ 13 files changed, 342 insertions(+), 9 deletions(-) create mode 100644 management/server/idp/jumpcloud.go create mode 100644 management/server/idp/jumpcloud_test.go diff --git a/client/internal/auth/pkce_flow.go b/client/internal/auth/pkce_flow.go index a3d0c130984..32f5383d36e 100644 --- a/client/internal/auth/pkce_flow.go +++ b/client/internal/auth/pkce_flow.go @@ -197,7 +197,13 @@ func (p *PKCEAuthorizationFlow) parseOAuthToken(token *oauth2.Token) (TokenInfo, tokenInfo.IDToken = idToken } - if err := isValidAccessToken(tokenInfo.GetTokenToUse(), p.providerConfig.Audience); err != nil { + // if a provider doesn't support an audience, use the Client ID for token verification + audience := p.providerConfig.Audience + if audience == "" { + audience = p.providerConfig.ClientID + } + + if err := isValidAccessToken(tokenInfo.GetTokenToUse(), audience); err != nil { return TokenInfo{}, fmt.Errorf("validate access token failed with error: %v", err) } diff --git a/client/internal/pkce_auth.go b/client/internal/pkce_auth.go index 2efbae97b5f..a35dacc77be 100644 --- a/client/internal/pkce_auth.go +++ b/client/internal/pkce_auth.go @@ -106,9 +106,6 @@ func GetPKCEAuthorizationFlowInfo(ctx context.Context, privateKey string, mgmURL func isPKCEProviderConfigValid(config PKCEAuthProviderConfig) error { errorMSGFormat := "invalid provider configuration received from management: %s value is empty. Contact your NetBird administrator" - if config.Audience == "" { - return fmt.Errorf(errorMSGFormat, "Audience") - } if config.ClientID == "" { return fmt.Errorf(errorMSGFormat, "Client ID") } diff --git a/go.mod b/go.mod index 7ecf61584ce..8be1599970a 100644 --- a/go.mod +++ b/go.mod @@ -29,6 +29,7 @@ require ( require ( fyne.io/fyne/v2 v2.1.4 + github.com/TheJumpCloud/jcapi-go v3.0.0+incompatible github.com/c-robinson/iplib v1.0.3 github.com/cilium/ebpf v0.10.0 github.com/coreos/go-iptables v0.7.0 diff --git a/go.sum b/go.sum index 1eb9d243dee..25182ca85df 100644 --- a/go.sum +++ b/go.sum @@ -61,6 +61,8 @@ github.com/PuerkitoBio/purell v1.0.0/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbt github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= +github.com/TheJumpCloud/jcapi-go v3.0.0+incompatible h1:hqcTK6ZISdip65SR792lwYJTa/axESA0889D3UlZbLo= +github.com/TheJumpCloud/jcapi-go v3.0.0+incompatible/go.mod h1:6B1nuc1MUs6c62ODZDl7hVE5Pv7O2XGSkgg2olnq34I= github.com/XiaoMi/pegasus-go-client v0.0.0-20210427083443-f3b6b08bc4c2 h1:pami0oPhVosjOu/qRHepRmdjD6hGILF7DBr+qQZeP10= github.com/XiaoMi/pegasus-go-client v0.0.0-20210427083443-f3b6b08bc4c2/go.mod h1:jNIx5ykW1MroBuaTja9+VpglmaJOUzezumfhLlER3oY= github.com/akavel/rsrc v0.8.0/go.mod h1:uLoCtb9J+EyAqh+26kdrTgmzRBFPGOolLWKpdxkKq+c= diff --git a/infrastructure_files/base.setup.env b/infrastructure_files/base.setup.env index 4bcec128df9..f610a9691bc 100644 --- a/infrastructure_files/base.setup.env +++ b/infrastructure_files/base.setup.env @@ -46,6 +46,14 @@ NETBIRD_TOKEN_SOURCE=${NETBIRD_TOKEN_SOURCE:-accessToken} # PKCE authorization flow NETBIRD_AUTH_PKCE_REDIRECT_URL_PORTS=${NETBIRD_AUTH_PKCE_REDIRECT_URL_PORTS:-"53000"} NETBIRD_AUTH_PKCE_USE_ID_TOKEN=${NETBIRD_AUTH_PKCE_USE_ID_TOKEN:-false} +NETBIRD_AUTH_PKCE_AUDIENCE=$NETBIRD_AUTH_AUDIENCE + +# Dashboard + +# The default setting is to transmit the audience to the IDP during authorization. However, +# if your IDP does not have this capability, you can turn this off by setting it to false. +NETBIRD_DASH_AUTH_USE_AUDIENCE=${NETBIRD_DASH_AUTH_USE_AUDIENCE:-true} +NETBIRD_DASH_AUTH_AUDIENCE=$NETBIRD_AUTH_AUDIENCE # exports export NETBIRD_DOMAIN @@ -86,4 +94,7 @@ export NETBIRD_TOKEN_SOURCE export NETBIRD_AUTH_DEVICE_AUTH_SCOPE export NETBIRD_AUTH_DEVICE_AUTH_USE_ID_TOKEN export NETBIRD_AUTH_PKCE_AUTHORIZATION_ENDPOINT -export NETBIRD_AUTH_PKCE_USE_ID_TOKEN \ No newline at end of file +export NETBIRD_AUTH_PKCE_USE_ID_TOKEN +export NETBIRD_AUTH_PKCE_AUDIENCE +export NETBIRD_DASH_AUTH_USE_AUDIENCE +export NETBIRD_DASH_AUTH_AUDIENCE \ No newline at end of file diff --git a/infrastructure_files/configure.sh b/infrastructure_files/configure.sh index 4e568b2fef4..3db79906827 100755 --- a/infrastructure_files/configure.sh +++ b/infrastructure_files/configure.sh @@ -164,6 +164,12 @@ done export NETBIRD_AUTH_PKCE_REDIRECT_URLS=${REDIRECT_URLS%,} +# Remove audience for providers that do not support it +if [ "$NETBIRD_DASH_AUTH_USE_AUDIENCE" = "false" ]; then + export NETBIRD_DASH_AUTH_AUDIENCE=none + export NETBIRD_AUTH_PKCE_AUDIENCE= +fi + env | grep NETBIRD envsubst docker-compose.yml diff --git a/infrastructure_files/docker-compose.yml.tmpl b/infrastructure_files/docker-compose.yml.tmpl index b70e4cb6e0b..c5ea3ae56b0 100644 --- a/infrastructure_files/docker-compose.yml.tmpl +++ b/infrastructure_files/docker-compose.yml.tmpl @@ -12,7 +12,7 @@ services: - NETBIRD_MGMT_API_ENDPOINT=$NETBIRD_MGMT_API_ENDPOINT - NETBIRD_MGMT_GRPC_API_ENDPOINT=$NETBIRD_MGMT_API_ENDPOINT # OIDC - - AUTH_AUDIENCE=$NETBIRD_AUTH_AUDIENCE + - AUTH_AUDIENCE=$NETBIRD_DASH_AUTH_AUDIENCE - AUTH_CLIENT_ID=$NETBIRD_AUTH_CLIENT_ID - AUTH_CLIENT_SECRET=$NETBIRD_AUTH_CLIENT_SECRET - AUTH_AUTHORITY=$NETBIRD_AUTH_AUTHORITY diff --git a/infrastructure_files/docker-compose.yml.tmpl.traefik b/infrastructure_files/docker-compose.yml.tmpl.traefik index 3c18ed9172c..cab471df646 100644 --- a/infrastructure_files/docker-compose.yml.tmpl.traefik +++ b/infrastructure_files/docker-compose.yml.tmpl.traefik @@ -12,7 +12,7 @@ services: - NETBIRD_MGMT_API_ENDPOINT=$NETBIRD_MGMT_API_ENDPOINT - NETBIRD_MGMT_GRPC_API_ENDPOINT=$NETBIRD_MGMT_API_ENDPOINT # OIDC - - AUTH_AUDIENCE=$NETBIRD_AUTH_AUDIENCE + - AUTH_AUDIENCE=$NETBIRD_DASH_AUTH_AUDIENCE - AUTH_CLIENT_ID=$NETBIRD_AUTH_CLIENT_ID - AUTH_CLIENT_SECRET=$NETBIRD_AUTH_CLIENT_SECRET - AUTH_AUTHORITY=$NETBIRD_AUTH_AUTHORITY diff --git a/infrastructure_files/management.json.tmpl b/infrastructure_files/management.json.tmpl index e74b93b32ac..e185faa6ebd 100644 --- a/infrastructure_files/management.json.tmpl +++ b/infrastructure_files/management.json.tmpl @@ -62,7 +62,7 @@ }, "PKCEAuthorizationFlow": { "ProviderConfig": { - "Audience": "$NETBIRD_AUTH_AUDIENCE", + "Audience": "$NETBIRD_AUTH_PKCE_AUDIENCE", "ClientID": "$NETBIRD_AUTH_CLIENT_ID", "ClientSecret": "$NETBIRD_AUTH_CLIENT_SECRET", "AuthorizationEndpoint": "$NETBIRD_AUTH_PKCE_AUTHORIZATION_ENDPOINT", diff --git a/infrastructure_files/setup.env.example b/infrastructure_files/setup.env.example index 9b03ccd2d77..f9ad638465f 100644 --- a/infrastructure_files/setup.env.example +++ b/infrastructure_files/setup.env.example @@ -8,6 +8,9 @@ NETBIRD_DOMAIN="" # e.g., https://example.eu.auth0.com/.well-known/openid-configuration # ------------------------------------------- NETBIRD_AUTH_OIDC_CONFIGURATION_ENDPOINT="" +# The default setting is to transmit the audience to the IDP during authorization. However, +# if your IDP does not have this capability, you can turn this off by setting it to false. +#NETBIRD_DASH_AUTH_USE_AUDIENCE=false NETBIRD_AUTH_AUDIENCE="" # e.g. netbird-client NETBIRD_AUTH_CLIENT_ID="" diff --git a/management/server/idp/idp.go b/management/server/idp/idp.go index 3b6a633c89e..7adb76f4044 100644 --- a/management/server/idp/idp.go +++ b/management/server/idp/idp.go @@ -176,7 +176,11 @@ func NewManager(config Config, appMetrics telemetry.AppMetrics) (Manager, error) CustomerID: config.ExtraConfig["CustomerId"], } return NewGoogleWorkspaceManager(googleClientConfig, appMetrics) - + case "jumpcloud": + jumpcloudConfig := JumpCloudClientConfig{ + APIToken: config.ExtraConfig["ApiToken"], + } + return NewJumpCloudManager(jumpcloudConfig, appMetrics) default: return nil, fmt.Errorf("invalid manager type: %s", config.ManagerType) } diff --git a/management/server/idp/jumpcloud.go b/management/server/idp/jumpcloud.go new file mode 100644 index 00000000000..0115b404982 --- /dev/null +++ b/management/server/idp/jumpcloud.go @@ -0,0 +1,257 @@ +package idp + +import ( + "context" + "fmt" + "net/http" + "strings" + "time" + + v1 "github.com/TheJumpCloud/jcapi-go/v1" + + "github.com/netbirdio/netbird/management/server/telemetry" +) + +const ( + contentType = "application/json" + accept = "application/json" +) + +// JumpCloudManager JumpCloud manager client instance. +type JumpCloudManager struct { + client *v1.APIClient + apiToken string + httpClient ManagerHTTPClient + credentials ManagerCredentials + helper ManagerHelper + appMetrics telemetry.AppMetrics +} + +// JumpCloudClientConfig JumpCloud manager client configurations. +type JumpCloudClientConfig struct { + APIToken string +} + +// JumpCloudCredentials JumpCloud authentication information. +type JumpCloudCredentials struct { + clientConfig JumpCloudClientConfig + helper ManagerHelper + httpClient ManagerHTTPClient + appMetrics telemetry.AppMetrics +} + +// NewJumpCloudManager creates a new instance of the JumpCloudManager. +func NewJumpCloudManager(config JumpCloudClientConfig, appMetrics telemetry.AppMetrics) (*JumpCloudManager, error) { + httpTransport := http.DefaultTransport.(*http.Transport).Clone() + httpTransport.MaxIdleConns = 5 + + httpClient := &http.Client{ + Timeout: 10 * time.Second, + Transport: httpTransport, + } + helper := JsonParser{} + + if config.APIToken == "" { + return nil, fmt.Errorf("jumpCloud IdP configuration is incomplete, ApiToken is missing") + } + + client := v1.NewAPIClient(v1.NewConfiguration()) + credentials := &JumpCloudCredentials{ + clientConfig: config, + httpClient: httpClient, + helper: helper, + appMetrics: appMetrics, + } + + return &JumpCloudManager{ + client: client, + apiToken: config.APIToken, + httpClient: httpClient, + credentials: credentials, + helper: helper, + appMetrics: appMetrics, + }, nil +} + +// Authenticate retrieves access token to use the JumpCloud user API. +func (jc *JumpCloudCredentials) Authenticate() (JWTToken, error) { + return JWTToken{}, nil +} + +func (jm *JumpCloudManager) authenticationContext() context.Context { + return context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ + Key: jm.apiToken, + }) +} + +// UpdateUserAppMetadata updates user app metadata based on userID and metadata map. +func (jm *JumpCloudManager) UpdateUserAppMetadata(_ string, _ AppMetadata) error { + return nil +} + +// GetUserDataByID requests user data from JumpCloud via ID. +func (jm *JumpCloudManager) GetUserDataByID(userID string, appMetadata AppMetadata) (*UserData, error) { + authCtx := jm.authenticationContext() + user, resp, err := jm.client.SystemusersApi.SystemusersGet(authCtx, userID, contentType, accept, nil) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountRequestStatusError() + } + return nil, fmt.Errorf("unable to get user %s, statusCode %d", userID, resp.StatusCode) + } + + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountGetUserDataByID() + } + + userData := parseJumpCloudUser(user) + userData.AppMetadata = appMetadata + + return userData, nil +} + +// GetAccount returns all the users for a given profile. +func (jm *JumpCloudManager) GetAccount(accountID string) ([]*UserData, error) { + authCtx := jm.authenticationContext() + userList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, nil) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountRequestStatusError() + } + return nil, fmt.Errorf("unable to get account %s users, statusCode %d", accountID, resp.StatusCode) + } + + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountGetAccount() + } + + users := make([]*UserData, 0) + for _, user := range userList.Results { + userData := parseJumpCloudUser(user) + userData.AppMetadata.WTAccountID = accountID + + users = append(users, userData) + } + + return users, nil +} + +// GetAllAccounts gets all registered accounts with corresponding user data. +// It returns a list of users indexed by accountID. +func (jm *JumpCloudManager) GetAllAccounts() (map[string][]*UserData, error) { + authCtx := jm.authenticationContext() + userList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, nil) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountRequestStatusError() + } + return nil, fmt.Errorf("unable to get all accounts, statusCode %d", resp.StatusCode) + } + + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountGetAllAccounts() + } + + indexedUsers := make(map[string][]*UserData) + for _, user := range userList.Results { + userData := parseJumpCloudUser(user) + indexedUsers[UnsetAccountID] = append(indexedUsers[UnsetAccountID], userData) + } + + return indexedUsers, nil +} + +// CreateUser creates a new user in JumpCloud Idp and sends an invitation. +func (jm *JumpCloudManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") +} + +// GetUserByEmail searches users with a given email. +// If no users have been found, this function returns an empty list. +func (jm *JumpCloudManager) GetUserByEmail(email string) ([]*UserData, error) { + searchFilter := map[string]interface{}{ + "searchFilter": map[string]interface{}{ + "filter": []string{email}, + "fields": []string{"email"}, + }, + } + + authCtx := jm.authenticationContext() + userList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, searchFilter) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountRequestStatusError() + } + return nil, fmt.Errorf("unable to get user %s, statusCode %d", email, resp.StatusCode) + } + + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountGetUserByEmail() + } + + usersData := make([]*UserData, 0) + for _, user := range userList.Results { + usersData = append(usersData, parseJumpCloudUser(user)) + } + + return usersData, nil +} + +// InviteUserByID resend invitations to users who haven't activated, +// their accounts prior to the expiration period. +func (jm *JumpCloudManager) InviteUserByID(_ string) error { + return fmt.Errorf("method InviteUserByID not implemented") +} + +// DeleteUser from jumpCloud directory +func (jm *JumpCloudManager) DeleteUser(userID string) error { + authCtx := jm.authenticationContext() + _, resp, err := jm.client.SystemusersApi.SystemusersDelete(authCtx, userID, contentType, accept, nil) + if err != nil { + return err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountRequestStatusError() + } + return fmt.Errorf("unable to delete user, statusCode %d", resp.StatusCode) + } + + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountDeleteUser() + } + + return nil +} + +// parseJumpCloudUser parse JumpCloud system user returned from API V1 to UserData. +func parseJumpCloudUser(user v1.Systemuserreturn) *UserData { + names := []string{user.Firstname, user.Middlename, user.Lastname} + return &UserData{ + Email: user.Email, + Name: strings.Join(names, " "), + ID: user.Id, + } +} diff --git a/management/server/idp/jumpcloud_test.go b/management/server/idp/jumpcloud_test.go new file mode 100644 index 00000000000..1bfdcefcc70 --- /dev/null +++ b/management/server/idp/jumpcloud_test.go @@ -0,0 +1,46 @@ +package idp + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/server/telemetry" +) + +func TestNewJumpCloudManager(t *testing.T) { + type test struct { + name string + inputConfig JumpCloudClientConfig + assertErrFunc require.ErrorAssertionFunc + assertErrFuncMessage string + } + + defaultTestConfig := JumpCloudClientConfig{ + APIToken: "test123", + } + + testCase1 := test{ + name: "Good Configuration", + inputConfig: defaultTestConfig, + assertErrFunc: require.NoError, + assertErrFuncMessage: "shouldn't return error", + } + + testCase2Config := defaultTestConfig + testCase2Config.APIToken = "" + + testCase2 := test{ + name: "Missing APIToken Configuration", + inputConfig: testCase2Config, + assertErrFunc: require.Error, + assertErrFuncMessage: "should return error when field empty", + } + + for _, testCase := range []test{testCase1, testCase2} { + t.Run(testCase.name, func(t *testing.T) { + _, err := NewJumpCloudManager(testCase.inputConfig, &telemetry.MockAppMetrics{}) + testCase.assertErrFunc(t, err, testCase.assertErrFuncMessage) + }) + } +} From 9131069d12a9c9c600751bad45ba8e1078cfa702 Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Wed, 4 Oct 2023 15:08:50 +0200 Subject: [PATCH 14/14] Improve updateAccountPeers by bypassing AM and using account directly (#1193) Improve updateAccountPeers performance by bypassing AM and using the account directly --- management/server/account.go | 4 +-- management/server/dns.go | 4 ++- management/server/group.go | 17 +++++++------ management/server/nameserver.go | 18 +++----------- management/server/peer.go | 43 ++++++++++----------------------- management/server/policy.go | 8 ++++-- management/server/route.go | 16 ++++-------- management/server/setupkey.go | 4 ++- management/server/user.go | 12 ++++----- 9 files changed, 49 insertions(+), 77 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index b8bfd5770e7..0b741797c49 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -1491,9 +1491,7 @@ func (am *DefaultAccountManager) GetAccountFromToken(claims jwtclaims.Authorizat if err := am.Store.SaveAccount(account); err != nil { log.Errorf("failed to save account: %v", err) } else { - if err := am.updateAccountPeers(account); err != nil { - log.Errorf("failed updating account peers while updating user %s", account.Id) - } + am.updateAccountPeers(account) for _, g := range addNewGroups { if group := account.GetGroup(g); group != nil { am.storeEvent(user.Id, user.Id, account.Id, activity.GroupAddedToUser, diff --git a/management/server/dns.go b/management/server/dns.go index 427ba40d120..252782aeaac 100644 --- a/management/server/dns.go +++ b/management/server/dns.go @@ -122,7 +122,9 @@ func (am *DefaultAccountManager) SaveDNSSettings(accountID string, userID string am.storeEvent(userID, accountID, accountID, activity.GroupRemovedFromDisabledManagementGroups, meta) } - return am.updateAccountPeers(account) + am.updateAccountPeers(account) + + return nil } func toProtocolDNSConfig(update nbdns.Config) *proto.DNSConfig { diff --git a/management/server/group.go b/management/server/group.go index 0aebc645403..9e5ede3f48b 100644 --- a/management/server/group.go +++ b/management/server/group.go @@ -84,10 +84,7 @@ func (am *DefaultAccountManager) SaveGroup(accountID, userID string, newGroup *G return err } - err = am.updateAccountPeers(account) - if err != nil { - return err - } + am.updateAccountPeers(account) // the following snippet tracks the activity and stores the group events in the event store. // It has to happen after all the operations have been successfully performed. @@ -229,7 +226,9 @@ func (am *DefaultAccountManager) DeleteGroup(accountId, userId, groupID string) am.storeEvent(userId, groupID, accountId, activity.GroupDeleted, g.EventMeta()) - return am.updateAccountPeers(account) + am.updateAccountPeers(account) + + return nil } // ListGroups objects of the peers @@ -281,7 +280,9 @@ func (am *DefaultAccountManager) GroupAddPeer(accountID, groupID, peerID string) return err } - return am.updateAccountPeers(account) + am.updateAccountPeers(account) + + return nil } // GroupDeletePeer removes peer from the group @@ -309,7 +310,9 @@ func (am *DefaultAccountManager) GroupDeletePeer(accountID, groupID, peerID stri } } - return am.updateAccountPeers(account) + am.updateAccountPeers(account) + + return nil } // GroupListPeers returns list of the peers from the group diff --git a/management/server/nameserver.go b/management/server/nameserver.go index 7025388baab..9af5b49adc8 100644 --- a/management/server/nameserver.go +++ b/management/server/nameserver.go @@ -7,7 +7,6 @@ import ( "github.com/miekg/dns" "github.com/rs/xid" - log "github.com/sirupsen/logrus" nbdns "github.com/netbirdio/netbird/dns" "github.com/netbirdio/netbird/management/server/activity" @@ -74,11 +73,7 @@ func (am *DefaultAccountManager) CreateNameServerGroup(accountID string, name, d return nil, err } - err = am.updateAccountPeers(account) - if err != nil { - log.Error(err) - return newNSGroup.Copy(), status.Errorf(status.Internal, "failed to update peers after create nameserver %s", name) - } + am.updateAccountPeers(account) am.storeEvent(userID, newNSGroup.ID, accountID, activity.NameserverGroupCreated, newNSGroup.EventMeta()) @@ -113,11 +108,7 @@ func (am *DefaultAccountManager) SaveNameServerGroup(accountID, userID string, n return err } - err = am.updateAccountPeers(account) - if err != nil { - log.Error(err) - return status.Errorf(status.Internal, "failed to update peers after update nameserver %s", nsGroupToSave.Name) - } + am.updateAccountPeers(account) am.storeEvent(userID, nsGroupToSave.ID, accountID, activity.NameserverGroupUpdated, nsGroupToSave.EventMeta()) @@ -147,10 +138,7 @@ func (am *DefaultAccountManager) DeleteNameServerGroup(accountID, nsGroupID, use return err } - err = am.updateAccountPeers(account) - if err != nil { - return status.Errorf(status.Internal, "failed to update peers after deleting nameserver %s", nsGroupID) - } + am.updateAccountPeers(account) am.storeEvent(userID, nsGroup.ID, accountID, activity.NameserverGroupDeleted, nsGroup.EventMeta()) diff --git a/management/server/peer.go b/management/server/peer.go index 4e9bbedfd58..8e4ad08e3d0 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -290,10 +290,7 @@ func (am *DefaultAccountManager) MarkPeerConnected(peerPubKey string, connected if oldStatus.LoginExpired { // we need to update other peers because when peer login expires all other peers are notified to disconnect from // the expired one. Here we notify them that connection is now allowed again. - err = am.updateAccountPeers(account) - if err != nil { - return err - } + am.updateAccountPeers(account) } return nil @@ -364,10 +361,7 @@ func (am *DefaultAccountManager) UpdatePeer(accountID, userID string, update *Pe return nil, err } - err = am.updateAccountPeers(account) - if err != nil { - return nil, err - } + am.updateAccountPeers(account) return peer, nil } @@ -433,7 +427,9 @@ func (am *DefaultAccountManager) DeletePeer(accountID, peerID, userID string) er return err } - return am.updateAccountPeers(account) + am.updateAccountPeers(account) + + return nil } // GetPeerByIP returns peer by its IP @@ -622,10 +618,7 @@ func (am *DefaultAccountManager) AddPeer(setupKey, userID string, peer *Peer) (* opEvent.Meta = newPeer.EventMeta(am.GetDNSDomain()) am.storeEvent(opEvent.InitiatorID, opEvent.TargetID, opEvent.AccountID, opEvent.Activity, opEvent.Meta) - err = am.updateAccountPeers(account) - if err != nil { - return nil, nil, err - } + am.updateAccountPeers(account) networkMap := account.GetPeerNetworkMap(newPeer.ID, am.dnsDomain) return newPeer, networkMap, nil @@ -740,10 +733,7 @@ func (am *DefaultAccountManager) LoginPeer(login PeerLogin) (*Peer, *NetworkMap, } if updateRemotePeers { - err = am.updateAccountPeers(account) - if err != nil { - return nil, nil, err - } + am.updateAccountPeers(account) } return peer, account.GetPeerNetworkMap(peer.ID, am.dnsDomain), nil } @@ -817,10 +807,7 @@ func (am *DefaultAccountManager) checkAndUpdatePeerSSHKey(peer *Peer, account *A } // trigger network map update - err = am.updateAccountPeers(account) - if err != nil { - return nil, err - } + am.updateAccountPeers(account) return peer, nil } @@ -865,7 +852,9 @@ func (am *DefaultAccountManager) UpdatePeerSSHKey(peerID string, sshKey string) } // trigger network map update - return am.updateAccountPeers(account) + am.updateAccountPeers(account) + + return nil } // GetPeer for a given accountID, peerID and userID error if not found. @@ -922,18 +911,12 @@ func updatePeerMeta(peer *Peer, meta PeerSystemMeta, account *Account) (*Peer, b // updateAccountPeers updates all peers that belong to an account. // Should be called when changes have to be synced to peers. -func (am *DefaultAccountManager) updateAccountPeers(account *Account) error { +func (am *DefaultAccountManager) updateAccountPeers(account *Account) { peers := account.GetPeers() for _, peer := range peers { - remotePeerNetworkMap, err := am.GetNetworkMap(peer.ID) - if err != nil { - return err - } - + remotePeerNetworkMap := account.GetPeerNetworkMap(peer.ID, am.dnsDomain) update := toSyncResponse(nil, peer, nil, remotePeerNetworkMap, am.GetDNSDomain()) am.peersUpdateManager.SendUpdate(peer.ID, &UpdateMessage{Update: update}) } - - return nil } diff --git a/management/server/policy.go b/management/server/policy.go index dde0b46d8b5..308a5c3c0db 100644 --- a/management/server/policy.go +++ b/management/server/policy.go @@ -350,7 +350,9 @@ func (am *DefaultAccountManager) SavePolicy(accountID, userID string, policy *Po } am.storeEvent(userID, policy.ID, accountID, action, policy.EventMeta()) - return am.updateAccountPeers(account) + am.updateAccountPeers(account) + + return nil } // DeletePolicy from the store @@ -375,7 +377,9 @@ func (am *DefaultAccountManager) DeletePolicy(accountID, policyID, userID string am.storeEvent(userID, policy.ID, accountID, activity.PolicyRemoved, policy.EventMeta()) - return am.updateAccountPeers(account) + am.updateAccountPeers(account) + + return nil } // ListPolicies from the store diff --git a/management/server/route.go b/management/server/route.go index cdd21420350..79c207c9bad 100644 --- a/management/server/route.go +++ b/management/server/route.go @@ -9,7 +9,6 @@ import ( "github.com/netbirdio/netbird/management/server/status" "github.com/netbirdio/netbird/route" "github.com/rs/xid" - log "github.com/sirupsen/logrus" ) // GetRoute gets a route object from account and route IDs @@ -185,11 +184,7 @@ func (am *DefaultAccountManager) CreateRoute(accountID, network, peerID string, return nil, err } - err = am.updateAccountPeers(account) - if err != nil { - log.Error(err) - return &newRoute, status.Errorf(status.Internal, "failed to update peers after create route %s", newPrefix) - } + am.updateAccountPeers(account) am.storeEvent(userID, newRoute.ID, accountID, activity.RouteCreated, newRoute.EventMeta()) @@ -250,10 +245,7 @@ func (am *DefaultAccountManager) SaveRoute(accountID, userID string, routeToSave return err } - err = am.updateAccountPeers(account) - if err != nil { - return err - } + am.updateAccountPeers(account) am.storeEvent(userID, routeToSave.ID, accountID, activity.RouteUpdated, routeToSave.EventMeta()) @@ -283,7 +275,9 @@ func (am *DefaultAccountManager) DeleteRoute(accountID, routeID, userID string) am.storeEvent(userID, routy.ID, accountID, activity.RouteRemoved, routy.EventMeta()) - return am.updateAccountPeers(account) + am.updateAccountPeers(account) + + return nil } // ListRoutes returns a list of routes from account diff --git a/management/server/setupkey.go b/management/server/setupkey.go index e857230a577..6e626d08411 100644 --- a/management/server/setupkey.go +++ b/management/server/setupkey.go @@ -317,7 +317,9 @@ func (am *DefaultAccountManager) SaveSetupKey(accountID string, keyToSave *Setup } }() - return newKey, am.updateAccountPeers(account) + am.updateAccountPeers(account) + + return newKey, nil } // ListSetupKeys returns a list of all setup keys of the account diff --git a/management/server/user.go b/management/server/user.go index ed22bca709b..3169c784f14 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -377,7 +377,9 @@ func (am *DefaultAccountManager) deleteRegularUser(account *Account, initiatorUs meta := map[string]any{"name": tuName, "email": tuEmail} am.storeEvent(initiatorUserID, targetUserID, account.Id, activity.UserDeleted, meta) - return am.updateAccountPeers(account) + am.updateAccountPeers(account) + + return nil } func (am *DefaultAccountManager) deleteUserPeers(initiatorUserID string, targetUserID string, account *Account) error { @@ -674,9 +676,7 @@ func (am *DefaultAccountManager) SaveUser(accountID, initiatorUserID string, upd return nil, err } - if err := am.updateAccountPeers(account); err != nil { - log.Errorf("failed updating account peers while updating user %s", accountID) - } + am.updateAccountPeers(account) } else { if err = am.Store.SaveAccount(account); err != nil { return nil, err @@ -870,9 +870,7 @@ func (am *DefaultAccountManager) expireAndUpdatePeers(account *Account, peers [] if len(peerIDs) != 0 { // this will trigger peer disconnect from the management service am.peersUpdateManager.CloseChannels(peerIDs) - if err := am.updateAccountPeers(account); err != nil { - return err - } + am.updateAccountPeers(account) } return nil }