lonnieezell / myth-auth

One-stop Auth package for CodeIgniter 4
MIT License
637 stars 208 forks source link

ISSUE redirect if use filter login at $globals #443

Closed mjamilasfihani closed 2 years ago

mjamilasfihani commented 3 years ago

We really don't need this : https://github.com/lonnieezell/myth-auth/blob/4c80469038d8535c40d777d9429cd0729e8bbeef/src/Filters/LoginFilter.php#L30-L35

Because these lines is replaced by : https://github.com/lonnieezell/myth-auth/blob/4c80469038d8535c40d777d9429cd0729e8bbeef/src/Filters/LoginFilter.php#L26

If we've decided which one is used, I can do PR for it 🛩️

mjamilasfihani commented 3 years ago

Uhhh, I am sorry the problem is not about ->setHost('') but at line 34 itself. =)

EDIT : if we remove line 34, the $current will be the segment itself, if $indexPage = ""; otherwise it'll become index.php/{segment}

lonnieezell commented 3 years ago

Please provide an example of how to recreate the issue. The solution you've provided ensures that is removed after a check for whether we're currently forcing a secure request. Neither one of those seem to have anything to do with the other....

mjamilasfihani commented 3 years ago

I am sorry if my explanation is bad,, okay this is the problem :

I've set the login filter in before request at $global. if I use http://localhost:8080/ and visit the login/register/forgot url, than the page could loaded as what I want, but if I use domain (here is the trouble) it will loop redirect to page itself.

Than I tried to comment the line 34 as I mention before and it's success to display the login page, sorry that's not the best solution for what I did but I'll try my best to fix what I start.

MGatner commented 3 years ago

The filter should be reworked to use the url_is helper function, probably still combined with route_to(). The filter predates that function but would benefit from it greatly and that would clean up some of this other code.

mjamilasfihani commented 3 years ago

that's a good idea, so we need to re worked again but keep on the mind that this auth need to support CI > 4.0.4 tell me if I am wrong :)

MGatner commented 3 years ago

I don't think that is a requirement for development. As long as composer.json is updated with the actual requirements (e.g. if you use a new feature from 4.1.4 then update CodeIgniter4/framework to ^4.1.4) then Composer will make sure nothing breaks. @lonnieezell can weigh in if he would like more backwards-compatibility than that.

lonnieezell commented 3 years ago

I agree. With this being the development branch, no need. Ensure requirements are correct in composer.json (although in this case it sounds like required is 4.0.4, right?

mjamilasfihani commented 3 years ago

I agree. With this being the development branch, no need. Ensure requirements are correct in composer.json (although in this case it sounds like required is 4.0.4, right?

4.0.5 exactly, I've compared and url_is function has added at 4.0.5

Edit : I will check the another filters too maybe need to re-code

mjamilasfihani commented 3 years ago

you can merge the PR and close this issue 🥳