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

docs: added websocket authentication #8462

Closed
wants to merge 8 commits into from

Conversation

juzhiyuan
Copy link
Member

@juzhiyuan juzhiyuan commented Dec 6, 2022

Description

This tutorial uses key-auth to authenticate before connecting to the upstream WebSocket server.

Rendered Version: https://github.com/apache/apisix/blob/docs/tutorial-websocket-authentication/docs/en/latest/tutorials/websocket-authentication.md

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@juzhiyuan juzhiyuan requested a review from pottekkat December 6, 2022 08:08
@juzhiyuan juzhiyuan requested a review from bzp2010 December 6, 2022 08:19
Copy link
Contributor

@bzp2010 bzp2010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content of the article looks good, but there is a problem here.

AFAIK, the browser's WebSocket does not provide an API to allow users to set their own initial request headers. It will affect browser users, but not users who build their own clients.

So for browsers, we'd better switch to using the query parameter as authentication to start a WS connection, otherwise it will be difficult for users to use the authentication plugin provided by APISIX in the browser.

Perhaps it would be better to provide a truly dedicated websocket plugin in the future. 😊


Connect `ws://127.0.0.1:9080/raw` without `key`, APISIX returns `401 Unauthorized` status code.

![Connect without Key](https://static.apiseven.com/2022/12/06/638ef6db9dd4b.png)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is a mosaic used in the picture? 🤔

1. Add one header `apikey` with value `this_is_the_key`;
2. Connect `ws://127.0.0.1:9080/raw` with `key`, it's successfully.

![Connect with key](https://static.apiseven.com/2022/12/06/638efac7c42b6.png)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto


To establish a WebSocket connection, the client sends a WebSocket **handshake** request, for which the server returns a WebSocket handshake response, see below:

**Client Request**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to change bolds to H3 heading?


Apache APISIX supports [WebSocket](https://en.wikipedia.org/wiki/WebSocket) traffic, but the WebSocket protocol doesn't handle authentication. This article guides you on how to configure authentication for WebSocket connections.

## WebSocket Protocol
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All headings should follow sentence case as stated in the style guide: https://apisix.apache.org/docs/general/documentation-style-guide/#formatting-punctuation-and-organization


When establishing one connection from Client to Server, in the **handshake** phase, APISIX first checks its authentication information, then chooses to proxy this request or deny it directly.

### Pre-requisite
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Pre-requisite
### Prerequisites


### Key Auth

#### Create one Route
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#### Create one Route
#### Create a Route

}'
```

#### Create one Consumer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#### Create one Consumer
#### Create a Consumer

Comment on lines +141 to +143
### Note

Other authentication methods are similar to this one.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this note to the top and use the Docusaurus admonition :::note.

Comment on lines +145 to +147
## Reference

1. [Wikipedia - WebSocket](https://en.wikipedia.org/wiki/WebSocket)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not generally have separate references section. You can just link it inline as you have done in the first sentence. We can remove this section.

@github-actions
Copy link

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Apr 23, 2023
@pottekkat
Copy link
Member

I will update this PR because I think @juzhiyuan is busy elsewhere.

@pottekkat
Copy link
Member

Opened new PR to add this tutorial. Closing this one.

See: #9369

@pottekkat pottekkat closed this Apr 25, 2023
@membphis membphis deleted the docs/tutorial-websocket-authentication branch August 2, 2023 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants