Add alias for the Guard contract to auth.driver

[wpillar]

@euantorano @JN-Jones when I created a new controller, I got a "Mybb\Auth\Contracts\Guard is not instantiable" exception from the container when it was trying to resolve MyBB\Core\Http\Controllers\Controller, this fixed it.

Did either of you have a similar problem at any stage? I can't really see how it ever would have worked?

[JN-Jones]

The current controllers use Laravel/Auth/Guard which is resolved to our own auth implementation.

[euantorano]

Odd. It's injected elsewhere, but it looks like most are just injecting Illuminate\Auth\Guard, which happens to work due to other DI registrations.

The method for extending Guard is a particularly painful one unfortunately, and is probably one of the most difficult parts of Laravel to extend.

[wpillar]

@JN-Jones Where is this contract bound into the container? https://github.com/mybb/mybb2/blob/master/app/Http/Controllers/Controller.php#L7

I couldn't find anywhere that it bound that contract to a concretion.

[JN-Jones]

It doesn't use the contract somewhere else. It uses laravels guard implementation instead. Which resolves to ours as laravels isn't initialed at all.

[euantorano]

If this fixes the issue though, we should merge it.

That contract is bound through other contracts (the Guard instance is created by the manager). It's a bit of a convoluted process, with manual instantiation in the Manager. This is how Laravel's core does it too.

[wpillar]

So we should be type-hinting for Illuminate\Auth\Guard everywhere rather than MyBB\Auth\Contracts\Guard?

[JN-Jones]

Yep. Also we should suggest to laravel to make extending easier. Extending three classes just to change the hashing algorithm is pretty stupid.

[JN-Jones]

If this commit fixes that and allows using the contract it should be changed

[euantorano]

IMO we should type hint MyBB\Auth\Contratcs\Guard as then it's clear which contract we're using. The current approach hides this and may lead to confusion further down the line.

[wpillar]

Agree with @euantorano.

The hashing algorithm we're using instead is still bcrypt right? Or rather, something much much better than md5 or sha* ?

[euantorano]

Yes, we use BCrypt by default.

The alternative hashing algorithms are meant to enable support for the future merge system as other boards use other hashing mechanisms. This package allows those mechanisms to still function after a merge without forcing password resets for all users.

[JN-Jones]

By default we're using bcrypt (laravels default class). However we also support different ones like mybb 1.x, phpbb, phpass etc. Everything the merge system may need

[euantorano]

EG: Support for old MyBB 1.x passwords so that upgraded boards don't require 100% password resets for all users.

[wpillar]

I see. Good chat.

If we could merge this that would be great 👍

We should also try and enforce hinting on our own contract rather than the Illuminate one.

[JN-Jones]

Yep, should be changed everywhere (only online from mobile so can't merge/test anything)

[wpillar]

Can we merge this @euantorano @JN-Jones ?

[JN-Jones]

Nothing against it - can you also change the controllers then?

[wpillar]

Sure, I'll do it once this is merged 👍