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

[WebSocket Client] Make the browser client support the token authentication #9886

Merged

Conversation

gaoran10
Copy link
Contributor

@gaoran10 gaoran10 commented Mar 11, 2021

Fixes issue #9854

Motivation

Currently, the WebSocket client uses the HTTP request header to transport the authentication params, but the browser javascript WebSocket client couldn't add new headers.

Modifications

Use the query param token to transport the authentication token for the browser javascript WebSocket client.

Verifying this change

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

Update the WebSocket client document.

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

Copy link
Member

@zymap zymap left a comment

Choose a reason for hiding this comment

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

Is it possible to use the java WebSocket client to add an integration test for this?

@gaoran10
Copy link
Contributor Author

Is it possible to use the java WebSocket client to add an integration test for this?

There is already a mock broker test, I could add a real token case for it.

@sijie
Copy link
Member

sijie commented Mar 12, 2021

@tuteng Can you take a look at this PR?

Copy link
Member

@tuteng tuteng left a comment

Choose a reason for hiding this comment

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

LGTM

@codelipenghui codelipenghui merged commit f1f272e into apache:master Mar 14, 2021
fmiguelez pushed a commit to fmiguelez/pulsar that referenced this pull request Mar 16, 2021
…cation (apache#9886)

Fixes issue apache#9854 

### Motivation

Currently, the WebSocket client uses the HTTP request header to transport the authentication params, but the browser javascript WebSocket client couldn't add new headers.

### Modifications

Use the query param `token` to transport the authentication token for the browser javascript WebSocket client.
@sijie
Copy link
Member

sijie commented Mar 16, 2021

@codelipenghui Can you make sure this change is cherry-picked to 2.7.2?

@codelipenghui
Copy link
Contributor

ok

@codelipenghui codelipenghui added cherry-picked/branch-2.7 Archived: 2.7 is end of life and removed cherry-picked/branch-2.7 Archived: 2.7 is end of life labels Mar 23, 2021
codelipenghui pushed a commit that referenced this pull request Mar 24, 2021
…cation (#9886)

Fixes issue #9854 

### Motivation

Currently, the WebSocket client uses the HTTP request header to transport the authentication params, but the browser javascript WebSocket client couldn't add new headers.

### Modifications

Use the query param `token` to transport the authentication token for the browser javascript WebSocket client.

(cherry picked from commit f1f272e)
@gaoran10 gaoran10 deleted the ws-client-broswer-token-auth-support branch March 30, 2021 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants