search

owncloud / twofactor_totp

🔑 Second factor TOTP (Google Authenticator) provider for ownCloud
GNU Affero General Public License v3.0
9 stars 9 forks source link

Acceptance tests failing #170

Closed phil-davis closed 4 years ago

phil-davis commented 4 years ago

The nightly test run had some fails. e.g. https://drone.owncloud.com/owncloud/twofactor_totp/732/13/11

  Scenario Outline: User provides wrong key on the verification page                                                        # /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUITwoFactorTOTP/keyVerification.feature:35
    Given user "newly-created-user" has logged in using the webUI                                                           # WebUILoginContext::theUserHasLoggedInUsingTheWebUI()
    And the user has browsed to the personal security settings page                                                         # WebUIPersonalSecuritySettingsContext::theUserBrowsesToThePersonalSecuritySettingsPage()
    And the user has activated TOTP Second-factor auth but not verified                                                     # TwoFactorTOTPContext::theUserHasActivatedTOTPSecondFactorAuthButNotVerified()
    When the user adds one-time key generated from the secret key using the webUI                                           # TwoFactorTOTPContext::theUserAddsVerificationKeyFromSecretKeyToVerifyUsingWebUI()
    Then the TOTP secret key should be verified on the webUI                                                                # TwoFactorTOTPContext::totpSecretKeyShouldBeVerifiedOnTheWebUI()
    When the user re-logs in as "newly-created-user" to the two-factor authentication verification page                     # TwoFactorTOTPContext::theUserReLogsInAsForTwoFactorAuthentication()
    And the user adds one-time key "<key>" on the verification page on the webUI                                            # TwoFactorTOTPContext::theUserAddsOneTimeKeyOnTheVerificationPageOnTheWebui()
    Then the user should see an error message on the verification page saying "An error occurred while verifying the token" # TwoFactorTOTPContext::theUserShouldSeeAnErrorMessageOnTheVerificationPageSaying()

    Examples:
      | key   |
      | abcde |
        The key could not be verified
        Failed asserting that false is true.

--- Failed scenarios:

    /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUITwoFactorTOTP/keyVerification.feature:46

1 scenario (1 failed)
10 steps (6 passed, 1 failed, 3 skipped)

After rerunning the acceptance tests, 2 scenarios failed, but one of them passed on the 2nd run. This one failed 2 times: https://drone.owncloud.com/owncloud/twofactor_totp/733/11/11

  Scenario: Totp workflow                                                                               # /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUITwoFactorTOTP/keyVerification.feature:25
    Given user "newly-created-user" has logged in using the webUI                                       # WebUILoginContext::theUserHasLoggedInUsingTheWebUI()
    And the user has browsed to the personal security settings page                                     # WebUIPersonalSecuritySettingsContext::theUserBrowsesToThePersonalSecuritySettingsPage()
    And the user has activated TOTP Second-factor auth but not verified                                 # TwoFactorTOTPContext::theUserHasActivatedTOTPSecondFactorAuthButNotVerified()
    When the user adds one-time key generated from the secret key using the webUI                       # TwoFactorTOTPContext::theUserAddsVerificationKeyFromSecretKeyToVerifyUsingWebUI()
    Then the TOTP secret key should be verified on the webUI                                            # TwoFactorTOTPContext::totpSecretKeyShouldBeVerifiedOnTheWebUI()
      The key could not be verified
      Failed asserting that false is true.
    When the user re-logs in as "newly-created-user" to the two-factor authentication verification page # TwoFactorTOTPContext::theUserReLogsInAsForTwoFactorAuthentication()
    And the user adds one-time key generated from the secret key on the verification page on the webUI  # TwoFactorTOTPContext::theUserAddsOneTimeKeyGeneratedOnVerificationPageOnTheWebui()
    Then the user should be redirected to a webUI page with the title "Files - %productname%"           # WebUIGeneralContext::theUserShouldBeRedirectedToAWebUIPageWithTheTitle()
