feat: authenticator

[brendt]

No description provided.

[coveralls]

Pull Request Test Coverage Report for Build 11137713319

Details

• 209 of 223 (93.72%) changed or added relevant lines in 28 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.4%) to 81.776%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/Tempest/Reflection/src/PropertyReflector.php	4	5	80.0%
src/Tempest/Framework/Testing/Http/TestResponseHelper.php	3	16	18.75%

Files with Coverage Reduction	New Missed Lines	%
src/Tempest/Mapper/src/Casters/EnumCaster.php	1	87.5%

Totals
Change from base Build 11135946327:	0.4%
Covered Lines:	6143
Relevant Lines:	7512

---

💛 - Coveralls

[brendt]

After this PR, we'll look into an authorization API (preferably with user roles built-in). All front-end stuff and user-management is left alone for now.

[innocenzi]

I think this is great, there shouldn't need to be more built-in (besides authorization) 👍

However, I wouldn't let Tempest take care of the user model itself and the migration, I would rather have Tempest use an interface that has to be implemented in userland

[brendt]

However, I wouldn't let Tempest take care of the user model itself and the migration, I would rather have Tempest use an interface that has to be implemented in userland

Yeah, that's a fair point.

[brendt]

While I want the user model to be configurable, I do want to provide a default implementation — I might reconsider that though.

Also, I'm ok with "the user" being tied to the database, I don't really care about other ways of storing user data for now.

[innocenzi]

Looking at the current code, it's unclear to me—is it possible to ignore the provided migration? Or would you recommend to add another one that would run after it and modify the table as we see fit?

That'd be one reason I was suggesting not to provide a default model/migration, but it seems low enough friction

[aidan-casey]

I'd love to see this use a repository to decouple retrieving the user details. If it were me personally, my criteria would be for this to be decoupled so that it could work entirely outside the framework. I understand that's a bit of an extreme view, though. 🙂

I don't really care about other ways of storing user data for now.

We have to think about the people storing passwords in the filesystem, @brendt!

[brendt]

Looking at the current code, it's unclear to me—is it possible to ignore the provided migration? Or would you recommend to add another one that would run after it and modify the table as we see fit?

Right now it's not possible, but this is something I already planned on figuring out before merging this PR :)

[brendt]

I understand that's a bit of an extreme view, though. 🙂

Yes, not within my scope for now :)

[brendt]

We have to think about the people storing passwords in the filesystem, @brendt!

I thought about it: they are free to roll their own auth layer separate from Tempest's default implementation 👍

[brendt]

How to deal with this circular dependency: auth requires http because of session management, but http also requires auth because of the middleware.

Since this auth layer is only concerned with http authentication, would it make sense to keep it all within the same component?

I know @aidan-casey will tell me otherwise :)

[innocenzi]

Is the circular dependency itself a technical issue? If not, I think keeping the tempest/auth package separate is a good idea

[brendt]

Apparently it is, composer can't deal with it (at least not in our CI setup). There's no issue in this branch because I haven't added the circular dependency yet, but we had the same issue once before between core and event-bus.

Maybe there's a solution for it, I'll do some digging.

[aidan-casey]

@brendt - I haven't looked through the code yet, but probably I would have said use a driver/repository for storing the auth data. 🙈 🙃

[brendt]

I solved the issue, it was pretty straight forward: just need to hook into the Kernel::BOOTED event and add the middleware like so

[brendt]

Ok so, this branch contains so much code that I want to merge it rather sooner than later. I'm still going to write an implementation for revokePermission, but after that I plan on merging it.