Patch #1237

Add support for two-factor authentication

Added by Sam McCoy over 11 years ago. Updated about 1 month ago.

Status:NewStart date:2008-05-14
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:Accounts / authentication
Target version:4.2.0

Description

Please add support for a one time password service, such as Yubikey, and add the ability to authenticate against a two-factor authentication system (as in RSA SecurID).

0002-2-factor-authentication-disabled-enabled-required.patch Magnifier (13.8 KB) Felix Schäfer, 2018-01-02 18:42

0001-2-factor-authentication-using-TOTP.patch Magnifier (30 KB) Felix Schäfer, 2018-01-02 18:42

0003-Backup-codes-for-2-factor-authentication.patch Magnifier (18.8 KB) Felix Schäfer, 2018-01-02 18:42

2fa-setting@2x.png (55.4 KB) Go MAEDA, 2018-01-04 07:30

2fa-my-account@2x.png (45.5 KB) Go MAEDA, 2018-01-04 07:34

2fa-enabling@2x.png (26.1 KB) Go MAEDA, 2018-01-04 07:35

2fa-enter-auth-code@2x.png (15.4 KB) Go MAEDA, 2018-01-04 07:39

ja-translation-2fa.diff Magnifier (4.21 KB) Go MAEDA, 2018-01-29 05:19

0001-2-factor-authentication-using-TOTP.patch Magnifier (27.6 KB) Felix Schäfer, 2019-08-14 13:04

0001-adds-two-factor-authentication-support.patch Magnifier (3.14 KB) Jens Krämer, 2019-08-17 15:46

0002-adds-a-setting-to-disable-enable-require-2fa-auth.patch Magnifier (3.15 KB) Jens Krämer, 2019-08-17 15:46

0003-backup-codes-for-2fa-auth.patch Magnifier (3.13 KB) Jens Krämer, 2019-08-17 15:46

0004-adds-integration-test-for-totp-two-factor-auth.patch Magnifier (3.15 KB) Jens Krämer, 2019-08-17 15:46

0002-adds-a-setting-to-disable-enable-require-2fa-auth.patch Magnifier (13.8 KB) Jens Krämer, 2019-08-17 16:05

0001-adds-two-factor-authentication-support.patch Magnifier (30.4 KB) Jens Krämer, 2019-08-17 16:05

0004-adds-integration-test-for-totp-two-factor-auth.patch Magnifier (4.83 KB) Jens Krämer, 2019-08-17 16:05

0003-backup-codes-for-2fa-auth.patch Magnifier (18.9 KB) Jens Krämer, 2019-08-17 16:05

0001-adds-two-factor-authentication-support.patch Magnifier (30.8 KB) Jens Krämer, 2019-08-20 08:43

0002-adds-a-setting-to-disable-enable-require-2fa-auth-r18593.patch Magnifier (12.6 KB) Go MAEDA, 2019-10-05 10:44

0003-backup-codes-for-2fa-auth-r18593.patch Magnifier (19.1 KB) Go MAEDA, 2019-10-05 10:44

0004-adds-integration-test-for-totp-two-factor-auth-r18593.patch Magnifier (5.26 KB) Go MAEDA, 2019-10-05 10:45

0001-adds-two-factor-authentication-support-r18859.patch Magnifier (31.9 KB) Go MAEDA, 2019-10-24 10:35


Related issues

Related to Redmine - Feature #699: OpenID login Closed 2008-02-20
Blocks Redmine - Patch #31920: require 2FA only for certain user groups New

History

#1 Updated by Thomas Lecavelier over 11 years ago

