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

Telefonica Proposal #3

Merged
merged 11 commits into from
Dec 20, 2022

Conversation

monamok
Copy link
Collaborator

@monamok monamok commented Nov 24, 2022

Telefónica Proposal based on the conversation here

@monamok monamok requested a review from bigludo7 as a code owner December 5, 2022 16:20
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

Hello
We have an API close to this proposal in Orange. 2 comments:

  1. Could we align the POST body with MC structure:
    { "mc_claims": { "device_msisdn": "+33640049871" } }
    (and I guess device_hashed_msisdn attribute for the second resource)
    It will not change the philosophy API and not add complexity but allow people familiar with this pattern to have it.

  2. We have in Orange another resource to retrieve directly the msisdn from the access token (GET operation). This is relevant for some UC - you do not have this requirement on your side ?

Thanks.

@DT-DawidWroblewski
Copy link
Collaborator

@bigludo7 for a resource call this is something that is common for vMSISDN, so we can align on a POST call to a resource that contains a body you mentioned:

{ "mc_claims": { "device_msisdn": "+33640049871" } }

for GET operation to extract MSISDN out of Access Token - this is one of the solutions available on a market. However, this call should take place during the device authentication process initiated with a request coming to /authorize endpoint. If MSISDN extraction is fine, then "code" is returned and we're following OIDC flow.

The reason why it should take place in auth process is thanks to this approach, we're hiding various means of extracting MSISDN out of...whatever (access token, specific token, HE). For the developer, this is handy, because there is a common flow (OpenID Connect) and MNOs hides complexity.

@monamok
Copy link
Collaborator Author

monamok commented Dec 7, 2022

Hello,

About the first comment:

1. Could we align the POST body with MC structure:
   `{   "mc_claims": { "device_msisdn": "+33640049871"   } }`
   (and I guess device_hashed_msisdn attribute for the second resource)
   It will not change the philosophy API and not add complexity but allow people familiar with this pattern to have it.

The reason why we prefer to separate them in two endpoints is to have control over the scopes and be able to manage the permissions separately. In order to have an intial version 0.X we can have them a single endpoint.
But I find it a bit confusing naming it "mc_claims" as it's a new API and not MC. I agree that there might be people familiar with the concept but not necesarily. My suggestion is to use a more user-fiendly name.

About the second point:

2. We have in Orange another resource to retrieve directly the msisdn from the access token (GET operation). This is relevant for some UC - you do not have this requirement on your side ?

This is related to issue #101 that is still under discussion. TEF is strongly against returning PI and sharing that information with 3d-parties. But if already have this UC (I think DT also mentioned that they do as well) we can include it as a separate GET endpoint getting the number from the access token.

Access token can be result of OIDC flow but we insist to have endpoints separeted from the authentication flow.

I will update the yaml and upload it so you can comment on it.

@monamok
Copy link
Collaborator Author

monamok commented Dec 7, 2022

I changed the yaml file so there is an single endpoint to verify the number using oneOf structure where it can receive plain phone number or hashed phone number.
Another option is to reuse the same parameter for clear number as well as hashed number and then have a flag to indicate if it's hashed or not. In that case the request body looks like this:
{
"phone_number": "xxx"
"hashed": "false"
}
As you can see in my previous comment, I think mentioning mc_claims in this API is not a good idea because we're trying to agree on having a resource specific API and there is no claims here that we should refer to.

I didn't include the share endpoint (getting the number from the token and returning it) as it's not in our roadmap but if you consider it necesarry you can include it to this version as it's a 0.x version and for the v1.0 we can decide later once issue #101 get resolved.

Please check out the new version.

@monamok monamok requested a review from bigludo7 December 7, 2022 17:13
paulocesardiaslima referenced this pull request in paulocesardiaslima/NumberVerification Dec 8, 2022
TIM Italy and TIM Brasil reviews points and contributions proposal:

Suggest to converge on a YAML from previous proposal A+B+C:
	A) Proposal by TEF (Telefonica Proposal #3)
	B) Proposal by DT (Initial content for Number Verify #2)
	C) Proposal initially by TIM Brasil (IP and Port header input parameters) 

Review points include in new YAML file version regarding path /authorize:
	IP and Port as optional input header parameters
	oAuth2 already mapped in DT (Initial content for Number Verify #2)
	Error code handling according to CAMARA others API (e.g., the Traffic Influence API)
paulocesardiaslima referenced this pull request in paulocesardiaslima/NumberVerification Dec 8, 2022
TIM Italy and TIM Brasil reviews points and contributions proposal:

Suggest to converge on a YAML from previous proposal A+B+C:
	A) Proposal by TEF (Telefonica Proposal #3)
	B) Proposal by DT (Initial content for Number Verify #2)
	C) Proposal initially by TIM Brasil (IP and Port header input parameters) 

