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

Which error send back when issue with Device Identifier provided by the API Consumer? #127

Closed
bigludo7 opened this issue Jan 30, 2024 · 46 comments · Fixed by #213
Closed
Labels
enhancement New feature or request

Comments

@bigludo7
Copy link
Collaborator

Problem description
A very small issue but looking for feedback. I guess we can have a common behavior.

In a lot of our API we use the device identifier schema to get the identifier of the line. That's fine, but which error to send back if the device identifier is not supported by the API Implementation? example, an implementation is only able to perform an API with a phone number or IP but not networkAccessIdentifier.

Possible evolution
Agreed to use for this cas error 501 with code DEVICE_IDENTIFIER_TYPE_NOT_MANAGED

Alternative solution
use 500 (with code DEVICE_IDENTIFIER_TYPE_NOT_MANAGED) ?
Let this at the hand of each implem?

Additional context

@bigludo7 bigludo7 added the enhancement New feature or request label Jan 30, 2024
@shilpa-padgaonkar
Copy link
Collaborator

Agree that we need a common behaviour for this. How about 501 with code DEVICE_IDENTIFIER_TYPE_NOT_SUPPORTED instead of DEVICE_IDENTIFIER_TYPE_NOT_MANAGED

@pjhac
Copy link
Collaborator

pjhac commented Feb 5, 2024

Hello,

I was looking at the RFC 9110 definition of 501: "the server does not support the functionality required to fulfill the request. This is the appropriate response when the server does not recognize the request method and is not capable of supporting it for any resource".

In the present case, I think (but I may be wrong in my understanding, please correct me if that's the case) that the server is actually able to handle the request, but not some of the parameters. If the supported parameters are known by the calling party, then I believe it should be a 400 as the calling party is able to change the request input parameters. Technically, a 501 cannot be fixed by the calling party (the code is not available on the server side to handle the request), as per the note inside https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/501.

To make an analogy, it would be like calling a function inside a program with a parameter that is unknown to the function; this is a caller error, as the function cannot be blamed for not supporting a parameter defined by the caller.

What do you think?

@uwerauschenbach
Copy link
Collaborator

I prefer a 422 status code (Unprocessable content), to indicate that there is an issue with the message content (payload body) provided. Similar handling could apply to other attributes or attribute values in the message content that are optional to be supported. Maybe we even make this error response more generic, as we are working on Commonalities here, such as Attribute Not Supported or Attribute Value Not Supported.

501, as I understand it, is more related to the whole protocol (e.g. method such as PATCH not implemented).

@pjhac
Copy link
Collaborator

pjhac commented Feb 5, 2024

Good catch, @uwerauschenbach, I am with you for 422.

The example in RFC 9110 expresses it this way: "for example, this status code can be sent if an XML request content contains well-formed (i.e., syntactically correct), but semantically erroneous XML instructions."

Having a syntactically correct JSON with an inappropriate parameter is a semantic error, so I am in favor of it.

@bigludo7
Copy link
Collaborator Author

bigludo7 commented Feb 5, 2024

Thanks for the suggestion. I'm fine with the 422 as well.
Waiting for other feedbacks before to make any PR ;)

@eric-murray
Copy link
Collaborator

I'm fine with 422 as well

@jlurien
Copy link
Contributor

jlurien commented Feb 27, 2024

Hi, this is very relevant topic to enhance our testing guidelines and compliance expectancy.

Currently device have 4 optional properties, with minItems=1. There are several error cases for device validation:

  • device not schema compliant -> 400
  • Schema compliant but no one of the provided properties are supported -> 422, which code ?
  • Schema compliant with several properties supported, but they identify different device, e.g. phoneNumber and IP address do not match -> 409 ? 422 different code ?
  • Schema compliant, properties supported but identified device does not match the one in the 3-legged access token -> 403
  • Device correctly identified but service not supported for this device, e.g. mobile vs fixed, B2B vs B2C, etc -> 5xx ? discussed in Proposal to add error response for not supported method for phone number #138

Should it be a validation order?
Apart from status, it would be good to assign specific codes for identified error cases, e.g. PHONE_NUMBER_REQUIRED, DEVICE_IDENTIFIERS_MISMATCH, INVALID_TOKEN_CONTEXT...

@pjhac
Copy link
Collaborator

pjhac commented Feb 27, 2024

Hi @jlurien

  • Schema compliant with several properties supported, but they identify different device, e.g. phoneNumber and IP address do not match -> 409 ? 422 different code ?

