Fix Cisco Duo OTP 2FA rejecting valid passcodes

What does this MR do and why?

A customer (Geotab) using Cisco Duo as a 2FA OTP provider could not sign in: a correct passcode was always rejected.

The Duo integration (lib/gitlab/auth/otp/strategies/duo_auth/manual_otp.rb) sent the passcode to the duo_api gem as otp_code.to_i, an integer, which caused two separate problems. First, otp_code.to_i discards leading zeros before the gem ever sees the value, so a code like 042315 was mangled to 42315. This was true under both the old and current gem versions, so leading-zero passcodes were never actually valid, not a regression introduced by the upgrade. Second, and this is the change that broke the customer's sign-in, the transport used to send the passcode changed between gem versions. Under 1.3.0, POST parameters were form-encoded and every value was stringified before being sent, so an integer passcode still reached Duo as a string and was accepted. Since 1.4.0, POST parameters are serialized into a JSON request body, so the integer is now sent as a JSON number instead of a string, which does not match the type Duo's Auth API expects. Because this affects every passcode, not just ones with leading zeros, it's what actually broke sign-in after the gem upgrade. The fix, otp_code.to_s, resolves both issues: the passcode is sent as a JSON string, and leading zeros are preserved.

A second issue in the same file: the denial branch called error(message: ...), but the base error(message, http_status = nil) method takes the message as a positional argument. Passing it as a keyword-style hash wrapped Duo's denial reason in a hash instead of surfacing it as a plain string. This is fixed by passing the message positionally.

The previous spec stubbed the gem's request method entirely, so it never exercised the real transport and could not catch this class of bug. It has been rewritten to stub the HTTP layer with WebMock, and now asserts that the passcode reaches Duo as a string with leading zeros preserved, that an "allow" response yields success, and that a "deny" response surfaces Duo's status message as a plain string. These new assertions were confirmed to fail against the pre-fix code.

References

Screenshots

How to set up and validate locally

  1. Run the unit spec:
    bundle exec rspec spec/lib/gitlab/auth/otp/strategies/duo_auth/manual_otp_spec.rb
  2. To validate end to end, you need a Cisco Duo tenant. Configure Duo OTP in gitlab.yml under duo_auth with integration_key, secret_key, hostname, and enabled: true.
  3. Sign in as a user enrolled with Duo, and enter a Duo passcode when prompted, including one with a leading zero.
  4. Confirm the passcode is accepted and sign-in succeeds.

This fix has been verified with unit tests only. It has not been validated end to end against a live Cisco Duo tenant, since the local environment has no Duo account configured. A reviewer or the customer with a Duo tenant should confirm the full sign-in flow before this is considered fully verified.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist.

Edited by Eduardo Sanz García

Merge request reports

Loading
Loading