codeigniter4 / shield

Authentication and Authorization for CodeIgniter 4
https://shield.codeigniter.com
MIT License
358 stars 128 forks source link

feat: Change session manipulation methods from `private` to `protected` #1113

Closed michalsn closed 4 months ago

michalsn commented 5 months ago

Description This PR changes the session manipulation methods from private to protected, so we can use them if we extend the Authentication\Authenticators\Session class.

Reference: https://github.com/codeigniter4/shield/discussions/1111

Checklist:

kenjis commented 5 months ago

This Session class is already very long, and would violate the SRP. Don't we need a new class for these methods?

michalsn commented 5 months ago

The methods related to the user session will give us about 50 lines of code. I don't see the point of moving them to a separate class - especially if they are to remain non-public.

kenjis commented 4 months ago

The session data manipulation is needed only for the Session authenticator. So extracting them into a class does not seem to help much in the future. If we extract classes, we should extract another functions, but so far there is no need.

By the way, is this an enhancement or a bug fix? In other words, should the next release be minor version up?

michalsn commented 4 months ago

I would say this is an enhancement.

kenjis commented 4 months ago

Then it should be released as v1.1.0.

michalsn commented 4 months ago

Okay. I'm not familiar with the Shield release cycle, so it's up to you when you'd like to merge this.