Skip to content

cleaning up repo#51

Closed
DT-DawidWroblewski wants to merge 0 commit intocamaraproject:mainfrom
DT-DawidWroblewski:main
Closed

cleaning up repo#51
DT-DawidWroblewski wants to merge 0 commit intocamaraproject:mainfrom
DT-DawidWroblewski:main

Conversation

@DT-DawidWroblewski
Copy link
Collaborator

@DT-DawidWroblewski DT-DawidWroblewski commented Jul 26, 2023

What type of PR is this?

  • cleanup

What this PR does / why we need it:

clean up repo

Which issue(s) this PR fixes:

Fixes #35
Fixes #29
Fixes #28

Special notes for reviewers:

cleans up repo

Changelog input

removes MobileConnect definition
creates CAMARA reference to Mobile Connect

Additional documentation

Copy link
Contributor

@diegogonmar diegogonmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the cleanup!
Some additional comments:

  • Mobile Connect files should be removed, not moved to archive. It's misleading to a reader to have a reference and then static content, more if archived info is from something not defined here but in a different standard. It would be also problematic to have obsolete information.
  • Regarding references, although I proposed alternative wording, I still think that info does not help at all developers and may distract them. In any case reference should be only in .md, not in yaml.

Also some extra cleanup is needed:
This file should be removed -->
https://github.com/camaraproject/NumberVerification/blob/main/code/API_definitions/README.MD
This folder is no longer needed, adds unneded folder hierarchy level: https://github.com/DT-DawidWroblewski/NumberVerification/tree/main/documentation/API_documentation/CAMARA
(info in the folder should be moved to upper level and empty folder removed)
This folder should be removed --> https://github.com/DT-DawidWroblewski/NumberVerification/tree/main/documentation/API_documentation/assets

description: |
Service Enabling Network Function API to verify that the provided **mobile phone number** is the one used in the device. It verifies that the user is using a device with the same *mobile phone number* as it is declared.

This API derives from the GSMA Mobile Connect Account Takeover Protection specification [Mobile Connect Number Verify](https://www.gsma.com/identity/wp-content/uploads/2022/12/IDY.54-Mobile-Connect-Verified-MSISDN-Definition-and-Technical-Requirements-1.0.pdf). For more about Mobile Connect, please see [about Mobile Connect](https://mobileconnect.io/).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yaml should not include this info. This info should be in the .md files within API documentation. Yaml is API spec and has to stick to API Definition

Copy link
Collaborator

@bigludo7 bigludo7 Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diegogonmar Up to my knowledge we decided to not have anymore md file and embed documentation in the yaml.

@DT-DawidWroblewski @diegogonmar For the wording I tend to prefer the one proposed by Diego "GSMA Mobile Connect Account Takeover Protection specification Mobile Connect Number Verify](https://www.gsma.com/identity/wp-content/uploads/2022/12/IDY.54-Mobile-Connect-Verified-MSISDN-Definition-and-Technical-Requirements-1.0.pdf).) was used as source of input for this API. For more about Mobile Connect, please see about Mobile Connect." which look to me less tight coupled than derived from but not a blocker for me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right @bigludo7, I didn't realize that. Suggestion then is to include the reference in the .yaml plus remove rest of documentation, meeting commonalities agreement


Number Verification API is used to verify that the provided **mobile phone number** is the one used in the device. It verifies that the user is using a device with the same *mobile phone number* as it is declared.
It also makes it possible for a Service provider to verify the number itself by returning the phone number associated to the authenticated user's access token.
It also makes it possible for a Service provider to verify the number itself by returning the phone number associated to the authenticated user's access token. This API derives from the GSMA Mobile Connect Account Takeover Protection specification [Mobile Connect Number Verify](https://www.gsma.com/identity/wp-content/uploads/2022/12/IDY.54-Mobile-Connect-Verified-MSISDN-Definition-and-Technical-Requirements-1.0.pdf). For more about Mobile Connect, please see [about Mobile Connect](https://mobileconnect.io/).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to be precise. It's not true that this API was derived from MC.
I propose this wording, alligned with Markus suggestion here (camaraproject/WorkingGroups#200 (comment)) reflecting an existing discussion and agreement.

GSMA Mobile Connect Account Takeover Protection specification Mobile Connect Number Verify](https://www.gsma.com/identity/wp-content/uploads/2022/12/IDY.54-Mobile-Connect-Verified-MSISDN-Definition-and-Technical-Requirements-1.0.pdf).) was used as source of input for this API. For more about Mobile Connect, please see about Mobile Connect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also please use references as in the rest of the document, pointing to a references chapter at the end of the doc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not true. Camara Number Verification API completely derives from "Number Verify" (www.numberverify.org explicitly stated on various presentaitons) that works on Mobile Connect Verified MSISDN spec.

|No|Version|Changelog|
|:---:|---:|:---|
|1|0.1.0|Camara & Mobile Connect flavours available|
|2|0.2.0|Camara only version<br>(Mobile Connect archived)|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
|2|0.2.0|Camara only version<br>(Mobile Connect archived)|
|2|0.2.0|Camara only version<br>(Mobile Connect removed)|


1. Based on CAMARA standardization guidelines, that enables API service on a dedicated endpoint and follows CAMARA AuthN/AuthZ concept. [More details available here](#details)
2. Based on GSMA Mobile Connect standardized family of Identity APIs, that delivers Verified MSISDN service. [More details available here](#details)
CAMARA Number Verify API aims to deliver proof of device possession, that is attached to SIM card and MSISDN, using mobile network authentication. This API derives from the GSMA Mobile Connect Account Takeover Protection specification [Mobile Connect Number Verify](https://www.gsma.com/identity/wp-content/uploads/2022/12/IDY.54-Mobile-Connect-Verified-MSISDN-Definition-and-Technical-Requirements-1.0.pdf). For more about Mobile Connect, please see [about Mobile Connect](https://mobileconnect.io/).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above regarding proposed re-wording

@diegogonmar
Copy link
Contributor

@DT-DawidWroblewski happy to help, if you're ok I can upload a commit with the extra cleanup and my comments

bigludo7
bigludo7 previously approved these changes Aug 16, 2023
Copy link
Collaborator

@bigludo7 bigludo7 Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we decided to embed directly the API documentation in the yaml I'm not sure we need to keep this file. It should be removed

description: |
Service Enabling Network Function API to verify that the provided **mobile phone number** is the one used in the device. It verifies that the user is using a device with the same *mobile phone number* as it is declared.

This API derives from the GSMA Mobile Connect Account Takeover Protection specification [Mobile Connect Number Verify](https://www.gsma.com/identity/wp-content/uploads/2022/12/IDY.54-Mobile-Connect-Verified-MSISDN-Definition-and-Technical-Requirements-1.0.pdf). For more about Mobile Connect, please see [about Mobile Connect](https://mobileconnect.io/).
Copy link
Collaborator

@bigludo7 bigludo7 Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diegogonmar Up to my knowledge we decided to not have anymore md file and embed documentation in the yaml.

@DT-DawidWroblewski @diegogonmar For the wording I tend to prefer the one proposed by Diego "GSMA Mobile Connect Account Takeover Protection specification Mobile Connect Number Verify](https://www.gsma.com/identity/wp-content/uploads/2022/12/IDY.54-Mobile-Connect-Verified-MSISDN-Definition-and-Technical-Requirements-1.0.pdf).) was used as source of input for this API. For more about Mobile Connect, please see about Mobile Connect." which look to me less tight coupled than derived from but not a blocker for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MC solution and guideline alignment sub attribute meaning & cardinality Question about compliance with IDY.54 for MC version

3 participants