Wrongful omission of required sp_esvcId query param in baseString when computing signature for MyInfo Person endpoint

[zionsg]

Created a demo application at https://github.com/zionsg/demo-mockpass-login to simultaneously start up MockPass and get opengovsg's SingPass/CorpPass/MyInfo client libraries working locally, especially the figuring out of which MockPass certificates to use for the clients.

Ran into problems with retrieving data from MyInfo Person endpoint. Traced it to the following:

• getPerson() in https://github.com/opengovsg/myinfo-gov-client/blob/develop/src/MyInfoGovClient.class.ts computes the signature with sp_esvcId query param in the URL for the MyInfo /person endpoint.

• pki() in https://github.com/opengovsg/mockpass/blob/master/lib/crypto/myinfo-signature.js omits the sp_esvcId query param when returning baseString for the MyInfo /person endpoint.

• This causes verify() in https://github.com/opengovsg/mockpass/blob/master/lib/express/myinfo/controllers.js to use the wrong baseString when computing the signature and results in /myinfo/${version}/person/:uinfin/ returning 401 error.

    {
      "code": 401,
      "message": "Signature verification failed, GET&http://host.docker.internal:5156/myinfo/v3/person/S9812379B/&app_id=clientId&attributes=name,sex,mobileno&client_id=clientId&nonce=Fm6ZUBDqVaZkbQejplmD8s9XhQQMmVGafc1/+SVp7oU=&signature_method=RS256&timestamp=1662655799222 does not result in dpP5+I2KRU0PSmwDpPbXMxHv2CcmK0yG7ylZWIlwxA7T6P89rShf03LNGCkr+LHPgTdCY6F84H4VVStQ2NM/O4cFt+QkDjqdDMslm612zeMS9bKZ5hs1OSHrNK7cvRUkINaz8bSFswAhmKHO6hgCj47ufN7xPvbG/RgY4qT7T6GUu2gl6d9pOB5HzscKMt+fOPHsuUpqnGkLKl+vSa00TBBPw3+pwZexRB4YbjEL1h6RwyYp9dMef43mGw2BxPwmOKMdKhsgjz3V54MVt0jtptqGim77HnxvqIUrsP9ZvfiYXYpyr9reI4qjJ/54Kk85d5l8YzmXiVdz+U+e8xWUrQ=="
    }
  

The MyInfo API docs at https://public.cloud.myinfo.gov.sg/myinfo/tuo/myinfo-tuo-specs.html#operation/getperson states that sp_esvcId query param is required, hence it should not be omitted in baseString when computing the signature.

Suggested solution (tried this and it worked):

• Remove check for req.path.includes('/person-basic') in apex() and pki() of https://github.com/opengovsg/mockpass/blob/master/lib/crypto/myinfo-signature.js so that sp_esvcId query is in baseString.

[LoneRifle]

The /person endpoint described here is meant to correspond to the MyInfo KYC endpoint, not the TUO (Tell Us Once) one you referred to. This won't be easy to solve given that MockPass conflates the two together, and there are users who are dependent on the KYC behaviour.

We will have to likely overhaul this in a major release, perhaps together with removing the SAML-based endpoints

[zionsg]

Oh, didn't realize that there were the KYC and TUO endpoints. Noted with thanks!