fix incorrect error handling logic when a JWT is created wrongly; closes pterodactyl/panel#3295

Prior to this logic not only was the error response incorrect for events, but we registered event listeners before the authentication event; so if auth failed we flooded the socket with tons of output that was never going to be sent anyways.

This change now waits to register listeners until the socket is fully authenticated and we're guaranteed to have a token present.
This commit is contained in:
Dane Everitt 2021-10-25 21:23:45 -07:00
parent 023d7ec1ec
commit 32d6594476
3 changed files with 36 additions and 28 deletions

View File

@ -5,7 +5,6 @@ import (
"encoding/json" "encoding/json"
"time" "time"
"emperror.dev/errors"
"github.com/gin-gonic/gin" "github.com/gin-gonic/gin"
ws "github.com/gorilla/websocket" ws "github.com/gorilla/websocket"
@ -64,17 +63,6 @@ func getServerWebsocket(c *gin.Context) {
} }
}() }()
go func() {
if err := handler.ListenForServerEvents(ctx); err != nil {
handler.Logger().Warn("error while processing server event; closing websocket connection")
if err := handler.Connection.Close(); err != nil {
handler.Logger().WithField("error", errors.WithStack(err)).Error("error closing websocket connection")
}
}
}()
go handler.ListenForExpiration(ctx)
for { for {
j := websocket.Message{} j := websocket.Message{}
@ -94,7 +82,7 @@ func getServerWebsocket(c *gin.Context) {
} }
go func(msg websocket.Message) { go func(msg websocket.Message) {
if err := handler.HandleInbound(msg); err != nil { if err := handler.HandleInbound(ctx, msg); err != nil {
handler.SendErrorJson(msg, err) handler.SendErrorJson(msg, err)
} }
}(j) }(j)

View File

@ -10,11 +10,37 @@ import (
"github.com/pterodactyl/wings/server" "github.com/pterodactyl/wings/server"
) )
// RegisterListenerEvents will setup the server event listeners and expiration
// timers. This is only triggered on first authentication with the websocket,
// reconnections will not call it.
//
// This needs to be called once the socket is properly authenticated otherwise
// you'll get a flood of error spam in the output that doesn't make sense because
// Docker events being output to the socket will fail when it hasn't been
// properly initialized yet.
//
// @see https://github.com/pterodactyl/panel/issues/3295
func (h *Handler) registerListenerEvents(ctx context.Context) {
h.Logger().Debug("registering event listeners for connection")
go func() {
if err := h.listenForServerEvents(ctx); err != nil {
h.Logger().Warn("error while processing server event; closing websocket connection")
if err := h.Connection.Close(); err != nil {
h.Logger().WithField("error", errors.WithStack(err)).Error("error closing websocket connection")
}
}
}()
go h.listenForExpiration(ctx)
}
// ListenForExpiration checks the time to expiration on the JWT every 30 seconds // ListenForExpiration checks the time to expiration on the JWT every 30 seconds
// until the token has expired. If we are within 3 minutes of the token expiring, // until the token has expired. If we are within 3 minutes of the token expiring,
// send a notice over the socket that it is expiring soon. If it has expired, // send a notice over the socket that it is expiring soon. If it has expired,
// send that notice as well. // send that notice as well.
func (h *Handler) ListenForExpiration(ctx context.Context) { func (h *Handler) listenForExpiration(ctx context.Context) {
// Make a ticker and completion channel that is used to continuously poll the // Make a ticker and completion channel that is used to continuously poll the
// JWT stored in the session to send events to the socket when it is expiring. // JWT stored in the session to send events to the socket when it is expiring.
ticker := time.NewTicker(time.Second * 30) ticker := time.NewTicker(time.Second * 30)
@ -54,12 +80,11 @@ var e = []string{
// ListenForServerEvents will listen for different events happening on a server // ListenForServerEvents will listen for different events happening on a server
// and send them along to the connected websocket client. This function will // and send them along to the connected websocket client. This function will
// block until the context provided to it is canceled. // block until the context provided to it is canceled.
func (h *Handler) ListenForServerEvents(pctx context.Context) error { func (h *Handler) listenForServerEvents(pctx context.Context) error {
var o sync.Once var o sync.Once
var err error var err error
ctx, cancel := context.WithCancel(pctx) ctx, cancel := context.WithCancel(pctx)
h.Logger().Debug("listening for server events")
callback := func(e events.Event) { callback := func(e events.Event) {
if sendErr := h.SendJson(&Message{Event: e.Topic, Args: []string{e.Data}}); sendErr != nil { if sendErr := h.SendJson(&Message{Event: e.Topic, Args: []string{e.Data}}); sendErr != nil {
h.Logger().WithField("event", e.Topic).WithField("error", sendErr).Error("failed to send event over server websocket") h.Logger().WithField("event", e.Topic).WithField("error", sendErr).Error("failed to send event over server websocket")

View File

@ -269,7 +269,7 @@ func (h *Handler) setJwt(token *tokens.WebsocketPayload) {
} }
// HandleInbound handles an inbound socket request and route it to the proper action. // HandleInbound handles an inbound socket request and route it to the proper action.
func (h *Handler) HandleInbound(m Message) error { func (h *Handler) HandleInbound(ctx context.Context, m Message) error {
if m.Event != AuthenticationEvent { if m.Event != AuthenticationEvent {
if err := h.TokenValid(); err != nil { if err := h.TokenValid(); err != nil {
h.unsafeSendJson(Message{ h.unsafeSendJson(Message{
@ -285,13 +285,6 @@ func (h *Handler) HandleInbound(m Message) error {
{ {
token, err := NewTokenPayload([]byte(strings.Join(m.Args, ""))) token, err := NewTokenPayload([]byte(strings.Join(m.Args, "")))
if err != nil { if err != nil {
// If the error says the JWT expired, send a token expired
// event and hopefully the client renews the token.
if err == jwt.ErrExpValidation {
h.SendJson(&Message{Event: TokenExpiredEvent})
return nil
}
return err return err
} }
@ -304,10 +297,7 @@ func (h *Handler) HandleInbound(m Message) error {
h.setJwt(token) h.setJwt(token)
// Tell the client they authenticated successfully. // Tell the client they authenticated successfully.
h.unsafeSendJson(Message{ h.unsafeSendJson(Message{Event: AuthenticationSuccessEvent})
Event: AuthenticationSuccessEvent,
Args: []string{},
})
// Check if the client was refreshing their authentication token // Check if the client was refreshing their authentication token
// instead of authenticating for the first time. // instead of authenticating for the first time.
@ -317,6 +307,11 @@ func (h *Handler) HandleInbound(m Message) error {
return nil return nil
} }
// Now that we've authenticated with the token and confirmed that we're not
// reconnecting to the socket, register the event listeners for the server and
// the token expiration.
h.registerListenerEvents(ctx)
// On every authentication event, send the current server status back // On every authentication event, send the current server status back
// to the client. :) // to the client. :)
state := h.server.Environment.State() state := h.server.Environment.State()