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

[Proposal] Close Websocket connections when the context is done/cancelled #1727

Closed
RobinCPel opened this issue Nov 25, 2021 · 1 comment
Closed

Comments

@RobinCPel
Copy link
Contributor

Scenario

What happened?

When the websocket's configured initialization function is ran, and returns a modified context with a deadline set to the expiration time of a JWT (JSON Web Token), the websocket is not disconnected when the context is done/cancelled.

What did you expect?

That the websocket would be disconnected.

Versions

  • gqlgen version? v0.14.0
  • go version? 1.17.3 darwin/amd64
  • dep or go modules? Modules!

The Actual Proposal

Let the websocket connection close when the context is done.

Example

Add the following function to the wsConnection type:

func (c *wsConnection) closeOnCancel(ctx context.Context) {
	<-ctx.Done()
	c.close(websocket.CloseNormalClosure, "terminated")
}

And then, call it from the wsConnection's run function like so:

go c.closeOnCancel(ctx)

More feedback = more better

For this reason it would also be nice to add a way to communicate back to the user that the token has expired. Maybe through a context value like this?

// A private key for context that only this package can access. This is important
// to prevent collisions between different context uses
var closeReasonCtxKey = &wsCloseReasonContextKey{"close-reason"}
type wsCloseReasonContextKey struct {
	name string
}

func AppendCloseReason(ctx context.Context, reason string) context.Context {
	return context.WithValue(ctx, closeReasonCtxKey, reason)
}

func closeReasonForContext(ctx context.Context) string {
	reason, _ := ctx.Value(closeReasonCtxKey).(string)
	return reason
}

func (c *wsConnection) closeOnCancel(ctx context.Context) {
	<-ctx.Done()

	if r := closeReasonForContext(ctx); r != "" {
		c.sendConnectionError(r)
	}
	c.close(websocket.CloseNormalClosure, "terminated")
}

This way, the application can add a reason to the context values like so:

func WebsocketAuthorization(ctx context.Context, initPayload transport.InitPayload) (context.Context, error) {
	payloadAuth := initPayload.Authorization()
	if payloadAuth == "" {
		return ctx, errors.New("the JWT is missing in the initialization payload")
	}

	jwt, err := authenticateJWT(payloadAuth)
	if err != nil {
		return ctx, err
	}

	// Add the JWT expiration as a deadline, and add the reason
	newCtx, _ := context.WithDeadline(transport.AppendCloseReason(ctx, "authentication token has expired"), time.Unix(jwt.ExpiresAt, 0))
	return newCtx, nil
}

Sidenote

This proposal would also solve this (pretty old) issue #774.

@RobinCPel
Copy link
Contributor Author

Implemented through this PR: #1728

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

1 participant