camaraproject / OTPValidation

Repository to describe, develop, document and test the OTP Validation API family
https://wiki.camaraproject.org/display/CAM/OTPValidation
Apache License 2.0
6 stars 13 forks source link

Telefonica Proposal #5

Closed monamok closed 1 year ago

monamok commented 1 year ago

This PR is related to our comments on here.

DT-DawidWroblewski commented 1 year ago

Hi Mona!

Thanks for sharing your thoughts about this.

I get the point.

Can I suggest to:

  1. Rename this API family to e.g. “Messaging API – SMS OTP” – currently this API family is confusing, as Number Verify solution promise to deliver seamless experience to end-user, so having SMS as a substitute is not the promise MNOs are trying to deliver to customers with Number Verify. Currently MNOs avoids having fallback to SMS and SPs/developers prefers errors returned in redirect (I get an impression that NumberVerificationSMS2FA was placed here by mistake, just to cover a scenario when end-user’s device is on WiFi and standard Verified MSISDN flow cannot be handled).
  2. Deprecate “device msisdn verification” for this API, and focus on messaging api that enables otp validation mechanism to developer

I’d appreciate your feedback.

Regards, David

Deutsche Telekom Technology Delivery International

Dawid Wróblewski Marynarska 12, 02-674 Warsaw +48 602 204 473 (phone) Email: @.**@.> www.telekom.comhttp://www.telekom.com/

@.***

Life is for sharing.

You can find the obligatory information on www.telekom.com/compulsory-statementhttp://www.telekom.com/compulsory-statement

