hydra icon indicating copy to clipboard operation
hydra copied to clipboard

feat: endpoint to delete login session by session id

Open aarmam opened this issue 4 years ago • 1 comments

This pull request introduces admin endpoint to delete login session by session id.

Use case:

  1. User logs in from device/browser 1 to client application A. Hydra has created login session 1 (remember=true) and authenticated user.
  2. User logs in from device/browser 1 to client application B
  3. Login provider displays UI page with message "You have active sessions in following applications: Application A" and options 3.1) "Logout from all sessions and login as different user", 3.2) "Resume active sessions"
  4. User selects 3.1 "Logout from all sessions and login as different user" 4.1 Login provider performs DELETE /oauth2/auth/sessions/consent?subject=user1&login_session_id=session1&trigger_backchannel_logout=true (related feature https://github.com/ory/hydra/pull/2844) 4.2 Login provider performs DELETE /oauth2/auth/sessions/login/session1 (current feature) 4.3 Login provider request GET /oauth2/auth/requests/login?login_challenge=a435b9e14cc04ee9a4b374b71e17397f and uses request_url to redirect user to initiate new login request (Just to clarify where redirect url comes from. Actual request is made in step 3)

Proposed feature allows to initiate new login with fewer redirects by skipping the usual logout flow.

Related issue(s)

https://github.com/ory/hydra/pull/2844

Checklist

  • [x] I have read the contributing guidelines.
  • [x] I have referenced an issue containing the design document if my change introduces a new feature.
  • [x] I am following the contributing code guidelines.
  • [x] I have read the security policy.
  • [x] I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security. vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
  • [x] I have added tests that prove my fix is effective or that my feature works.
  • [x] I have added or changed the documentation.

aarmam avatar Nov 30 '21 12:11 aarmam

Codecov Report

Merging #2876 (4a7c08f) into master (6e1f545) will increase coverage by 0.04%. The diff coverage is 100.00%.

:exclamation: Current head 4a7c08f differs from pull request most recent head 6294aa6. Consider uploading reports for the commit 6294aa6 to get more accurate results

@@            Coverage Diff             @@
##           master    #2876      +/-   ##
==========================================
+ Coverage   76.85%   76.90%   +0.04%     
==========================================
  Files         124      124              
  Lines        9164     9183      +19     
==========================================
+ Hits         7043     7062      +19     
  Misses       1672     1672              
  Partials      449      449              
Impacted Files Coverage Δ
driver/registry.go 24.24% <ø> (ø)
consent/handler.go 68.43% <100.00%> (+1.66%) :arrow_up:
driver/registry_base.go 86.09% <100.00%> (+0.10%) :arrow_up:

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Mar 30 '22 21:03 codecov[bot]

Code looks good, but there are conflicts with master. Rebasing and regenerating the SDKs probably is enough to fix it!

aeneasr avatar Nov 04 '22 07:11 aeneasr

@aeneasr aside from being out of date with master, is there anything holding this body of work back?

web-kat avatar Apr 05 '23 14:04 web-kat

@web-kat @aeneasr I am closing this pull request because a similar pull request, #3450, has already been merged.

aarmam avatar Apr 17 '23 06:04 aarmam