Skip to content
This repository has been archived by the owner on Nov 16, 2020. It is now read-only.

Using token and session auth should not run both #68

Closed
grahamburgsma opened this issue Jun 3, 2019 · 9 comments
Closed

Using token and session auth should not run both #68

grahamburgsma opened this issue Jun 3, 2019 · 9 comments
Labels
enhancement New feature or request

Comments

@grahamburgsma
Copy link

When using TokenAuthenticatable and SessionAuthenticatable both seem to be run on each request which just does more queries and so is not ideal.

If middleware is set as User.tokenAuthMiddleware(), User.authSessionsMiddleware(), User.guardAuthMiddleware() and on a request, I only provide a bearer token and no cookies, I would expect only the tokenAuthMiddleware to run. Or if authSessionsMiddleware runs, that it exits early without doing any queries.

Using the Vapor 3 auth-template, then enabled sessions (and setup cache etc.), once user is logged in and a request is run, these are the logs:

SELECT * FROM "UserToken" WHERE ("UserToken"."expiresAt" IS NULL OR "UserToken"."expiresAt" > (?)) AND "UserToken"."string" = (?) LIMIT 1 OFFSET 0 [1559561285.008395, "bzmCb/FWPE2l/4ItxasXbw=="]
SELECT * FROM "User" WHERE "User"."id" = (?) LIMIT 1 OFFSET 0 [1]
SELECT * FROM "fluentcache" WHERE "fluentcache"."key" = (?) LIMIT 1 OFFSET 0 ["19OwkNmfnogOE6gwiUUqSA=="]
INSERT INTO "fluentcache" ("key", "data") VALUES (?, ?) ["19OwkNmfnogOE6gwiUUqSA==", 0x7b2264617461223a7b225f5573657253657373696f6e223a2231227d7d]

I wouldn't expect the 2 fluentcache queries since only a bearer token was given in the request. Also should be same for the opposite, if there is a vapor-session cookie, the token middleware should exit early and not perform any queries.

Would really appreciate a fix for Vapor 3 since Vapor 4 seems far off still (for production). Or some pointers on how I could contribute on this.

@0xTim
Copy link
Member

0xTim commented Jun 6, 2019

Out of curiosity - what's the use case for having a route that is protected by both a token or a cookie? Since normally a cookie (browser) request returns a View and a token request returns JSON.

The middleware is behaving as expected unfortunately - there's no way to 'skip' a middleware in the chain. If you want to do that you'll need to write a custom middleware that checks for an authenticated user before trying to do a look up again. Or wraps the two middlewares and works out which one to use.

@grahamburgsma
Copy link
Author

I'm not super familiar with front end stuff so if this isn't the way to do it please let me know. But I have a Vapor API (all JSON routes) that supports an iOS app and two React frontends. I want to use cookie based for the frontends as that is more secure (using an http only cookie). For the iOS app, token based is more preferred. I would have thought this is a common use case, but I guess not!

@0xTim
Copy link
Member

0xTim commented Jun 6, 2019

Ah ok, I see what you mean. So (I think) the usual way of doing it is to provide a token to the front end that it uses to make requests. Note that if you use HTTP only cookies, you won't be able to use them with your React front-end (since that's JS obviously and it won't have access to the cookies)

@grahamburgsma
Copy link
Author

I was doing it that way, but there is not secure way to store a token in web storage (that I could find). Any JS can read the local storage and have the token.
I do actually have it working with React and an http cookie, (using axios at least) it is included automatically using the withCredentials option.

@tanner0101 tanner0101 added the enhancement New feature or request label Jun 11, 2019
@tanner0101 tanner0101 added this to To Do in Vapor 4 via automation Jun 11, 2019
@tanner0101
Copy link
Member

This seems like it could be an oversight in the session auth middleware. The rest of the middleware have a guard statement at the beginning that check to see if authentication has already been performed:

https://github.com/vapor/auth/blob/2/Sources/Authentication/Basic/BasicAuthenticationMiddleware.swift#L16-L20

But this is lacking in the session auth middleware:

https://github.com/vapor/auth/blob/2/Sources/Authentication/Persist/AuthenticationSessionsMiddleware.swift#L7

@tanner0101 tanner0101 added this to To Do in Vapor 3 via automation Jun 11, 2019
@tanner0101
Copy link
Member

Let me know if this seems like it will do the trick: #69

@grahamburgsma
Copy link
Author

@tanner0101 that fixes it! Thank you 👏

@tanner0101
Copy link
Member

Great, I merged and tagged as 2.0.4 :)

Vapor 3 automation moved this from To Do to Done Jun 11, 2019
Vapor 4 automation moved this from To Do to Done Jun 11, 2019
@MarkMurphy
Copy link

I think it's worth adding something about this use case to the docs as an example. A lot of people are building APIs to accommodate both mobile and web based apps using libraries/frameworks like React, Vue, Angular, etc. It's convenient (and more secure) to leverage cookies for authentication when we're building web apps.

It's easy to pass a flag to fetch or an XHR to tell the browser to include credentials (ie. cookies) with any api requests.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
Vapor 4
  
Done
Vapor 3
  
Done
Development

No branches or pull requests

4 participants