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

Add postLogout hook to finish sessions from external session managers… #3940

Merged
merged 1 commit into from Mar 20, 2017

Conversation

@MorrisJobke
Member

MorrisJobke commented Mar 20, 2017

Add postLogout hook to finish sessions from external session managers…
… (#27048)

* Add postLogout hook to finish sessions from external session managers like CAS

* Add postLogout hook to finish sessions from external session managers like CAS

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Mar 20, 2017

@MorrisJobke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ChristophWurst, @icewind1991 and @LukasReschke to be potential reviewers.

mention-bot commented Mar 20, 2017

@MorrisJobke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ChristophWurst, @icewind1991 and @LukasReschke to be potential reviewers.

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Mar 20, 2017

Codecov Report

Merging #3940 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #3940      +/-   ##
============================================
- Coverage     54.22%   54.22%   -0.01%     
  Complexity    21133    21133              
============================================
  Files          1304     1304              
  Lines         80731    80732       +1     
  Branches       1282     1282              
============================================
- Hits          43779    43778       -1     
- Misses        36952    36954       +2
Impacted Files Coverage Δ Complexity Δ
lib/private/User/Session.php 72.75% <0%> (-0.24%) 111 <0> (ø)
lib/private/Files/Cache/Propagator.php 94.93% <0%> (-1.27%) 16% <0%> (ø)

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 528a903...e7dc1f4. Read the comment docs.

codecov-io commented Mar 20, 2017

Codecov Report

Merging #3940 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #3940      +/-   ##
============================================
- Coverage     54.22%   54.22%   -0.01%     
  Complexity    21133    21133              
============================================
  Files          1304     1304              
  Lines         80731    80732       +1     
  Branches       1282     1282              
============================================
- Hits          43779    43778       -1     
- Misses        36952    36954       +2
Impacted Files Coverage Δ Complexity Δ
lib/private/User/Session.php 72.75% <0%> (-0.24%) 111 <0> (ø)
lib/private/Files/Cache/Propagator.php 94.93% <0%> (-1.27%) 16% <0%> (ø)

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 528a903...e7dc1f4. Read the comment docs.

@@ -796,6 +798,7 @@ public function logout() {
$this->setToken(null);
$this->unsetMagicInCookie();
$this->session->clear();
$this->manager->emit('\OC\User', 'postLogout');

This comment has been minimized.

@ChristophWurst

ChristophWurst Mar 20, 2017

Member

on a side note, would it make sense to use the ::class notation for event namespacing too?

@ChristophWurst

ChristophWurst Mar 20, 2017

Member

on a side note, would it make sense to use the ::class notation for event namespacing too?

This comment has been minimized.

@MorrisJobke

MorrisJobke Mar 20, 2017

Member

I would not do this, because those are basically random strings, that are just by "incident" the same as the classname, to have them nicely namespace ;)

@MorrisJobke

MorrisJobke Mar 20, 2017

Member

I would not do this, because those are basically random strings, that are just by "incident" the same as the classname, to have them nicely namespace ;)

This comment has been minimized.

@ChristophWurst

ChristophWurst Mar 20, 2017

Member

Okay, fine by me.

@ChristophWurst

ChristophWurst Mar 20, 2017

Member

Okay, fine by me.

@LukasReschke LukasReschke merged commit 373bfea into master Mar 20, 2017

2 of 4 checks passed

codecov/patch 0% of diff hit (target 54.22%)
Details
codecov/project 54.22% (-0.01%) compared to 528a903
Details
Scrutinizer 1 new issues
Details
continuous-integration/drone/pr the build was successful
Details

@LukasReschke LukasReschke deleted the downstream-27048 branch Mar 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment