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

set withCredentials to true before xhr to fix authentication #335

Merged
merged 1 commit into from
Nov 22, 2011
Merged

set withCredentials to true before xhr to fix authentication #335

merged 1 commit into from
Nov 22, 2011

Conversation

gavinuhma
Copy link
Contributor

set withCredentials to true before xhr to fix authentication. Bug #333 and server bug socketio/socket.io#625

@3rd-Eden
Copy link
Contributor

Looks good to me.

@3rd-Eden
Copy link
Contributor

@gavinuhma actually, you also need to set the Access-Control-Allow-Credientials or you will get a shitload of Credentials flag is true, but Access-Control-Allow-Credentials is not "true". messages.

@gavinuhma
Copy link
Contributor Author

@3rd-Eden that happens in socket.io server already. This pull request requires the pull request I submitted here:
socketio/socket.io#630

Here's the commit:
https://github.com/gavinuhma/socket.io/commit/e4a9342e8b029d8dc251f13b6320500e809c7921

@3rd-Eden
Copy link
Contributor

Yeh I might have posted it in the wrong repo, but I implemented you fix in my reconnect branch (3rd-Eden@5927d41#L4R178) and it does not pass the test suite as it's giving that message Credentials flag is true, but Access-Control-Allow-Credentials is not "true". the only fix for it is remove

https://github.com/LearnBoost/socket.io/pull/630/files#L0R725 that cookie check if statement.

@gavinuhma
Copy link
Contributor Author

aw that sounds right. our tests always had a cookie so we wouldn't have ran into that case

rauchg added a commit that referenced this pull request Nov 22, 2011
set withCredentials to true before xhr to fix authentication
@rauchg rauchg merged commit e7fc25e into socketio:master Nov 22, 2011
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

Successfully merging this pull request may close these issues.

3 participants