vapor / auth

👤 Authentication and Authorization framework for Fluent.
53 stars 34 forks source link

Using token and session auth should not run both #68

Closed grahamburgsma closed 5 years ago

grahamburgsma commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Let me know if this seems like it will do the trick: https://github.com/vapor/auth/pull/69

grahamburgsma commented 5 years ago

@tanner0101 that fixes it! Thank you 👏

tanner0101 commented 5 years ago

Great, I merged and tagged as 2.0.4 :)

MarkMurphy commented 4 years ago

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.