Sim swap subscriptions alignement with commonalities 0.5 #179

bigludo7 · 2024-12-23T10:43:51Z

What type of PR is this?

Add one of the following kinds:

enhancement/feature

What this PR does / why we need it:

Alignement of sim_swap-subscriptions.yaml & sim_swap-subscriptions.feature with commonalities 0.5 rules.
See detail in release note.

Which issue(s) this PR fixes:

Fixes #161

Special notes for reviewers:

This is breaking change.

Changelog input

 release-note
sim_swap-subscriptions.yaml:
- Change error structure definition to normalize error & status
- Update error code to introduce 429 & 422 - removed 5xx, 410 and 415 error
- Follow principle of data minimization by removing sinkCredential in POST & GET response
- Adapt request for 3-legs usage featuring identification of the phoneNumber
- Improve documentation part
- Add 3-legs examples

sim_swap-subscriptions.feature:
- Align test scenario with commonalities 0.5 improvements

Additional documentation

This section can be blank.

docs

Align sim-swap-subscriptions.yaml with comm 0.5

Align sim-swap-subscriptions.feature with comm 0.5

github-actions · 2024-12-23T10:44:59Z

🦙 MegaLinter status: ✅ SUCCESS

Descriptor	Linter	Files	Errors	Elapsed time
✅ ACTION	actionlint	2	0	0.02s
✅ OPENAPI	spectral	2	0	3.2s
✅ REPOSITORY	git_diff	yes	no	0.01s
✅ REPOSITORY	secretlint	yes	no	0.8s
✅ YAML	yamllint	2	0	0.58s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by

Mega-Linter - remove trailing spaces

Try to fix examples for subscriptionDetail

bigludo7 · 2024-12-23T11:02:28Z

Hello @rartych
I will need your help for the megalinter issue.

Megalinter is unhappy because in the 3LEGS examples the subscriptionDetail is null.
This attribute is mandatory but in these examples we will have an empty object.

I tried
subscriptionDetail: null
or
subscriptionDetail:

but neither work. Any idea?

Thanks for you help.

code/API_definitions/sim-swap-subscriptions.yaml

Co-authored-by: Rafal Artych <[email protected]>

gregory1g · 2025-01-06T06:56:36Z

code/API_definitions/sim-swap-subscriptions.yaml

@@ -1194,3 +1351,17 @@ components:
          startsAt: '2024-06-07T16:10:15.302Z'
          expiresAt: '2024-06-07T16:10:15.302Z'
          status: ACTIVATION_REQUESTED
+    SUBSCRIPTIONS_3LEGS:


How one is expected to use a 3-legged token to read all subscriptions? A 3-legged token is valid for a single phon number only.

Therefore,

either the response must include only subscription for this exactly phone number, what creates an implicit filtering functionality AND reading subscription without specifying an ID must also support 2-legged token to really read all subscriptions

or reading subscription without specifying an ID must support 2-legged auth only .

This is a good comment. I propose to update the documentation part:

These rules apply for subscription:

If 3-legged access token is used:

For POST and GET{id} request & responses must not provide any device identifier phoneNumber.

For GET (list) only subscription for this exactly phone number is retrieved (implicit filtering)

If 2-legged access token is used:

For POST and GET{id} the presence of a device identifier phoneNumber in the request t& response is mandatory.

For GET (list) the phoneNumber could be used as filtering criteria.

WDYT?

Makes sense for me.
But

for usual operation it is up to the MNO to decide if it wants/can support 3-legged or 2-legged option.

meanwhile for GET (list) MNO must always support client credentials (with or without 3-legged option in addition), because otherwise this API is hardly usable - one cannot get a complete list of subscription it has created.

I got your point for the GET (list) but I'm wondering if we have to strictly enforce that MNO must support client credential. I will prefer to keep it as a recommandation and probably waiting for live implementation to get feedbacks. If we enforce it now it will be tough from a certification perspective.

gregory1g · 2025-01-06T07:00:43Z

code/Test_definitions/sim-swap-subscriptions.feature

@@ -63,7 +63,7 @@ Feature: CAMARA sim swap subscriptions  API, v0.1.1
  Scenario: Check existing subscription is retreived in list
    Given valid subscriptions are existing
    And use BaseURL
-    When get sim swap subscription
+    When the HTTP "GET" request is sent


This PQ makes response depending on the token type. As a result statement "And the response body complies with an array of OAS schema defined at "#/components/schemas/Subscription"" does not describe the expected result clearly enough anymore. Separate test cases are needed for 2 and 3 legged requests.

And for 3-legged case one need to specify which exactly subscriptions must be returned.

