Numberverify verify Test definition

[AxelNennker]

What type of PR is this?

Add one of the following kinds:

• tests

What this PR does / why we need it:

This PR provides tests for the path verify.
Tests for path device-phone-number are TBD.

Which issue(s) this PR fixes:

Fixes #104

[github-actions]

🦙 MegaLinter status: ✅ SUCCESS

Descriptor	Linter	Files	Fixed	Errors	Elapsed time
✅ ACTION	actionlint	2		0	0.04s
✅ OPENAPI	spectral	1		0	1.65s
✅ REPOSITORY	git_diff	yes		no	0.01s
✅ REPOSITORY	secretlint	yes		no	0.73s
✅ YAML	yamllint	1		0	0.31s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by

[trehman-gsma]

It's just the first step of the Authorization Code flow (/auth) that requires a mobile connection, right?

The /token request and the /verify request can be executed from a backend? Flow diagram here.

If so, what do you think about moving the mobile connection requirement to the Background section?

[AxelNennker]

I think about that. Usually, it is a security issue if several requests are not bound together and/or come from different sources.
https://www.rfc-editor.org/rfc/rfc7636
https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#name-sender-constrained-access-t
https://openid.bitbucket.io/fapi/fapi-2_0-security-profile.html

I wanted to add sender-constrained tokens to Camara, but I was told that operators have never heard of that and too few implemented it, so ICM should not demand it.
Same with DPoP.
The slowest boot determines the speed of the convoy.

Thanks for the comment.

[bigludo7]

Hello @AxelNennker
Thanks for the contribution.
Few comments for your consideration.

[trehman-gsma]

Hi @AxelNennker,

Are tests for hashedPhoneNumber TBD too?

We could probably repeat most of the plain phoneNumber tests for hashedPhoneNumber, and have an additional test that expects a 400 when phoneNumber and hashedPhoneNumber are both passed in (because maxProperties=1).

[AxelNennker]

I added NumberVerification_verify203_both_phone_number_and_hashed_in_request

[fernandopradocabrillo]

The way of describing the scenarios is a bit different from other test plans of other CAMARA APIs.
The nomenclature of the test is a bit confusing, but that's probably a me problem haha

I think we should include the common 401 test. For example these are the ones from sim swap:

# Generic 401 errors

  @check_sim_swap_401.1_no_authorization_header
  Scenario: No Authorization header
    Given the header "Authorization" is removed
    And the request body is set to a valid request body
    When the request "checkSimSwap" is sent
    Then the response status code is "401"
    And the response property "$.status" is 401
    And the response property "$.code" is "UNAUTHENTICATED"
    And the response property "$.message" contains a user friendly text

  @check_sim_swap_401.2_expired_access_token
  Scenario: Expired access token
    Given the header "Authorization" is set to an expired access token
    And the request body is set to a valid request body
    When the request "checkSimSwap" is sent
    Then the response status code is "401"
    And the response property "$.status" is 401
    And the response property "$.code" is "UNAUTHENTICATED"
    And the response property "$.message" contains a user friendly text

  @check_sim_swap_401.3_invalid_access_token
  Scenario: Invalid access token
    Given the header "Authorization" is set to an invalid access token
    And the request body is set to a valid request body
    When the request "checkSimSwap" is sent
    Then the response status code is "401"
    And the response property "$.status" is 401
    And the response property "$.code" is "UNAUTHENTICATED"
    And the response property "$.message" contains a user friendly text

[fernandopradocabrillo]

i think this one should be match_hashed_false

[fernandopradocabrillo]

or UNAUTHENTICATED, still don't know which one should apply in this case

[fernandopradocabrillo]

Test included for veryfy operation:

1. ✅ @NumberVerification_verify0_phoneNumber_does_not_match_schema --> Invalid phone number
2. ✅ @NumberVerification_verify100_match_true -->Token associated with phoneNumber1 matches phoneNumber1 sent in the body
3. ✅ @NumberVerification_verify300_match_hashed_true --> Token associated with phoneNumber1 matches hashedPhoneNumber1 sent in the body
4. ✅ @NumberVerification_verify101_match_false --> Token associated with phoneNumber2 doesn't match phoneNumber1 in the body
5. ⚠️ @NumberVerification_verify301_match_ hashed _false --> Token associated with phoneNumber2 doesn't match hashedPhoneNumber1 in the body
6. ⚠️ @NumberVerification_verify200_missing_phone_number_in_request --> neither phoneNumber nor hashedPhoneNumber
7. ⚠️ @NumberVerification_verify201_missing_scope
8. ⚠️ @NumberVerification_verify202_expired_access_token
9. ✅ @NumberVerification_verify203_both_phone_number_and_hashed_in_request

Pending test to be included:

1. ❌ The access token is valid but it has been generated with an auth method other than network auth (user/password, otp-sms, etc) --> error NUMBER_VERIFICATION.USER_NOT_AUTHENTICATED_BY_MOBILE_NETWORK
2. ❌ The phone number cannot be deducted from access token --> error INVALID_TOKEN_CONTEXT

cc: @bigludo7 @AxelNennker

[fernandopradocabrillo]

Test included for share operation:

1. ✅ @NumberVerification_phone_number_share100_match_true
2. ⚠️ @NumberVerification_phone_number_share201_missing_scope
3. ✅ @NumberVerification_phone_number_share202_expired_access_token

Pending test to be included:

1. ❌ The access token is valid but it has been generated with an auth method other than network auth (user/password, otp-sms, etc) --> error NUMBER_VERIFICATION.USER_NOT_AUTHENTICATED_BY_MOBILE_NETWORK
2. ❌ The phone number cannot be deducted from access token --> error INVALID_TOKEN_CONTEXT

cc: @bigludo7 @AxelNennker

[AxelNennker]

regarding "pending tests" 1.

Maybe the authorization server should never create an access token if the scope is a "number verification"-scope and never did network authentication?! There is probably no way for the resource server / API endpoint to identify the authentication method used.

[AxelNennker]

regarding "pending tests" 2.

This also sounds like an internal authorization server error and should never happen - like the access token creation without network authentication for nv-scopes.

The resource server / API-endpoint can detect this condition

[AxelNennker]

https://github.com/camaraproject/NumberVerification/blob/main/code/API_definitions/number_verification.yaml#L270 talks about AMR
As Camara does not specify the access token nor the information associated with it other than the API's subject like phoneNumber, this would be a NV requirement on the telco issuing access tokens

[fernandopradocabrillo]

wdyt @bigludo7? I don't know if we can leave that validation to the authserver as something external to the API.
The auth process will validate if a phone number has access to certain scopes but I don't know if it can also check the auth method at token generation time.
And being Auth Code where the authentication is not done by the authserver itself but maybe something like an IDP.
I tend to think that is better to include the test mainly because is an error specified in the API yaml and we should cover it.

[bigludo7]

I'm not as expert that both of you on this specific point but as we have the error in the YAML we must have a test case covered it.
By preference it to have it and then we can always discuss in the future to remove it.

[AxelNennker]

I added a test to "verify" that uses USER_NOT_AUTHENTICATED_BY_MOBILE_NETWORK

[AxelNennker]

Added a test to "share" that uses USER_NOT_AUTHENTICATED_BY_MOBILE_NETWORK

https://github.com/camaraproject/NumberVerification/blob/test_definition/code/Test_Definitions/NumberVerification_device_phone_number_share.feature#L90

[bigludo7]

LGTM
Plus I've check @fernandopradocabrillo requests and as far as I checked the request change from Fernando are taken into account.