From: Mona Mokhber @.> Sent: Monday, December 12, 2022 12:41 PM To: camaraproject/NumberVerificationSMS2FA @.> Cc: Wróblewski Dawid @.>; Mention @.> Subject: Re: [camaraproject/NumberVerificationSMS2FA] Telefonica Proposal (PR #5)

SECURITY WARNING: Ta wiadomość pochodzi z zewnętrznego źródła - uważaj na załączniki i linki. Jeśli wiadomość wyda Ci się podejrzana zgłoś incydent. This email is from an external source - be careful of attachments and links. Please follow good practices and report suspicious emails.

@monamok commented on this pull request.


In code/API_definitions/number_verification_sms.yamlhttps://github.com/camaraproject/NumberVerificationSMS2FA/pull/5#discussion_r1045719997:

  • description: |-

Thank you @DT-DawidWroblewskihttps://github.com/DT-DawidWroblewski for your comment. We prefer to keep them seperated as two individual APIs. Our idea is to have this API usable without forcing to use it as a fallback of the other API.

— Reply to this email directly, view it on GitHubhttps://github.com/camaraproject/NumberVerificationSMS2FA/pull/5#discussion_r1045719997, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A3XZEQPXTYZ37H4WGPXOWWTWM4FOLANCNFSM6AAAAAASUBGICA. You are receiving this because you were mentioned.Message ID: @.**@.>>

T-MOBILE POLSKA S.A. z siedzibą w Warszawie Adres: ul. Marynarska 12, 02-674 Warszawa Zarząd Spółki: Andreas Maierhofer - Prezes Zarządu; Juraj Andráš - Członek Zarządu, Dyrektor ds. Finansowych; Dorota Kuprianowicz-Legutko – Członek Zarządu, Dyrektor ds. Polityki Personalnej; Goran Marković – Członek Zarządu, Dyrektor ds. Rynku Prywatnego; Alexander Jenbar – Członek Zarządu, Dyrektor ds. Technologii i Innowacji; Agnieszka Rynkowska - Członek Zarządu, Dyrektor ds. Rynku Biznesowego. Spółka zarejestrowana w Sądzie Rejonowym dla m.st. Warszawy w Warszawie, XIII Wydział Gospodarczy Krajowego Rejestru Sądowego. KRS 0000391193 | NIP 526-10-40-567 | Regon 011417295 Kapitał zakładowy 711.210.000 złotych, kapitał wpłacony w całości.

DUŻE ZMIANY ZACZYNAJĄ SIĘ OD MAŁYCH - CHROŃ PLANETĘ, NIE DRUKUJ TEGO E-MAILA, JEŻELI NIE MUSISZ.

Ta wiadomość i jej treść są zastrzeżone w szczegółowym zakresie dostępnym na http://www.t-mobile.pl/stopka This e-mail and its contents are subject to a DISCLAIMER with important RESERVATIONS: see http://www.t-mobile.pl/stopka

DT-DawidWroblewski commented 1 year ago

Hi Ludovic,

One Time Password API name looks good and does not require any explanation 😊

This API supports OTP mechanism that some developers are willing to utilize – fine for me.

Deutsche Telekom Technology Delivery International

Dawid Wróblewski Marynarska 12, 02-674 Warsaw +48 602 204 473 (phone) Email: @.**@.> www.telekom.comhttp://www.telekom.com/

@.***

Life is for sharing.

You can find the obligatory information on www.telekom.com/compulsory-statementhttp://www.telekom.com/compulsory-statement

From: Ludovic Robert @.> Sent: Monday, December 12, 2022 2:11 PM To: camaraproject/NumberVerificationSMS2FA @.> Cc: Wróblewski Dawid @.>; Mention @.> Subject: Re: [camaraproject/NumberVerificationSMS2FA] Telefonica Proposal (PR #5)

SECURITY WARNING: Ta wiadomość pochodzi z zewnętrznego źródła - uważaj na załączniki i linki. Jeśli wiadomość wyda Ci się podejrzana zgłoś incydent. This email is from an external source - be careful of attachments and links. Please follow good practices and report suspicious emails.

@bigludo7 commented on this pull request.

Hello I'm fine on this definition. The only point for me is the title of the API as highlighted by @DT-DawidWroblewskihttps://github.com/DT-DawidWroblewski . Number Verification is confusing with the other API if we keep them separately (and agreed with you Mona to keep them separate). What about One Time Password API as it stands for OTP and it is very clear for a developer?

— Reply to this email directly, view it on GitHubhttps://github.com/camaraproject/NumberVerificationSMS2FA/pull/5#pullrequestreview-1213518434, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A3XZEQKQZ75GTAGZU3NJTQDWM4P5ZANCNFSM6AAAAAASUBGICA. You are receiving this because you were mentioned.Message ID: @.**@.>>

T-MOBILE POLSKA S.A. z siedzibą w Warszawie Adres: ul. Marynarska 12, 02-674 Warszawa Zarząd Spółki: Andreas Maierhofer - Prezes Zarządu; Juraj Andráš - Członek Zarządu, Dyrektor ds. Finansowych; Dorota Kuprianowicz-Legutko – Członek Zarządu, Dyrektor ds. Polityki Personalnej; Goran Marković – Członek Zarządu, Dyrektor ds. Rynku Prywatnego; Alexander Jenbar – Członek Zarządu, Dyrektor ds. Technologii i Innowacji; Agnieszka Rynkowska - Członek Zarządu, Dyrektor ds. Rynku Biznesowego. Spółka zarejestrowana w Sądzie Rejonowym dla m.st. Warszawy w Warszawie, XIII Wydział Gospodarczy Krajowego Rejestru Sądowego. KRS 0000391193 | NIP 526-10-40-567 | Regon 011417295 Kapitał zakładowy 711.210.000 złotych, kapitał wpłacony w całości.

DUŻE ZMIANY ZACZYNAJĄ SIĘ OD MAŁYCH - CHROŃ PLANETĘ, NIE DRUKUJ TEGO E-MAILA, JEŻELI NIE MUSISZ.

Ta wiadomość i jej treść są zastrzeżone w szczegółowym zakresie dostępnym na http://www.t-mobile.pl/stopka This e-mail and its contents are subject to a DISCLAIMER with important RESERVATIONS: see http://www.t-mobile.pl/stopka

monamok commented 1 year ago

Hello @DT-DawidWroblewski and @bigludo7,

This is what was agreed and signed about the definition of this API at the first place:

MoU

I think it's aligned with the yaml defenition, it sends a message and then verifies the code in order to verify the number. Do you still think we must change the name?

bigludo7 commented 1 year ago

I should confess that from start I was not comfortable for using this same close name that bring confusion. I'm still on favor to have a simplest name like one time password. Thanks

monamok commented 1 year ago

Thank you for the feedbacks. I renamed the API to "One Time Password SMS" and aligned the descriptions and errors with the new name. Although I would like to make sure that we are all at the same page and this change is just a change of how we call the API and the functionality remains the same:

It enables a Service Provider (SP) to verify a phone number (MSISDN) by sending a code by SMS and validating the introduced code in order to make sure that the user has access to the given phone number.

So there wouldn't be any other API in the CAMARA to cover this use case. Are you ok with it?

bigludo7 commented 1 year ago

Thanks @monamok I have one last comment is about 'Draft Version' mention in the API title. Probably better to remove it before merging the PR?

monamok commented 1 year ago

Thanks @monamok I have one last comment is about 'Draft Version' mention in the API title. Probably better to remove it before merging the PR?

Thank you @bigludo7. You're right. I removed it.

monamok commented 1 year ago

Thanks @monamok We're good to go for my perspective.

Thank you @bigludo7.

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

monamok commented 1 year ago

@DT-DawidWroblewski I made the change you'd requested. Could you please review it again and merge it?