Move profile resolution to the authz gate

client/internal/ipcauth/authz_gate.go

@@ -18,11 +18,12 @@ 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. The error says what was wrong with the
-	// handle itself.
-	OwnsProfile(id Identity, handle string) (bool, error)
+	// ResolveTarget resolves the profile a request names to a concrete profile
+	// and reports whether the caller may address it. An empty handle is the
+	// active profile.
+	//
+	// The error says what was wrong with the handle itself.
+	ResolveTarget(id Identity, handle string) (Target, error)
 }
 
 // AuthzGate authorizes every RPC call before its handler run.
@@ -118,8 +119,9 @@ func denyPolicyLevel(r Request, p MethodPolicy) error {
 // target-scoped.
 func (g *AuthzGate) StreamPolicyInterceptor() grpc.StreamServerInterceptor {
 	return func(srv any, ss grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error {
-		authErr := g.authorize(ss.Context(), info.FullMethod, nil)
-		if authErr != nil {
+		// A stream's context cannot be replaced from here. We use context to pass
+		// resolved target profile and no streaming method may be target-scoped.
+		if _, authErr := g.authorize(ss.Context(), info.FullMethod, nil); authErr != nil {
 			return authErr
 		}
 		return handler(srv, ss)
@@ -129,7 +131,7 @@ func (g *AuthzGate) StreamPolicyInterceptor() grpc.StreamServerInterceptor {
 // UnaryPolicyInterceptor authorizes each unary RPC before the handler runs.
 func (g *AuthzGate) UnaryPolicyInterceptor() grpc.UnaryServerInterceptor {
 	return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp any, err error) {
-		authErr := g.authorize(ctx, info.FullMethod, req)
+		ctx, authErr := g.authorize(ctx, info.FullMethod, req)
 		if authErr != nil {
 			return nil, authErr
 		}
@@ -137,55 +139,68 @@ func (g *AuthzGate) UnaryPolicyInterceptor() grpc.UnaryServerInterceptor {
 	}
 }
 
-func (g *AuthzGate) authorize(ctx context.Context, method string, msg any) error {
+func (g *AuthzGate) authorize(ctx context.Context, method string, msg any) (context.Context, error) {
 	id, ok := CallerIdentity(ctx)
 	if !ok {
 		log.Warnf("ipc authz: DENY %s, caller identity unavailable", method)
-		return status.Error(codes.PermissionDenied,
+		return ctx, status.Error(codes.PermissionDenied,
 			"caller identity could not be verified on the daemon control channel")
 	}
 	st := g.state()
 	if st == nil {
 		log.Warnf("ipc authz: DENY %s for %s, daemon state not attached", method, id)
-		return status.Error(codes.Unavailable, "daemon not initialized")
+		return ctx, status.Error(codes.Unavailable, "daemon not initialized")
 	}
 	policy := methodPolicyFor(method)
 
-	// Only a target-scoped method reads a profile off the request.
-	var target string
+	// Only a target-scoped method reads a profile off the request. Everything
+	// else acts on the active profile, which an empty handle resolves to.
+	var handle string
 	if policy.TargetsProfile {
 		named, ok := targetProfile(msg)
 		if !ok {
-			return status.Errorf(codes.Internal, "%s is declared target-scoped but names no profile", method)
+			return ctx, status.Errorf(codes.Internal, "%s is declared target-scoped but names no profile", method)
 		}
-		target = named
+		handle = named
 	}
 
-	level, resolveErr := resolveLevel(id, target, st)
+	target, handleErr := st.ResolveTarget(id, handle)
+
+	level := resolveLevel(id, target, st)
+
+	if handleErr != nil && handle != "" {
+		level = AuthzLevelIdentified
+	}
 
 	req := Request{
 		Identity: id,
 		Level:    level,
-		Target:   target,
+		Target:   handle,
 		Method:   method,
 		State:    st,
 		Msg:      msg,
 	}
 	if req.Level < policy.Level {
 		log.Warnf("ipc authz: DENY %s for %s (%s), requires %s", method, id, req.Level, policy.Level)
-		if resolveErr != nil {
-			return resolveErr
+		if presentable := presentableHandleError(handle, handleErr); presentable != nil {
+			return ctx, presentable
 		}
-		return denyPolicyLevel(req, policy)
+		return ctx, denyPolicyLevel(req, policy)
 	}
 	for _, rule := range policy.Rules {
 		if err := rule(req); err != nil {
 			log.Warnf("ipc authz: DENY %s for %s (%s): error", method, id, req.Level)
-			return err
+			return ctx, err
 		}
 	}
 	if policy.Audit {
 		log.Infof("ipc authz: allow %s for %s (%s)", method, id, req.Level)
 	}
-	return nil
+	if !target.Owned {
+		// Reaching here means the method was open to the caller's level
+		// without owning anything, so there is no authorized profile to hand
+		// the handler.
+		return ctx, nil
+	}
+	return ContextWithTarget(ctx, target.Path), nil
 }

client/internal/ipcauth/authz_gate_test.go

@@ -35,9 +35,9 @@ func switchTo(handle string) *proto.SwitchProfileRequest {
 // exists and belongs to somebody, which a mistyped handle does not.
 func TestAuthorizeSurfacesWhatIsWrongWithTheHandle(t *testing.T) {
 	notFound := gstatus.Errorf(codes.NotFound, "profile %q not found", "asdfasdfasdf")
-	g := gateFor(t, stubState{ownsErr: notFound})
+	g := gateFor(t, stubState{targetErr: notFound})
 
-	err := g.authorize(transportCtx(unprivUser, nil), servicePath+"SwitchProfile", switchTo("asdfasdfasdf"))
+	_, err := g.authorize(transportCtx(unprivUser, nil), servicePath+"SwitchProfile", switchTo("asdfasdfasdf"))
 	require.Error(t, err)
 
 	st := gstatus.Convert(err)
@@ -52,9 +52,9 @@ func TestAuthorizeSurfacesWhatIsWrongWithTheHandle(t *testing.T) {
 // error, and the CLI reformats it into a hint. It has to reach the CLI.
 func TestAuthorizeSurfacesAnAmbiguousHandle(t *testing.T) {
 	ambiguous := gstatus.Errorf(codes.InvalidArgument, "handle %q matches 2 profiles", "ab")
-	g := gateFor(t, stubState{ownsErr: ambiguous})
+	g := gateFor(t, stubState{targetErr: ambiguous})
 
-	err := g.authorize(transportCtx(unprivUser, nil), servicePath+"SwitchProfile", switchTo("ab"))
+	_, err := g.authorize(transportCtx(unprivUser, nil), servicePath+"SwitchProfile", switchTo("ab"))
 	require.Error(t, err)
 	assert.Equal(t, codes.InvalidArgument, gstatus.Convert(err).Code())
 }
@@ -64,9 +64,9 @@ func TestAuthorizeSurfacesAnAmbiguousHandle(t *testing.T) {
 // the refusal stays about who the profile belongs to.
 func TestAuthorizeBlamesOwnershipForTheActiveProfile(t *testing.T) {
 	notFound := gstatus.Errorf(codes.NotFound, "profile %q not found", "active-profile-id")
-	g := gateFor(t, stubState{ownsErr: notFound})
+	g := gateFor(t, stubState{targetErr: notFound})
 
-	err := g.authorize(transportCtx(unprivUser, nil), servicePath+"SwitchProfile", switchTo(""))
+	_, err := g.authorize(transportCtx(unprivUser, nil), servicePath+"SwitchProfile", switchTo(""))
 	require.Error(t, err)
 
 	denial, ok := DenialFrom(err)
@@ -78,9 +78,9 @@ func TestAuthorizeBlamesOwnershipForTheActiveProfile(t *testing.T) {
 // A daemon-side failure is not something the caller can correct, and putting it
 // on the wire would describe the daemon rather than the request.
 func TestAuthorizeKeepsADaemonFailureOffTheWire(t *testing.T) {
-	g := gateFor(t, stubState{ownsErr: errors.New("read profile directory: permission denied")})
+	g := gateFor(t, stubState{targetErr: errors.New("read profile directory: permission denied")})
 
-	err := g.authorize(transportCtx(unprivUser, nil), servicePath+"SwitchProfile", switchTo("some-profile"))
+	_, err := g.authorize(transportCtx(unprivUser, nil), servicePath+"SwitchProfile", switchTo("some-profile"))
 	require.Error(t, err)
 
 	denial, ok := DenialFrom(err)
@@ -93,14 +93,15 @@ func TestAuthorizeKeepsADaemonFailureOffTheWire(t *testing.T) {
 // identified caller may make. A failure there must not take those down.
 func TestAuthorizeAllowsIdentifiedMethodsDespiteAResolveFailure(t *testing.T) {
 	notFound := gstatus.Errorf(codes.NotFound, "profile %q not found", "active-profile-id")
-	g := gateFor(t, stubState{ownsErr: notFound})
+	g := gateFor(t, stubState{targetErr: notFound})
 
 	for _, method := range []string{"ListProfiles", "AddProfile", "GetActiveProfile", "GetFeatures"} {
 		t.Run(method, func(t *testing.T) {
 			require.Equal(t, AuthzLevelIdentified, methodPolicies[servicePath+method].Level,
 				"fixture is wrong: %s is no longer open to any identified caller", method)
 
-			assert.NoError(t, g.authorize(transportCtx(unprivUser, nil), servicePath+method, nil))
+			_, err := g.authorize(transportCtx(unprivUser, nil), servicePath+method, nil)
+			assert.NoError(t, err)
 		})
 	}
 }
@@ -108,32 +109,61 @@ func TestAuthorizeAllowsIdentifiedMethodsDespiteAResolveFailure(t *testing.T) {
 // Ownership is the gate's answer, never the error's: a resolution that failed is
 // a no whatever it returned alongside.
 func TestAuthorizeRefusesWhenResolutionFails(t *testing.T) {
-	g := gateFor(t, stubState{owns: false, ownsErr: gstatus.Error(codes.NotFound, "profile not found")})
+	g := gateFor(t, stubState{targetErr: gstatus.Error(codes.NotFound, "profile not found")})
 
-	err := g.authorize(transportCtx(unprivUser, nil), servicePath+"SwitchProfile", switchTo("some-profile"))
+	_, err := g.authorize(transportCtx(unprivUser, nil), servicePath+"SwitchProfile", switchTo("some-profile"))
 	assert.Error(t, err, "an error from the resolution cannot be read as ownership")
 }
 
-// A resolution that failed established nothing about the profile, so no level
-// returned alongside the error may be acted on. This is the invariant the gate
-// clamps, pinned at the function that has to hold it.
+// A resolution that failed established nothing about the profile, so ownership
+// reported alongside the error may not be acted on. A state that answers both
+// at once is exactly what this refuses to trust.
 func TestResolveLevelNeverRaisesTheLevelOnAFailure(t *testing.T) {
-	asDaemon(t, root)
-
 	notFound := gstatus.Error(codes.NotFound, "profile not found")
+	owned := Target{Path: "/profiles/some-profile.json", Owned: true}
 
 	for _, tc := range []struct {
 		name string
 		st   stubState
 	}{
-		{"a live session it reports as owned", stubState{owns: true, running: true, ownsErr: notFound}},
-		{"an idle daemon it reports as owned", stubState{owns: true, ownsErr: notFound}},
-		{"a daemon-side failure it reports as owned", stubState{owns: true, ownsErr: errors.New("read profile directory")}},
+		{"a live session it reports as owned", stubState{target: owned, running: true, targetErr: notFound}},
+		{"an idle daemon it reports as owned", stubState{target: owned, targetErr: notFound}},
+		{"a daemon-side failure it reports as owned", stubState{target: owned, targetErr: errors.New("read profile directory")}},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			level, _ := resolveLevel(unprivUser, "some-profile", tc.st)
-			assert.Equal(t, AuthzLevelIdentified, level,
-				"a failed resolution cannot confer %s", level)
+			g := gateFor(t, tc.st)
+
+			_, err := g.authorize(transportCtx(unprivUser, nil), servicePath+"SwitchProfile", switchTo("some-profile"))
+			assert.Error(t, err, "a failed resolution conferred a level it had no business conferring")
 		})
 	}
 }
+
+// The profile the gate resolved is what the handler acts on, so it has to reach
+// the handler. Resolving the handle a second time downstream is what this
+// exists to make unnecessary.
+func TestAuthorizeCarriesTheResolvedTargetToTheHandler(t *testing.T) {
+	g := gateFor(t, stubState{target: Target{Path: "/profiles/abcd1111.json", Owned: true}})
+
+	ctx, err := g.authorize(transportCtx(unprivUser, nil), servicePath+"SwitchProfile", switchTo("work"))
+	require.NoError(t, err)
+
+	got, ok := TargetFromContext(ctx)
+	require.True(t, ok, "the handler has no profile to act on")
+	assert.Equal(t, "/profiles/abcd1111.json", got,
+		"the handler would act on a different profile than the one authorized")
+}
+
+// A privileged caller skips the ownership question but still needs the profile
+// their handle named, or every target-scoped RPC breaks under sudo.
+func TestAuthorizeCarriesTheTargetForAPrivilegedCaller(t *testing.T) {
+	g := gateFor(t, stubState{target: Target{Path: "/profiles/abcd1111.json", Owned: true}})
+
+	ctx, err := g.authorize(transportCtx(root, nil), servicePath+"ClaimProfile",
+		&proto.ClaimProfileRequest{Handle: "work"})
+	require.NoError(t, err)
+
+	got, ok := TargetFromContext(ctx)
+	require.True(t, ok, "root resolved nothing to act on")
+	assert.Equal(t, "/profiles/abcd1111.json", got)
+}

client/internal/ipcauth/authz_level.go

@@ -47,41 +47,42 @@ func (l AuthzLevel) String() string {
 }
 
 // resolveLevel is the authority the caller holds over the profile the request
-// names. The second return is what was wrong with the handle, when that is
-// worth showing the caller instead of a refusal. It never raises the level: a
-// resolution that failed still denies.
-func resolveLevel(id Identity, target string, st DaemonState) (AuthzLevel, error) {
+// resolved to. A profile the caller does not own confers nothing beyond being
+// identified, which is also what an unresolved handle leaves them with.
+func resolveLevel(id Identity, target Target, st DaemonState) AuthzLevel {
 	if !id.Known() {
-		return AuthzLevelNone, nil
+		return AuthzLevelNone
 	}
 	if IsPrivilegedCaller(id) {
-		return AuthzLevelPrivileged, nil
+		return AuthzLevelPrivileged
 	}
-	ownsProfile, err := st.OwnsProfile(id, target)
-	if err != nil {
-		return AuthzLevelIdentified, presentableHandleError(target, err)
-	}
-	if !ownsProfile {
-		return AuthzLevelIdentified, nil
+	if !target.Owned {
+		return AuthzLevelIdentified
 	}
 	if holder, running := st.SessionHolder(); !running || holder.Matches(id) {
-		return AuthzLevelSessionHolder, nil
+		return AuthzLevelSessionHolder
 	}
-	return AuthzLevelProfileOwner, nil
+	return AuthzLevelProfileOwner
 }
 
 // presentableHandleError keeps a resolution failure only when the gate can put
 // it in front of the caller in place of its own refusal. Everything else is
 // dropped, and the caller gets the refusal their level earned.
-func presentableHandleError(target string, err error) error {
-	// An empty target is the active profile rather than something the caller
+func presentableHandleError(handle string, err error) error {
+	if err == nil {
+		return nil
+	}
+
+	// An empty handle is the active profile rather than something the caller
 	// typed, so a failure to resolve it is not theirs to correct.
-	if target == "" {
+	if handle == "" {
 		return nil
 	}
 
 	// Only a gRPC status reaches the caller as a sentence the CLI and the UI
-	// render.
+	// render. A plain error is a daemon-side failure, and putting it on the
+	// wire would tell the caller about the daemon rather than about the handle
+	// they gave.
 	if _, ok := gstatus.FromError(err); !ok {
 		return nil
 	}

client/internal/ipcauth/privilege_denial_test.go

@@ -102,14 +102,15 @@ func TestDenyPolicyLevelWithoutGuidanceStaysBare(t *testing.T) {
 
 // stubState stands in for the daemon so a denial can be built without a server.
 type stubState struct {
-	holder  Principal
-	running bool
-	owns    bool
-	ownsErr error
+	holder    Principal
+	running   bool
+	target    Target
+	targetErr error
 }
 
-func (s stubState) SessionHolder() (Principal, bool)           { return s.holder, s.running }
-func (s stubState) OwnsProfile(Identity, string) (bool, error) { return s.owns, s.ownsErr }
+func (s stubState) SessionHolder() (Principal, bool) { return s.holder, s.running }
+
+func (s stubState) ResolveTarget(Identity, string) (Target, error) { return s.target, s.targetErr }
 
 // A refusal caused by somebody else's connection explains itself and offers no
 // command, since the caller cannot end a session that is not theirs.

client/internal/ipcauth/target.go

@@ -0,0 +1,26 @@
+package ipcauth
+
+import "context"
+
+// Target is the profile a request resolved to.
+type Target struct {
+	Path  string
+	Owned bool
+}
+
+type targetKey struct{}
+
+// ContextWithTarget carries the profile the gate resolved, so a handler acts on
+// the profile that was authorized rather than resolving the caller's handle a
+// second time.
+func ContextWithTarget(ctx context.Context, path string) context.Context {
+	return context.WithValue(ctx, targetKey{}, path)
+}
+
+// TargetFromContext returns the file of the profile the gate resolved for this
+// request. It reports false when nothing resolved, which a handler acting on a
+// named profile must treat as a refusal rather than as the active profile.
+func TargetFromContext(ctx context.Context) (string, bool) {
+	path, ok := ctx.Value(targetKey{}).(string)
+	return path, ok && path != ""
+}