socket-controllers icon indicating copy to clipboard operation
socket-controllers copied to clipboard

fix: Middlewares not executed when setting up with `useSocketServer`

Open sittingbool opened this issue 3 years ago • 8 comments

Description

Hi, I wanted to use a middleware for doing authentication.

The problem is: The middleware is never ever being called.

Following setup:

const io = require('socket.io')(server, {
      cors: {
          origin: '*', // ['http://localhost:4200', 'http://localhost:63342'],
          credentials: true
      }
  });

  useSocketServer(io, {
      controllers: [__dirname + '/controller/socket/*.js'],
      middlewares: [SocketAuthenticationMiddleware]
  });
@Middleware()
export class SocketAuthenticationMiddleware implements MiddlewareInterface {

    use(socket: Socket, next: express.NextFunction): void {
        const token = <string>socket.request.headers['x-api-token']; // break point here -> never called
        if(stringIsEmpty(token)) {
            console.error('denied connection due to  missing token ' + token);
            next(new Error(UserErrors.tokenInvalid));
        }
        console.log('Successfully connected socket for ' + token);
        next();
    }
}
@SocketController('/chat')
export class ChatController extends RootController {
    @OnConnect()
    connection(@ConnectedSocket() socket: Socket): void {
        console.log('client connected to chat: ' + socket.id); // <<< this one is called despite missing token
    }

Expected behavior

Middleware code being executed

Actual behavior

Middleware code not being executed

sittingbool avatar May 25 '21 09:05 sittingbool

I have a similar problem

yakovets-evgeny avatar Jun 01 '21 21:06 yakovets-evgeny

Me too

ltrillaud avatar Jun 23 '21 15:06 ltrillaud

This is now almost a year old. I am having this again in another project. Using socket.io 4. Ist this project still being maintained?

sittingbool avatar Apr 06 '22 11:04 sittingbool

Yes, but unfortunately the time we can allocate is limited. I’m planning some refactoring and support for latest socket.io in the near future.

attilaorosz avatar Apr 06 '22 11:04 attilaorosz

I am doing some research currently.

My setup: Node LTS, express server.

The problem is located here: https://github.com/typestack/socket-controllers/blob/eea8d9593c025502bc95350e8f36c5b248bc1db8/src/SocketControllerExecutor.ts#L74

The middleware is not called, because the socket.io middleware in which it is wrapped is not called. So this might be a problem with the combination socket.io on an express server. I will dig deeper.

sittingbool avatar Apr 06 '22 11:04 sittingbool

One other suggestion: I am using a custom namespace. It seems like the middleware is namespace-specific:

https://github.com/socketio/socket.io/issues/3082

sittingbool avatar Apr 06 '22 11:04 sittingbool

Confirmed last statement:

Tested with code like this:




    io.of('/my-namespace')
        .use((socket, next) => {
            console.log('middleware running with namespace...');
            next();
        });


    io.use((socket, next) => {
        console.log('middleware running in general...');
        next();
    });

    useSocketServer(io, {
        controllers: [__dirname + '/controllers/*.ctrl.js'],
        middlewares: [__dirname + '/middlewares/*.mw.js']
    });

leads to following output:

Listening on port 3000
Database loaded
middleware running with namespace...

sittingbool avatar Apr 06 '22 11:04 sittingbool

I'll propose a solution in a PR

sittingbool avatar Apr 06 '22 11:04 sittingbool

Closing as this has been resolved in #511

attilaorosz avatar Dec 22 '22 16:12 attilaorosz

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

github-actions[bot] avatar Jan 22 '23 01:01 github-actions[bot]