From 76a39c1dcbc165408b6e5059f2882e68edd8d718 Mon Sep 17 00:00:00 2001 From: Alisdair MacLeod Date: Tue, 3 Feb 2026 10:03:38 +0000 Subject: [PATCH] Revert "add management side of OIDC authentication" This reverts commit 02ce918114d02e6aa4058ad6161c3066390be698. --- .../modules/reverseproxy/manager/manager.go | 23 ++------------- .../modules/reverseproxy/reverseproxy.go | 19 ++++-------- management/internals/shared/grpc/proxy.go | 29 +++++-------------- shared/management/http/api/openapi.yml | 3 -- shared/management/http/api/types.gen.go | 3 -- 5 files changed, 16 insertions(+), 61 deletions(-) diff --git a/management/internals/modules/reverseproxy/manager/manager.go b/management/internals/modules/reverseproxy/manager/manager.go index 9960fd4be..5ed089c84 100644 --- a/management/internals/modules/reverseproxy/manager/manager.go +++ b/management/internals/modules/reverseproxy/manager/manager.go @@ -145,12 +145,7 @@ func (m *managerImpl) CreateReverseProxy(ctx context.Context, accountID, userID return nil, fmt.Errorf("failed to create setup key for reverse proxy: %w", err) } - idp, err := m.getIdentityProvider(ctx, accountID, userID, reverseProxy.Auth.BearerAuth) - if err != nil { - return nil, fmt.Errorf("failed to get identity provider: %w", err) - } - - m.proxyGRPCServer.SendReverseProxyUpdate(reverseProxy.ToProtoMapping(reverseproxy.Create, key.Key, idp)) + m.proxyGRPCServer.SendReverseProxyUpdate(reverseProxy.ToProtoMapping(reverseproxy.Create, key.Key)) return reverseProxy, nil } @@ -196,12 +191,7 @@ func (m *managerImpl) UpdateReverseProxy(ctx context.Context, accountID, userID m.accountManager.StoreEvent(ctx, userID, reverseProxy.ID, accountID, activity.ReverseProxyUpdated, reverseProxy.EventMeta()) - idp, err := m.getIdentityProvider(ctx, accountID, userID, reverseProxy.Auth.BearerAuth) - if err != nil { - return nil, fmt.Errorf("failed to get identity provider: %w", err) - } - - m.proxyGRPCServer.SendReverseProxyUpdate(reverseProxy.ToProtoMapping(reverseproxy.Update, "", idp)) + m.proxyGRPCServer.SendReverseProxyUpdate(reverseProxy.ToProtoMapping(reverseproxy.Update, "")) return reverseProxy, nil } @@ -235,14 +225,7 @@ func (m *managerImpl) DeleteReverseProxy(ctx context.Context, accountID, userID, m.accountManager.StoreEvent(ctx, userID, reverseProxyID, accountID, activity.ReverseProxyDeleted, reverseProxy.EventMeta()) - m.proxyGRPCServer.SendReverseProxyUpdate(reverseProxy.ToProtoMapping(reverseproxy.Delete, "", nil)) + m.proxyGRPCServer.SendReverseProxyUpdate(reverseProxy.ToProtoMapping(reverseproxy.Delete, "")) return nil } - -func (m *managerImpl) getIdentityProvider(ctx context.Context, accountID, userID string, bearerAuth *reverseproxy.BearerAuthConfig) (*types.IdentityProvider, error) { - if bearerAuth == nil || !bearerAuth.Enabled || bearerAuth.IdentityProviderID == "" { - return nil, nil - } - return m.accountManager.GetIdentityProvider(ctx, accountID, bearerAuth.IdentityProviderID, userID) -} diff --git a/management/internals/modules/reverseproxy/reverseproxy.go b/management/internals/modules/reverseproxy/reverseproxy.go index 585e4acdf..9c8f92ea8 100644 --- a/management/internals/modules/reverseproxy/reverseproxy.go +++ b/management/internals/modules/reverseproxy/reverseproxy.go @@ -6,11 +6,9 @@ import ( "net/url" "strconv" - "github.com/coreos/go-oidc/v3/oidc" "github.com/rs/xid" log "github.com/sirupsen/logrus" - "github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/proto" ) @@ -45,7 +43,6 @@ type PINAuthConfig struct { type BearerAuthConfig struct { Enabled bool `json:"enabled"` - IdentityProviderID string `json:"identity_provider_id,omitempty"` DistributionGroups []string `json:"distribution_groups,omitempty" gorm:"serializer:json"` } @@ -102,7 +99,6 @@ func (r *ReverseProxy) ToAPIResponse() *api.ReverseProxy { authConfig.BearerAuth = &api.BearerAuthConfig{ Enabled: r.Auth.BearerAuth.Enabled, DistributionGroups: &r.Auth.BearerAuth.DistributionGroups, - IdentityProviderId: &r.Auth.BearerAuth.IdentityProviderID, } } @@ -136,7 +132,7 @@ func (r *ReverseProxy) ToAPIResponse() *api.ReverseProxy { } } -func (r *ReverseProxy) ToProtoMapping(operation Operation, setupKey string, idp *types.IdentityProvider) *proto.ProxyMapping { +func (r *ReverseProxy) ToProtoMapping(operation Operation, setupKey string) *proto.ProxyMapping { pathMappings := make([]*proto.PathMapping, 0, len(r.Targets)) for _, target := range r.Targets { if !target.Enabled { @@ -173,12 +169,12 @@ func (r *ReverseProxy) ToProtoMapping(operation Operation, setupKey string, idp auth.Pin = true } - if r.Auth.BearerAuth != nil && r.Auth.BearerAuth.Enabled && idp != nil { + if r.Auth.BearerAuth != nil && r.Auth.BearerAuth.Enabled { auth.Oidc = &proto.OIDC{ - OidcProviderUrl: idp.Issuer, - OidcClientId: idp.ClientID, - OidcClientSecret: idp.ClientSecret, - OidcScopes: []string{oidc.ScopeOpenID, "profile", "email"}, + OidcProviderUrl: "", // TODO: + OidcClientId: "", // TODO: + OidcClientSecret: "", // TODO: + OidcScopes: nil, // TODO: DistributionGroups: r.Auth.BearerAuth.DistributionGroups, } } @@ -251,9 +247,6 @@ func (r *ReverseProxy) FromAPIRequest(req *api.ReverseProxyRequest, accountID st bearerAuth := &BearerAuthConfig{ Enabled: req.Auth.BearerAuth.Enabled, } - if req.Auth.BearerAuth.IdentityProviderId != nil { - bearerAuth.IdentityProviderID = *req.Auth.BearerAuth.IdentityProviderId - } if req.Auth.BearerAuth.DistributionGroups != nil { bearerAuth.DistributionGroups = *req.Auth.BearerAuth.DistributionGroups } diff --git a/management/internals/shared/grpc/proxy.go b/management/internals/shared/grpc/proxy.go index daefa210f..d48ed1564 100644 --- a/management/internals/shared/grpc/proxy.go +++ b/management/internals/shared/grpc/proxy.go @@ -27,10 +27,9 @@ type reverseProxyStore interface { GetReverseProxyByID(ctx context.Context, lockStrength store.LockingStrength, accountID string, serviceID string) (*reverseproxy.ReverseProxy, error) } -type accountStore interface { +type keyStore interface { GetGroupByName(ctx context.Context, groupName string, accountID string) (*types.Group, error) CreateSetupKey(ctx context.Context, accountID string, keyName string, keyType types.SetupKeyType, expiresIn time.Duration, autoGroups []string, usageLimit int, userID string, ephemeral bool, allowExtraDNSLabels bool) (*types.SetupKey, error) - GetIdentityProvider(ctx context.Context, accountID, idpID, userID string) (*types.IdentityProvider, error) } // ProxyServiceServer implements the ProxyService gRPC server @@ -46,8 +45,8 @@ type ProxyServiceServer struct { // Store of reverse proxies reverseProxyStore reverseProxyStore - // Store for accounts and authentication information - accountStore accountStore + // Store for client setup keys + keyStore keyStore // Manager for access logs accessLogManager accesslogs.Manager @@ -65,11 +64,11 @@ type proxyConnection struct { } // NewProxyServiceServer creates a new proxy service server -func NewProxyServiceServer(store reverseProxyStore, keys accountStore, accessLogMgr accesslogs.Manager) *ProxyServiceServer { +func NewProxyServiceServer(store reverseProxyStore, keys keyStore, accessLogMgr accesslogs.Manager) *ProxyServiceServer { return &ProxyServiceServer{ updatesChan: make(chan *proto.ProxyMapping, 100), reverseProxyStore: store, - accountStore: keys, + keyStore: keys, accessLogManager: accessLogMgr, } } @@ -140,7 +139,7 @@ func (s *ProxyServiceServer) sendSnapshot(ctx context.Context, conn *proxyConnec continue } - group, err := s.accountStore.GetGroupByName(ctx, rp.Name, rp.AccountID) + group, err := s.keyStore.GetGroupByName(ctx, rp.Name, rp.AccountID) if err != nil { log.WithFields(log.Fields{ "proxy": rp.Name, @@ -150,7 +149,7 @@ func (s *ProxyServiceServer) sendSnapshot(ctx context.Context, conn *proxyConnec } // TODO: should this even be here? We're running in a loop, and on each proxy, this will create a LOT of setup key entries that we currently have no way to remove. - key, err := s.accountStore.CreateSetupKey(ctx, + key, err := s.keyStore.CreateSetupKey(ctx, rp.AccountID, rp.Name, types.SetupKeyReusable, @@ -170,25 +169,11 @@ func (s *ProxyServiceServer) sendSnapshot(ctx context.Context, conn *proxyConnec continue } - var idp *types.IdentityProvider - if rp.Auth.BearerAuth != nil && rp.Auth.BearerAuth.Enabled && rp.Auth.BearerAuth.IdentityProviderID != "" { - idp, err = s.accountStore.GetIdentityProvider(ctx, rp.AccountID, rp.Auth.BearerAuth.IdentityProviderID, activity.SystemInitiator) - if err != nil { - log.WithFields(log.Fields{ - "proxy": rp.Name, - "account": rp.AccountID, - "idp_id": rp.Auth.BearerAuth.IdentityProviderID, - }).WithError(err).Warn("Failed to get identity provider for reverse proxy, but bearer auth is enabled. Bearer auth will be disabled") - rp.Auth.BearerAuth.Enabled = false - } - } - if err := conn.stream.Send(&proto.GetMappingUpdateResponse{ Mapping: []*proto.ProxyMapping{ rp.ToProtoMapping( reverseproxy.Create, // Initial snapshot, all records are "new" for the proxy. key.Key, - idp, ), }, }); err != nil { diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml index 8db7deaab..573924a44 100644 --- a/shared/management/http/api/openapi.yml +++ b/shared/management/http/api/openapi.yml @@ -2962,9 +2962,6 @@ components: enabled: type: boolean description: Whether bearer auth is enabled - identity_provider_id: - type: string - description: ID of the identity provider to use for OIDC authentication distribution_groups: type: array items: diff --git a/shared/management/http/api/types.gen.go b/shared/management/http/api/types.gen.go index aa4c18476..2290e7e1f 100644 --- a/shared/management/http/api/types.gen.go +++ b/shared/management/http/api/types.gen.go @@ -474,9 +474,6 @@ type BearerAuthConfig struct { // Enabled Whether bearer auth is enabled Enabled bool `json:"enabled"` - - // IdentityProviderId ID of the identity provider to use for OIDC authentication - IdentityProviderId *string `json:"identity_provider_id,omitempty"` } // BundleParameters These parameters control what gets included in the bundle and how it is processed.