-
-
Notifications
You must be signed in to change notification settings - Fork 178
Add default implementation for CredentialsAuthenticatable #711
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
Changes from all commits
64c76d2
9b55c3a
f3cd85b
8c3d8ac
c09a465
5b2cf71
7913b7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import Vapor | ||
|
|
||
| public protocol ModelCredentialsAuthenticatable: Model, Authenticatable { | ||
| static var usernameKey: KeyPath<Self, Field<String>> { get } | ||
| static var passwordHashKey: KeyPath<Self, Field<String>> { get } | ||
| func verify(password: String) throws -> Bool | ||
| } | ||
|
|
||
| extension ModelCredentialsAuthenticatable { | ||
| public static func credentialsAuthenticator( | ||
| database: DatabaseID? = nil | ||
| ) -> Authenticator { | ||
| ModelCredentialsAuthenticator<Self>(database: database) | ||
| } | ||
|
|
||
| var _$username: Field<String> { | ||
| self[keyPath: Self.usernameKey] | ||
| } | ||
|
|
||
| var _$passwordHash: Field<String> { | ||
| self[keyPath: Self.passwordHashKey] | ||
| } | ||
| } | ||
|
|
||
| public struct ModelCredentials: Content { | ||
| public let username: String | ||
| public let password: String | ||
|
|
||
| public init(username: String, password: String) { | ||
| self.username = username | ||
| self.password = password | ||
| } | ||
| } | ||
|
|
||
| private struct ModelCredentialsAuthenticator<User>: CredentialsAuthenticator | ||
| where User: ModelCredentialsAuthenticatable | ||
| { | ||
| typealias Credentials = ModelCredentials | ||
|
|
||
| public let database: DatabaseID? | ||
|
|
||
| func authenticate(credentials: ModelCredentials, for request: Request) -> EventLoopFuture<Void> { | ||
| User.query(on: request.db(self.database)).filter(\._$username == credentials.username).first().flatMapThrowing { foundUser in | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe, since this is a default implementation that a lot of people will end up using, we should do something like this: .all()
.flatMapThrowing {
#if DEBUG
assert($0.count <= 1 , "Usernames must be unique in your database")
#else
guard $0.count <= 1 else throw Abort(.conflict, reason: "Found more than one matching user")
#endif
return $0.first
}I'd hate for it to turn into another potential footgun for the unsuspecting 😕. Granted, this would only catch the case where someone actually did end up with multiple of the same username, but at least it's something.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given this affects all of the authenticators, we should probably do that as a separate PR |
||
| guard let user = foundUser else { | ||
| return | ||
| } | ||
| guard try user.verify(password: credentials.password) else { | ||
| return | ||
| } | ||
| request.auth.login(user) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| import XCTFluent | ||
| import XCTVapor | ||
| import Fluent | ||
| import Vapor | ||
|
|
||
| final class CredentialTests: XCTestCase { | ||
|
|
||
| func testCredentialsAuthentication() throws { | ||
| let app = Application(.testing) | ||
| defer { app.shutdown() } | ||
|
|
||
| // Setup test db. | ||
| let testDB = ArrayTestDatabase() | ||
| app.databases.use(testDB.configuration, as: .test) | ||
|
|
||
| // Configure sessions. | ||
| app.middleware.use(app.sessions.middleware) | ||
|
|
||
| // Setup routes. | ||
| let sessionRoutes = app.grouped(CredentialsUser.sessionAuthenticator()) | ||
|
|
||
| let credentialRoutes = sessionRoutes.grouped(CredentialsUser.credentialsAuthenticator()) | ||
| credentialRoutes.post("login") { req -> Response in | ||
| guard req.auth.has(CredentialsUser.self) else { | ||
| throw Abort(.unauthorized) | ||
| } | ||
| return req.redirect(to: "/protected") | ||
| } | ||
|
|
||
| let protectedRoutes = sessionRoutes.grouped(CredentialsUser.redirectMiddleware(path: "/login")) | ||
| protectedRoutes.get("protected") { req -> HTTPStatus in | ||
| _ = try req.auth.require(CredentialsUser.self) | ||
| return .ok | ||
| } | ||
|
|
||
| // Create user | ||
| let password = "password-\(Int.random())" | ||
| let passwordHash = try Bcrypt.hash(password) | ||
| let testUser = CredentialsUser(id: UUID(), username: "user-\(Int.random())", password: passwordHash) | ||
| testDB.append([TestOutput(testUser)]) | ||
| testDB.append([TestOutput(testUser)]) | ||
| testDB.append([TestOutput(testUser)]) | ||
| testDB.append([TestOutput(testUser)]) | ||
|
|
||
| // Test login | ||
| let loginData = ModelCredentials(username: testUser.username, password: password) | ||
| try app.test(.POST, "/login", beforeRequest: { req in | ||
| try req.content.encode(loginData, as: .urlEncodedForm) | ||
| }) { res in | ||
| XCTAssertEqual(res.status, .seeOther) | ||
| XCTAssertEqual(res.headers[.location].first, "/protected") | ||
| let sessionID = try XCTUnwrap(res.headers.setCookie?["vapor-session"]?.string) | ||
|
|
||
| // Test accessing protected route | ||
| try app.test(.GET, "/protected", beforeRequest: { req in | ||
| var cookies = HTTPCookies() | ||
| cookies["vapor-session"] = .init(string: sessionID) | ||
| req.headers.cookie = cookies | ||
| }) { res in | ||
| XCTAssertEqual(res.status, .ok) | ||
| } | ||
| } | ||
|
|
||
|
|
||
| } | ||
| } | ||
|
|
||
| final class CredentialsUser: Model { | ||
| static let schema = "users" | ||
|
|
||
| @ID(key: .id) | ||
| var id: UUID? | ||
|
|
||
| @Field(key: "username") | ||
| var username: String | ||
|
|
||
| @Field(key: "password") | ||
| var password: String | ||
|
|
||
| init() { } | ||
|
|
||
| init(id: UUID? = nil, username: String, password: String) { | ||
| self.id = id | ||
| self.username = username | ||
| self.password = password | ||
| } | ||
| } | ||
|
|
||
|
|
||
| extension CredentialsUser: ModelCredentialsAuthenticatable { | ||
| static let usernameKey = \CredentialsUser.$username | ||
| static let passwordHashKey = \CredentialsUser.$password | ||
|
|
||
| func verify(password: String) throws -> Bool { | ||
| try Bcrypt.verify(password, created: self.password) | ||
| } | ||
| } | ||
| extension CredentialsUser: ModelSessionAuthenticatable {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouch, I don't like that this was missing at all 😕