Redmine

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.

+1 for dual-factor authentication support

Thanks!

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

+1 for Two Factory Authentication support

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

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.

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

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

Felix, could you add tests for this feature?

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 %>

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.

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.

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

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.

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.

Go MAEDA wrote:

I have emailed Jean-Philippe Lang and got his approval to commit this patch for Redmine 4.2.0.

Great news!

Go MAEDA wrote:

I have emailed Jean-Philippe Lang and got his approval to commit this patch for Redmine 4.2.0.

That's so cool! Thank you for your efforts.