HttpAuthorizer only accepts auth responses with status 200 OK

[hovsater]

Currently, the HttpAuthorizer only accepts auth responses that have the HTTP status 200 OK. See https://github.com/pusher/pusher-websocket-java/blob/master/src/main/java/com/pusher/client/util/HttpAuthorizer.java#L141.

If an auth endpoint returns a correct auth signature but the response status is 201 Created, the HttpAuhorizer will simply fail the authorization. I went through the documentation of auth signatures but couldn't find any references to what status code the auth endpoint is expected to return.

If the only valid status code is 200 OK, I'd think it would be great to add it to the documentation. If, 201 OK is considered a valid status code as well, I'd be happy to create a pull request for it.

Cheers!

[zimbatm]

Thanks. This is a bug in the library, the authorizer should accept any valid response.

[mdpye]

I think we've had this one before, and I respectfully disagree. There are two arguments:

One is an appeal to the general principal: there should be exactly one correct way to do something.

I realise this can't be shown to be a universal truth, so the other argument is an appeal to follow the HTTP spec: http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

Under my reading, 201 should not be used unless a hypertext resource has been created. That's something you can reference with a URI - 201 responses should have a Location header to give a reference to the created resource.

Our authentication tokens are not resources in this sense, the response should be 200.

The documentation should specify the 200 requirement.

[hovsater]

@mdpye fair argument. I still suggest that this is documented as a requirement in the Pusher documentation. Currently there are no references regarding this at all.

In a perfect world, this would not have been necessary. Unfortunately, HTTP status codes are a subject of great abuse and I can imagine people having a hard time debugging this when the actual HTTP body includes a correct signature but it fails due to incorrect response status.

What do you think?

[mdpye]

I completely agree about the documentation and I've opened a pull request internally to add a short section on response codes and body format to https://pusher.com/docs/authenticating_users

[hovsater]

@mdpye I think it's safe to say that we can close this now since the documentation is updated. Right?

[mdpye]

Agreed.

[mukeshsolanki]

@mdpye There seems to be inconsistent with this and LibPusher used for iOS. Please try to make it consistent.

Snippet form LibPusher
if ([URLResponse isKindOfClass:[NSHTTPURLResponse class]]) { _authorized = ([(NSHTTPURLResponse *)URLResponse statusCode] == 200 || [(NSHTTPURLResponse *)URLResponse statusCode] == 201); }

[mdpye]

As we've already come to the conclusion that 201 doesn't make sense and the documentation reflects the behaviour here, we would make it consistent by closing the loophole in libPusher. But this would be a breaking change for people who only use that, which would require them to change their server-side endpoint to fix.

There would need to hear a strong argument for disrupting those people.

[mukeshsolanki]

@mdpye I can understand your concern about making a lot of people change their code, but having inconsistency between iOS and Android already forces sever side code to make the change anyways and that being said is a strong enough argument to keep it consistent.

[mdpye]

OK, compatibility trumps correctness: #120

I do encourage you to read the spec for a given status code before choosing to return it from your web app in future 😛