yjs / y-websocket

Websocket Connector for Yjs
https://docs.yjs.dev/ecosystem/connection-provider/y-websocket
MIT License
495 stars 256 forks source link

Authentication testing #7

Closed dmonad closed 3 years ago

dmonad commented 4 years ago

The proposed auth method should be tested. A client shouldn't try to reconnect after auth has been rejected and the server should properly close the connection.

From @WinstonFasset in the Gitter channel:

I got auth working a while back based on that marker in bin/server.js, but it's been a bit of a struggle and is still a bit of a mess. If anyone else has had any success I'd love to know what they did. One question is how do you authenticate? A cookie? A request parameter? Send the message on a socket? I went with an access_token querystring parameter (over SSL) the other thing I wasn't sure of is how to reject access. I tried closing/destroying/terminating the socket and it caused the browser to start spooling up infinite new websockets, so I set a 10s timeout before closing. also once I added this I started seeing server crashes caused by what I think was a memory leak here's the code I ended up with. apologies for the messiness. some of it may not be strictly necessary:

server.on('upgrade', (request, socket, upgradeHead) => {
  var head = new Buffer(upgradeHead.length);
  upgradeHead.copy(head);
  upgradeHead = null
  // You may check auth of request here..
  // or check cookie here
  // console.log('CHECK AUTH', request.url, request.headers.cookie)
  /**
   * @param {any} ws
   */
  const handleAuth = async ws => {
    const parsed = url.parse(request.url, true)
    const access_token = parsed.query.access_token
    const docName = parsed.pathname
    // console.log('parsed', parsed)
    request.url = (parsed.origin||'') + docName
    const access = await verifyAccess(access_token, docName)
    const { user } = access
    if (user) {
      // console.log('connecting inner request url', request.url)
      request.user = user
      wss.emit('connection', ws, request)
      // release ref?
      socket.unref()
      socket = null
    }
    // if (true || request.headers.cookie.indexOf('tehsecretpassword')> -1) {
      // } 
    else {
      console.log('access denied', request.url)
      // socket.close();
      setTimeout(() => {
        socket.destroy();
        socket = null;
      }, 10000)
      // socket.close()
      // socket.terminate()
      return;
    }
  }
  wss.handleUpgrade(request, socket, head, handleAuth)
})
WinstonFassett commented 4 years ago

I've been running with the above approach for several months and it works, including reconnecting but it's got some issues.

So I set about rewriting last night and while researching, stumbled across this issue. So here's my new approach.

Authenticate websocket connections over websockets

Today I rewrote my authentication implementation to authenticate over websockets before handing off to YJS and I like it a lot better.

Basically, on server:

same on client:

Unfortunately it's a bit invasive, but at this point I have my own local embedded fork of y-websocket server code rewritten as a class, so I tried it and it worked.

Sample Code

Allow the connection, wire up to the message handler and set conn.authenticated = false. But do NOT call setupWSConnection yet.

  _onConnection (conn, req, { docName = req.url.slice(1), gc = true } = {}) {
    conn.authenticated = false;
    conn.on('message', message => {
      this._onMessage(conn, conn.doc, message);
    }); 
  }  

In message handler, if the message is a string, authenticate with it. Once authenticated, forward messages to original YJS message handler.

  async _onMessage (conn, doc, message) {
    if (typeof message === 'string') {
      const authenticated = await this._authenticate(conn, message);
      conn.authenticated = authenticated;
      if (authenticated) {        
        conn.send('authenticated');
        this._setupWSConnection(conn, conn.docName, conn.gc);
      } else {
        conn.send('access-denied');
        conn.close();
      }
    } else if (conn.authenticated) {
      this._onCollabMessage(conn, doc, new Uint8Array(message));
    } else {    
      // being precautious but not actually using this yet.  may not need it
      conn.preauthenticatedMessages.push(message);
    }
  }

On the client I currently monkey-patch provider.ws to plug in the auth handlers but I might move this into the y-websocket client:

  const provider = new WebsocketProvider(VUE_APP_YS_ENDPOINT, docName , ydoc);

  const onConnecting = () => {
    const provider_onmessage = provider.ws.onmessage;
    provider.ws.onmessage = event => {
      const { data } = event
      if (typeof data === 'string') {
        switch (data) {
          case 'authenticated':
            provider.ws.onmessage = provider_onmessage
            provider_onopen();
            break;
          case 'authentication-failed':
            provider.disconnect();
            break;
          default:
            break;
        }
      }
    }    
    const provider_onopen = provider.ws.onopen
    provider.ws.onopen = event => {      
      provider.ws.send(accessToken);
    }
  }
  onConnecting();

