Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doesn't appear to work.... #1

Open
ProductOfAmerica opened this issue Nov 26, 2017 · 9 comments
Open

Doesn't appear to work.... #1

ProductOfAmerica opened this issue Nov 26, 2017 · 9 comments
Labels

Comments

@ProductOfAmerica
Copy link

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.

@Root-Core Root-Core added the bug label Nov 28, 2017
@Root-Core
Copy link
Owner

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

@ProductOfAmerica
Copy link
Author

ProductOfAmerica commented Nov 29, 2017

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();
   }
}

@Root-Core
Copy link
Owner

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?

@ProductOfAmerica
Copy link
Author

ProductOfAmerica commented Dec 1, 2017

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.

@iamjochem
Copy link

hi,

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

@Root-Core
Copy link
Owner

@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'.

@stephen-dahl
Copy link

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.

@ProductOfAmerica
Copy link
Author

I would agree with @FreakinaBox

@ProductOfAmerica
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants