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

Allow upgrade hook to return custom response #16

Closed
1 task
JCtapuk opened this issue Feb 26, 2024 · 7 comments
Closed
1 task

Allow upgrade hook to return custom response #16

JCtapuk opened this issue Feb 26, 2024 · 7 comments

Comments

@JCtapuk
Copy link

JCtapuk commented Feb 26, 2024

Describe the feature

Here you also need to add an authentication check during the upgrade handshake, for example, only authorized users can enter the socket in order to then save the client user information
Examle event:

const onAuth = defineEventHandler(event => {
  // valid token or more check
  event.context.user = {} // save user
})

export default defineWebSocketHandler({
  upgrade(req) {
    return false // failed connect example JWT.verify(req.headers['Token'])
    return {headers:{}} success connect
  }
  // or new method auth (before the upgrade event)
  auth(event) {
    const {token} = getQuery(event)
    if (!token) {
      throw createError('Not authorized')
    }
    
    return true // or {headers: {}}
  }
})

This is just an example since in this code there is no way to refuse clients who have passed

// Upgrade
    async upgrade(req) {
      const [res1, res2] = await Promise.all([
        opts.hooks?.upgrade?.(req),
        await resolveHook(req, "upgrade").then((h) => h?.(req)),
      ]);
      const headers = new Headers(res1?.headers);
      if (res2?.headers) {
        for (const [key, value] of new Headers(res2?.headers)) {
          headers.append(key, value);
        }
      }
      return { headers };
    },

Additional information

  • Would you be willing to help implement this feature?
@JCtapuk JCtapuk changed the title Implement intermediate ones after upgrade Implement intermediate ones before upgrade Feb 26, 2024
@pi0
Copy link
Member

pi0 commented Feb 26, 2024

Maybe by allows to throw an error we can support this?

@JCtapuk
Copy link
Author

JCtapuk commented Feb 26, 2024

Maybe by allows to throw an error we can support this?

Yes. It's possible, but we can't change the Response like that.

@pi0
Copy link
Member

pi0 commented Feb 26, 2024

We might expose a new createError({ status, body }) util to support status controllng or maybe simply something like this:

export default defineWebSocketHandler({
  async upgrade(req) {
    const authenticated = await authenticate(req);
    if (!authenticated) {
      return {
        error: { status: 401, statusMessage: 'Unauthorized' }
      };
    }
    return {
      headers
    }
  }
})

The thrown error could also be attached with those keys BTW (example createError util in h3). So we can make it compatible with 3rd party errors to control code.

@pi0 pi0 changed the title Implement intermediate ones before upgrade Implement a method to abort upgrade (auth) Feb 26, 2024
@JCtapuk
Copy link
Author

JCtapuk commented Feb 26, 2024

@pi0 good sound

@frenzzy
Copy link

frenzzy commented Mar 12, 2024

Authentication is not complete without accessing the session information for a peer.

Is there a way to, for example, bind the authenticated userId to a peer object?

Or is it supposed to be handled manually via peer.headers.cookie when necessary?
For instance, through the H3 getSession helper, which currently expects an H3Event as the first argument.

@ml1nk
Copy link

ml1nk commented Jul 2, 2024

Is there any progress on this one?

I also want to authenticate on upgrade, because doing it later on is somewhat tricky.

@pi0 pi0 changed the title Implement a method to abort upgrade (auth) Allow upgrade hook to return custom response Jul 31, 2024
@pi0
Copy link
Member

pi0 commented Aug 6, 2024

Added in #55. It will be possible in next release to use return new Response() inside upgrade hook in order to fail upgrade with an auth error.

@pi0 pi0 closed this as completed Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants