Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: unauthorized users for groups and permissions are not redirected to the previous url #799

Conversation

sammyskills
Copy link
Contributor

Fixes #798

Copy link
Collaborator

@datamweb datamweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am on mobile. I did not check the issue practically.

But the explanation and how to implement it seems logical.

@sammyskills
Copy link
Contributor Author

Thanks @datamweb. Feel free to check when you can.

@sammyskills
Copy link
Contributor Author

The option for adding arguments to filter configuration has been added to CI v4.4.0, see docs.

This PR might need to be updated to either:

  1. Update the docs and encourage developers to upgrade to v4.4.0
  2. Maintain this PR to cover all use cases.

WDYT?

@datamweb
Copy link
Collaborator

The option for adding arguments to filter configuration has been added to CI v4.4.0, see docs.

Yes, I remembered, I had reviewed this one😀.

  1. Update the docs and encourage developers to upgrade to v4.4.0

Shield requires CodeIgniter 4.2.7+ at this point I am not very inclined to upgrade to 4.4.0.
The difference between CodeIgniter 4.2.7+ and 4.4.0 is huge, 4.4.0 is great, but I think we shouldn't limit the user community at this point.

@sammyskills
Copy link
Contributor Author

Hi @kenjis, @lonnieezell, @MGatner can you take a look?

Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kenjis kenjis added the bug Something isn't working label Sep 5, 2023
@datamweb
Copy link
Collaborator

datamweb commented Sep 5, 2023

@sammyskills is aware of this, I haven't played with the code yet, I'll try to do it tomorrow.

Comment on lines +35 to +36
$session = session();
$session->setTempdata('beforeLoginUrl', current_url(), 300);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just came up with this code, but it is a copy and paste.
Since it is the same knowledge (save the URL for 300 seconds in the session), it might be better to put it together in a method if possible.

But there doesn't seem to be an appropriate class to put it in now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm currently merge this. Maybe we can refactor later.

Copy link
Collaborator

@datamweb datamweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sammyskills I came back after a million days 😃. Sorry.
Thank you!

@datamweb datamweb merged commit 319b5ef into codeigniter4:develop Sep 7, 2023
28 checks passed
@sammyskills sammyskills deleted the fix-unauthorized-users-before-login-url branch September 7, 2023 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: beforeLoginUrl is not stored in session if a user is logged out while in a protected group route.
3 participants