-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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: list consent sessions by session id #2853
Conversation
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.
The idea of this PR is to only return a subset of the user’s consents?
From implementation standpoint it looks good :)
Let me know if ready for 👀 by clicking "ready for review"! |
b70391d
to
26a6230
Compare
Codecov Report
@@ Coverage Diff @@
## master #2853 +/- ##
==========================================
+ Coverage 76.82% 76.95% +0.13%
==========================================
Files 123 123
Lines 8976 9044 +68
==========================================
+ Hits 6896 6960 +64
- Misses 1652 1655 +3
- Partials 428 429 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
26a6230
to
0cc8f84
Compare
f3fee3a
to
b918410
Compare
e38a27a
to
319ba6a
Compare
c56c5f2
to
76cf185
Compare
db77826
to
5f9223b
Compare
a8596e4
to
e73baaf
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.
This looks very good! There is a small conflict with the v2 SDK naming but once that’s resolved I’ll merge this asap :)
e73baaf
to
4c29937
Compare
6298d9d
to
1b307ba
Compare
1b307ba
to
20d5bbc
Compare
This pull request introduces feature to list subject consent sessions by session id.
Use case:
When authentication is initiated without
prompt
parameter from multiple devices, we would like to distinguish in what session was the consent given.Current situation:
GET /oauth2/auth/sessions/consent returns single consent and it references to the session id, where the consent was initially given. Furthermore if logout is performed from device initially gave the consent, the reference to login session id is cleared.
If
prompt=consent
were to be used, separate consents with separate session id would be returned and you would not have same problems. So this endpoint behaves differently in relation to how login session id is referenced.Proposed solution:
Add additional query parameter
login_session_id
for GET /oauth2/auth/sessions/consent to return consents related to requested session id.This solution does not change how login session id is referenced in result.
Checklist
introduces a new feature.
contributing code guidelines.
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.
works.
Further Comments
Tests and documentation will be commited after inital acceptance of the proposed feature.