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

feat: endpoint to delete login session by session id #2876

Closed
wants to merge 6 commits into from

Conversation

aarmam
Copy link
Contributor

@aarmam aarmam commented Nov 30, 2021

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 feat: revoke consent by session id. trigger back channel logout. #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)

#2844

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • 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
    security@ory.sh) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

@aarmam aarmam requested a review from aeneasr as a code owner November 30, 2021 12:28
@aarmam aarmam changed the title feat: entpoint to delete login session by session id feat: endpoint to delete login session by session id Dec 1, 2021
@aarmam aarmam marked this pull request as draft December 1, 2021 17:03
@aarmam aarmam force-pushed the feature/delete-session-enpoint branch from eaa7ab5 to 5187c19 Compare December 8, 2021 09:41
@aarmam aarmam force-pushed the feature/delete-session-enpoint branch from 5187c19 to a7011fc Compare March 15, 2022 12:33
@aarmam aarmam force-pushed the feature/delete-session-enpoint branch 3 times, most recently from 89b26f5 to 624997a Compare March 25, 2022 12:01
@codecov
Copy link

codecov bot commented Mar 30, 2022

Codecov Report

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

❗ 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%) ⬆️
driver/registry_base.go 86.09% <100.00%> (+0.10%) ⬆️

... 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.

@aarmam aarmam force-pushed the feature/delete-session-enpoint branch from 706e11d to 66dd1d1 Compare March 30, 2022 21:43
@aarmam aarmam marked this pull request as ready for review April 4, 2022 06:05
@aarmam aarmam force-pushed the feature/delete-session-enpoint branch 2 times, most recently from 1947271 to 610851c Compare April 19, 2022 13:46
@aarmam aarmam force-pushed the feature/delete-session-enpoint branch from 610851c to 8fe10c9 Compare May 11, 2022 07:27
@burkov burkov mentioned this pull request Jun 23, 2022
7 tasks
@aarmam aarmam marked this pull request as draft September 6, 2022 10:47
@aarmam aarmam force-pushed the feature/delete-session-enpoint branch 3 times, most recently from 921f258 to af42dee Compare November 1, 2022 16:51
@aarmam aarmam marked this pull request as ready for review November 1, 2022 17:34
@aarmam aarmam force-pushed the feature/delete-session-enpoint branch from af42dee to 8cb6323 Compare November 2, 2022 08:19
aeneasr
aeneasr previously approved these changes Nov 4, 2022
Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

🙏

@aeneasr
Copy link
Member

aeneasr commented Nov 4, 2022

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

@aarmam aarmam force-pushed the feature/delete-session-enpoint branch 2 times, most recently from 7765091 to 976f40b Compare November 4, 2022 08:09
@aarmam aarmam requested a review from aeneasr November 4, 2022 08:32
@aarmam aarmam force-pushed the feature/delete-session-enpoint branch 2 times, most recently from 1b30b54 to ed7b4fa Compare December 21, 2022 15:20
@aarmam aarmam marked this pull request as draft December 21, 2022 16:16
@aarmam aarmam marked this pull request as ready for review December 21, 2022 16:16
@web-kat
Copy link

web-kat commented Apr 5, 2023

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

@aarmam aarmam force-pushed the feature/delete-session-enpoint branch from 99e6fe4 to 6d362d5 Compare April 5, 2023 14:22
@aarmam aarmam force-pushed the feature/delete-session-enpoint branch from 8305036 to 6294aa6 Compare April 6, 2023 10:32
@aarmam
Copy link
Contributor Author

aarmam commented Apr 17, 2023

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

@aarmam aarmam closed this Apr 17, 2023
alarkvell pushed a commit to e-gov/GovSSO-Session that referenced this pull request Nov 28, 2023
alarkvell pushed a commit to e-gov/GovSSO-Session that referenced this pull request Nov 29, 2023
alarkvell pushed a commit to e-gov/GovSSO-Session that referenced this pull request Nov 29, 2023
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.

None yet

3 participants