Skip to content

Commit

Permalink
Display more user friendly error for invalid os logins in Web UI (#47420
Browse files Browse the repository at this point in the history
) (#47603)

Host resolution no longer happens prior to connections in the Web
UI and all dial attempts are by UUID. When an invalid login is
attempted the error message constructed only contained the info
provided to the dial request: a login and a Host UUID. To make
this error more user friendly access denied errors are now
augmented with the hostname if the user has permissions to that
host and the error occurs only due to an invalid login.

Closes #23719
  • Loading branch information
rosstimothy authored Oct 17, 2024
1 parent 4ddc069 commit c786a24
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 0 deletions.
4 changes: 4 additions & 0 deletions lib/web/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7451,6 +7451,10 @@ type authProviderMock struct {
server types.ServerV2
}

func (mock authProviderMock) ListUnifiedResources(ctx context.Context, req *authproto.ListUnifiedResourcesRequest) (*authproto.ListUnifiedResourcesResponse, error) {
return nil, nil
}

func (mock authProviderMock) GetNodes(ctx context.Context, n string) ([]types.Server, error) {
return []types.Server{&mock.server}, nil
}
Expand Down
17 changes: 17 additions & 0 deletions lib/web/terminal.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net"
"net/http"
Expand Down Expand Up @@ -100,6 +101,7 @@ type AuthProvider interface {
IsMFARequired(ctx context.Context, req *authproto.IsMFARequiredRequest) (*authproto.IsMFARequiredResponse, error)
GenerateUserSingleUseCerts(ctx context.Context) (authproto.AuthService_GenerateUserSingleUseCertsClient, error)
MaintainSessionPresence(ctx context.Context) (authproto.AuthService_MaintainSessionPresenceClient, error)
ListUnifiedResources(ctx context.Context, req *authproto.ListUnifiedResourcesRequest) (*authproto.ListUnifiedResourcesResponse, error)
}

// NewTerminal creates a web-based terminal based on WebSockets and returns a
Expand Down Expand Up @@ -885,6 +887,21 @@ func (t *sshBaseHandler) connectToNode(ctx context.Context, ws terminal.WSConn,
// The close error is ignored instead of using [trace.NewAggregate] because
// aggregate errors do not allow error inspection with things like [trace.IsAccessDenied].
_ = conn.Close()

// Since connection attempts are made via UUID and not hostname, any access denied errors
// will not contain the resolved host address. To provide an easier troubleshooting experience
// for users, attempt to resolve the hostname of the server and augment the error message with it.
if trace.IsAccessDenied(err) {
if resp, err := t.authProvider.ListUnifiedResources(ctx, &authproto.ListUnifiedResourcesRequest{
SortBy: types.SortBy{Field: types.ResourceKind},
Kinds: []string{types.KindNode},
Limit: 1,
PredicateExpression: fmt.Sprintf(`resource.metadata.name == "%s"`, t.sessionData.ServerID),
}); err == nil && len(resp.Resources) > 0 {
return nil, trace.AccessDenied("access denied to %q connecting to %v", sshConfig.User, resp.Resources[0].GetNode().GetHostname())
}
}

return nil, trace.Wrap(err)
}

Expand Down

0 comments on commit c786a24

Please sign in to comment.