-
Notifications
You must be signed in to change notification settings - Fork 56
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
[82122] updates /revoke to take device_secret param #16676
Conversation
ee54d65
to
935cb93
Compare
5104507
to
7439515
Compare
7439515
to
3bf40a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I tested this and confirmed it removes all sessions with a shared 'hashed_device_secret'
Also I confirmed a refresh token is necessary to perform this, which eventually we may want to consider whether it's necessary or not in a 'device_secret' revoke, but for now we'll just make refresh token required
Summary
/revoke
to take optionaldevice_secret
parameterdevice_secret
is presentSessionRevoker
will attempt to use to it to look up and destroy any other sessions that have a matchinghashed_device_secret
propertyRelated issue(s)
Testing done
Testing
shared_sessions
totrue
.SignIn::OAuthSession
s, then modify a session to have a matchinghashed_device_secret
to the current session you possess thedevice_secret
for:/revoke
call with the access token for your current session and thedevice_secret
passed as a url param:What areas of the site does it impact?
/revoke
Acceptance criteria
Requested Feedback
(OPTIONAL)What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?