SCENARIO RESULT: (fail)

--- Failed scenarios:

    /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUITwoFactorTOTP/keyVerification.feature:25

1 scenario (1 failed)
10 steps (6 passed, 1 failed, 3 skipped)

The 3rd run of the tests had 3 fails. 2 of those passed the 2nd time, leaving 1 failure at the end: https://drone.owncloud.com/owncloud/twofactor_totp/735/31/13

  Scenario Outline: User provides wrong key on the verification page                                                        # /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUITwoFactorTOTP/keyVerification.feature:35
    Given user "newly-created-user" has logged in using the webUI                                                           # WebUILoginContext::theUserHasLoggedInUsingTheWebUI()
    And the user has browsed to the personal security settings page                                                         # WebUIPersonalSecuritySettingsContext::theUserBrowsesToThePersonalSecuritySettingsPage()
    And the user has activated TOTP Second-factor auth but not verified                                                     # TwoFactorTOTPContext::theUserHasActivatedTOTPSecondFactorAuthButNotVerified()
    When the user adds one-time key generated from the secret key using the webUI                                           # TwoFactorTOTPContext::theUserAddsVerificationKeyFromSecretKeyToVerifyUsingWebUI()
    Then the TOTP secret key should be verified on the webUI                                                                # TwoFactorTOTPContext::totpSecretKeyShouldBeVerifiedOnTheWebUI()
    When the user re-logs in as "newly-created-user" to the two-factor authentication verification page                     # TwoFactorTOTPContext::theUserReLogsInAsForTwoFactorAuthentication()
    And the user adds one-time key "<key>" on the verification page on the webUI                                            # TwoFactorTOTPContext::theUserAddsOneTimeKeyOnTheVerificationPageOnTheWebui()
    Then the user should see an error message on the verification page saying "An error occurred while verifying the token" # TwoFactorTOTPContext::theUserShouldSeeAnErrorMessageOnTheVerificationPageSaying()

    Examples:
      | key   |
      | 11111 |
        The key could not be verified
        Failed asserting that false is true.

--- Failed scenarios:

    /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUITwoFactorTOTP/keyVerification.feature:47

1 scenario (1 failed)
10 steps (6 passed, 1 failed, 3 skipped)
phil-davis commented 4 years ago

Note: PR #168 "Bump khanamiryan/qrcode-detector-decoder from 1.0.2 to 1.0.3" was merged recently. Maybe that has caused some different timing.

The diff of that dependency bump is https://github.com/khanamiryan/php-qrcode-detector-decoder/compare/1.0.2...1.0.3

But I don't see anything in that code that might be a problem.

jasson99 commented 4 years ago

I tried to run the acceptance tests after reverting PR #168 . It seems like that PR did not resulted in the tests failure.(at least not only by that PR).. We can see the tests failing still after reverting the changes in that PR as: https://drone.owncloud.com/owncloud/twofactor_totp/745

phil-davis commented 4 years ago

https://drone.owncloud.com/owncloud/twofactor_totp/794/35/13

  Scenario Outline: Administrator tries to verify OTP key for user using correct key                            # /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUIapiTwoFactorTOTP/keyVerificationByAPI.feature:14
    Given using OCS API version "<ocs_api_version>"                                                             # FeatureContext::usingOcsApiVersion()
    And user "user0" has logged in using the webUI                                                              # WebUILoginContext::theUserHasLoggedInUsingTheWebUI()
    And the user has browsed to the personal security settings page                                             # WebUIPersonalSecuritySettingsContext::theUserBrowsesToThePersonalSecuritySettingsPage()
    And the user has activated TOTP Second-factor auth but not verified                                         # TwoFactorTOTPContext::theUserHasActivatedTOTPSecondFactorAuthButNotVerified()
    When the administrator tries to verify with the one-time key generated from the secret key for user "user0" # TwoFactorTOTPContext::theAdministratorTriesToVerifyTheOtpKeyForUserUsingTheCorrectKey()
    Then the OCS status code should be "<ocs-code>"                                                             # OCSContext::theOCSStatusCodeShouldBe()
    And the HTTP status code should be "<http-code>"                                                            # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And the result of the last verification request should be true                                              # TwoFactorTOTPContext::theResultOfTheLastVerificationRequestShouldBeTrue()

    Examples:
      | ocs_api_version | ocs-code | http-code |
      | 2               | 100      | 200       |
        Invalid OTP (Exception)

