8000 Throw event when changing session id by wouterverstuyf · Pull Request #3377 · forkcms/forkcms · GitHub
[go: up one dir, main page]

Skip to content

Throw event when changing session id#3377

Merged
carakas merged 3 commits intoforkcms:masterfrom
wouterverstuyf:ProfilesChangedSessionIdEvent
May 3, 2021
Merged

Throw event when changing session id#3377
carakas merged 3 commits intoforkcms:masterfrom
wouterverstuyf:ProfilesChangedSessionIdEvent

Conversation

@wouterverstuyf
Copy link
Contributor
@wouterverstuyf wouterverstuyf commented Apr 24, 2021

Type

  • Enhancement

Resolves the following issues

When a public profile login/logout the session id changes.
Could be handy for other modules to hook in through an event, to get the new Session ID.

Pull request description

You can create a listener in your own module that detects the login/logout action. In the listener you can get the new session ID and do your stuff

/Frontend/Modules/MyModule/EventListener/ProfilesSessionIdChangedListener.php

namespace Frontend\Modules\MyModule\EventListener;

use Common\Events\ForkSessionIdChangedEvent;

class ForkSessionIdChangedListener
{
    public function onForkSessionIdChanged(ForkSessionIdChangedEvent $event): void
    {
        // do your stuff
        // $event->getSessionId()
        // $event->getOldSessionId()
    }
}

@codecov
Copy link
codecov bot commented Apr 24, 2021

Codecov Report

Merging #3377 (327f8c2) into master (9c2446b) will increase coverage by 0.01%.
The diff coverage is 47.82%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3377      +/-   ##
============================================
+ Coverage     27.74%   27.76%   +0.01%     
- Complexity     8028     8031       +3     
============================================
  Files           568      569       +1     
  Lines         30624    30647      +23     
============================================
+ Hits           8497     8508      +11     
- Misses        22127    22139      +12     
Flag Coverage Δ Complexity Δ
functional 23.70% <47.82%> (+0.01%) 8031.00 <3.00> (+3.00)
installer 3.85% <0.00%> (-0.01%) 8031.00 <3.00> (+3.00) ⬇️
unit 7.64% <30.43%> (+0.01%) 8031.00 <3.00> (+3.00)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
...rontend/Modules/Profiles/Engine/Authentication.php 0.00% <0.00%> (ø) 17.00 <0.00> (ø)
src/Common/Events/ForkSessionIdChangedEvent.php 42.85% <42.85%> (ø) 3.00 <3.00> (?)
src/Backend/Core/Engine/Authentication.php 91.72% <100.00%> (+0.52%) 41.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c2446b...327f8c2. Read the comment docs.

@wouterverstuyf
Copy link
Contributor Author

Question : when you login/logout in the backend section, the Session ID also changes. Should this be event be triggered from the backend login/logout function also? Is it ok to trigger a frontend event from within the backend section?

@carakas
Copy link
Member
carakas commented Apr 26, 2021

@wouterverstuyf I would add it in the Common namespace, not in the profiles one. It is tied to the session, not a profile, and then you can throw the event in the backend as well

public static function logout(): void
{
$session = FrontendModel::getSession();
$oldSession = FrontendModel::getSession()->getId();
Copy link
Member

Choose a reason for hiding this comment

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

you can use $session->getId() here

@wouterverstuyf
Copy link
Contributor Author

@wouterverstuyf I would add it in the Common namespace, not in the profiles one. It is tied to the session, not a profile, and then you can throw the event in the backend as well

Yes, got the same feeling this morning, I check it ;)

public function __construct(string $oldSessionId)
{
$this->oldSessionId = $oldSessionId;
$this->sessionId = FrontendModel::getSession()->getId();
Copy link
Member

Choose a reason for hiding this comment

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

Pass this one in the constructor as well.

@carakas carakas added this to the 5.10.0 milestone May 3, 2021
@carakas carakas changed the title WIP: Initialize ProfilesSessionIdChanged event Throw event when changing session id May 3, 2021
@carakas carakas merged commit 851fec3 into forkcms:master May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0