vapor / auth

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

Protected routes can still be hit after calling `unauthenticate(_:)` #45

Closed ilyahal closed 6 years ago

ilyahal commented 6 years ago

Protected routes can still be hit after calling unauthenticate(_:)

Steps to reproduce

Configurate:

// ...
try services.register(AuthenticationProvider())
// ...
var migrations = MigrationConfig()
migrations.add(model: User.self, database: .sqlite)
migrations.add(model: Token.self, database: .sqlite)
// ...

Models:

final class User: SQLiteUUIDModel, Migration, Content, Parameter, BasicAuthenticatable, TokenAuthenticatable {
    var id: UUID?
    var username: String
    var password: String

    init(username: String, password: String) {
        self.username = username
        self.password = password
    }

    static let usernameKey: UsernameKey = \.username
    static let passwordKey: PasswordKey = \.password
}
final class Token: SQLiteUUIDModel, Migration, Content, Authentication.Token, BearerAuthenticatable {
    var id: UUID?
    var token: String
    var userID: User.ID

    init(token: String, userID: User.ID) {
        self.token = token
        self.userID = userID
    }

    typealias UserType = User
    static let userIDKey: UserIDKey = \.userID

    static let tokenKey: TokenKey = \.token

    static func prepare(on connection: SQLiteConnection) -> Future<Void> {
        return Database.create(self, on: connection) { builder in
            try addProperties(to: builder)
            try builder.addReference(from: \.userID, to: \User.id)
        }
    }

    static func generate(for user: User) throws -> Token {
        let random = try CryptoRandom().generateData(count: 16)
        return try Token(token: random.base64EncodedString(), userID: user.requireID())
    }
}

Controller:

func boot(router: Router) throws {
    let usersRoutes = router.grouped("users")

    usersRoutes.post(User.self, use: createHandler)

    let basicAuthMiddleware = User.basicAuthMiddleware(using: BCryptDigest())
    let basicAuthGroup = usersRoutes.grouped(basicAuthMiddleware)

    basicAuthGroup.post("login", use: loginHandler)

    let tokenAuthMiddleware = User.tokenAuthMiddleware()
    let tokenAuthGroup = usersRoutes.grouped(tokenAuthMiddleware)

    tokenAuthGroup.delete(use: logoutHandler)
}

func createHandler(_ req: Request, user: User) throws -> Future<User> {
    let hasher = try req.make(BCryptDigest.self)
    user.password = try hasher.hash(user.password)

    return user.save(on: req)
}

func loginHandler(_ request: Request) throws -> Future<Token> {
    let user = try request.requireAuthenticated(User.self)
    let token = try Token.generate(for: user)

    return token.save(on: request)
}

func logoutHandler(_ request: Request) throws -> HTTPStatus {
    guard try request.isAuthenticated(User.self) else { throw Abort(.unauthorized) }

    try request.unauthenticate(User.self)
    return .ok
}

Steps:

  1. Call POST /users
  2. Call POST /users/login
  3. Call DELETE /users
  4. Call DELETE /users

Expected behavior

401 Unauthorized

Actual behavior

200 OK

Environment

0xTim commented 6 years ago

This is 'expected' behaviour - your logout function needs to invalidate the token rather than unauthenticating the user. Doing that is for the sessions authentication

ilyahal commented 6 years ago

How can I invalidate the token in logoutHandler(_:)?

0xTim commented 6 years ago

Get the token from the request and delete it from the database. Next time a user uses that token it will be invalid and they'll get a 401

ilyahal commented 6 years ago

Thank you!

func logoutHandler(_ request: Request) throws -> Future<HTTPStatus> {
    let token = try request.requireAuthenticated(Token.self)
    return token.delete(on: request).transform(to: .ok)
}