--- Failed scenarios:

    /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUIapiTwoFactorTOTP/keyVerificationByAPI.feature:26

1 scenario (1 failed)
9 steps (8 passed, 1 failed)
0m35.06s (19.62Mb)

Sadly there was 1 test scenario that failed twice in a row last night. So PR #172 is an improvement, but not perfect.

phil-davis commented 4 years ago

https://drone.owncloud.com/owncloud/twofactor_totp/796/44/13

  Scenario Outline: Administrator tries to verify OTP key for user using correct key                            # /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUIapiTwoFactorTOTP/keyVerificationByAPI.feature:14
    Given using OCS API version "<ocs_api_version>"                                                             # FeatureContext::usingOcsApiVersion()
    And user "user0" has logged in using the webUI                                                              # WebUILoginContext::theUserHasLoggedInUsingTheWebUI()
    And the user has browsed to the personal security settings page                                             # WebUIPersonalSecuritySettingsContext::theUserBrowsesToThePersonalSecuritySettingsPage()
    And the user has activated TOTP Second-factor auth but not verified                                         # TwoFactorTOTPContext::theUserHasActivatedTOTPSecondFactorAuthButNotVerified()
    When the administrator tries to verify with the one-time key generated from the secret key for user "user0" # TwoFactorTOTPContext::theAdministratorTriesToVerifyTheOtpKeyForUserUsingTheCorrectKey()
    Then the OCS status code should be "<ocs-code>"                                                             # OCSContext::theOCSStatusCodeShouldBe()
    And the HTTP status code should be "<http-code>"                                                            # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And the result of the last verification request should be true                                              # TwoFactorTOTPContext::theResultOfTheLastVerificationRequestShouldBeTrue()

    Examples:
      | ocs_api_version | ocs-code | http-code |
      | 2               | 100      | 200       |
        Invalid OTP (Exception)

--- Failed scenarios:

    /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUIapiTwoFactorTOTP/keyVerificationByAPI.feature:26

1 scenario (1 failed)
9 steps (8 passed, 1 failed)

The fail is the same in keyVerificationByAPI.feature:26 - and was the same the night before.

So the same scenario has failed 3 nights in a row.

phil-davis commented 4 years ago

https://drone.owncloud.com/owncloud/twofactor_totp/816/25/11

  Background:                                                                       # /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUIapiTwoFactorTOTP/keyVerificationByAPI.feature:9
    Given these users have been created with default attributes and skeleton files: # FeatureContext::theseUsersHaveBeenCreated()
      | username |
      | user0    |

SCENARIO RESULT: (fail)
  Scenario Outline: Administrator tries to verify OTP key for user using correct key                            # /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUIapiTwoFactorTOTP/keyVerificationByAPI.feature:14
    Given using OCS API version "<ocs_api_version>"                                                             # FeatureContext::usingOcsApiVersion()
    And user "user0" has logged in using the webUI                                                              # WebUILoginContext::theUserHasLoggedInUsingTheWebUI()
    And the user has browsed to the personal security settings page                                             # WebUIPersonalSecuritySettingsContext::theUserBrowsesToThePersonalSecuritySettingsPage()
    And the user has activated TOTP Second-factor auth but not verified                                         # TwoFactorTOTPContext::theUserHasActivatedTOTPSecondFactorAuthButNotVerified()
    When the administrator tries to verify with the one-time key generated from the secret key for user "user0" # TwoFactorTOTPContext::theAdministratorTriesToVerifyTheOtpKeyForUserUsingTheCorrectKey()
    Then the OCS status code should be "<ocs-code>"                                                             # OCSContext::theOCSStatusCodeShouldBe()
    And the HTTP status code should be "<http-code>"                                                            # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And the result of the last verification request should be true                                              # TwoFactorTOTPContext::theResultOfTheLastVerificationRequestShouldBeTrue()

    Examples:
      | ocs_api_version | ocs-code | http-code |
      | 2               | 100      | 200       |
        Invalid OTP (Exception)