Review points include in new YAML file version regarding path /authorize:
	IP and Port as optional input header parameters
	oAuth2 already mapped in DT (Initial content for Number Verify #2)
	Error code handling according to CAMARA others API (e.g., the Traffic Influence API)
@monamok
Copy link
Collaborator Author

monamok commented Dec 14, 2022

Dear all, could you please review my previous comment and the last changes that were made. Today is the deadline and from Telefónica we hope we can meet it together.

@monamok
Copy link
Collaborator Author

monamok commented Dec 15, 2022

Dear all, as it was agreed in this morning meeting I updated the proposal.

@bigludo7 I added a new endpoint for the share operation as you requested. It's a GET operation and returns the phone number. As I already mentioned, sharing PI is not in Telefónica roadmap but having in mind that you have this use case and the commonality issue for the PI is still open and under discussion, we can have it in this version and we'll adjust it later for v1.0, depending on that issue's result.
Please check it out if it covers your requirements. Thank you.

bigludo7
bigludo7 previously approved these changes Dec 15, 2022
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

Thanks @monamok - This is good for my perspective.

@monamok
Copy link
Collaborator Author

monamok commented Dec 15, 2022

Thanks @monamok - This is good for my perspective.

Thank you @bigludo7.

@DT-DawidWroblewski could you please review it and if you agree please go ahead and merge it.

@monamok
Copy link
Collaborator Author

monamok commented Dec 19, 2022

Dear @DT-DawidWroblewski, would it be possible to get your comments/approval-merge today?
Thanks.

@DT-DawidWroblewski
Copy link
Collaborator

@monamok working on it...

title: Number Verification
description: |-
Service Enabling Network Function API to verify that the provided **phone number** is the one used in the device. It verifies that the user is using a device with the same *phone number* as it is declared.
It also make it possible for a Service provider to verify the number itself by returning the phone number retrieved from the authenticated user's access token.
Copy link
Collaborator

Choose a reason for hiding this comment

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

are we assuming here that MSISDN is somehow returned in Access Token? should it be JWT? where MSISDN is then returned?

Copy link
Collaborator Author

@monamok monamok Dec 19, 2022

Choose a reason for hiding this comment

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

The API doesn't return the Token. In order to use the API, a previous step of three legged authentication is needed and the phone number that is associated to the access token will be used to compare with the input data.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @monamok !

this particular sentence makes confision:

...verify the number itself by returning the phone number retrieved from the authenticated user's access token.

I read it like:

as a sw developer i can retrieve phone number from authenticated user's access token, so that I can verify phone number.

maybe sth like this is better:

as a sw developer I can retrieve information about phone number of the device, so that I can confirm that end user possess the device.

what do yo think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I rephrased it. Please take a look :)

code/API_definitions/number_verification.yaml Outdated Show resolved Hide resolved
code/API_definitions/number_verification.yaml Show resolved Hide resolved

# Resources and Operations overview
This API currently provides two endpoints where both require a **3-legged token** and authentication via **mobile network** (excluding for example by SMS/OTP or user/password as an authentication method):
- The first one checks if the user phone number matches the MSISDN associated with the mobile device. It can receive either a hashed phone number or a clear phone number as input and it compares the received input with the authenticated user's MSISDN retrieved from the access token in order to respond **true/false**.
- The first one checks if the user mobile phone number matches the phone number associated with the mobile device. It can receive either a hashed phone number or a clear phone number as input and it compares the received input with the authenticated user's phone number retrieved from the access token in order to respond **true/false**.
Copy link
Collaborator

Choose a reason for hiding this comment

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

phone number retrieved from the access token --> phone number associated to the access token

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Edited

@@ -96,7 +96,7 @@ paths:
description: |-
Returns the phone number so the API clients can verify the number themselves:
- It will be done for the user that has authenticated via mobile network and so their `sub` is in the access token
- It returns the authenticated user's `MSISDN` retrieved from the access token
- It returns the authenticated user's `device phone number` retrieved from the access token
Copy link
Collaborator

Choose a reason for hiding this comment

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

device phone number retrieved from the access token --> device phone number associated to the access token

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Edited

Copy link
Collaborator

@DT-DawidWroblewski DT-DawidWroblewski left a comment

Choose a reason for hiding this comment

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

thx!

@DT-DawidWroblewski
Copy link
Collaborator

merging under condition that Number Verify based Mobile Connect Verified MSISDN yaml is also available inside PR, although I'm not able to merge it by myself and waiting for @bigludo7 approval.

@monamok monamok requested a review from bigludo7 December 20, 2022 09:45
@DT-DawidWroblewski DT-DawidWroblewski merged commit f4f99b4 into camaraproject:main Dec 20, 2022
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.

3 participants