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

Attempt to authenticate tokens without X-Token-Type #7

Merged
merged 10 commits into from
Sep 26, 2019

Conversation

djones6
Copy link
Collaborator

@djones6 djones6 commented Sep 23, 2019

This PR aims to simplify use of this plugin with third party clients that send a standard Authorization header but do not supply our custom X-token-type header.
Resolves #6

Current Credentials behavior is:

  • No plugins registered: does nothing (calls next())
  • One or more plugins registered: calls plugins in order, until one succeeds, fails, or they all pass (in which case an unauthorized response is sent)

The token plugins decide whether to pass or attempt authentication based on the X-token-type header:

  • If X-token-type == JWT, it will attempt to authenticate the Authorization header and either succeed or fail.
  • If X-token-type is a value other than JWT, it will pass (defer to other plugins).
  • If X-token-type is not set, it will attempt to authenticate the Authorization header and either succeed, or pass (defer to other plugins). This is the new behavior introduced here.

It is important that it passes (rather than failing) in the case where the X-token-type is not set, as this can be considered a 'speculative' attempt which could really be intended for another plugin.

@djones6 djones6 added the jazzy-doc When applied to a PR, instructs Package-Builder to regenerate the Jazzy documentation label Sep 25, 2019
Copy link
Contributor

@CameronMcWilliam CameronMcWilliam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good.

Add test for multi-auth skipping JWT

Restructure test code into separate sources
@djones6
Copy link
Collaborator Author

djones6 commented Sep 25, 2019

@CameronMcWilliam FYI, I realised I had not added a test for type-safe JWT middleware in the absence of the X-Token-Type header.

While adding that I discovered that the test for skipping authentication was wrong: it was actually rejecting the JWT because we hadn't set up a verifier with .unauthorized, but then the second Codable route was invoked which overwrote that response with the one the test expected.

Having two Codable routes registered to a path isn't a good idea as you could end up with multiple responses in the same body, and the last one that executes dictates the response status. So I've replaced those routes with a single route that uses TypeSafeMultiCredentials to test whether the JWT middleware passes to the next middleware (a trivial type-safe HTTP Basic middleware).

@djones6 djones6 merged commit 87dc061 into master Sep 26, 2019
@djones6 djones6 deleted the issue.notokentype branch September 26, 2019 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jazzy-doc When applied to a PR, instructs Package-Builder to regenerate the Jazzy documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Idea: Attempt to authenticate tokens without X-Token-Type
3 participants