OpenConext / Stepup-SelfService

Stepup Self-service interface
Apache License 2.0
2 stars 5 forks source link

SMS prove possession route not found #288

Closed MKodde closed 1 year ago

MKodde commented 1 year ago

The ss_registration_sms_prove_possession might have been skipped by the SMS RT proof op posession where it was removed. During that action this route was also removed. But it was still used in the token registration of an SMS token.

The other commit states that: When the max attempts are expired, the view vars did not include the verifyEmail property. That was fixed in this boy scout commit.

See: https://www.pivotaltracker.com/story/show/184750138

MKodde commented 1 year ago

Short summary of our IRL review session:

  1. Ruben noticed the lack of automated tests in this PR, and I explained we do not have a very practical way to write tests for controller logic. The one possibility is to add behat tests in the openconext deploy repository. And to my shame I have to admit we run them seldom.
  2. We discussed different test types and how to utilize them
  3. Looking at the controller logic we touched on the SOLID design principles and measured the controller logic against them. Ouch..