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

Suggested Changes/Updates #13

Open
calebkleveter opened this issue Jul 26, 2018 · 0 comments
Open

Suggested Changes/Updates #13

calebkleveter opened this issue Jul 26, 2018 · 0 comments
Assignees
Labels
enhancement New feature or request

Comments

@calebkleveter
Copy link
Contributor

  1. After the change, JWTVerificationMiddleware is essentially the same as JWTStorageMiddleware.
    The differences I see: it's non-generic, uses JWTService.verify instead of JWT.verify, and doesn't touch the request's private container. That's pretty redundant, maybe it could be removed altogether.
    (That being said, I don't know the purpose of JWTService.)

  2. The Authorization header missing usually doesn't result in a 400 Bad Request error, but 401 or 403 (depending on the school one subscribes to).
    I'm leaning towards 401 myself. (Also for the invalid token case.)

  3. This is more of a vapor/jwt thing probably, but it would be pretty rad to have an RFCPayload struct that contains all the standard claims (https://tools.ietf.org/html/rfc7519#section-4.1) as Optionals, and can verify them automatically, if they are present in the payload.

@calebkleveter calebkleveter added the enhancement New feature or request label Jul 26, 2018
@calebkleveter calebkleveter self-assigned this Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant