nats-io / stan.go

NATS Streaming System
https://nats.io
Apache License 2.0
706 stars 117 forks source link

NATS ErrAuthorization error hidden by STAN ErrTimeout #288

Closed Will2817 closed 5 years ago

Will2817 commented 5 years ago

stan.go version: 0.5.0

Example: Start up a NATS Streaming server with nats configuration with a user that doesn't have permission to publish to a channel. Using the client connect to NATS Streaming as the above user and attempt to publish to the channel. This results in a stan.ErrTimeout error and hides the nats.ErrAuthorization error and makes an authorization error look like publish timeout error.

kozlovic commented 5 years ago

Unfortunately, not sure if this can be solved. The low level publish is not a req/reply, so the message is dropped even before reaching the streaming "server" (that is, the message is not processed by the streaming server layer). If the library owns the NATS Connection, we could set up an async error callback, but I am not sure how well we would be able to correlate the auth violation with the publish calls. @derekcollison This is something that may affect JS too since if the publish provides a reply to it is expected to get an ACK but again, since message will be dropped early in NATS Server, the app may still wait until timeout (if doing req/reply).

derekcollison commented 5 years ago

This will be handled naturally in JetStream since it will just be a Publish or Request (where you want the pub ack) to a normal, non-munged subject. So will be straightforward to detect authorization violations.

kozlovic commented 5 years ago

Not that straightforward because the publisher would do nc.Request("foo", []byte("msg"), time.Second) and would still have the timeout here. In the err callback there will be the notification that a publish failed. It is true for any request that we do today, just wanted to raise that in the context of a "persistent" publish.

derekcollison commented 5 years ago

image

derekcollison commented 5 years ago

That is how we show auth errors in the nats-req example for the Go client.

kozlovic commented 5 years ago

@Will2817 Would above work for you? I did test and yes, the Publish() will have to timeout, but using code above (note that you can get the low level NATS connection with stan.Conn.NatsConn()), you would get something like this:

$ go run examples/stan-pub/main.go -s nats://user:pass@localhost:4222 foo msg1
Error during publish: nats: Permissions Violation for Publish to "_STAN.pub.A27HSoVWiUyq2VqNIxUmmU.foo"
exit status 1
Will2817 commented 5 years ago

If I'm using stanClient.AsyncPublish() and get nc.LastError() in the callback is there a chance the last error may actually be for a different publish attempt?

kozlovic commented 5 years ago

@Will2817 There is always a risk that the last error changes depending on the events occurring. That being said, I would think that if your application is publishing to a channel that it is not allowed to, it is likely a misconfiguration and your app needs to be fixed. So capturing once this error should alert you that the application should be looked at. In other words, it is almost a catch of a fatal error (I would think). So I agree that the solution is far from perfect (especially if your app have many go routines publishing to different channels, or if there is a NATS disconnect, etc..), but it should allow you to catch the permission error (by the way, you could check specifically for publish permission violation when checking the LastError(), and not assume that if there is a LastError() it is a publish perm violation: as seen above, the text does say that it is a publish perm error and gives you the subject).

Will2817 commented 5 years ago

@kozlovic fair point, this should be a rare occurrence, and I only need to catch it once to know something is wrong. Thanks.

kozlovic commented 5 years ago

Thank you for your understanding. Shall we close the issue?