CredentialsError always return 500 server error?

[kgn]

I'm working on implementing user login with Vapor, I'm using the built in Turnstyle CredentialsErrors, however they don't seem to implement the correct HTTP error codes. For example, when I setup my ProtectMiddleware.

ProtectMiddleware(error: IncorrectCredentialsError())

The response I get on the client is a 500 with the message IncorrectCredentialsError: Invalid Credentials:

HTTP/1.1 500 Internal Server Error
Content-Type: application/json; charset=utf-8
Content-Length: 100

{"code":500,"error":true,"message":"IncorrectCredentialsError: Invalid Credentials","metadata":null}

I would expect 403 Forbidden.

[edjiang]

Hey @kgn, have you looked at https://github.com/stormpath/Turnstile-Vapor-Example?

In Vapor, uncaught exceptions cause a 500. The idea is that you would have an "error rendering" middleware that transforms exceptions into the error code that you want. The errors don't have an error code in of itself.

See this documentation: https://vapor.github.io/documentation/guide/middleware.html#errors

Also, the ProtectMiddleware error passed in is meant to be the "Unauthorized" error, for people who don't pass authentication. You would pass in what error type that you'd like to use to represent that error. You could use Vapor's Abort.unauthorized to represent a 401.

403 should be used for when the user is authenticated, but doesn't have permission to access the resource.

Hope this helps!

[kgn]

Thanks for the pointers @edjiang! Is the "error rendering" middleware something included with Turnstile? I didn't see it used in the example app.

[edjiang]

No problem! All of the Vapor+Turnstile integration code are in the Vapor.Auth framework, so you won't be seeing any Vapor specific stuff in this repo.

Vapor already has error rendering & stuff defined by default, so Vapor.Auth piggybacks on this functionality. In Vapor, the default "error rendering" middleware is AbortMiddleware, which takes any Abort.* errors and renders it in either HTML or JSON, depending on the client's accept headers. This middleware is added by default. If you don't like this behavior, you can implement it yourself.

AbortMiddleware: https://github.com/vapor/vapor/blob/master/Sources/Vapor/Error/AbortMiddleware.swift Abort errors: https://github.com/vapor/vapor/blob/master/Sources/Vapor/Error/Abort.swift

[kgn]

Would it make sense go add a code property to the CredentialsErrors so the HTTP code can be defined a long with the message?

In the case of IncorrectCredentialsError, UnsupportedCredentialsError, AccountTakenError should they all have the code 403 Forbidden?

[edjiang]

If you want to add an extension for CredentialsError, you are welcome to do so. However the intention is that your app should be handling those errors.

In specifics in terms of HTTP codes,

IncorrectCredentialsError would probably be 401 -- the user is not authenticated. UnsupportedCredentialsError would probably be a 500 -- since this is developer error, and should not be hit unless the developer made a mistake. AccountTakenError should be handled by the developer, but if you were exposing it to an API, might be something like 409 Conflict