--- Failed scenarios:

    /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUIapiTwoFactorTOTP/keyVerificationByAPI.feature:26

1 scenario (1 failed)
9 steps (8 passed, 1 failed)

It has not failed for a while!

phil-davis commented 4 years ago

https://drone.owncloud.com/owncloud/twofactor_totp/826/23/11

  Background:                                                                       # /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUIapiTwoFactorTOTP/keyVerificationByAPI.feature:9
    Given these users have been created with default attributes and skeleton files: # FeatureContext::theseUsersHaveBeenCreated()
      | username |
      | Alice    |

SCENARIO RESULT: (fail)
  Scenario Outline: Administrator tries to verify OTP key for user using correct key                            # /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUIapiTwoFactorTOTP/keyVerificationByAPI.feature:14
    Given using OCS API version "<ocs_api_version>"                                                             # FeatureContext::usingOcsApiVersion()
    And user "Alice" has logged in using the webUI                                                              # WebUILoginContext::theUserHasLoggedInUsingTheWebUI()
    And the user has browsed to the personal security settings page                                             # WebUIPersonalSecuritySettingsContext::theUserBrowsesToThePersonalSecuritySettingsPage()
    And the user has activated TOTP Second-factor auth but not verified                                         # TwoFactorTOTPContext::theUserHasActivatedTOTPSecondFactorAuthButNotVerified()
    When the administrator tries to verify with the one-time key generated from the secret key for user "Alice" # TwoFactorTOTPContext::theAdministratorTriesToVerifyTheOtpKeyForUserUsingTheCorrectKey()
    Then the OCS status code should be "<ocs-code>"                                                             # OCSContext::theOCSStatusCodeShouldBe()
    And the HTTP status code should be "<http-code>"                                                            # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And the result of the last verification request should be true                                              # TwoFactorTOTPContext::theResultOfTheLastVerificationRequestShouldBeTrue()

    Examples:
      | ocs_api_version | ocs-code | http-code |
      | 1               | 100      | 200       |
        Invalid OTP (Exception)

--- Failed scenarios:

    /var/www/owncloud/testrunner/apps/twofactor_totp/tests/acceptance/features/webUIapiTwoFactorTOTP/keyVerificationByAPI.feature:25
karakayasemi commented 4 years ago

This commit has touched the ~key~ qr image generation logic https://github.com/owncloud/twofactor_totp/commit/0de4518651ccde953e4037b6959c97437ab57e0e . So, it may have broken something, I will investigate.

phil-davis commented 4 years ago

@karakayasemi thanks. The test fail is intermittent. So it might still be some test timing of when the new OTP is generated, and when it is immediately used in the UI, or???

karakayasemi commented 4 years ago

I suspect from the encoding format of the generated key or something similar. Since this problem did not exist in the past, I do not think any timing related problem.

karakayasemi commented 4 years ago

https://github.com/owncloud/twofactor_totp/pull/167 has added a casting operation to fix phpstan errors. However, it looks like there is a problem in the upstream method signature of this method https://github.com/ChristianRiesen/otp/blob/master/src/OtpInterface.php#L71

When the key starts with '0', typecasting to int operation loses that leading '0' and this situation leads to failure.

Fix PR in here for our repo: https://github.com/owncloud/twofactor_totp/pull/182