Because the y-websocket client calls setupWS on every reconnection attempt, I have to monkey-patch it every time to get it to go through authentication again. So I have y-websocket emit a 'status' event with a status of 'connecting', and I use that to re-apply the authentication handlers as new websockets are created.

  provider.on('status', ({status}) => {
    if (status === 'connecting') {
      onConnecting()
    }
  })

This was the shortest distance I could find to a working implementation.

The server is difficult to extend from the outside, with the binary protocol and lack of class methods to override or options to override behavior. I'm not sure how best to extend it or whether it is intended to be forked rather than extended, but here are some ideas:

ivan-nemtinov commented 3 years ago

To do auth and stop client reconnections we can use websocket's connection.close(CUSTOM_CODE). It's fast, because websockets are not used to send auth messages.

On the server side:

export const WEBSOCKET_AUTH_FAILED = 4000;

this.wss.on("connection", this.onConnection); protected onConnection = async (connection: WebSocket, request: http.IncomingMessage) => { console.log("socket on connection");

// authenticate can get JWT from URL or cookie
if (!this.authenticate(request)) {
    connection.close(WEBSOCKET_AUTH_FAILED);
}

...

}

On the client side (modify y-websocket.js), see // Do not reconnect if auth failed:

websocket.onclose = (e) => {
  provider.ws = null
  provider.wsconnecting = false
  if (provider.wsconnected) {
    provider.wsconnected = false
    provider.synced = false
    // update awareness (all users except local left)
    awarenessProtocol.removeAwarenessStates(provider.awareness, Array.from(provider.awareness.getStates().keys()).filter(client => client !== provider.doc.clientID), provider)
    provider.emit('status', [{
      status: 'disconnected'
    }])
  } else {
    provider.wsUnsuccessfulReconnects++
  }

  // Do not reconnect if auth failed
  if(e.code === WEBSOCKET_AUTH_FAILED) {
      console.log("Auth failed", e.code);
      return;
  }

  // Start with no reconnect timeout and increase timeout by
  // log10(wsUnsuccessfulReconnects).
  // The idea is to increase reconnect timeout slowly and have no reconnect
  // timeout at the beginning (log(1) = 0)
  setTimeout(setupWS, math.min(math.log10(provider.wsUnsuccessfulReconnects + 1) * reconnectTimeoutBase, maxReconnectTimeout), provider)
}
janaka commented 1 year ago

Hey @WinstonFassett, thanks for your details post on authN. I you changes to y-protocol and y-websocket to add support didn't get merged into the main project right?

Is forking the best approach here? just want to make sure I'm not missing a trick.

raineorshine commented 1 year ago

I took @WinstonFassett's code and made a fork that supports generic access token authentication. The server exports an authenticate method, and the client adds the auth field to the WebsocketProvider constructor. Pass your access token on auth and it will be forwarded to the authenticate method:

Server:

const { createServer } = require('y-websocket-auth/server')

const server = createServer({ 
  // accessToken is passed as { auth: ACCESS_TOKEN } 
  // in the WebsocketProvider constructor on the client-side
  authenticate: (accessToken: string) => {
    // do authentication
    return true
  }
})

server.listen(port, host, () => {
  console.log(`running at '${host}' on port ${port}`)
})

Client:

const wsProvider = new WebsocketProvider(
  'ws://localhost:1234', 
  'my-roomname', 
  ydoc, 
  { auth: ACCESS_TOKEN }
)

Hope this helps others. Happy to accept contributions if you'd like to extend the API.

jiangxiaoqiang commented 11 months ago

I took @WinstonFassett's code and made a fork that supports generic access token authentication. The server exports an authenticate method, and the client adds the auth field to the WebsocketProvider constructor. Pass your access token on auth and it will be forwarded to the authenticate method:

Server:

const { createServer } = require('y-websocket-auth/server')

const server = createServer({ 
  // accessToken is passed as { auth: ACCESS_TOKEN } 
  // in the WebsocketProvider constructor on the client-side
  authenticate: (accessToken: string) => {
    // do authentication
    return true
  }
})

server.listen(port, host, () => {
  console.log(`running at '${host}' on port ${port}`)
})

Client:

const wsProvider = new WebsocketProvider(
  'ws://localhost:1234', 
  'my-roomname', 
  ydoc, 
  { auth: ACCESS_TOKEN }
)

Hope this helps others. Happy to accept contributions if you'd like to extend the API.

the demo did not mention the auth failed logic. the key is how to handle the failed of auth.