Skip to content

Conversation

@AxelNennker
Copy link
Collaborator

@AxelNennker AxelNennker commented Mar 13, 2025

What type of PR is this?

Add one of the following kinds:

  • enhancement/feature

What this PR does / why we need it:

Currently NumberVerification relies on network-based authentication, which means that the user's phone needs to be on a mobile connection because the phone's IP-address is mapped to subscriber's MSISDN.
This means that NumberVerification does not work over WiFI.

This PR does not change the NumberVerification API but changes how the API Consumer gets an access token for the API.

Fixes #86 #165

Changelog input

 Enable NumberVerification for off-net scenarios e.g. if the user's device is connected to the Internet via WiFi.

Additional documentation

GSMA TS.43 standard on temporary tokens and "operations"
https://www.gsma.com/get-involved/working-groups/gsma_resources/ts-43-v12-0-service-entitlement-configuration/

docs

AxelNennker and others added 6 commits March 14, 2025 11:44
Co-authored-by: Jorge Garcia Hospital <129095857+jgarciahospital@users.noreply.github.com>
Co-authored-by: Jorge Garcia Hospital <129095857+jgarciahospital@users.noreply.github.com>
Co-authored-by: Jorge Garcia Hospital <129095857+jgarciahospital@users.noreply.github.com>
@AxelNennker
Copy link
Collaborator Author

@jgarciahospital many thanks for your review and suggestions. I am trying to address them all and commit your suggestions if I agree with them. Then I "resolve the conversation". I hope my way of doing this is OK with you.
Please review again.

Copy link
Contributor

@tanjadegroot tanjadegroot left a comment

Choose a reason for hiding this comment

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

looks good, some rewording / clarifications added

Co-authored-by: Tanja de Groot <87864067+tanjadegroot@users.noreply.github.com>
AxelNennker and others added 5 commits March 20, 2025 09:25
Co-authored-by: Tanja de Groot <87864067+tanjadegroot@users.noreply.github.com>
Co-authored-by: Tanja de Groot <87864067+tanjadegroot@users.noreply.github.com>
@ECORMAC
Copy link

ECORMAC commented Mar 20, 2025

I've tried to include a comment directly (didn't work for me-sorry): but could we please add a clarification before the flow, indication that an aggregator may also be part of the flow->"Note that the diagram shows just an example of a direct integration from developer's application and the API Provider's auth server (an aggregator may also be part of the flow):

@hdamker
Copy link
Contributor

hdamker commented Mar 20, 2025

I've tried to include a comment directly (didn't work for me-sorry): but could we please add a clarification before the flow, indication that an aggregator may also be part of the flow->"Note that the diagram shows just an example of a direct integration from developer's application and the API Provider's auth server (an aggregator may also be part of the flow):

@ECORMAC, my view:

  • the flow picture is unchanged, it shows only the network authentication case
  • the work "example" already implies that there are other variants possible
  • it talks about "API Provider" which does not imply necessarily a network operator

My suggestion is to address further changes in this section within #94, which we agreed to include in a later version.

@bigludo7
Copy link
Collaborator

I've tried to include a comment directly (didn't work for me-sorry): but could we please add a clarification before the flow, indication that an aggregator may also be part of the flow->"Note that the diagram shows just an example of a direct integration from developer's application and the API Provider's auth server (an aggregator may also be part of the flow):

This is fine for me to add this - @hdamker @AxelNennker @fernandopradocabrillo any view on your side?

@hdamker
Copy link
Contributor

hdamker commented Mar 20, 2025

This is fine for me to add this - @hdamker @AxelNennker @fernandopradocabrillo any view on your side?

Fine for me, but maybe not necessary and not in scope of this PR.

@fernandopradocabrillo
Copy link
Collaborator

I've tried to include a comment directly (didn't work for me-sorry): but could we please add a clarification before the flow, indication that an aggregator may also be part of the flow->"Note that the diagram shows just an example of a direct integration from developer's application and the API Provider's auth server (an aggregator may also be part of the flow):

This is fine for me to add this - @hdamker @AxelNennker @fernandopradocabrillo any view on your side?

I prefer to leave it out of this PR and address it later since we are going to do some rework. Axel mentioned to also include diagrams with mermaid, etc. I think we can plan it with more time so it'll be more detailed.

@ECORMAC
Copy link

ECORMAC commented Mar 20, 2025

Ok that sounds like a plan.

bigludo7
bigludo7 previously approved these changes Mar 20, 2025
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.

lgtm :)

Copy link
Collaborator

@fernandopradocabrillo fernandopradocabrillo left a comment

Choose a reason for hiding this comment

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

LGTM

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.

LGTM

@bigludo7 bigludo7 merged commit 9b4a72b into camaraproject:main Mar 20, 2025
2 checks passed
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.

integration to on device application (EAP-AKA)

9 participants