-
Notifications
You must be signed in to change notification settings - Fork 60
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
List sessions endpoint for quality-on-demand #325
Conversation
- The access token may be either 2-legged or 3-legged. If a 3-legged access token is used, the end user associated with the session must also be associated with the access token. | ||
- If no QoS session is found for the device, an empty array is returned. |
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.
I think we have to distinct the cases of 2-legged and 3-legged token even more:
- with 2-legged token the API Client can get only the sessions created by the same API Client
- with 3-legged token all session created for the device associated with the access token are returned (at least that was my understanding of a use case at Telefonica?) [update based on the discussion below: I don't think that this use case can be covered with the same endpoint, it would require a different scope]
As we will make device optional as parameter: should there be a valid return if 2-legged token is used and no device is given? As far as I remember we don't want to return the complete list of sessions created by the API Client, even if this was the original intention of #101. One reason is that we then need to introduce paging.
The text for the two lines could get changed as following:
- The access token may be either 2-legged or 3-legged. If a 3-legged access token is used, the end user associated with the session must also be associated with the access token. | |
- If no QoS session is found for the device, an empty array is returned. | |
- The access token may be either 2-legged or 3-legged. | |
- If a 3-legged access token is used, the end user associated with the session must also be associated with the access token. In this case it is recommended to not use the device parameter (see API description for details). | |
- If a 2-legged access token is used, all returned sessions must be created by the API client given in the access token. In this case the device parameter must be used and specify a device. | |
- If no QoS session is found for the device, an empty array is returned. |
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.
I haven't read the last #101 (comment) in #101 before making my comment. Fine for me to postpone this discussion into the context of #313 and #326.
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.
Relation with client_id is something that we may have to consider also for the rest of endpoints accessing created sessions, even if we have pragmatically avoided the topic relying on the knowledge of the sessionId as a secret. Case is:
client_id_A creates a session, client_id_B, with a valid token, tries to retrieve or delete that session. Should we return success or an error? Should we have a different behaviour if input is sessionId or device?
Returning an error, implies that the "owner" client_id has to be kept as a property of the session by the implementation, and only owner client_id can access or operate over a created session. I think that now we are not considering this requirement, but we may need to make explicit the expected behaviour for each scenario, for example to consider the scenario in a test plan.
There are several scenarios to decide upon:
Case | Type of token | input | same client_id | Expected behaviour |
---|---|---|---|---|
1 | 3-legged | session_id | Y | 200 + sessionInfo |
2 | 3-legged | session_id | N | 200 + sessionInfo, new client_id needs permission from user |
3 | 2-legged | session_id | Y | 200 + sessionInfo, but after checking client_id ownership ? |
4 | 2-legged | session_id | N | Now: 200 + sessionInfo, but should we check client_id ownership and return 403? |
5 | 3-legged | device | Y | 200 + sessionInfo, checking that device is coherent with token |
6 | 3-legged | device | N | 200 + sessionInfo, checking that device is coherent with token, new client_id needs permission from user |
7 | 2-legged | device | Y | TBD: 200 + sessionInfo, after checking client_id ownership ? |
8 | 2-legged | device | N | TBD: 403 after after checking client_id ownership or return session without any check on client_id ownership ? |
9 | 3-legged | empty (device deducted from token) | Y | As 5 |
10 | 3-legged | empty (device deducted from token) | N | As 6 |
11 | 3-legged | empty (device not deducted from token) | Y | 403 INVALID_CONTEXT |
12 | 3-legged | empty (device not deducted from token) | N | 403 INVALID_CONTEXT |
13 | 2-legged | empty | Y | TDB: Return all sessions for the client_id, or avoid |
14 | 2-legged | empty | N | TBD: To be avoided (return all sessions for any client) |
Even if cases 9 and 10 are clear, I tend to think that we should make the device required by now, to avoid the other corner cases (13 and 14). And as as commented in #313 making device optional has other implications.
I have doubts with cases 4 and 7/8, after what I wrote above about implications on client_id ownership: should we keep a different behaviour depending on the type of input or have a common approach (checking or not checking ownership at all)? I tend to think here that we should have the same guideline for scenarios 4 and 8.
It may add flexibility to distinguish the endpoint (retrieve session by device) with a different scope, so providers can restrict the usage of 2-legged tokens for certain apps or to certain endpoints.
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.
If we keep the scope of the endpoint to "retrieve session by device", meaning that there need to be an identified device, then we can still make the parameter device optional, as for the cases 13 and 14 there is a clear behaviour defined in the appendix of the Design Guidelines (which I copied as required into the info.description within #326):
Error handling for unidentifiable devices:
- If the
device
object is not included in the request and the device information cannot be derived from the 3-legged access token, the server will return a 422UNIDENTIFIABLE_DEVICE
error.Restrictions for tokens without an associated authenticated identifier:
- For scenarios which do not have a single device identifier associated to the token during the authentication flow, e.g. 2-legged access tokens, the
device
object MUST be provided in the API request. This ensures that the device identification is explicit and valid for each API call made with these tokens.
BTW: within 11 and 12 the error code would be UNIDENTIFIABLE_DEVICE. INVALID_TOKEN_CONTEXT can happen in 5, 6, 7 and 8. And I'm in favour to check the ownership of a session, I don't see cases where a client_id_B should manipulate the session of a client_id_A.
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.
And I'm in favour to check the ownership of a session, I don't see cases where a client_id_B should manipulate the session of a client_id_A.
A use case that we have for this is when the user uses an App to manage or know about the existence of sessions that may have been created from another channel. But 3-legged tokens are required as the App must have consent from the user to manage their sessions. What I don't foresee is a use case where an App using 2-legged tokens can manage sessions created by other apps for any user.
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.
Hi @eric-murray
That is indeed the question, what "quality-on-demand:sessions:read" scope means. If we explicitly indicate that the scope give access to a client to read sessions only created by that client, only clients that created sessions can get or delete those sessions. It is also possible to create different scopes "quality-on-demand:sessions:read:self" and "quality-on-demand:sessions:read:all" to differentiate the behaviour and only assign the later to privileged apps.
Ultimately we have to define a test scenario such as:
Given a clientId_A created a session with sessionId=X
And clientId_B has an an access token with scope "quality-on-demand:sessions:read"
When clientId_B requests the session for sessionId=X
Then the response is ??
Implications on implementation are not trivial, as Auth servers granting and checking scopes will not generally know anything about sessionId and owner of that session. It would have to be the service implementation the one enforcing the rule and for that it needs to attach the clientId during session creation. But clientId is not part of the API, it is part of the token, so info contained in the access token has to be populated from the Auth server to the service implementation. This affects any API dealing with resources, such as subcriptions. So we probably need an explicit guideline.
For the short term, we have to take a decision regarding this endpoint and its scopes.
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.
My proposal would be to be restrictive here:
- We require that the returned sessions are
- associated with the client_id presented in the access token
- in case of 3-legged tokens the end user (or device) associated with the session must also be associated with the access token
- We require that a device must be identifiable for the operation (read: we don't list all sessions associated with a client_id)
- We define an own scope for the operation, not using
read
With that we would support only the cases 5,7,9 from the table above for the retrieve operation (session is not an input option here). Operators can decide separately from the read
scope if they grant access.
And I suppose my suggested text from the begin of the discussion thread might fit to these restrictions:
- The access token may be either 2-legged or 3-legged.
- If a 3-legged access token is used, the end user associated with the session must also be associated with the access token. In this case it is recommended to not use the device parameter (see API description for details).
- If a 2-legged access token is used, all returned sessions must be created by the API client given in the access token. In this case the device parameter must be used and specify a device.
- If no QoS session is found for the device, an empty array is returned.
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 proposal makes sense to me. I think it is reasonable to restrict the current scope "read" only to the clientId that created the sessions. We would have to make that more explicit for the whole API.
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.
I will not add by now any reference to the optionality of device, as that is not specific to this PR and we should have consistent texts for that across all endpoints, so I'll just add:
- If a 2-legged access token is used, all returned sessions must have been created by the same API client given in the access token.
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 requirement that "all returned sessions must have been created by the same API client given in the access token" is independent of 2- or 3-legged token. So also my text proposal was wrong:
- The access token may be either 2-legged or 3-legged.
- all returned sessions must have been created by the same API client given in the access token
- If a 3-legged access token is used, the end user associated with the session must also be associated with the access token. In this case it is recommended to not use the device parameter (see API description for details).
- If a 2-legged access token is used the device parameter must be used and specify a device.
- If no QoS session is found for the device, an empty array is returned.
"500": | ||
$ref: "#/components/responses/Generic500" | ||
"503": | ||
$ref: "#/components/responses/Generic503" | ||
|
||
/sessions/retrieve: |
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.
I also raised this point for the QoS provisioning proposal, but the design guidelines could be interpreted as meaning this endpoint must be named POST /retrieve-sessions
.
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.
I also commented there, that both names work for me, so no strong opinion here
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.
I have changed it to /retrieve-sessions
- device | ||
properties: | ||
device: | ||
$ref: "#/components/schemas/Device" |
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.
If a session was created using, for example, IP address / port, should we be allowed to search for it by other parameters, such as MSISDN? Would we just leave this up to implementations?
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.
It is hard to limit that in an API. It depends on how implementations attach device info to the session and are able to work with or exchange different type of identifiers. They may locate the session or not depending on this.
But the concept of Device as property of current SessionInfo triggers many questions that we probably need to tackle for future releases. So far we ask the device to be returned as part of sessionInfo in responses, but we have not stated explicitly if the identifiers in the response must always be the same identifiers that were indicated in the creation request. Moreover, IP addresses may be reassigned and they may no longer identify the same device at all moments. Also, if we decide to map to a permanent identifier, such a MSISDN, and we return it, we may be disclosing private and unnecessary information to the client.
- Renamed path to /retrieve-sessions - Independent scope: quality-on-demand:sessions-retrieve-by-device - Specify usage of 2-legged token for same clientId
We need to consider if adding of the operation with the restrictions defined in the above discussion thread (see this comment as summary) does still make sense. The original issue #101 asked for "A /get endpoint offering a paginated response of all sessions that the currently authenticated user created and are still active." This we excluded, it would a different operation if offered at all. Then there was the idea of an app on the device which would allow the device user to see all active QoD session, independent of who created them. This we excluded as well. I would argue that this would be an operation which needs to be very carefully designed and potentially not provided beyond operator owned apps. If these two use cases are not covered by the current proposal ... what value is left for developers with this new operation? |
I (hopefully) have adapted the new endpoint to the same principles in #326 |
Co-authored-by: Herbert Damker <herbert.damker@telekom.de>
I should be ready to be merged after #326 |
Remove duplicated responses
@eric-murray @RandyLevensalor also here the question if you have any additional comments? |
@hdamker please feel free to merge it once you get all needed OKs |
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.
LGTM
What type of PR is this?
What this PR does / why we need it:
Adds a new endpoint to return a list of sessions for a given device, thus it will be possible to look for sessions for a given device without knowing the assigned sessionId.
Which issue(s) this PR fixes:
Fixes #101
Special notes for reviewers:
Deprecates PR #228
I have made device required in the input, to be aligned with the rest of the API in this PR. There is a separate issue #313 to decide on the alignment of the API with the guidelines regarding device identification and making it optional in the request body.
Changelog input
Additional documentation