Yes make sense
I've reworked the proposal with @sim_swap_subscription_retrieve_04_retrieve_list_2legs, @sim_swap_subscription_retrieve_07_retrieve_list_3legs and @sim_swap_subscription_retrieve_08_retrieve_empty_list_3legs

Specify distinct TC for GET (list) in 2 or 3 legs

Clarify in API presence of phoneNumber in 2 or 3 legs

bigludo7 · 2025-01-09T07:25:20Z

@fernandopradocabrillo @gregory1g @rartych Following camaraproject/Commonalities#369 discussion I think it's fair to remove 403 INVALID_TOKEN_CONTEXT example in the yaml.

May I ask you to review this new proposal soon as we have to prepare the RC candidate before Jan17th. We can still modify after but we have to achieve the milestone.

gregory1g · 2025-01-13T06:56:50Z

code/API_definitions/sim-swap-subscriptions.yaml

+      #### phoneNumber presence:
+      These rules apply for subscription:
+
+      - If 3-legged access token is used, the POST and GET{id} requests & responses must not provide any device identifier (``phoneNumber``). For GET (list) only subscription(s) for this exactly phone number is/are retrieved (implicit filtering).


"any device identifier": what other device identifiers are supported and again an API targets subscription (MISDN), not a device. Even if the API would accept device ID in input, it will have to find target MSISDN using provided Device ID first.

Agreed. I will fix it.

gregory1g · 2025-01-13T07:11:28Z

code/API_definitions/sim-swap-subscriptions.yaml

+      These rules apply for subscription:
+
+      - If 3-legged access token is used, the POST and GET{id} requests & responses must not provide any device identifier (``phoneNumber``). For GET (list) only subscription(s) for this exactly phone number is/are retrieved (implicit filtering).
+      - If 2-legged access token is used, for POST and GET{id}, the presence of a device identifier (``phoneNumber``) in the request & response is mandatory. For GET (list) the device identifier (``phoneNumber``) could be used as filtering criteria.


This is a very short description and it keeps some questions opened.
for example:

it says that one can use phoneNumber filtering criteria. But the GET "/subscriptions" specification does not support filtering.

what GET subscription returns if one does not use filtering criteria in 2-legged token case?

what happens if one uses 3-legged token for "GET /subscriptions/{subscriptionId}", where the target subscription which is not associated with a phoneNumber from the token?

what happens if one tries to use 2-legged token towards a subscription created by another API invoker?

@gregory1g Fair questions. We have added a piece in commonalities here to target your questions 2. 3. & 4.
For question 1. this should be here.

As these questions are not specific to this API I suggest to park them to block the merging.

gregory1g · 2025-01-13T07:15:59Z

code/Test_definitions/sim-swap-subscriptions.feature

    Then the response property "$.status" is 200
    And the response header "Content-Type" is "application/json"
    And the response header "x-correlator" has same value as the request header "x-correlator"
    And the response body complies with an array of OAS schema defined at "#/components/schemas/Subscription"
    And all valid subscriptions are listed

+  @sim_swap_subscription_retrieve_07_retrieve_list_3legs
+  Scenario: Check existing subscriptions are retrieved in list
+    Given valid subscription is existing for a phoneNumber


How "valid subscription" is defined and how one can list "not valid" subscription to, for example, delete them?

Yes this 'valid' wording is not defined and could be confusing as we are not supposed to store/manage not valid subscription. I removed this unnecessary valid wording & make some ajustements in the wording.

@gregory1g

As suggested by @gregory1g

fernandopradocabrillo · 2025-01-17T12:49:03Z

code/Test_definitions/sim-swap-subscriptions.feature

@@ -306,39 +331,24 @@ Feature: CAMARA sim swap subscriptions  API, v0.1.1
 # Error Code 403
 ##################

-  @sim_swap_subscription_creation_60_phone_number_token_mismatch
+  @sim_swap_subscription_retrieval_61_phone_number_token_mismatch


@bigludo7 I checked with Pedro regarding the SUBSCRIPTION_MISMATCH error because I wasn't fully sure if were right about the meaning. And spoiler: we weren't.
So this error is designed to cover the scenario where an API Client requests the creation of an event subscription for an event type for which it does not have permission. The token is valid and the client can consume the endpoint, but only for the events that were requested in the authentication.

Maybe with an example for device status this is clearer. There we have a scope for roaming on and another one for roaming off. If the token was generated for roaming on and the client requests the creation of a subscription for roaming off, there we should return the SUBSCRIPTION_MISMATCH error.

And this is why it is only present in the POST for creation

@fernandopradocabrillo Whaou got it ! now I remind ! - Kudos to Pedro
So I remove the tests for GET & DELETE
I'm not sure we need a Test for POST as we have only one event type.

