camaraproject / SimSwap

Repository to describe, develop, document and test the Sim Swap API family
https://wiki.camaraproject.org/x/DgSeAQ
Apache License 2.0
23 stars 23 forks source link

Add MonitoredPeriod into the API Response if a Telco cannot show the date / data for for Privacy Reasons #124

Open QaunainM opened 4 months ago

QaunainM commented 4 months ago

Problem / Context

To Reproduce

Expected behavior As the restriction on the data being sent back is due to the Telcos privacy reasons, it seems it would be better for:

QaunainM commented 4 months ago

As a lot of these Network APIs will have privacy concerns depending on the consent that has been captured or based on the historical duration of the event that the developer is trying to retrieve, I think CAMARA should add the 451 error code to its standard.

451 Unavailable For Legal Reasons (RFC 7725) A server operator has received a legal demand to deny access to a resource or to a set of resources that includes the requested resource.

bigludo7 commented 3 months ago

Hi @QaunainM This is a fair point. As of now I was expecting the API to send a 400 error for this case (there is discussion for this test case here #70 ).

Regarding use of 451, as it should be considered globally within all CAMARA API, and 451 is not defined in our design document may I ask you to trigger an issue in Commonalities?

QaunainM commented 3 months ago

@bigludo7, thanks I raised it in the Commonalities and made the description more holistic as it will cover all or most CAMARA APIs https://github.com/camaraproject/Commonalities/issues/258

jgarciahospital commented 3 months ago

Hi both,

For this specific case, bear in mind that the decision of including a 200 ok with empty body was concluded because that scenario is indeed providing valuable information to the developer.

If the telco is saying that it cannot provide the exact date of the last Sim Swap because it occurred more than X months ago, the telco is saying exactly this, that the last Sim Swap occurred more than X months ago. No application would need to use the check endpoint with maxage=xMonths, and telcos will be providing useful, valuable (and private) information with an error response.

That is mainly why it was decided to maintain 200 ok with empty body, to avoid this strange situation.

hdamker commented 3 months ago

I don't think we should use the 451 here. It was introduced to make the blocking of content transparent (hence the number from "Fahrenheit 451") and was in addition used (according to the linked Wikipedia article) "because the user's country adds regulatory requirements that the publisher refuses to comply with [e.g. GDPR]". If we would use it now in cases where the resource provider complies with privacy regulations, it would be kind of ironic. In addition the API client couldn't do anything about it ... with 451 there is at least the option to work around with VPNs, TOR etc if the error was returned by an intermediate.

I see two cases concrete for SimSwap:

Another reason not to return an error: for the majority of /retrieveDate requests the last sim swap date will be outside of the time window ... so the API would generate mainly error responses. Not a nice case for colleagues monitoring the operation ;-)

bigludo7 commented 3 months ago

Thanks @jgarciahospital for the refresh ! I've forgot this one - sorry. Indeed we have a discussion in the past and retrieved it: https://github.com/camaraproject/SimSwap/issues/16

I should confess that I liked a lot the proposal by @gregory1g to add a monitoredPeriod attribute but not sure why we did not select this solution. I saw that @hdamker proposed something similar.

QaunainM commented 3 months ago

Hi all, thanks for the context, I read through the comments on #16 , if we focus purely on /retrieve-date

Having something like this as suggested in the previous issue will give the application developers a way of knowing that the null was because there was a SIM swap but it can't be shown as the Telco only returns data for the monitoredPeriod of days, each Telco will have a different monitoredPeriod. Then it also makes it more understandable as to why they are being charged.

{ latestSimChange = null
monitoredPeriod = 90 # days }
gregory1g commented 3 months ago

@bigludo7 If I recall correctly the reason to skip monitoredPeriod was something like: during onboarding an ASP signs terms and conditions where this monitoredPeriod is mentioned.

However, it looks like the current solution is a source of confusions and possible misunderstandings - 200+null response really hard to understand without digging deep into SimSwap anti-fraud purpose and simswap attack techniques.

Moreover, here we discuss GDPR driven limitation, but someone can implement such a limitation purely based on technical reasons: there is no need to support 20 years of simswap history to support anti-simswap attach which has attack time window of few days at most.

Therefore, it could make sense to reconsider "monitoredPeriod" attribute in the response again. The impact on the API and the implementations should be very small.

QaunainM commented 3 months ago

Do we need to make a new issue in this Project or the Commonalities project regarding the monitoredPeriod idea?

bigludo7 commented 3 months ago

Hello @QaunainM We mentioned this topic during the OTP/SIM Swap/Number verification call yesterday. The idea to rethink introduction of monitoredPeriod was welcomed from my understanding. Probably we could introduce it once the rush for the meta release will be over.

Perhaps we can just edit the title of this issue to a more generic form "What to do when the SIM Swap date is greater than the max historic days that the Telco allows"?

AxelNennker commented 3 months ago

I am not the biggest fan of "that the Telco allows", because I think we should avoid that one telco allows 90 days and another 30.

Is it possible to write: "What to do when the SIM Swap date is greater than the max historic days that the Camara Terms and Conditions allows"?

QaunainM commented 3 months ago

Hi Axel, the issue I've seen is that each Telco has different max historic days they want to abide by, so its not a standardized number

gregory1g commented 3 months ago

for the "check" API we define maxAge upper limit to 2400 hours (aka 100 days) on the camara API level - so everyone is expected to support it independent on GDPR (at least I do not see any statement saying that local regulations can be applied on top of schema).

Why cannot we define the same "common" limit to retrieve-date API? This will make it both APIs logically consistent. Consistency is even more important considering the fact that APIs are rather interchangeable: one can use check API to detect rather exact swap date using binary search method. Therefore, if retrieve-date is limited to 30 days, one still can find out swap date back to 100 days using "check" API and binary search.

Or, if we expect that local regulation can reduce /check monitoring window to 60 or 30 days - it makes sense to describe this option explicitly and apply the same logic for both APIs

ToshiWakayama-KDDI commented 3 months ago

Hi @QaunainM , @bigludo7 , @gregory1g , @hdamker , @AxelNennker ,

Thank you for this discussion. I support the idea monitoredPeriod or maxTimeWindow, because it seems each telco has different maxAge limitation for whatever reasons. In the first place, I am not quite sure how come the current maxAge limitation of 100 days was defined.

Anyway, I understand monitoredPeriod or maxTimeWindow will not be available for Fall24 Meta-Release, so, is it correct understanding that a Telco is supposed to reply Error 400 when the maxAge requested in the request body is larger than the telco supports? I am not quite sure of it, as I have not been involvd in this discussion and the test case discussion PR #70.

Many thanks, Toshi

QaunainM commented 3 months ago

@ToshiWakayama-KDDI @gregory1g and all, it sounds like there is consensus on adding in a monitoredPeriodor maxTimeWindow. How do we get this onto the agenda so it can be added into an upcoming version/release.

ToshiWakayama-KDDI commented 3 months ago

Thank you, @QaunainM.

Sorry, I am a bit confused. I am talking about 'Check SIM Swap' API's maxAge parameter, but am I correct?

Assuimng I am correct, I discussed this internally with our product team, and now we have the following opinions - in general, it is better not to rush :

Best regards, Toshi

hdamker commented 3 months ago

@ToshiWakayama-KDDI I think there is confusion about two different discussions here in the issue (using the term "monitoredPeriod" here):

So both proposals make sense to me:

hdamker commented 3 months ago

Is it possible to write: "What to do when the SIM Swap date is greater than the max historic days that the Camara Terms and Conditions allows"?

@AxelNennker there is no such thing like "Camara Terms and Conditions", we define only the technical interfaces. T&C have to be agreed between the API Provider and its customer. Please be aware that an API Provider can be an operator or an aggregator who itself is a customer of multiple operators.

But see my previous comment ... we should already technically define the API in a way, that all API Providers can response to valid requests (if they are offering the API at all).

AxelNennker commented 3 months ago

Is it possible to write: "What to do when the SIM Swap date is greater than the max historic days that the Camara Terms and Conditions allows"?

@AxelNennker there is no such thing like "Camara Terms and Conditions", we define only the technical interfaces. T&C have to be agreed between the API Provider and its customer. Please be aware that an API Provider can be an operator or an aggregator who itself is a customer of multiple operators.

But see my previous comment ... we should already technically define the API in a way, that all API Providers can response to valid requests (if they are offering the API at all).

I wanted to get rid of "that the Telco allows", because this issue is not what one Telco should decide, I think. My hope is that for all telcos that fall under same jurisdiction, the value for max historic days is the same.

Preferably, there is one global constant camara_max_historic_days.

I think it is undesirable if an API consumer calls the SimSwap API for subscription1 by telco1 and gets an error due to max historic day, but if they call the API for subscription2 by telco2 they do not get an error. Or an error for a different request.

QaunainM commented 3 months ago

Currently some countries, i.e. Germany make their rules so it can be interpreted not to disclose SIM Swap data greater than 30 days, but other countries don't don't have that limit and can offer a greater duration, so unfortunately there is a disparity between nations.

bigludo7 commented 2 months ago

Is there still any blocker to introduce a monitorPeriod attribute as an optional attribute in the response of the retrieveSimSwapDate operation for next version of this API (not for this meta release)?

Rules proposal:

gregory1g commented 2 months ago
  • If operator cannot provide latest sim swap change, the latestSimChange is not present but the operator confirm than no sim swap occurred during its monitored period, by valuing monitorPeriod in the response.

I would suggest latestSimChange=null + monitorPeriod - this way monitorPeriod will be pure extension of the current response syntax and semantic.

bigludo7 commented 2 months ago

Agree @gregory1g

Rules proposal:

hdamker commented 2 months ago
  • Note: An empty response 200 is still allowed to avoid breaking change (I'm not sure about this latest rule)

The current spec allows indeed to return an empty response -- but the description does not define in which case that is allowed. And: I think making an optional response parameter mandatory wouldn't be a breaking change for the API Client, so it can be done.

QaunainM commented 2 months ago

I'm just thinking about the optional vs mandatory scenario. If this is not going to be implemented until after September a part of a a new API version, would the CSP / Telco know that one condition of using that updated version is to include this monitorPeriod when there is no latestSimChange value?

bigludo7 commented 2 months ago

Seems that we are aligned on the rationale to add this attribute. Now we are in the 'tech' design. If no objection I guess we can initialize a PR to discuss on code. @QaunainM do you want to initialize this PR of prefer I did it? In any case I suggest we wait merge of the current release before to contribute any code (the merge release should be before this week end).

QaunainM commented 2 months ago

Great news, if you don't mind, it would be great if you initialized the PR and process

bigludo7 commented 2 months ago

Thanks @QaunainM for your answer. We'll see tomorrow during our call if we all share that we're ready to work on the PR and if yes I will initialize it.