I have asked some colleagues who propose 404 for this specific case, as it is like trying to identify a unique resource with a composite key whose sub-elements point to different resources but intersect to none. It also has the advantage of not telling anything about which property is wrong or right and/or to what it is pointing, as it is the kind of information one might not want to see revealed (a bit like when you enter a bad login/password combination, you don't know which is wrong, just that it failed).

What do you think?

Best regards,
Pierre

@eric-murray
Copy link
Collaborator

@pjhac
I think that, as long as we allow the API consumer to submit multiple device identifiers, we need a separate error response when these identifiers identify different users (neither of which is "wrong", they just don't agree). A 404 should mean "no valid end user found", rather than "multiple valid end users found and I don't know which one you really meant". There is a case to be made for error responses being "helpful" to the developer.

Now I agree this possibly allows the API consumer to play games by, for example, fixing the IP address and varying the MSISDN to try and find who is using a particular IP address. But they could equally do that with a 404, and really it is up to the API provider to try and stop this type of abuse. And there is anyway no obligation on the API provider to ensure that the identifiers match. More likely, they will accept their "preferred" identifier and ignore the rest. So a positive response does not mean that all provided identifiers agree.

Ongoing, I think this problem will anyway go away, as the end user must match that associated with the access token, and only one end user identifier can be used to obtain an access token.

@pjhac
Copy link
Collaborator

pjhac commented Feb 28, 2024

Hi @eric-murray

I think that, as long as we allow the API consumer to submit multiple device identifiers, we need a separate error response when these identifiers identify different users (neither of which is "wrong", they just don't agree). A 404 should mean "no valid end user found", rather than "multiple valid end users found and I don't know which one you really meant". There is a case to be made for error responses being "helpful" to the developer.

My understanding from @jlurien 's post was that the fields, if more than one provided, were all expected to match the same and unique device; in other words, using more than one field should be interpreted as a composite key pointing to a single device. If they are unrelated to each other at provider side, it looks like I may be wrong here.

And there is anyway no obligation on the API provider to ensure that the identifiers match. More likely, they will accept their "preferred" identifier and ignore the rest. So a positive response does not mean that all provided identifiers agree.

How will the end-user know which field is preferred, if that is API provider specific / implementation specific? I am wondering what impact it has in terms of user experience, if one has to provide several fields and only one appears to be used in the end. Or is there a way to tell which is preferred? I would expect that reusing the same API calls from provider to provider would give the same results without having to know which parameter(s) provider A needs / wants, then provider B... I may be misunderstanding your point and will welcome additional feedback you can provide.

Thanks and best regards,
Pierre

@diegogonmar
Copy link

Hi,
I want to provide a different point of view. We're discussing of error codes in case some essential input to the API is not supported. But CAMARA API program key for success is interoperability. It's a must in OpenGateway. With existing solution for device, and assuming that each implementation will support a subset of values, we are going against interoperability. A developer may need to code their App to collect and send every available value to maximize/ensure success, or have logic of type if Operator 1 send this/ If Operator 2 send other... Both options are weird and to be honest, we could not really say that CSPs are offering same API

Shouldn't we rethink the device object solution instead? Maybe we should better choose 1-2 options and then support for those becomes mandatory in the server side. Some ideas:

  • It seems that phone_number serves in most scenarios. It may not be valid in M2M, in such cases maybe IMSI or ICCID would work?. I see no reason for not mandating every implementation to support phone_number.
  • networkAccessIdentifier may be not very developer friendly. I have doubts about this one, but if we maintain it maybe we should clearly define format and mandate its support
  • Regarding IPv4 / IPv6, if this info is available it means that there is Mobile/Fixed CSP connectivity, so the best is to use Frontend Flow in any case, as described here https://github.com/camaraproject/IdentityAndConsentManagement/blob/main/documentation/CAMARA-API-access-and-user-consent.md#authorization-code-flow-frontend-flow. Then the API Client need not gather the IP from the device, as its automatically bound to the Access Token. No matter Ipv4 or IPv6, if the connection is valid flow will success (Identity Provider performing Network Auth can just perform the redirect to IPv6 or IPv4 endpoint if only one is supported, App FE will handle it)

So, wouldn't be enough with phone_number and an alternative to solve M2M scenarios e.g.: IMSI or ICCID? Both mandatory for implementations.

@eric-murray
Copy link
Collaborator

@pjhac
To summarise my comment, it is an error if the identifiers don't identify the same end user, so we are only discussing what that error should be. I prefer making error responses useful to developers, as that encourages API adoption, rather than prioritising thwarting hackers by giving as little information as possible. That is just my preference.

My understanding from @jlurien 's post was that the fields, if more than one provided, were all expected to match the same and unique device

So, yes, that is the expectation. They should all identify the same end user. But the identifiers are independent and could identify different end users, rather than being a composite key where the end user can only be identified once we have all the pieces.

It is an error if the identifiers don't match, and one the API consumer needs to resolve. Likely they made an error in one of the fields, So I think telling the API consumer that the identifiers were valid but contradictory is more useful than just saying there was an error but no give clues as to what it was.

Another point is that we cannot mandate that the API provider enforces that all identifiers match the same end user. It may just not be possible for some API providers to evaluate some identifiers.

How will the end-user know which field is preferred

In one sense, it doesn't matter. The API consumer provides what they know and the API provider processes as they prefer. That could be using one and ignoring the others (so not even validating that the other identifiers identify the same user). It is not practical to mandate that the API provider validates all identifiers passed. For example, determining the end user from a given IP and port (in real-time) is not functionality that all networks support. It is therefore not practical for such networks to support identifying devices by IPv4 address.

If the API consumer wishes to know which identifiers a given API provider supports, they would need to read the API provider's documentation. Alternatively, there is a proposal for a mechanism to advertise which parameters are supported.

But likely the API consumer is accessing the API via an aggregator, and does not know which network operator will be fulfilling the API call. That is why the API consumer should provide all the parameters they know, as it may be one API provider prefers phoneNumber and another prefers networkAccessIdentifier and a third does not support ipv4Address. By providing all these parameters, it does not matter which API provider eventually fulfils the API call. But if an API provider does check that all provided identifiers match the same end user, I still think it useful to have a separate error to distinguish this from the case that the identifier(s) evaluated by the API provider does not match any end user.

@eric-murray
Copy link
Collaborator

I see no reason for not mandating every implementation to support phone_number.

Not all mobile subscriptions will have an associated MSISDN. And a fixed connectivity CPE is also likely not have a phone number. So supporting phoneNumber cannot be mandated.

@diegogonmar
Copy link

diegogonmar commented Feb 28, 2024

I see no reason for not mandating every implementation to support phone_number.

Not all mobile subscriptions will have an associated MSISDN. And a fixed connectivity CPE is also likely not have a phone number. So supporting phoneNumber cannot be mandated.

I don't agree with your statement. My point is that implementation of the API must support that if phone_number is included, is processed. I'm not saying having phone_number as required input field in .yaml, I'm saying that if included, there is no option to answer "error I have not implemented support for phone_numbers". Of course if phone_number do not exist (I also indicated example of M2M SIMCards) can't be used by API Clients for that use case.

@diegogonmar
Copy link

Another point is that we cannot mandate that the API provider enforces that all identifiers match the same end user. It may just not be possible for some API providers to evaluate some identifiers.

This is an interesting one. If API Client sends 2 values not matching the same user and API provider do not support one of the two, request will succeed (failing will be worse?) but API Client will not know which one was used. This reflects that having N options and each API provider supporting different sub-set of them makes scenario more complicated.

@diegogonmar
Copy link

diegogonmar commented Feb 28, 2024

But likely the API consumer is accessing the API via an aggregator, and does not know which network operator will be fulfilling the API call. That is why the API consumer should provide all the parameters they know, as it may be one API provider prefers phoneNumber and another prefers networkAccessIdentifier and a third does not support ipv4Address. By providing all these parameters, it does not matter which API provider eventually fulfils the API call. But if an API provider does check that all provided identifiers match the same end user, I still think it useful to have a separate error to distinguish this from the case that the identifier(s) evaluated by the API provider does not match any end user.

This is the best option with current approach, but I wouldn't say it's a Developer Friendly approach. Should developer ask user for phone_number just in case? Maybe use case does not fit well with asking the user to provide phone_number. Or collect both IPv4 and IPv6 to be 100% sure it will work?

@eric-murray
Copy link
Collaborator

I agree there are many complexities in device identification!

Implementation of the API must support that if phone_number is included, is processed.

Even this may be problematic if the use case requires the IP address of the device and the network has no way to determine this from the MSISDN. I agree that there should be a general expectation that phoneNumber is evaluated if provided, but I don't think that can be mandated as that may exclude implementations that API consumers are perfectly happy to use.

but API Client will not know which one was used

Some APIs include the device identifier in the response, and so can indicate which identifier was used, but this is not the general case. I agree this is a potential issue for other APIs, and can be resolved in the same way, by including the used device identifier in the response if it is required to avoid such ambiguity.

Should developer ask user for phone_number just in case?

Maybe the aggregator themselves can resolve this. If they know that all southbound implementations support, say, phoneNumber, then they can inform (and maybe mandate) that API consumers of their implementation provide that, and only that, identifier. If multiple identifiers are required for full coverage, then that set can be mandated.

@diegogonmar
Copy link

I agree there are many complexities in device identification!

Implementation of the API must support that if phone_number is included, is processed.

Even this may be problematic if the use case requires the IP address of the device and the network has no way to determine this from the MSISDN. I agree that there should be a general expectation that phoneNumber is evaluated if provided, but I don't think that can be mandated as that may exclude implementations that API consumers are perfectly happy to use.

Not sure I get your point. You mean use case in an API or the API itself? In which example are you thinking?
I'd say that if an API requires IP, then then IP has to be used, not a device object with other options. As I said before, probably for the IP cases (v4 vs v6) the best always-valid option is to use AuthCode Flow which will get a token based on Network Auth (either IPv4, IPv6 or other mechanism that will result to the same)
Mandating phoneNumber will not exclude other implementations for API consumers, because it's not a mandate for API Consumer, its a mandate for support by API Implementation.

but API Client will not know which one was used

Some APIs include the device identifier in the response, and so can indicate which identifier was used, but this is not the general case. I agree this is a potential issue for other APIs, and can be resolved in the same way, by including the used device identifier in the response if it is required to avoid such ambiguity.

Yes, this is an option. In my opinion not a good one in terms of API design as adds yet another complexity to API Consumer. In addition to the collect all and send them its the check responses to find out what was used. Also some APIs may not be designed to have a good fit for this approach. E.g.: QoD has a session concept with the info, there we can say that identifier included is the one used among the ones in the request. But for Location Verification the fit of this info is unnecessary in terms of API design and needs.

Should developer ask user for phone_number just in case?

Maybe the aggregator themselves can resolve this. If they know that all southbound implementations support, say, phoneNumber, then they can inform (and maybe mandate) that API consumers of their implementation provide that, and only that, identifier. If multiple identifiers are required for full coverage, then that set can be mandated.

For me this seems that we in CAMARA do not create a standard fully-interoperable APIs and then add a requirement to others (aggregators) to solve the issue. There will also exist scenarios where partners directly go to implementations, without an aggregator, in that case we would also need to ask them to either send all, identify whats needed for full coverage or made the If operator 1 / if operator 2.

@eric-murray
Copy link
Collaborator

Not sure I get your point. You mean use case in an API or the API itself? In which example are you thinking?

An example would be Simple Edge Discovery, where what matters in determining the closest edge site is the allocated device IP address, and not its MSISDN. If CAMARA has a rule that mandates accepting MSISDN as the device identifier if provided, then implementations will be forced to implement MSISDN to IP lookup, which is not trivial.

Far better to make it optional, so that those API providers who can implement this do support MSISDN as the device identifier, and those who cannot do not. That way, at least API consumers will get an API that accepts IP address (which may be sufficient for them) rather than nothing at all because an API provider choses not to provide the API rather than implement MSISDN to IP lookup.

@pjhac
Copy link
Collaborator

pjhac commented Feb 29, 2024

@eric-murray Thank you for your reply addressing my last post.

I also think that having optional parameters for the reasons detailed above makes sense; it may look "less constraining", but this can also be an advantage as it gives more freedom while still locking providers to a specific subset of possible fields which, although optional, have well-defined names.

(as a side note, an alternative approach is the { "key": "x", "value": "y" } array item structure, which can be repeated as many times as needed inside an array, and virtually accepts any parameter. The problem with this approach is its difficult validation (avoiding repetitions within the array), enforcement of specific key names (unless enumerated are used as only valid key names) and their formats. On less constraining systems where "quantity" of possible combinations matters over "quality" (well-defined, well-scoped field names), this makes sense, but in the present discussion, the low amount of parameters does not really call for such an approach.)

Best regards,
Pierre

@diegogonmar
Copy link

@eric-murray Thank you for your reply addressing my last post.

I also think that having optional parameters for the reasons detailed above makes sense; it may look "less constraining", but this can also be an advantage as it gives more freedom while still locking providers to a specific subset of possible fields which, although optional, have well-defined names.

(as a side note, an alternative approach is the { "key": "x", "value": "y" } array item structure, which can be repeated as many times as needed inside an array, and virtually accepts any parameter. The problem with this approach is its difficult validation (avoiding repetitions within the array), enforcement of specific key names (unless enumerated are used as only valid key names) and their formats. On less constraining systems where "quantity" of possible combinations matters over "quality" (well-defined, well-scoped field names), this makes sense, but in the present discussion, the low amount of parameters does not really call for such an approach.)

Best regards, Pierre

I have a different view about implications of optional parameters
More options in an API input--> flexibility, freedom, etc.
More options in an API input + allowing partial support to them --> unpredictability, non-universal working, extra explanations to developers/aggregators needed.
So I think options are good, but for the API Consumer to choose, not to be restricted due to options availability.

@diegogonmar
Copy link

Not sure I get your point. You mean use case in an API or the API itself? In which example are you thinking?

An example would be Simple Edge Discovery, where what matters in determining the closest edge site is the allocated device IP address, and not its MSISDN. If CAMARA has a rule that mandates accepting MSISDN as the device identifier if provided, then implementations will be forced to implement MSISDN to IP lookup, which is not trivial.

Far better to make it optional, so that those API providers who can implement this do support MSISDN as the device identifier, and those who cannot do not. That way, at least API consumers will get an API that accepts IP address (which may be sufficient for them) rather than nothing at all because an API provider choses not to provide the API rather than implement MSISDN to IP lookup.

Ok, I took a look to that API, as I was not very familiar with it.
Before talking about the specific API I also want to state that in my opinion we should have the general rules, but some APIs may require specific solution. E.g.: we have CIBA, Client Credentials and AuthCode as options and some rules, but Number Verification states that it's designed only for AuthCode. So maybe Edge or some other need specific solution, device object may not be the solution for every API.

Regarding Simple Edge Discovery, the API states that it is valid for a "network attached device", so IP connectivity is a must. As you said, the IP is what it matters in determination so maybe in this API it's phone_number which has to be removed and make it always work with IP. I'd say that given the clear characteristics of this API, the use of identifiers other than the IP should actually be different API, this API refers specifically to existing connection of the device.
So in this case you suggest to make phone_number optional, but IP must be mandatory. Let's provide developers with smaller APIs and without partial support or "ifs". The always work scenario should be an API all the operators can say that implement exactly the same.

@bigludo7
Copy link
Collaborator Author

bigludo7 commented Mar 5, 2024

Thanks for all inputs.
@rartych as requested:
As we discussed it yesterday in Commonalities call, with @eric-murray probably some post should be detached in another discussion.

Reseting the conversation back to the initial topic and leveraging @jlurien post to have more cases covered. Here a proposal to manage device identifier errors with valuable information send back to the consumer.
I've added @mhfoo input from #138 for completion.

If the provided device identifier is not compliant
Managed under 400 and following example should described:

NotSupported:
   description: Invalid Argument
   content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     example:
      status: 400
      code: INVALID_DEVICE_IDENTIFIER_FORMAT
      message: Invalid argument for the device identifier

(other example could be added)

If a compliant device identifier is provided but the implementation did not manage this identifier type
If several compliant identifers are provided but do not target same device

Managed under 422 and following examples should described:

UnprocessableEntity:
   description: Unprocessable Entity
   content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     examples:
        IdentifierTypeNotManaged:
            value:
                status: 422
                code: NOT_MANAGED_IDENTIFIER_TYPE
                message: Identifier type not managed by implementation
        IdentifierMismatch:
            value:
                status: 422
                code: DEVICE_IDENTIFIERS_MISMATCH
                message: unprocessable entity with multiple inconsistent identifiers

(other example could be added)
For this mismatch one I noticed proposal by @pjhac to use 404. I've still a preference for 422 as more explicit but probably a matter of taste & color so open to discuss if Pierre you are considering this point as a blocker.

If the identifier did not match the one got from the 3-legs access token
Managed under 403 and following examples should described (already defined as such in Number Verification API):

Forbidden:
    description:  Client does not have sufficient permission
    content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     example:
        status: 403
        code: NUMBER_VERIFICATION.INVALID_TOKEN_CONTEXT
        message: Phone number cannot be deducted from access token context

(other example could be added)

If the provided service identifier is compliant but not supported for this service (#145 and #138)
Managed under 501 and following examples should described:

NotSupported:
   description: Not Supported
   content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     example:
      status: 501
      code: NOT_SUPPORTED
      message: Service not supported for this phone number

(other example could be added)
I saw @sachinvodafone that you proposed 204 for this UC but I let the discussion rolling in #138.

@pjhac
Copy link
Collaborator

pjhac commented Mar 5, 2024

Hi @bigludo7

For this mismatch one I noticed proposal by @pjhac to use 404. I've still a preference for 422 as more explicit but probably a matter of taste & color so open to discuss if Pierre you are considering this point as a blocker.

Following this comment extract from @eric-murray: "A 404 should mean "no valid end user found", rather than "multiple valid end users found and I don't know which one you really meant" -> thinking further about it, I believe that having several fields pointing to more than one device makes the entity "unprocessable" and as such would call for a 422 rather than a 404, I think it makes sense for this particular situation, so I'll follow Eric's comment.

Now, I have another question: if none of the provided fields point to a valid / existing device, I believe we would be responding with a 404, is my understanding correct?

To summarize, we would have:

  • all provided fields point to the same device -> OK
  • some provided fields point to more than one unique device -> 422
  • no provided fields point to an existing device -> 404

I see another case:

  • some provided fields point to a unique device, but some other provided fields point to non-existent devices -> 422? or we ignore and continue? Was it covered and I missed it? Or it may just fall under "unprocessable entity with multiple inconsistent identifiers" case? I think I'd be fine with it if that's the case.

These are not blockers for me, I just want to be sure I'm understanding all cases correctly.

Best regards,
Pierre

@bigludo7
Copy link
Collaborator Author

bigludo7 commented Mar 5, 2024

@pjhac
Thanks for your feedback.

I was thinking same than you about the case where none of the provided field point to an existent device with 404 but checking QoD API and find that:

                CannotIdentifyDevice:
                  summary: No device can be identified from provided parameters
                  value:
                    status: 400
                    code: INVALID_ARGUMENT
                    message: "Unable to identify device from specified parameters: device"

So probably we need an agreement for this point.

for the other case with the mismatch I would be fine with 422 with message unprocessable entity with multiple inconsistent identifiers.

@jlurien
Copy link
Contributor

jlurien commented Mar 6, 2024

Hi, some comments on the latest responses:

  • Regarding the original discussion about getting a request without any identifier being supported, the debate started by @diegogonmar points to the root problem, which is the device model that we currently have. In TEF we consider that the current model is not adequate for interoperability if all identifiers in the spec are not fully supported, or if we don't agree on certain rules about a common subset to be fully supported. Agreeing on error codes to overcome this is not the solution for the long term but just a patch to the indefinition that we have right now. We suggest to open a dedicate issue for that. If we manage to simplify and rationalise the device identification, the error use cases will be simplified as well.

  • The proposed statuses, 400, 403, are quite clear. For 501 I suggest to consolidate the discussion in Proposal to add error response for not supported method for phone number #138 here. For 422, most people have expressed preference for a 4xx code than a 5xx, so 422 is fine in that case. Again, hopefully we can avoid these error cases in the future with a better device identification rules.

  • Apart from status, I think is key to enhance the guidelines about specification and usage of code if we want to have consistent implementations. Long time ago we decided not to specify codes as an enum within specs and rely on examples, which adds flexibility, but examples are not normative and now it is not clear to which extend those codes have to be used. We probably have to maintain in Commonalities a set of agreed codes to be used per identified and transversal error cases, similar to examples listed above by @bigludo7, and indicate in the guidelines that those codes have to be used for those error cases. When we define the Test plans we must create specific scenarios for those error codes. Additionally, each API or WG may define other codes which are specific to that WG. It would be useful to agree on some format, for example API_NAME.ERROR_CODE for the specific error codes, and no prefix for the common ones, or maybe CAMARA.ERROR_CODE.

  • Another aspect to consider in general is to make errors informative enough to help clients, but to avoid disclosing unnecessary information by means of errors. For example, if we validate consistency between identifiers and we return errors in case of mismatch, APIs can be used to check whether certain device is assigned some IP address. Maybe this is a bit "paranoiac" but is a collateral effect of allowing several identifiers to coexist.

@bigludo7
Copy link
Collaborator Author

Hello
In order to move forward on this topic:
(looking for team feedback)

  • Discuss about the mandatory constraint for all API server to accommodate use of any device identifier is not part of this issue and not considered in this proposal.
  • As stated by @jlurien (third point) we need also to underline in the guidelines the management of the code --> José could you please raises a specific issue for that?
  • Thinking about also José point about disclosing unnecessary information perhaps we can answer with 404 when the request has multiple identifiers but not consistent (instead of 422 as initially proposed) - managed in case 3.

New proposal with sorting:

Case 0:
If the identifier did not match the one got from the 3-legs access token
Managed under 403 and following examples must be described
(already defined as such in Number Verification API):

Forbidden:
    description:  Client does not have sufficient permission
    content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     example:
        status: 403
        code: NUMBER_VERIFICATION.INVALID_TOKEN_CONTEXT
        message: Phone number cannot be deducted from access token context

(other example could be added)

Case 1:
If at least one provided device identifier is not compliant with pattern
Managed under 400 and following example must be described:

NotSupported:
   description: Invalid Argument
   content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     example:
      status: 400
      code: INVALID_DEVICE_IDENTIFIER_FORMAT
      message: Invalid argument for the device identifier

(other example could be added)

Case 2:
If the request provides one or several device identifier(s) and the implementation did not manage at least one of this(ese) identifier type.
Note: This case will be dropped if we decide to mandate use of all identifier type
Managed under 422 and following example must be described:

UnprocessableEntity:
   description: Unprocessable Entity
   content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     examples:
        IdentifierTypeNotManaged:
            value:
                status: 422
                code: NOT_MANAGED_IDENTIFIER_TYPE
                message: Identifier type not managed by implementation

(other example could be added)

Case 3
if none of the provided field(s) point to an existing device or if request with multiple inconsistent identifiers (a msisdn and a ip v4 not targeting same device for example).
Managed under 404 and following examples must be described

NotFound:
    description: NotFound
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     example:
        status: 404
        code: INVALID_ARGUMENT
        message: "Unable to identify device from specified parameters: device"

Case 4
If the provided service identifier is compliant but not supported for this service (#145 and #138)
(example of UC: Telco is able to provide a given API only for mass market line and not fleet line. Response for a request on a fleet line)
Managed under 501 and following examples should described:

NotSupported:
   description: Not Supported
   content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     example:
      status: 501
      code: NOT_SUPPORTED
      message: Service not supported for this phone number

(other example could be added)

@pjhac
Copy link
Collaborator

pjhac commented Mar 22, 2024

Hello @bigludo7,

Thanks a lot for bringing all these together.

I think I am fine with cases 0 & 1.

For case 4, I would only say "service not supported" and align the remainder of the error message (if any needed) based on Jose's point about disclosure of information.

For cases 2 & 3, I am a little torn.

  • First, please note I support the error definition for "none of the fields point to a valid device" with code 404.
  • Second, and based on my previous comment (that contained a reference to a reply from Eric), I keep my previous statement about "request with multiple inconsistent identifiers (a msisdn and a ip v4 not targeting same device for example)" with code 422. To me (and this is my personal interpretation of the case, not a general convention), this is similar in essence to "the implementation did not manage at least one of this(ese) identifier type(s)", especially if reworded like this: "the implementation is unable to manage (or handle) at least one of this/these identifier type(s)". With that in mind, a 422 would work very well for any combination of fields & values that cannot either be managed / handled by the implementation because the field(s) is (are) unsupported OR the fact that the values are inconsistent across all provided fields.

In other words, the end result is still the same for those cases, they are unmanageable. As such, providing an "unmanageable entity" for both should work and not reveal anything about the exact scope of the problem (either unsupported or unable to manage a inconsistency), staying compliant with a response that doesn't reveal too much info. At the end of the day, the user has to provide the appropriate information, in both situations.

Best regards,
Pierre

@bigludo7
Copy link
Collaborator Author

Thanks @pjhac
No sure to fully understand ;)
Specifically on "request with multiple inconsistent identifiers (a msisdn and a ip v4 not targeting same device for example)" if we manage a 422 you mean that we did not have to provide a meaningful codes like:

UnprocessableEntity:
   description: Unprocessable Entity
   content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     examples:
        IdentifierTypeNotManaged:
            value:
                status: 422
                code: NOT_MANAGED_IDENTIFIER_TYPE
                message: Identifier type not managed by implementation
         InconsistentIdentifier:
            value:
                status: 422
                code: INCONSISTENT_IDENTIFIER
                message: Inconsistent identifier provided

but instead keep it 'vague' like:

UnprocessableEntity:
   description: Unprocessable Entity
   content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     examples:
        Unprocessable Entity:
            value:
                status: 422
                code: UNPROCESSABLE_ENTITY
                message: Unprocessable Entity provided

I'm not sure to get it because in case of Identifier type not managed we have to be explicit, right?

@pjhac
Copy link
Collaborator

pjhac commented Mar 25, 2024

Hi @bigludo7,

You got it right, that's what I meant. This way we remain consistent with regards to Jose's comment, we don't disclose what happened. I see it (to some extent) the same way we deal with invalid credentials, by not saying which failed / was incorrect.

To your question, I think that being more explicit on the field not managed by the implementer does not really bring much value -- it is not supported, so we can't do anything with it (and the caller will have to change their client call implementation anyway). Moreover, the documentation link should be made available such that the end-user knows which fields should be provided. Having to wait for the error "message" to know what can be provided or not is not really friendly in my opinion. It should be made available through the documentation (technically available at all times even outside the scope of an API call -- and, ideally, repeated as a reference in the appropriate error field, like "referenceError"). Now, with that said, the "message" field could be extended to "Unprocessable entity provided, please check the documentation", so at least the caller knows there is something to check to get the appropriate info.

I hope this makes sense. Let me know if I'm still not clear or if I missed something important.

Best regards,
Pierre

@bigludo7
Copy link
Collaborator Author

Thanks @pjhac
I though initially that we have to be meaningful in case of Identifier type not managed but your argument about the fact that the caller will have to change their client call implementation anyway makes a lot of sense.

As of now the proposal is the following:

Case 0:
If the identifier did not match the one got from the 3-legs access token
Managed under 403 and following examples must be described
(already defined as such in Number Verification API):

Forbidden:
    description:  Client does not have sufficient permission
    content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     example:
        status: 403
        code: NUMBER_VERIFICATION.INVALID_TOKEN_CONTEXT
        message: Phone number cannot be deducted from access token context

(other example could be added)

Case 1:
If at least one provided device identifier is not compliant with pattern
Managed under 400 and following example must be described:

NotSupported:
   description: Invalid Argument
   content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     example:
      status: 400
      code: INVALID_DEVICE_IDENTIFIER_FORMAT
      message: Invalid argument for the device identifier

(other example could be added)

Case 2:
If the request provides one or several device identifier(s) and the implementation did not manage at least one of this(ese) identifier type. Note:This case will be dropped if we decide to mandate use of all identifier type
If the request provideds multiple inconsistent identifiers (a msisdn and a ip v4 not targeting same device for example).
Managed under 422 and following example must be described:

UnprocessableEntity:
   description: Unprocessable Entity
   content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     examples:
        Unprocessable Entity:
            value:
                status: 422
                code: UNPROCESSABLE_ENTITY
                message: Unprocessable entity provided, please check the documentation

(other example could be added)

Case 3
if none of the provided field(s) point to an existing device or
Managed under 404 and following examples must be described

NotFound:
    description: NotFound
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     example:
        status: 404
        code: INVALID_ARGUMENT
        message: "Unable to identify device from specified parameters: device"

Case 4
If the provided service identifier is compliant but not supported for this service (#145 and #138)
(example of UC: Telco is able to provide a given API only for mass market line and not fleet line. Response for a request on a fleet line)
Managed under 501 and following examples should described:

NotSupported:
   description: Not Supported
   content:
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     example:
      status: 501
      code: NOT_SUPPORTED
      message: Service not supported

(other example could be added)

@pjhac
Copy link
Collaborator

pjhac commented Mar 26, 2024

Thank you @bigludo7 for putting together the suggested changes, you addressed all my comments.

@PedroDiez
Copy link
Collaborator

PedroDiez commented Mar 27, 2024

Hi,

Thanks for the clarification thread. Will align with @jlurien next week.

One comment, just to have in mind. For the Case 3 where any of the identitifiers provided do not point to an existing device, would not be NOT_FOUND code?. (Not sure whether here could apply a traversal error in the fashion CAMARA.DEVICE_NOT_FOUND, prefer to check this with Jose who has more context about this topic)

NotFound:
    description: NotFound
    application/json:
     schema:
      $ref: "#/components/schemas/ErrorInfo"
     example:
        status: 404
        code: INVALID_ARGUMENT
        message: "Unable to identify device from specified parameters: device"

@bigludo7
Copy link
Collaborator Author

bigludo7 commented Apr 3, 2024

Hi,

Thanks for the clarification thread. Will align with @jlurien next week.

One comment, just to have in mind. For the Case 3 where any of the identitifiers provided do not point to an existing device, would not be NOT_FOUND code?.

Thank @PedroDiez - Yes make sense ! probably better to have NOT_FOUND code for case 3.

@Kevsy
Copy link
Collaborator

Kevsy commented Apr 3, 2024

Hi @bigludo7 -

For Case 1, has 401 Unauthorized been considered? Per RFC 9110:

"If the request included authentication credentials, then the 401 response indicates that authorization has been refused for those credentials. The user agent MAY repeat the request with a new or replaced Authorization header field."

For Case 4, as touched upon earlier, 501 means "Not Implemented" for any resource, but Case 4 relates to not supporting certain resources.

Per RFC 9110:

"The 501 (Not Implemented) status code indicates that the server does not support the functionality required to fulfill the request. This is the appropriate response when the server does not recognize the request method and is not capable of supporting it for any resource."

I think a more appropriate code would be 403 Forbidden,

"The 403 (Forbidden) status code indicates that the server understood the request but refuses to fulfill it. A server that wishes to make public why the request has been forbidden can describe that reason in the response content (if any).

If authentication credentials were provided in the request, the server considers them insufficient to grant access. The client SHOULD NOT automatically repeat the request with the same credentials. The client MAY repeat the request with new or different credentials. However, a request might be forbidden for reasons unrelated to the credentials."

(in Case 4, it is indeed a request forbidden for reasons unrelated to credentials).

@jlurien
Copy link
Contributor

jlurien commented Apr 9, 2024

Hi @bigludo7 -

For Case 1, has 401 Unauthorized been considered? Per RFC 9110:

"If the request included authentication credentials, then the 401 response indicates that authorization has been refused for those credentials. The user agent MAY repeat the request with a new or replaced Authorization header field."

For Case 4, as touched upon earlier, 501 means "Not Implemented" for any resource, but Case 4 relates to not supporting certain resources.

Per RFC 9110:

"The 501 (Not Implemented) status code indicates that the server does not support the functionality required to fulfill the request. This is the appropriate response when the server does not recognize the request method and is not capable of supporting it for any resource."

I think a more appropriate code would be 403 Forbidden,

"The 403 (Forbidden) status code indicates that the server understood the request but refuses to fulfill it. A server that wishes to make public why the request has been forbidden can describe that reason in the response content (if any).
If authentication credentials were provided in the request, the server considers them insufficient to grant access. The client SHOULD NOT automatically repeat the request with the same credentials. The client MAY repeat the request with new or different credentials. However, a request might be forbidden for reasons unrelated to the credentials."

(in Case 4, it is indeed a request forbidden for reasons unrelated to credentials).

For case 1 I think that error is 400. 401 is to be used in authorization phase, while validating the Authorization header and access token
For case 5, I agree that 501 may not be the right one, when the operation is implemented but the problem is with the value in the request. 403 may be a valid one, as we assume that device in the request must be coherent with the device identified in the access token, and in this case the client would probably need to repeat the request with a different set of credentials. Another possibility is to use also 422 for this, which is more generic (the syntax of the request content is correct, but the was unable to process the contained instructions, because that device is not supported). Independently of the status code, I think we should use a dedicated code to identify this scenario precisely.

@jlurien
Copy link
Contributor

jlurien commented Apr 9, 2024

Summarizing the discussions in the thread above, I have tried to create a new proposal:

Considerations

  • Assuming schema is common for all errors (current agreement). If CAMARA is to enforce certain common status and code values for identified scenarios we'd have to adapt schema per error scenario or keep documentation as example, but remark that the values in examples are normative. Test plans should validate that the normative values are implemented.
    • For some cases, generic codes can be used (i.e. INVALID_ARGUMENT, but for others it is convenient to specify a dedicated one, to identify a very specific error case affecting the device)
  • Assuming current status-quo where several optional device identifiers can be provided and not all of them must be supported by the implementation. There is a separate issue #171 created to discuss simplifications of the device object, which may simplify as well the error logic here.
  • Cases order reflects the validation order and errors precedence, so I have changed the numbers of cases used above.
    • Validation of schema is not specific for device object. Implementations must validate the schema even for identifier properties that are not supported.
    • If several supported identifiers are provided, all of them must be resolved to a device by the implementation
    • For case 5, two options are considered (403 or 422) but we have to choose one.

Summary

Case # case_key validation_description status_value code_value message_example
0 InvalidArgument The request body does not comply with the schema 400 INVALID_ARGUMENT device does not comply with the schema
1 UnsupportedDeviceIdentifiers None of the provided device identifiers is supported by the implementation 422 UNSUPPORTED_DEVICE_IDENTIFIERS phoneNumber is required
2 DeviceIdentifierNotFound Some identifier cannot be matched to a device 404 DEVICE_NOT_FOUND Device identifier not found
3 InconsistentDeviceIdentifiers Device identifiers mismatch 422 DEVICE_IDENTIFIERS_MISMATCH Provided device identifiers are not consistent
4 InvalidTokenContext Invalid access token context 403 INVALID_TOKEN_CONTEXT Device identifiers are not consistent with access token
5 DeviceNotSupported Service not applicable to the device 403 or 422 DEVICE_NOT_APPLICABLE The service is not available for the provided device

Templates

response template

A response will group all examples for same operation and status code
Schema is common for all errors

description: |
  The examples section includes the list of subcases for this status error code to be implemented. In each example `status` and `code` are normative for the specific error case. `message` may be adjusted or localized by the implementation.
headers: 
  {{response_headers}}
content:
  application/json:
    schema:
      $ref: "#/components/schemas/ErrorInfo"
    examples:
      {{case_key_1}}:
        $ref: ""#/components/examples/{{case_key_1}}"
      {{case_key_2}}:
        $ref: ""#/components/examples/{{case_key_2}}"      

example template

One case will be needed per row in the table above, following this template:

components:
  examples:
    {{case_key}}:
      summary: {{validation_description}}
      value:
        status: {{status_value}}
        code: {{code_value}}
        message: {{message_example}}

@gmuratk
Copy link
Collaborator

gmuratk commented Apr 9, 2024

just looking for guidance from the group...

Which of the above cases, if any, will be applicable for the case where the device schema is compliant, provider supports the parameters submitted, and identifier(s) match a subscribed device, but it just so happens that device is not online, and the requested operation relies on session information.

An example use case was encountered in a multi-operator Device Status activity, where the queried device (e.g. for roaming) was not online, and the involved operators had differing opinions on what response to send back.

Given this is a thread for Commonalities, so maybe I should expect each API handle their special cases separately, but documenting clearly and stating a normative way so developers get common behavior across different API providers?

@jlurien
Copy link
Contributor

jlurien commented Apr 9, 2024

just looking for guidance from the group...

Which of the above cases, if any, will be applicable for the case where the device schema is compliant, provider supports the parameters submitted, and identifier(s) match a subscribed device, but it just so happens that device is not online, and the requested operation relies on session information.

If all the validations that are being discussed here are passed, it is probably a different case not related with device identification.

An example use case was encountered in a multi-operator Device Status activity, where the queried device (e.g. for roaming) was not online, and the involved operators had differing opinions on what response to send back.

I think that kind of scenarios have to be treated at Subproject level. In the example above it is not even clear that it has to be considered an error, or it should be covered in a 2xx response with some dedicated property.

Given this is a thread for Commonalities, so maybe I should expect each API handle their special cases separately, but documenting clearly and stating a normative way so developers get common behavior across different API providers?

I would expect that as well, specially for scenarios closely related to the API logic

@bigludo7
Copy link
Collaborator Author

bigludo7 commented Apr 9, 2024

Thanks for all comments & @jlurien for the proposal

for case 1 None of the provided device identifiers is supported by the implementation to be sure ... suppose your implem only manage phoneNumber. You get a request with a phoneNumber and an Ipv4 --> no 422 here but we use the phoneNumber without considering the Ip, right?

For case 5 I have preference for 422. Indeed we have UC where the /authorize is done for a device to the AS, Authorization performed and then when the API request is done with the token the BE is not able to provide the information for this particular device (token info matches the payload). I will keep 403 when we have token mismatch.

@jlurien
Copy link
Contributor

jlurien commented Apr 12, 2024

Thanks for all comments & @jlurien for the proposal

for case 1 None of the provided device identifiers is supported by the implementation to be sure ... suppose your implem only manage phoneNumber. You get a request with a phoneNumber and an Ipv4 --> no 422 here but we use the phoneNumber without considering the Ip, right?

Yes

For case 5 I have preference for 422. Indeed we have UC where the /authorize is done for a device to the AS, Authorization performed and then when the API request is done with the token the BE is not able to provide the information for this particular device (token info matches the payload). I will keep 403 when we have token mismatch.

I think it is important to agree also on relevant codes, because if we use 422 for different error cases, it would not be easy for developers to distinguish cases otherwise

@bigludo7
Copy link
Collaborator Author

Thanks @jlurien for the clarification

I think it is important to agree also on relevant codes, because if we use 422 for different error cases, it would not be easy for developers to distinguish cases otherwise

Agree. I'm fine with your code proposal in table above to distinguish case.

@mhfoo
Copy link
Collaborator

mhfoo commented Apr 16, 2024

@jlurien

For case 5, two options are considered (403 or 422) but we have to choose one.

Is the above referring to #138?

This is a server side error which does not support the method and the client has no way to "correct" it.

See 400 vs 500

@hdamker
Copy link
Collaborator

hdamker commented May 4, 2024

@mhfoo

This is a server side error which does not support the method and the client has no way to "correct" it.

See 400 vs 500

I don't agree with your interpretation. It is actually a semantic error which only the client can "correct" by providing a different phone number.

To say it with the words of ChatGPT:

... in the scenario you described, where the server understands the request and can process it but chooses not to because of specific conditions (such as the service not being available for a certain type of phone number), 501 would not accurately convey the situation. The server does have the capability to process the request, but it chooses not to due to the nature of the phone number provided.

Therefore, using 501 would likely be misleading to the client, as it implies a limitation on the server's capabilities rather than a specific condition related to the request data. Using a status code like 422 Unprocessable Entity or possibly 403 Forbidden would be more appropriate in this case.

Also your reference 400 vs 500 discourage the use of 501, as it states:

  1. Never return 500 errors intentionally. The only type of errors you should be showing to the user intentionally is validation (400) errors. 500 codes are all about something you don’t anticipate to happen.

That a phone number can't be supported is anticipated by the server implementation (e.g. landline number for a service which applies only for mobile numbers). The user can correct it by providing another - supported - phone number.

BTW: I vote for 422 over 403.

@eric-murray
Copy link
Collaborator

@mhfoo

This is a server side error which does not support the method and the client has no way to "correct" it.

See 400 vs 500

I think 501 is only appropriate if the functionality will (or can) be implemented at some point in the future. For scenarios where the requested phone number does not and never will make sense (e.g. requesting SIM Swap status for a landline) and so will never be implemented, a 4XX error is more appropriate. My preference is also 422 in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.