bigludo7 added 2 commits December 23, 2024 11:33

Align sim-swap-subscriptions.yaml with comm 0.5

9aeb8a6

Align sim-swap-subscriptions.yaml with comm 0.5

Align sim-swap-subscriptions.feature with comm 0.5

672e3b3

Align sim-swap-subscriptions.feature with comm 0.5

bigludo7 requested a review from fernandopradocabrillo as a code owner December 23, 2024 10:43

bigludo7 added 2 commits December 23, 2024 11:51

Mega-Linter - remove trailing spaces

c023094

Mega-Linter - remove trailing spaces

Try to fix examples for subscriptionDetail

064e096

Try to fix examples for subscriptionDetail

rartych reviewed Dec 23, 2024

View reviewed changes

code/API_definitions/sim-swap-subscriptions.yaml Outdated Show resolved Hide resolved

rartych reviewed Dec 23, 2024

View reviewed changes

code/API_definitions/sim-swap-subscriptions.yaml Outdated Show resolved Hide resolved

rartych reviewed Dec 23, 2024

View reviewed changes

code/API_definitions/sim-swap-subscriptions.yaml Outdated Show resolved Hide resolved

bigludo7 and others added 3 commits December 23, 2024 14:26

Update code/API_definitions/sim-swap-subscriptions.yaml

2f7393d

Co-authored-by: Rafal Artych <[email protected]>

Update code/API_definitions/sim-swap-subscriptions.yaml

d11cf95

Co-authored-by: Rafal Artych <[email protected]>

Update code/API_definitions/sim-swap-subscriptions.yaml

3ebe458

Co-authored-by: Rafal Artych <[email protected]>

gregory1g reviewed Jan 6, 2025

View reviewed changes

bigludo7 added 4 commits January 6, 2025 13:59

Specify distinct TC for GET (list) in 2 or 3 legs

bad275d

Specify distinct TC for GET (list) in 2 or 3 legs

Clarify in API presence of phoneNumber in 2 or 3 legs

d172cf0

Clarify in API presence of phoneNumber in 2 or 3 legs

Remove INVALID_TOKEN_CONTEXT example

9bf6966

Remove 403 INVALID_TOKEN_CONTEXT TC

788d040

update SUBSCRIPTION_MISMATCH TC description

65c636f

gregory1g reviewed Jan 13, 2025

View reviewed changes

bigludo7 added 2 commits January 15, 2025 10:32

Improved lines 54-55 documentation

f9dc420

As suggested by @gregory1g

Improve @sim_swap_subscription_retrieve_04_retrieve_list_2legs

81b5334

bigludo7 requested review from gregory1g and rartych January 15, 2025 09:43

fernandopradocabrillo reviewed Jan 17, 2025

View reviewed changes

Remove 403 SUBSCRIPTION_MISMATCH test cases

3aa3db9

fernandopradocabrillo approved these changes Jan 21, 2025

View reviewed changes

bigludo7 merged commit 77dde1f into main Jan 22, 2025
2 checks passed

bigludo7 deleted the sim-swap-sub-alig-comm0.5 branch January 22, 2025 08:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sim swap subscriptions alignement with commonalities 0.5 #179

Sim swap subscriptions alignement with commonalities 0.5 #179

bigludo7 commented Dec 23, 2024

github-actions bot commented Dec 23, 2024 •

edited

Loading

bigludo7 commented Dec 23, 2024

gregory1g Jan 6, 2025

bigludo7 Jan 6, 2025 •

edited

Loading

gregory1g Jan 6, 2025

bigludo7 Jan 6, 2025

gregory1g Jan 6, 2025

bigludo7 Jan 6, 2025

bigludo7 commented Jan 9, 2025

gregory1g Jan 13, 2025

bigludo7 Jan 15, 2025

bigludo7 Jan 15, 2025

gregory1g Jan 13, 2025

bigludo7 Jan 15, 2025

gregory1g Jan 13, 2025

bigludo7 Jan 15, 2025

fernandopradocabrillo Jan 17, 2025

bigludo7 Jan 17, 2025

Sim swap subscriptions alignement with commonalities 0.5 #179

Sim swap subscriptions alignement with commonalities 0.5 #179

Conversation

bigludo7 commented Dec 23, 2024

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for reviewers:

Changelog input

Additional documentation

github-actions bot commented Dec 23, 2024 • edited Loading

🦙 MegaLinter status: ✅ SUCCESS

bigludo7 commented Dec 23, 2024

Choose a reason for hiding this comment

bigludo7 Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bigludo7 commented Jan 9, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 23, 2024 •

edited

Loading

bigludo7 Jan 6, 2025 •

edited

Loading