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

[BUG]: Phalcon v5 forces strict session ID syntax #16170

Closed
niden opened this issue Oct 19, 2022 Discussed in #16169 · 3 comments · Fixed by #16173
Closed

[BUG]: Phalcon v5 forces strict session ID syntax #16170

niden opened this issue Oct 19, 2022 Discussed in #16169 · 3 comments · Fixed by #16173
Assignees
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: medium Medium

Comments

@niden
Copy link
Member

niden commented Oct 19, 2022

Discussed in #16169

Originally posted by wurst-hans October 19, 2022
Really?

I'm going to upgrade my middleware and projects from v4 to v5 and have been looking for hours, why my custom session handler does not work anymore. It causes PHP to create a new session ID on every request.
My session handler uses Random to generate a UUID v4 to be used as session ID.

After hours I found out, that Phalcon session manager checks session cookie value with following regex /^[a-z0-9]+$/iD. That explains, why my UUID is being ignored. That's a pity.

It should not be your (i.e. Phalcons) task to decide if a session ID is valid. When implementing a custom session handler (like I did) one can implement the SessionIdInterface which provides not only the create_sid() method but also a validateId() method that can be used for validating session ID.

@niden
Copy link
Member Author

niden commented Oct 19, 2022

@wurst-hans Here if you do not mind

@niden niden self-assigned this Oct 19, 2022
@niden niden added bug A bug report status: medium Medium 5.0 The issues we want to solve in the 5.0 release labels Oct 19, 2022
@niden niden added this to Phalcon v5 Oct 19, 2022
@niden niden moved this from Backlog to In Progress in Phalcon v5 Oct 19, 2022
@niden niden moved this to Backlog in Phalcon v5 Oct 19, 2022
@niden
Copy link
Member Author

niden commented Oct 19, 2022

Sorry, can't post my real code here due to NDA. But it should be simple to extend your stream adapter, like:

use Phalcon\Encryption\Security\Random;

class Stream extends Noop implements \SessionHandlerInterface, \SessionIdInterface
{
    [...]

    public function create_sid(): string
    {
        return (new Random())->uuid();
    }

    public function validateId(string $session): bool
    {
        return (bool)preg_match('/^[\da-f]{8}-[\da-f]{4}-4[\da-f]{3}-[89ab][\da-f]{3}-[\da-f]{12}$/iu', $session);
    }
}

As workaround, I've extended Phalcons session manager now and overwrite start() method and commented out the regex check.

@niden niden linked a pull request Oct 19, 2022 that will close this issue
5 tasks
@niden
Copy link
Member Author

niden commented Oct 19, 2022

Resolved in #16173

Thank you @wurst-hans

@niden niden closed this as completed Oct 19, 2022
Repository owner moved this from In Progress to Implemented in Phalcon v5 Oct 19, 2022
@niden niden moved this from Implemented to Backlog in Phalcon v5 Oct 24, 2022
@niden niden moved this from Backlog to Released in Phalcon v5 Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: medium Medium
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant