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

Add a tip about model middleware to authentication example #863

Merged
merged 6 commits into from
Aug 5, 2023

Conversation

sbeitzel
Copy link
Contributor

@sbeitzel sbeitzel commented Jul 20, 2023

The example we use for ModelAuthenticatable is a User identified by an email address. Email addresses are case-insensitive, while the ModelAuthenticatable that ships with Fluent is case sensitive. This means that an application that follows the example naively would consider SomeUser@example.com and someuser@example.com to be two different users. This is easily addressed with model middleware that converts the email address to lowercase before saving a User, but that example is elsewhere in the documentation.

This change simply calls out this possibility in a tip section and links to the model middleware section of the docs.

@@ -329,6 +329,9 @@ Don't forget to add the migration to `app.migrations`.
app.migrations.add(User.Migration())
```

!!! tip
Because email addresses are not case sensitive, you may want to add a [`Middleware`](../fluent/model.md#lifecycle) that coerces the email address to lowercase before saving it to the database.
Copy link
Member

Choose a reason for hiding this comment

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

So overall I agree with this change but I'd also like to add a note about ensuring the email is lowercased everywhere. A common example is on form logins with autocorrect where it capitalises the first letter of the email - that causes issues all the time so a note here to ensure the client lowercases the email as well or using a custom authenticator is probably needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. In my project I worked around that by creating an EmailAuthenticatable protocol based on ModelAuthenticatable but which lowercases whatever string is coming in before comparing it:

public protocol EmailAuthenticatable: Model, Authenticatable {
  static var emailKey: KeyPath<Self, Field<String>> { get }
  static var passwordHashKey: KeyPath<Self, Field<String>> { get }
  func verify(password: String) throws -> Bool
}

extension EmailAuthenticatable {
  public static func authenticator(
    database: DatabaseID? = nil
  ) -> Authenticator {
    EmailAuthenticator<Self>(database: database)
  }

  // swiftlint:disable:next identifier_name
  var _$email: Field<String> {
    self[keyPath: Self.emailKey]
  }

  // swiftlint:disable:next identifier_name
  var _$passwordHash: Field<String> {
    self[keyPath: Self.passwordHashKey]
  }
}

private struct EmailAuthenticator<User>: BasicAuthenticator
where User: EmailAuthenticatable {
  public let database: DatabaseID?

  public func authenticate(
    basic: BasicAuthorization,
    for request: Request
  ) -> EventLoopFuture<Void> {
    User.query(on: request.db(self.database))
      .filter(User.emailKey == basic.username.lowercased())
      .first()
      .flatMapThrowing {
        guard let user = $0 else {
          return
        }
        guard try user.verify(password: basic.password) else {
          return
        }
        request.auth.login(user)
      }
  }
}

@sbeitzel sbeitzel requested a review from 0xTim July 29, 2023 06:49
Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

Thanks!

@0xTim 0xTim enabled auto-merge (squash) August 5, 2023 16:35
@0xTim 0xTim requested a review from gwynne as a code owner August 5, 2023 16:40
@0xTim 0xTim merged commit 98ef20c into vapor:main Aug 5, 2023
1 check passed
@penny-for-vapor penny-for-vapor bot mentioned this pull request Aug 5, 2023
8 tasks
@sbeitzel sbeitzel deleted the feature/auth_help branch August 7, 2023 01:21
@redsun redsun mentioned this pull request Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants