Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Throw a useful error when authorization fails #2553

Closed
analogrelay opened this issue Jun 27, 2018 · 3 comments
Closed

Throw a useful error when authorization fails #2553

analogrelay opened this issue Jun 27, 2018 · 3 comments
Labels
resolved: Not Repro Closed because we could not reproduce this issue. type: Enhancement
Milestone

Comments

@analogrelay
Copy link
Contributor

When authorization fails in the JS client the error we get is "Error". Super useful 😄 .

We should detect when authentication fails and throw a useful error like "Authentication failed". The easiest way to detect this is if the negotiate request returns 401. Also, the transports should individually try to detect similar failures and throw the same error, in case authentication fails mid-connection.

Might need something similar in the .NET Client.

@analogrelay analogrelay added this to the 2.2.0 milestone Jul 2, 2018
@analogrelay analogrelay added type: Enhancement cost: S Will take up to 2 days to complete labels Jul 2, 2018
@analogrelay analogrelay added the PRI: 2 - Preferred Preferably should be handled during the milestone. label Aug 10, 2018
@BrennanConroy
Copy link
Member

Current behavior in TS client:

[2018-08-16T20:31:17.812Z] Error: Failed to complete negotiation with the server: Error: Unauthorized 
[2018-08-16T20:31:17.813Z] Error: Failed to start the connection: Error: Unauthorized

Current behavior in .NET Client:

Connecting to http://localhost:54543/broadcast
warn: Microsoft.AspNetCore.Http.Connections.Client.Internal.LoggingHttpMessageHandler[2]
      Unsuccessful HTTP response 401 return from POST 'http://localhost:54543/broadcast/negotiate'.
fail: Microsoft.AspNetCore.Http.Connections.Client.HttpConnection[10]
      Failed to start connection. Error getting negotiation response from 'http://localhost:54543/broadcast'.
System.Net.Http.HttpRequestException: Response status code does not indicate success: 401 (Unauthorized).
   at System.Net.Http.HttpResponseMessage.EnsureSuccessStatusCode()
   at Microsoft.AspNetCore.Http.Connections.Client.HttpConnection.NegotiateAsync(Uri url, HttpClient httpClient, ILogger logger) in C:\Github\SignalR\src\Microsoft.AspNetCore.Http.Connections.Client\HttpConnection.cs:line 425

Both mention not being authorized. Do we want even more? Haven't tried out in the middle of a connection yet. That's next.

@BrennanConroy
Copy link
Member

BrennanConroy commented Aug 16, 2018

Current behavior for token expiring in the middle of the connection:

  • Websockets - Does nothing, can keep sending and receiving (we have https://github.com/aspnet/SignalR/issues/1159 filed already)
  • ServerSentEvents - Can no longer send, but can still receive forever (TS only, .NET Closes connection)
  • LongPolling - Can no longer send, can receive one more message or until current poll times out (TS only, .NET Closes connection)

We might want to file issues for the SSE and LongPolling behaviors, but I think this issue can be closed as the logs do say that the connection/request was Unauthorized

@anurse thoughts?

@analogrelay
Copy link
Contributor Author

Agreed. I updated #1159 to cover these cases. Closing this in lieu of that issue

@analogrelay analogrelay added resolved: Not Repro Closed because we could not reproduce this issue. and removed PRI: 2 - Preferred Preferably should be handled during the milestone. cost: S Will take up to 2 days to complete labels Aug 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
resolved: Not Repro Closed because we could not reproduce this issue. type: Enhancement
Projects
None yet
Development

No branches or pull requests

2 participants