Good idea. Since OpenID (#699) should be implemented too, it could be an idea to build a plugin system for authentication, to help introduction of a new authentification systems.

#2 Updated by Etienne Massip about 8 years ago

  • Category set to Accounts / authentication

#3 Updated by Nathanael Hansen about 5 years ago

+1 for dual-factor authentication support

Thanks!

#4 Updated by Blagoy Chepelov over 4 years ago

+1 for two factory auth, YubiKey for instance it is open source and well documented

#5 Updated by Ouss Orange over 4 years ago

+1 for Two Factory Authentication support

#6 Updated by eric c almost 4 years ago

+1 for Two Factory Authentication, maybe using the one-time-password with the Google Authenticator App

#7 Updated by Stephen Yeargin over 3 years ago

This page (http://developers-club.com/posts/168063/) describes a potential method of using a low-level service to achieve this. It looks fairly complicated to set up, but it may help somebody. Would love to hear if it works.

#8 Updated by Fabián Rodríguez almost 3 years ago

There is a plugin for 2FA including support for SMS, Telegram and authenticator apps:
http://www.redmine.org/plugins/redmine_2fa

#9 Updated by Felix Schäfer almost 2 years ago

Please find attached a proposed 2-factor authentication implementation for Redmine. The core logic for the TOTP scheme is implemented using the rotp gem.

The first of the 3 attached patches is an extendable implementation of 2-factor authentication including a default TOTP scheme. The second patch adds an option to require, optionally enable or disable 2-factor authentication for all users. The third patch adds support for creating and using backup codes.

The feature is implemented so that new 2-factor authentication schemes other than TOTP can easily be added, for example in a plugin. All scheme-specific localisation strings have a __#{scheme}__ identifier, generally this is easier to do with namespaces, i.e. twofa.#{scheme}.some_string instead of twofa__#{scheme}__some_string, but the current Redmine tooling only supports top-level localisation keys.

Please furthermore note that the base 2-factor authentication is structured so that it can accommodate computed second factors (TOTP, HOTP, Keyfobs, …) or sent second factors (sending the code via SMS, telegram, …). Those sent second factors can contain a link with the code as query parameter so that the user can click on the link instead of copying the code into the text field in Redmine, which is why all actions that accept a 2-factor authentication code are also available via the GET verb.

We have rolled out this feature to Planio a few weeks ago, the customers that use this feature have been happy with it so far.

Last but not least, we have created a plugin implementing a 2-factor email scheme to demonstrate how additional schemes can be implemented, and how a scheme sending the one-time-password instead of using a generated one can work with the patches proposed above. Please note that this plugin is only for demonstration purposes and should probably not be used in production, refer to the "Security considerations" section of the README for more details.

#10 Updated by Jan from Planio www.plan.io almost 2 years ago

  • Target version set to Candidate for next major release

#11 Updated by Jan from Planio www.plan.io almost 2 years ago

  • Tracker changed from Feature to Patch
  • Status changed from New to Needs feedback

#12 Updated by Go MAEDA almost 2 years ago

Thank Felix and Planio for sharing the patch. I think this is an outstanding enhancement because 2FA is helpful to keep accounts secure and it is a must-have feature for modern cloud-based applications. Many users should welcome this feature.

I tried out the patch. It works very fine.

2FA is optional by default. Users can choose whether to use 2FA or not:

Users can enable 2FA on "My account" page:

Enabling 2FA:

One time password is required as well as login id and password when you sign in:

#13 Updated by Go MAEDA almost 2 years ago

  • Target version changed from Candidate for next major release to 4.1.0

Setting target version to 4.1.0.

#14 Updated by Mayank Ahuja almost 2 years ago

What is the release date of 4.1.0. any tentative dates ?

#15 Updated by Go MAEDA almost 2 years ago

  • File ja-translation-2fa.diff added

I have translated messages on this patch.

#16 Updated by Go MAEDA almost 2 years ago

This is an updated Japanese translation. There were 2 untranslated strings in the previous patch.

#17 Updated by Go MAEDA almost 2 years ago

  • File deleted (ja-translation-2fa.diff)

#18 Updated by Go MAEDA almost 2 years ago

Felix, could you add tests for this feature?

#19 Updated by Enziin System over 1 year ago

Thanks Felix!

When applying for the admin account, everything is OK.

But if I applying for normal account user, its error.


Started POST "/my/twofa/totp/activate/init" for 127.0.0.1 at 2018-04-10 06:15:23 +0000
Processing by TwofaController#activate_init as HTML
Parameters: {"authenticity_token"=>"Yf7MaaWDeovRvA9IoRYE7hyAZs6aWjgvKB1ELuytNPfzuhN0IhwcIzWQ+df6llld8ksBuclPKCVepCwo0OsAhA==", "scheme"=>"totp"}
Current user: kevin-nguyen (id=6)
Redirected to https://www.enziin.com/my/twofa/totp/activate/confirm
Completed 302 Found in 73ms (ActiveRecord: 12.5ms)
Started GET "/my/twofa/totp/activate/confirm" for 127.0.0.1 at 2018-04-10 06:15:23 +0000
Processing by TwofaController#activate_confirm as HTML
Parameters: {"scheme"=>"totp"}
Current user: kevin-nguyen (id=6)
Rendered twofa/totp/_new.html.erb (422.5ms)
Rendered my/_sidebar.html.erb (14.1ms)
Rendered twofa/activate_confirm.html.erb within layouts/base (459.4ms)
Completed 500 Internal Server Error in 478ms (ActiveRecord: 10.3ms)

ActionView::Template::Error (No route matches {:action=>"destroy", :controller=>"twofa", :scheme=>"totp"}):
4: <%=l(:field_created_on)%>: <%= format_time(@user.created_on) ></p>
5:
6: <
if @user.own_account_deletable? >
7: <p><
= link_to(l(:button_delete_my_account), {:action => 'destroy'}, :class => 'icon icon-del') ></p>
8: <
end >
9:
10: <h4><
= l(:label_feeds_access_key) %></h4>
app/views/my/_sidebar.html.erb:7:in `_app_views_my__sidebar_html_erb___731617039739943373_70003011594380'
app/views/twofa/activate_confirm.html.erb:22:in `block in app_views_twofa_activate_confirm_html_erb_76021097380460310_46976643820260'
app/views/twofa/activate_confirm.html.erb:21:in `_app_views_twofa_activate_confirm_html_erb__76021097380460310_46976643820260'
lib/redmine/sudo_mode.rb:63:in `sudo_mode'


I fixed it:

In the "my/_sidebar.html.erb" by adding :controller=>"my"

<% if @user.own_account_deletable? >
<p><
= link_to(l(:button_delete_my_account), {:action => 'destroy', :controller=>"my"}, :class => 'icon icon-del') ></p>
<
end %>

#20 Updated by Go MAEDA 6 months ago

  • Target version changed from 4.1.0 to 4.2.0

#21 Updated by Felix Schäfer 4 months ago

The rotp gem has changed its API, the attached 0001 patch replaces the original patch and fixes the API changes with rotp.

#22 Updated by Felix Schäfer 4 months ago

Enziin System wrote:

But if I applying for normal account user, its error.

I can see what causes the error. The method to create the link the user can use to delete the user's own account expects the my/_sidebar.html.erb partial to be used only on /my/account or /my/password pages.

I think this should be made more explicit/fixed in Redmine core so that other plugins that may use my/_sidebar.html.erb do not break either. Maeda-san, do you agree? If yes I can open an issue and propose a patch for my/_sidebar.html.erb.

#23 Updated by Jens Krämer 4 months ago

I rebased the patch series on top of current master and added an integration test covering the basic activation / totp and backup code functionality. I also fixed the above problem by making the account deletion link more explicit.

#24 Updated by Marius BALTEANU 4 months ago

Jens Krämer wrote:

I rebased the patch series on top of current master and added an integration test covering the basic activation / totp and backup code functionality. I also fixed the above problem by making the account deletion link more explicit.

Jens, I think something went wrong because your patches are empty.

#26 Updated by Jens Krämer 4 months ago

the fifth patch of the series actually adds a new feature on top of this so I created a separate issue for it: #31920

#27 Updated by Go MAEDA 4 months ago

  • Blocks Patch #31920: require 2FA only for certain user groups added

#28 Updated by Jens Krämer 4 months ago

Updated first patch that adds a require statement necessary for the patch to work in production.

#29 Updated by Go MAEDA 2 months ago

  • File 0001-adds-two-factor-authentication-support-r18547.patch added
  • File 0002-adds-a-setting-to-disable-enable-require-2fa-auth-r18547.patch added

I have slightly fixed two of the four patches to apply to the latest trunk, r18547. You can add two-factor authentication by applying the following patches.

#30 Updated by Go MAEDA 2 months ago

  • Status changed from Needs feedback to New
  • Assignee set to Jean-Philippe Lang
  • Target version changed from 4.2.0 to 4.1.0

Two-factor authentication is becoming a must-have feature nowadays. Thanks to Planio, now there is a complete and ready-to-commit patch with test code.

I think it is the time to commit the patch. Setting the target version to 4.1.0.

#31 Updated by Marius BALTEANU 2 months ago

Go MAEDA wrote:

Two-factor authentication is becoming a must-have feature nowadays. Thanks to Planio, now there is a complete and ready-to-commit patch with test code.

I thik it is the time to commit the patch. Setting the target version to 4.1.0.

Totally agree with you regarding the need for this feature.

Regarding the patches, I've just tested the entire work and it looks very good to me. All the tests pass on MySQL 5.7 and Postgres 10.9 (https://gitlab.com/redmine-org/redmine/pipelines/85148957).

I've found only some minor things:

Code related:
1. The db migration version should be 5.2 in the migration files. Also, the filenames should be updated to the current date before committing.

Mariuss-MacBook-Pro:redmine mariusbalteanu$ git diff
diff --git a/db/migrate/20170711134351_add_twofa_scheme_to_user.rb b/db/migrate/20170711134351_add_twofa_scheme_to_user.rb
index ac3f49717..ea0b48fc8 100644
--- a/db/migrate/20170711134351_add_twofa_scheme_to_user.rb
+++ b/db/migrate/20170711134351_add_twofa_scheme_to_user.rb
@@ -1,4 +1,4 @@
-class AddTwofaSchemeToUser < ActiveRecord::Migration[4.2]
+class AddTwofaSchemeToUser < ActiveRecord::Migration[5.2]
   def change
     add_column :users, :twofa_scheme, :string
   end
diff --git a/db/migrate/20170711134352_add_totp_to_user.rb b/db/migrate/20170711134352_add_totp_to_user.rb
index d24bdba86..6842878e3 100644
--- a/db/migrate/20170711134352_add_totp_to_user.rb
+++ b/db/migrate/20170711134352_add_totp_to_user.rb
@@ -1,4 +1,4 @@
-class AddTotpToUser < ActiveRecord::Migration[4.2]
+class AddTotpToUser < ActiveRecord::Migration[5.2]
   def change
     add_column :users, :twofa_totp_key, :string
     add_column :users, :twofa_totp_last_used_at, :integer

2. The following new Rubocop warnings are introduced:

Offenses:

app/controllers/account_controller.rb:255:5: C: Rails/Blank: Use if @user.blank? instead of unless @user.present?.
    unless @user.present?
    ^^^^^^^^^^^^^^^^^^^^^
app/controllers/twofa_backup_codes_controller.rb:1:1: C: Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
class TwofaBackupCodesController < ApplicationController
^
app/controllers/twofa_controller.rb:1:1: C: Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
class TwofaController < ApplicationController
^
app/controllers/twofa_controller.rb:78:5: C: Rails/Blank: Use if twofa_scheme.blank? instead of unless twofa_scheme.present?.
    unless twofa_scheme.present?
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/helpers/twofa_helper.rb:1:1: C: Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
module TwofaHelper
^
lib/redmine/twofa.rb:1:1: C: Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
module Redmine
^
lib/redmine/twofa.rb:25:5: W: Lint/UselessAccessModifier: Useless private access modifier.
    private
    ^^^^^^^
lib/redmine/twofa/base.rb:1:1: C: Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
module Redmine
^
lib/redmine/twofa/base.rb:97:29: C: Style/ParenthesesAroundCondition: Don't use parentheses around the condition of an unless.
        return false unless (@user.present? && @user == user_from_code)
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/redmine/twofa/totp.rb:1:1: C: Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
module Redmine
^
test/integration/twofa_test.rb:1:1: C: Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
require File.expand_path('../../test_helper', __FILE__)
^
test/integration/twofa_test.rb:124:1: C: Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body end.

3. Redmine copywriting "# Redmine - project management software etc" is missing from the files added by these patches.

Functional:
4. Users should not be allowed to disable 2FA authentication when the "Two-factor authentication" is set to "required" in administration. Currently, the user can disable it, but at the next login, he is be prompted again to configure 2FA. I propose to not allow users to disable 2FA authentication from the beginning.

#32 Updated by Marius BALTEANU 2 months ago

  • Assignee changed from Jean-Philippe Lang to Go MAEDA
  • Target version changed from 4.1.0 to 4.2.0

Please reassign this to 4.1.0 after the above issues are fixed.

#33 Updated by Go MAEDA 2 months ago

Marius, thank you for reviewing the patch series. Here are patches to commit:

Marius BALTEANU wrote:

1. The db migration version should be 5.2 in the migration files. Also, the filenames should be updated to the current date before committing.

Applied the patch you posted in #1237#note-31. But the filenames are not changed yet.

2. The following new Rubocop warnings are introduced:

All RuboCop warnings are fixed.

3. Redmine copywriting "# Redmine - project management software etc" is missing from the files added by these patches.

Done.

4. Users should not be allowed to disable 2FA authentication when the "Two-factor authentication" is set to "required" in administration. Currently, the user can disable it, but at the next login, he is be prompted again to configure 2FA. I propose to not allow users to disable 2FA authentication from the beginning.

I think the behavior should be kept. It is necessary to allow users to discard the current 2FA config and set up a new config by themself.

Assume that a user lost their authentication device but they have backup codes, and they want to set up a new authentication device. They can discard the 2FA config for the lost device and configure 2FA for the new device without the help of admins by the following steps:

1. Login with id, password, and a backup code
2. Go to "My account" and Disable 2FA (another backup code is required)
3. Logout
4. Login with id and password. Redmine requires the user to configure 2FA
5. Configure 2FA for the new device

#34 Updated by Marius BALTEANU about 1 month ago

Thanks for updating the patches, I can confirm that all code related issues have been fixed.

One more thing, it seems (at least on my environment) that the patches doesn't apply cleanly anymore:


mariusbalteanu@Mariuss-MacBook-Pro redmine % curl http://www.redmine.org/attachments/download/24068/0001-adds-two-factor-authentication-support-r18593.patch | patch -p1 
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 32734    0 32734    0     0   138k      0 --:--:-- --:--:-- --:--:--  138k
patching file Gemfile
Hunk #1 succeeded at 21 (offset 3 lines).
patching file app/controllers/account_controller.rb
patching file app/controllers/twofa_controller.rb
patching file app/models/user.rb
patching file app/views/account/twofa_confirm.html.erb
patching file app/views/my/_sidebar.html.erb
patching file app/views/my/account.html.erb
patching file app/views/twofa/activate_confirm.html.erb
patching file app/views/twofa/deactivate_confirm.html.erb
patching file app/views/twofa/totp/_new.html.erb
patching file app/views/users/_form.html.erb
patching file config/locales/de.yml
Hunk #2 FAILED at 1299.
1 out of 2 hunks FAILED -- saving rejects to file config/locales/de.yml.rej
patching file config/locales/en.yml
Hunk #1 succeeded at 1101 (offset 1 line).
Hunk #2 succeeded at 1281 (offset 1 line).
patching file config/routes.rb
patching file db/migrate/20170711134351_add_twofa_scheme_to_user.rb
patching file db/migrate/20170711134352_add_totp_to_user.rb
patching file lib/redmine.rb
patching file lib/redmine/twofa.rb
patching file lib/redmine/twofa/base.rb
patching file lib/redmine/twofa/totp.rb
patching file public/stylesheets/application.css
patching file public/stylesheets/responsive.css

Can you check, please? I think it is because of r18596.

#35 Updated by Go MAEDA about 1 month ago

  • File 0001-adds-two-factor-authentication-support-r18640.patch added
  • Assignee changed from Go MAEDA to Jean-Philippe Lang

Marius, thank you for reviewing the patch series. I have updated one of the four patches.

Patches to commit:

#36 Updated by Go MAEDA about 1 month ago

  • File deleted (0001-adds-two-factor-authentication-support-r18640.patch)

#37 Updated by Go MAEDA about 1 month ago

  • File deleted (0001-adds-two-factor-authentication-support-r18593.patch)

#38 Updated by Go MAEDA about 1 month ago

  • File deleted (0001-adds-two-factor-authentication-support-r18547.patch)

#39 Updated by Go MAEDA about 1 month ago

  • File deleted (0002-adds-a-setting-to-disable-enable-require-2fa-auth-r18547.patch)

#41 Updated by Jean-Philippe Lang about 1 month ago

  • Target version changed from 4.1.0 to 5.0.0

Reassigning this huge change to the next major.

#42 Updated by Go MAEDA about 1 month ago

  • Target version changed from 5.0.0 to 4.2.0

Jean-Philippe Lang wrote:

Reassigning this huge change to the next major.

I think the next major release is 4.2.0.

Also available in: Atom PDF