From 2d5ee900d056405fc9a015b31e3d73dfd2f73e48 Mon Sep 17 00:00:00 2001 From: "Theodor S. Midtlien" Date: Tue, 15 Sep 2026 15:22:41 +0200 Subject: [PATCH] Replace RequireFlowInitiator with AuthzLevel and clear flow on switch profile --- client/internal/ipcauth/authz_gate.go | 10 +++------- client/internal/ipcauth/methods.go | 14 +++++++------- client/server/server.go | 25 +++++++++++++++++++++---- client/server/server_connect_test.go | 11 +++++++---- 4 files changed, 38 insertions(+), 22 deletions(-) diff --git a/client/internal/ipcauth/authz_gate.go b/client/internal/ipcauth/authz_gate.go index 313b64b9f..2cb200059 100644 --- a/client/internal/ipcauth/authz_gate.go +++ b/client/internal/ipcauth/authz_gate.go @@ -18,6 +18,9 @@ type DaemonState interface { // whether one is held. SessionHolder() (Principal, bool) + // OwnsProfile reports whether id owns the profile a request names. An empty + // handle is the active profile, which is what a method that acts on the + // live session resolves against. OwnsProfile(id Identity, handle string) bool } @@ -74,13 +77,6 @@ func RequireLevel(want AuthzLevel) Rule { } } -// RequireFlowInitiator binds a pending authentication flow to the identity that -// started it. -func RequireFlowInitiator(r Request) error { - // TODO: needs the flow registry keyed by initiator. - return nil -} - func denyLevel(r Request, want AuthzLevel) error { return status.Errorf(codes.PermissionDenied, "%s requires %s, caller %s is %s", r.Method, want, r.Identity, r.Level) diff --git a/client/internal/ipcauth/methods.go b/client/internal/ipcauth/methods.go index d1817b3c6..502eaa7d3 100644 --- a/client/internal/ipcauth/methods.go +++ b/client/internal/ipcauth/methods.go @@ -53,11 +53,6 @@ var methodPolicies = map[string]MethodPolicy{ servicePath + "GetFeatures": {Level: AuthzLevelIdentified}, servicePath + "WailsUIReady": {Level: AuthzLevelIdentified}, - // Pending flows: bound to the principal that started them, at any level. - servicePath + "WaitSSOLogin": {Level: AuthzLevelIdentified, Rules: []Rule{RequireFlowInitiator}, Audit: true}, - servicePath + "WaitJWTToken": {Level: AuthzLevelIdentified, Rules: []Rule{RequireFlowInitiator}, Audit: true}, - servicePath + "WaitExtendAuthSession": {Level: AuthzLevelIdentified, Rules: []Rule{RequireFlowInitiator}}, - // Owner of the profile the request names. servicePath + "GetConfig": {Level: AuthzLevelProfileOwner, TargetsProfile: true, Audit: true}, servicePath + "SetConfig": {Level: AuthzLevelProfileOwner, TargetsProfile: true, Audit: true}, @@ -67,12 +62,14 @@ var methodPolicies = map[string]MethodPolicy{ servicePath + "RemoveProfile": {Level: AuthzLevelProfileOwner, TargetsProfile: true, Audit: true}, servicePath + "SwitchProfile": {Level: AuthzLevelSessionHolder, TargetsProfile: true, Audit: true}, - // Owner of some profile + // Owner of the active profile, which is what an empty target resolves to. servicePath + "GetLogLevel": {Level: AuthzLevelProfileOwner}, servicePath + "ListStates": {Level: AuthzLevelProfileOwner}, servicePath + "GetInstallerResult": {Level: AuthzLevelProfileOwner}, - // Session holder: the live engine and everything daemon-wide. + // Session holder: the live engine and everything daemon-wide. A pending + // authentication flow belongs to the profile it was started for, so each + // Wait sits at the level of the RPC that starts it. servicePath + "Up": {Level: AuthzLevelSessionHolder, TargetsProfile: true, Audit: true}, servicePath + "Down": {Level: AuthzLevelSessionHolder, Audit: true}, servicePath + "SubscribeStatus": {Level: AuthzLevelSessionHolder}, @@ -85,7 +82,10 @@ var methodPolicies = map[string]MethodPolicy{ servicePath + "ExposeService": {Level: AuthzLevelSessionHolder, Audit: true}, servicePath + "GetPeerSSHHostKey": {Level: AuthzLevelSessionHolder}, servicePath + "RequestJWTAuth": {Level: AuthzLevelSessionHolder, Audit: true}, + servicePath + "WaitJWTToken": {Level: AuthzLevelSessionHolder, Audit: true}, servicePath + "RequestExtendAuthSession": {Level: AuthzLevelSessionHolder}, + servicePath + "WaitExtendAuthSession": {Level: AuthzLevelSessionHolder}, + servicePath + "WaitSSOLogin": {Level: AuthzLevelSessionHolder, Audit: true}, servicePath + "DismissSessionWarning": {Level: AuthzLevelSessionHolder}, servicePath + "DebugBundle": {Level: AuthzLevelSessionHolder, Audit: true}, servicePath + "SetLogLevel": {Level: AuthzLevelSessionHolder}, diff --git a/client/server/server.go b/client/server/server.go index 708ef87b2..cc6547a61 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -165,9 +165,9 @@ type oauthAuthFlow struct { info auth.AuthFlowInfo // cacheGeneration is the SSH JWT cache's generation as of the start of the - // request that created this flow. The flow outlives a profile switch, so - // reading the generation any later — when the IdP has answered, or when the - // token finally arrives — would read the new session's one and let the old + // request that created this flow. A logout or a profile switch clears the + // flow, but the IdP may already have been polled by then, so reading the + // generation any later would read the new session's one and let the old // session's token into the new session's cache. cacheGeneration uint64 @@ -1277,6 +1277,7 @@ func (s *Server) SwitchProfile(callerCtx context.Context, msg *proto.SwitchProfi s.localMetrics.Reconcile(config.LocalMetricsEnabled, config.LocalMetricsAddress) s.jwtCache.clear() + s.clearPendingAuthFlows() if msg != nil && msg.ProfileName != nil { s.publishProfileListChanged(*msg.ProfileName) @@ -1335,9 +1336,25 @@ func (s *Server) Down(ctx context.Context, _ *proto.DownRequest) (*proto.DownRes return &proto.DownResponse{}, nil } -func (s *Server) cleanupConnection() error { +// clearPendingAuthFlows drops both pending authentication flows and wakes their +// waiters. A flow is only ever authorized against the profile that was active +// when it started, so leaving one behind across a switch or a logout would hand +// its result to whoever owns the profile that comes next. +// +// The caller holds s.mutex. +func (s *Server) clearPendingAuthFlows() { + if s.oauthAuthFlow.waitCancel != nil { + s.oauthAuthFlow.waitCancel() + } s.oauthAuthFlow = oauthAuthFlow{} + s.extendAuthSessionFlow.CancelWait() + s.extendAuthSessionFlow.Clear() +} + +func (s *Server) cleanupConnection() error { + s.clearPendingAuthFlows() + if s.actCancel == nil { return ErrServiceNotUp } diff --git a/client/server/server_connect_test.go b/client/server/server_connect_test.go index dc191a44f..171473cfe 100644 --- a/client/server/server_connect_test.go +++ b/client/server/server_connect_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "github.com/netbirdio/netbird/client/internal" + "github.com/netbirdio/netbird/client/internal/auth" "github.com/netbirdio/netbird/client/internal/peer" "github.com/netbirdio/netbird/client/proto" ) @@ -18,10 +19,12 @@ func newTestServer() *Server { return &Server{ rootCtx: context.Background(), statusRecorder: peer.NewRecorder(""), - // New always populates the SSH JWT cache and the logout and - // profile-switch paths call into it unconditionally, so a Server - // assembled field by field has to populate it too. - jwtCache: newJWTCache(), + // New always populates the SSH JWT cache and the pending extend-session + // flow, and the logout and profile-switch paths call into both + // unconditionally, so a Server assembled field by field has to populate + // them too. + jwtCache: newJWTCache(), + extendAuthSessionFlow: auth.NewPendingFlow(), } }