vapor / auth

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

Remove unnecessary Model conformance on Authenticatable #35

Closed jseibert closed 6 years ago

jseibert commented 6 years ago

Vapor's approach to authentication is designed to be flexible, and the ability to authenticate arbitrary classes on each Request via try req.authenticated(T.self) is powerful.

However, there is no logical or technical reason that Authenticatable classes must be Model's. In fact, it's easy to imagine cases (e.g. God roles) where they shouldn't be.

This makes that possible.

tanner0101 commented 6 years ago

@jseibert it might be even more flexible to not require Model on any of the protocols and instead require it in the extensions and middleware that actually rely on Fluent.

jseibert commented 6 years ago

@tanner0101 even better! That makes total sense.

jseibert commented 6 years ago

@tanner0101 Ok, I just got time to update this. I removed the Model conformance on as many of the protocols as possible. Some, like Session and Token, require it implicitly, so we can't strip it from them.

jseibert commented 6 years ago

Interesting, it builds on macOS but not Linux. Looking into it...

jseibert commented 6 years ago

@tanner0101 OK, compilation finally works. This is probably the furthest it can be abstracted without significant rewrites to TokenAuthenticatable.

Test are now failing for an unrelated reason, so I'll put the ball back in your court :)

penny-coin commented 6 years ago

Hey @jseibert, you just merged a pull request, have a coin!

You now have 1 coins.