Skip to content

Align error list & model with comm 0.5#161

Merged
bigludo7 merged 3 commits intomainfrom
error-code-align-comm-0.5
Jan 17, 2025
Merged

Align error list & model with comm 0.5#161
bigludo7 merged 3 commits intomainfrom
error-code-align-comm-0.5

Conversation

@bigludo7
Copy link
Collaborator

@bigludo7 bigludo7 commented Dec 19, 2024

What type of PR is this?

  • enhancement/feature

What this PR does / why we need it:

Updates number-verification API with the changes derived from Commonalities updates for error schema heritage & enums.
Remove 500, 503 & 504 error

Which issue(s) this PR fixes:

Fixes #

Special notes for reviewers:

This update did not break compatibility

Changelog input

 release-note
* Error schema updated with enums
* Remove 500, 503 & 504 error

Additional documentation

This section can be blank.

docs

Align error model with comm 0.5
@github-actions
Copy link

github-actions bot commented Dec 19, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 2 0 0.02s
✅ OPENAPI spectral 1 0 1.58s
✅ REPOSITORY git_diff yes no 0.01s
✅ REPOSITORY secretlint yes no 0.66s
✅ YAML yamllint 1 0 0.33s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

enum:
- PERMISSION_DENIED
- NUMBER_VERIFICATION.USER_NOT_AUTHENTICATED_BY_MOBILE_NETWORK
- INVALID_TOKEN_CONTEXT
Copy link
Collaborator

Choose a reason for hiding this comment

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

@bigludo7 this one is no longer needed I think.
Per current design, the token must 3-legged identifying a subscription and if the phone number in the request doesn't match we return a false in the response.

We are removing this one in almost all apis

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@fernandopradocabrillo Just to be sure before we remove it - it means that test case @NumberVerification_phone_number_verify204_no_phonenumber_associated_with_access_token is not relevant anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@fernandopradocabrillo I'm a bit reluctant to remove this code example because it is also use in /device-phone-number operation if for any reason we're not able to 'read' the phone number from the access token.
Any thoughts on the topic @AxelNennker ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

mm that is a good question. In order to be a valid access token it has to be associated with a phone number somehow (the internal structure of the access token is up to the Operator). Do you have an scenario or an example where you can have a valid access token but not being able to get the associated phone number?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding @AxelNennker to get his view.
I see this a safety bucket "in case of" but probably 403 NUMBER_VERIFICATION.USER_NOT_AUTHENTICATED_BY_MOBILE_NETWORK coud make the job?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding @trehman-gsma to get conformance perspective.
Toyeeb, make sens if we remove this 403 INVALID_TOKEN_CONTEXT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

A valid 3-legged access token implies that an identifier is associated with the access token which can be used to identify the subscription. This identifier can be a MSISDN but it can be anything the API provider seems fit e.g. IMSI or ICCID.

I think that the case where there is a valid access token that is not associated with a subscription identifier (phone number) does not exist if the access token was created using a 3-legged flow.

I suggest that Commonalities defines an error for all APIs that means that the API provider tried network-based authentication but failed. I would go with
{
"status": 400,
"code": "BAD REQUEST",
"message": "identifying the subscription failed"
}
if the API consumer failed to send the code request over a CSP connection controlled by the API provider.

If CIBA is used, then this case can not happen, because the login_hint is backed into the access token. But CIBA does not really identify the end-user, which I think is important in most jurisdictions.

Regarding CIBA vs OIDC authorization code flow with Age Verification, I think the API description should have some text explaining to the API consumer that CIBA uses an authentication device and if a minor has access to their parent's or to some other adult's mobile phone then CIBA relies on the adult's ability to keep their mobile phone out of the hands of the minor.
OIDC authorization code flow could make use of network-based authentication but then the age of the subscriber is verified and not the age of the end-user. If Age Verification promises to the API consumer that it verifies the age of the end-user then, OIDC authorization code flow with max_age=0 is the only solution, I think.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that, speaking only about Number Verification API, the INVALID_TOKEN_CONTEXT error cannot happen. This API is intended to use with Auth Code Network auth, which means 3-legged and, as Axel said, is not possible to have a valid access token not associated with an identifier.

The invalid token context error is checking the same as the core functionality of the API, which is that the phone number used for the network based authentication is the same one included in the request body. If they don't match, number verification returns a false.

What we wanted to check with @trehman-gsma is how this error has been tested in previous versions. I think it was excluded based on technical limitations.

If we have no use for this error, maybe it is best to remove it, even if we have to upgrade the major version..

Copy link

@trehman-gsma trehman-gsma Jan 16, 2025

Choose a reason for hiding this comment

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

Hi @bigludo7 @fernandopradocabrillo,

From my perspective, it's fine to remove INVALID_TOKEN_CONTEXT from this API. We were unable to simulate the error, and the test case @NumberVerification_phone_number_verify204_no_phonenumber_associated_with_access_token was excluded from certification based on technical limitations.

In the edge case of GET /device-phone-number being called with an access token where it's not possible to read the phone number, would a 403 PERMISSION_DENIED suffice?

enum:
- PERMISSION_DENIED
- NUMBER_VERIFICATION.USER_NOT_AUTHENTICATED_BY_MOBILE_NETWORK
- INVALID_TOKEN_CONTEXT
Copy link
Collaborator

Choose a reason for hiding this comment

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

A valid 3-legged access token implies that an identifier is associated with the access token which can be used to identify the subscription. This identifier can be a MSISDN but it can be anything the API provider seems fit e.g. IMSI or ICCID.

I think that the case where there is a valid access token that is not associated with a subscription identifier (phone number) does not exist if the access token was created using a 3-legged flow.

I suggest that Commonalities defines an error for all APIs that means that the API provider tried network-based authentication but failed. I would go with
{
"status": 400,
"code": "BAD REQUEST",
"message": "identifying the subscription failed"
}
if the API consumer failed to send the code request over a CSP connection controlled by the API provider.

If CIBA is used, then this case can not happen, because the login_hint is backed into the access token. But CIBA does not really identify the end-user, which I think is important in most jurisdictions.

Regarding CIBA vs OIDC authorization code flow with Age Verification, I think the API description should have some text explaining to the API consumer that CIBA uses an authentication device and if a minor has access to their parent's or to some other adult's mobile phone then CIBA relies on the adult's ability to keep their mobile phone out of the hands of the minor.
OIDC authorization code flow could make use of network-based authentication but then the age of the subscriber is verified and not the age of the end-user. If Age Verification promises to the API consumer that it verifies the age of the end-user then, OIDC authorization code flow with max_age=0 is the only solution, I think.

@bigludo7
Copy link
Collaborator Author

@AxelNennker @fernandopradocabrillo I think the discussion about INVALID_TOKEN_CONTEXT is out of the scope of this PR and we're not in the same page. This PR targets removal of 5xx code & align error definition. Let's move forward on this.

I suggest we focus to this and create a specific issue to removal/replacing INVALID_TOKEN_CONTEXT

AxelNennker
AxelNennker approved these changes Jan 17, 2025
version: 1.0.0
x-camara-commonalities: 0.4.0
version: wip
x-camara-commonalities: 0.5.0
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
x-camara-commonalities: 0.5.0
x-camara-commonalities: 0.5

@bigludo7 bigludo7 merged commit f0b558b into main Jan 17, 2025
1 check passed
@bigludo7 bigludo7 deleted the error-code-align-comm-0.5 branch January 17, 2025 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants