bug: Call to a member function inGroup() on null after logging out

[cornejobarraza]

I have declared the following under my Auth.php file:

public function logoutRedirect(): string
{
    $url = auth()->user()->inGroup('admin')
        ? 'login'
        : setting('Auth.redirects')['logout'];

    return $this->getUrl($url);
}

And get a Call to a member function inGroup() on null error when navigating to /logout.

After some debugging I realized the logout action function under LoginController.php logs the user out before redirecting, causing the redirect() helper to get an invalid value.

To overcome this, I first capture the URL in a variable and pass it to the redirect function after the auth is logged out:

public function logoutAction(): RedirectResponse
{
    $url = config('Auth')->logoutRedirect();

    auth()->logout();

    return redirect()->to($url)->with('message', lang('Auth.successLogout'));
}

This is my first time contributing to CodeIgniter, I'm not sure if I did this correctly so please let me know if I should change something, or even if this should not be considered an issue as it's not something CI Shield sets up out of the box.

Thank you and have a good one!

[kenjis]

Thank you for the contribution!

This PR makes sense to me.

We need your GPG sign to your commits. Please sign.
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#signing
and https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#gpg-signing-old-commits

[cornejobarraza]

Thank you, I didn't know about that. Just signed the commit and pushed the update, let me know if anything else is needed

[datamweb]

@cornejobarraza Thanks, can you write the php unit test for this PR?

see :
https://github.com/codeigniter4/CodeIgniter4/tree/develop/tests#resources

[cornejobarraza]

@cornejobarraza Thanks, can you write the php unit test for this PR?

see : https://github.com/codeigniter4/CodeIgniter4/tree/develop/tests#resources

Hi @datamweb, I think I ran the tests correctly and some files were generated, but those seem to be in the .gitignore file. I'm new to this, so please enlighten me on how to proceed, thank you!

[datamweb]

I think I ran the tests correctly

Running a test is different from writing a test.

but those seem to be in the .gitignore file.

We have:

shield/.gitignore

Lines 53 to 56 in ca7335c

	#-------------------------
	# Test Files
	#-------------------------
	tests/coverage*

so this issue does not create a problem for the new test. I did not understand your point in this regard.

When you execute the test command, the existing tests are executed.
Therefore, you should write a new test related to this PR.
Have you written a test for this PR? If not, see here.

I'm new to this!

Welcome. Don't worry, this PR will remain open until you write the corresponding test.

[kenjis]

@datamweb The code is covered by existing test code.

So I think this PR is okay with no additional test case.

If we write a test case that reproduces the bug, we need to write an anonymous class which extends Config\Auth and override the logoutRedirect(), and inject it to the Factories.
I don't think we need such a test because the change is very small.
And it is not easy for test beginners.

[datamweb]

So I think this PR is okay with no additional test case.

I agree., my emphasis was because of label teste needed.

[kenjis]

LGTM!

[datamweb]

@cornejobarraza thanks, merged.