client/web: split errTaggedSelf resp from getTailscaleBrowserSession

Previously returned errTaggedSource in the case that of any tagged
source. Now distinguishing whether the source was local or remote.
We'll be presenting the two cases with varying copy on the frontend.

Updates tailscale/corp#14335

Signed-off-by: Sonia Appasamy <sonia@tailscale.com>
This commit is contained in:
Sonia Appasamy 2023-10-24 15:15:10 -04:00 committed by Sonia Appasamy
parent e0a4a02b35
commit d79e0fde9c
2 changed files with 44 additions and 24 deletions

View File

@ -289,10 +289,11 @@ func (s *Server) serveLoginAPI(w http.ResponseWriter, r *http.Request) {
} }
var ( var (
errNoSession = errors.New("no-browser-session") errNoSession = errors.New("no-browser-session")
errNotUsingTailscale = errors.New("not-using-tailscale") errNotUsingTailscale = errors.New("not-using-tailscale")
errTaggedSource = errors.New("tagged-source") errTaggedRemoteSource = errors.New("tagged-remote-source")
errNotOwner = errors.New("not-owner") errTaggedLocalSource = errors.New("tagged-local-source")
errNotOwner = errors.New("not-owner")
) )
// getTailscaleBrowserSession retrieves the browser session associated with // getTailscaleBrowserSession retrieves the browser session associated with
@ -304,8 +305,13 @@ func (s *Server) serveLoginAPI(w http.ResponseWriter, r *http.Request) {
// //
// - (errNoSession) The request does not have a session. // - (errNoSession) The request does not have a session.
// //
// - (errTaggedSource) The source is a tagged node. Users must use their // - (errTaggedRemoteSource) The source is remote (another node) and tagged.
// own user-owned devices to manage other nodes' web clients. // Users must use their own user-owned devices to manage other nodes'
// web clients.
//
// - (errTaggedLocalSource) The source is local (the same node) and tagged.
// Tagged nodes can only be remotely managed, allowing ACLs to dictate
// access to web clients.
// //
// - (errNotOwner) The source is not the owner of this client (if the // - (errNotOwner) The source is not the owner of this client (if the
// client is user-owned). Only the owner is allowed to manage the // client is user-owned). Only the owner is allowed to manage the
@ -318,26 +324,25 @@ func (s *Server) serveLoginAPI(w http.ResponseWriter, r *http.Request) {
// The WhoIsResponse is always populated, with a non-nil Node and UserProfile, // The WhoIsResponse is always populated, with a non-nil Node and UserProfile,
// unless getTailscaleBrowserSession reports errNotUsingTailscale. // unless getTailscaleBrowserSession reports errNotUsingTailscale.
func (s *Server) getTailscaleBrowserSession(r *http.Request) (*browserSession, *apitype.WhoIsResponse, error) { func (s *Server) getTailscaleBrowserSession(r *http.Request) (*browserSession, *apitype.WhoIsResponse, error) {
whoIs, err := s.lc.WhoIs(r.Context(), r.RemoteAddr) whoIs, whoIsErr := s.lc.WhoIs(r.Context(), r.RemoteAddr)
status, statusErr := s.lc.StatusWithoutPeers(r.Context())
switch { switch {
case err != nil: case whoIsErr != nil:
return nil, nil, errNotUsingTailscale return nil, nil, errNotUsingTailscale
case statusErr != nil:
return nil, whoIs, statusErr
case status.Self == nil:
return nil, whoIs, errors.New("missing self node in tailscale status")
case whoIs.Node.IsTagged() && whoIs.Node.StableID == status.Self.ID:
return nil, whoIs, errTaggedLocalSource
case whoIs.Node.IsTagged(): case whoIs.Node.IsTagged():
return nil, whoIs, errTaggedSource return nil, whoIs, errTaggedRemoteSource
case !status.Self.IsTagged() && status.Self.UserID != whoIs.UserProfile.ID:
return nil, whoIs, errNotOwner
} }
srcNode := whoIs.Node.ID srcNode := whoIs.Node.ID
srcUser := whoIs.UserProfile.ID srcUser := whoIs.UserProfile.ID
status, err := s.lc.StatusWithoutPeers(r.Context())
switch {
case err != nil:
return nil, whoIs, err
case status.Self == nil:
return nil, whoIs, errors.New("missing self node in tailscale status")
case !status.Self.IsTagged() && status.Self.UserID != srcUser:
return nil, whoIs, errNotOwner
}
cookie, err := r.Cookie(sessionCookieName) cookie, err := r.Cookie(sessionCookieName)
if errors.Is(err, http.ErrNoCookie) { if errors.Is(err, http.ErrNoCookie) {
return nil, whoIs, errNoSession return nil, whoIs, errNoSession

View File

@ -151,15 +151,15 @@ func TestGetTailscaleBrowserSession(t *testing.T) {
tags := views.SliceOf([]string{"tag:server"}) tags := views.SliceOf([]string{"tag:server"})
tailnetNodes := map[string]*apitype.WhoIsResponse{ tailnetNodes := map[string]*apitype.WhoIsResponse{
userANodeIP: { userANodeIP: {
Node: &tailcfg.Node{ID: 1}, Node: &tailcfg.Node{ID: 1, StableID: "1"},
UserProfile: userA, UserProfile: userA,
}, },
userBNodeIP: { userBNodeIP: {
Node: &tailcfg.Node{ID: 2}, Node: &tailcfg.Node{ID: 2, StableID: "2"},
UserProfile: userB, UserProfile: userB,
}, },
taggedNodeIP: { taggedNodeIP: {
Node: &tailcfg.Node{ID: 3, Tags: tags.AsSlice()}, Node: &tailcfg.Node{ID: 3, StableID: "3", Tags: tags.AsSlice()},
}, },
} }
@ -240,11 +240,26 @@ func TestGetTailscaleBrowserSession(t *testing.T) {
wantError: errNotOwner, wantError: errNotOwner,
}, },
{ {
name: "tagged-source", name: "tagged-remote-source",
selfNode: &ipnstate.PeerStatus{ID: "self", UserID: userA.ID}, selfNode: &ipnstate.PeerStatus{ID: "self", UserID: userA.ID},
remoteAddr: taggedNodeIP, remoteAddr: taggedNodeIP,
wantSession: nil, wantSession: nil,
wantError: errTaggedSource, wantError: errTaggedRemoteSource,
},
{
name: "tagged-local-source",
selfNode: &ipnstate.PeerStatus{ID: "3"},
remoteAddr: taggedNodeIP, // same node as selfNode
wantSession: nil,
wantError: errTaggedLocalSource,
},
{
name: "not-tagged-local-source",
selfNode: &ipnstate.PeerStatus{ID: "1", UserID: userA.ID},
remoteAddr: userANodeIP, // same node as selfNode
cookie: userASession.ID,
wantSession: userASession,
wantError: nil, // should not error
}, },
{ {
name: "has-session", name: "has-session",