socketio-jwt icon indicating copy to clipboard operation
socketio-jwt copied to clipboard

Doesn't appear to work....

Open ProductOfAmerica opened this issue 8 years ago • 9 comments

I have submitted the wrong JWT to my server, and my server responds with an 'error', and sends a JSON object as a result instead of emitting 'unauthorized'. The instructions say to do this:

Client side:

Add a callback client-side to execute socket disconnect server-side.

socket.on('unauthorized', function(error, callback) {

This .on('unauthorized') on my client never gets called, however the .on('error') gets called.

ProductOfAmerica avatar Nov 26 '17 10:11 ProductOfAmerica

Could you provide a very small example? I will look into it, but i'm really busy atm.

Root-Core avatar Nov 28 '17 22:11 Root-Core

Thanks for even getting back to me, a lot of people don't, so I appreciate that. No matter if you're busy or not, take your time. I'm also fairly new to JavaScript (proficient in Java, but I'm teaching myself JS), so I might be completely wrong here.

So I hope you know Java, since I'm using it for this example.

My first test works perfectly fine: goodjwt

But as soon as I remove the good JWT, nothing happens. Please correct me if I'm wrong, but my understanding of this middleware was that if there is a bad JWT passed, .on('unauthorized') should be called on the client?

So here it is with bad jwt, not getting a response at all: badjwt

And my code for both:

Server side (JS, using your fork):

var express = require('express');
var app = express();
var server = require('http').createServer(app);
var io = require('socket.io')(server);
var socketioJwt = require('socketio-jwt');

app.get('/', function (req, res) {
  res.status(200).end();
});

io.use(socketioJwt.authorize({
  secret: 'ilovecookies',
  handshake: true
}));

io.on('connection', function (socket) {
  console.log('hello! ', socket.decoded_token);
});

server.listen(9000);

Here's my package.json, just to prove I'm using the most current version:

{
  "name": "sockets_test",
  "version": "0.0.1",
  "dependencies": {
    "express": "^4.16.2",
    "socket.io": "^2.0.4",
    "socketio-jwt": "root-core/socketio-jwt"
  }
}

Client side (just using the 1.0 maven repository library)

import io.socket.client.IO;
import io.socket.client.Socket;

import java.net.URISyntaxException;

public class RootCoreTest {
   public static void main(String[] args) throws URISyntaxException {
      String testToken = "badjwt";

      IO.Options options = new IO.Options();
      options.forceNew = true;
      options.query = "token=" + testToken;

      Socket socket = IO.socket("http://localhost:9000", options);

      socket.on(Socket.EVENT_CONNECT, objects -> {
         System.out.println("connected");
      }).on("unauthorized", objects -> {
         System.out.println("unauthorized");
      });

      socket.connect();
   }
}

ProductOfAmerica avatar Nov 29 '17 09:11 ProductOfAmerica

Okay, i have tested some stuff. You are right, response is send as "error", not as "unauthorized". I will try to fix this behavior and / or the documentation. Both "error" types are errors after all, so it might be better to send "error" in all cases?

Root-Core avatar Nov 29 '17 19:11 Root-Core

I'm glad that I'm not the only one getting problems! I guess the solution is to use the .on('error') listener, but it's more of a personal preference thing I guess. Personally, I think it'd be a little more organized to have a listener for an unauthorized call, but that's just me. Otherwise, I'm having to filter through more than one error in the error listener, for example:

socket.on(Socket.EVENT_CONNECT, objects -> {
   System.out.println("connected");
}).on(Socket.EVENT_ERROR, objects -> {
   for (Object object : objects) {
      switch (object.toString()) {
         case "unauthorized":
            // do something here
            break;
         case "someothererror":
            // do something else here
            break;
         case "someothererror2":
            // do something else here 2
            break;
      }
   }
});

So it's a difference of conglomerating all errors (including unauthorized) into .on('error') or just moving 'unauthorized' to it's own listener.

ProductOfAmerica avatar Dec 01 '17 04:12 ProductOfAmerica

hi,

it looks like the 'unauthorized" events are only emitted if the handshake option is set to FALSE.

iamjochem avatar Dec 09 '17 11:12 iamjochem

@iamjochem Thanks for investigation, we should fix this. But first of all we should determine, if the 'unauthorized' or 'error' should be triggered. I would vote for 'unauthorized'.

Root-Core avatar Dec 09 '17 20:12 Root-Core

you should not use 'error'. uncaught 'error' events will bring down the whole node process and I don't think we want that to happen if someone fails to log in.

stephen-dahl avatar Dec 11 '17 16:12 stephen-dahl

I would agree with @FreakinaBox

ProductOfAmerica avatar Dec 18 '17 22:12 ProductOfAmerica

@Root-Core Any word on updates? I know you said you were busy.

ProductOfAmerica avatar Dec 29 '17 13:12 ProductOfAmerica