feat(auth): add various improvements to auth and oauth installer

[iamdadmin]

(Replaces PR 1900 with clean code...)

Closes #1888 various improvements to auth/oauth installers, as follows

• (Installer) Asks whether to install OAuth first and provides different UserModel and Migration stubs with pre-populated fields matching the OAuthController
• Adds a very basic MustBeAuthenticated middleware stub, I've tested it in my own app, it's something that can be built upon but should give a working system
• Adds a very basic LoginController stub to show the login view (view NOT PROVIDED, comment explains this), post login (database query NOT PROVIDED, comment explains this), handle logout
• Implements Tempest\Http\Session\PreviousUrl to set/get the intended URL and redirect there after auth
• Update installer tests for revised questions/answers format

[iamdadmin]

Tests passing, accidentally committed a migration file into src/ which was generated while testing the install command interactively, and should not be there.

[innocenzi]

Waiting for @brendt's approval but LGTM!

[NeoIsRecursive]

One thought I have, what about naming the LoginController to SessionController, and using crud "actions" (view, store, delete etc.) since that's essentially what we are doing (managing the user session)?

Or using invokable single action controllers, uri([LoginController::class, 'logout']) doesn't really make sense, for me at least.

Sorry if this is a bit nitpicky but I assume this will be used pretty widely among (new) tempest users 😅

[iamdadmin]

One thought I have, what about naming the LoginController to SessionController, and using crud "actions" (view, store, delete etc.) since that's essentially what we are doing (managing the user session)?

Or using invokable single action controllers, uri([LoginController::class, 'logout']) doesn't really make sense, for me at least.

Sorry if this is a bit nitpicky but I assume this will be used pretty widely among (new) tempest users 😅

I get where you're coming from, and the best name in that case would be AuthenticationController::class but not SessionController::class. This isn't a session. This is an authentication, which USES a session to work.

Also, there's absolutely no reason why you can't just rename the generated class to whatever you want, such as PumpkinController::class if you wish. These are just intended as partial starters as there's no fixed opinionated way of doing it - and both are valid.

Happy to make the change to AuthenticationController if there's support for it?

[NeoIsRecursive]

Also, there's absolutely no reason why you can't just rename the generated class to whatever you want, such as PumpkinController::class if you wish. These are just intended as partial starters as there's no fixed opinionated way of doing it - and both are valid.

Yes, ofcourse, my thinking was that the naming should reflect what is considered best practice in tempest as it will become something (I assume) most people will refer to when looking up how to they should structure their controllers etc.

Sorry if it came out a bit harsh 🥶

This isn't a session. This is an authentication, which USES a session to work.

Id argue that it is, you are creating(store) an (Authenticated)Session on login and when you logout you destroy the session. :)

[iamdadmin]

Sorry if it came out a bit harsh 🥶

It wasn't at all!

This isn't a session. This is an authentication, which USES a session to work.

Id argue that it is, you are creating(store) an (Authenticated)Session on login and when you logout you destroy the session. :)

When you use the logout action provided, It de-authenticates an Authenticatable model. That's stored in a session. Other things can be stored in a session, which would mean the session itself would persist. If there's nothing else in the session, it may also be destroyed too, I honestly haven't looked at the code for Tempest\Http\Session to know that well enough. Sure, destroying the session would destroy the authentication. But it'd also destroy the session and anything else that was stored in it.

However someone interacting with your app can have a session without an authentication. See https://tempestphp.com/3.x/essentials/routing#flashing-values for just one example.

And authentication requires a session, a session does not require an authentication. This controller is performing authentication actions, with a dependency of a session. It wouldn't work without a session. But it's separate to just a session.

Login and Logout are Authentication actions, and thus I can change it to AuthenticationController, as Login is already misleading since there's a logout action in there too.

[innocenzi]

I actually agree with @NeoIsRecursive here. Laravel names it AuthenticatedUserController. This makes sense and would be fine by me. Other options like single action controllers would also be fine (LogInController, LogOutController (notice the casing).

At the end of the day, the user can make changes themselves (that's why we publish stuff), but providing good practices by default can't hurt!

[NeoIsRecursive]

And authentication requires a session, a session does not require an authentication. This controller is performing authentication actions, with a dependency of a session. It wouldn't work without a session. But it's separate to just a session.

Yes, I know, but you should regenerate the session on login and invalidate it on logout so in that sense it works (technically you "crud" an "AuthenticatedSession", not a normal Session so there you are correct I guess).

But lets agree to disagree! 😄

At the end of the day, the user can make changes themselves (that's why we publish stuff), but providing good practices by default can't hurt!

Is there anything in the docs for "best practices building with tempest" atm (other than the contributing section)? Or is it up to the example applications to display that?

Sorry for out-of-this-pr-scope question 😬

[iamdadmin]

Okie no worries, I definitely see the logic in avoiding a single function name, it's really not a single function. Updated naming convention to AuthenticateUserController since we're authenticating a user model specifically, and there could be other types of auth like API etc! Tests passing locally, should be ready for approval once more :)

Is there anything in the docs for "best practices building with tempest" atm (other than the contributing section)? Or is it up to the example applications to display that?

Sorry for out-of-this-pr-scope question 😬

Docs and example apps are what there is at the moment I believe, but hanging out in the discord you can raise discussions on topics or generally chat about that kinda thing if you're not already in there!

Invite here if you need it! https://discord.com/invite/pPhpTGUMPQ

[iamdadmin]

Looks like the tests all failed to run likely due to the PHPUnit CVE. Are they something I can re-trigger or does it need more permissions?