FEATURE: require admins to re-validate their email addresses if they haven't been seen for a number of days, configurable with the invalidate_inactive_admin_email_after_days site setting. Social logins are also revoked. Default is 365 days.

FEATURE: require admins to re-validate their email addresses if they haven't been seen for a number of days, configurable with the invalidate_inactive_admin_email_after_days site setting. Social logins are also revoked. Default is 365 days.
diff --git a/app/jobs/scheduled/invalidate_inactive_admins.rb b/app/jobs/scheduled/invalidate_inactive_admins.rb
new file mode 100644
index 0000000..c31cea4
--- /dev/null
+++ b/app/jobs/scheduled/invalidate_inactive_admins.rb
@@ -0,0 +1,25 @@
+module Jobs
+
+  class InvalidateInactiveAdmins < Jobs::Scheduled
+    every 1.day
+
+    def execute(_)
+      return if SiteSetting.invalidate_inactive_admin_email_after_days == 0
+
+      User.human_users
+        .where(admin: true)
+        .where('last_seen_at < ?', SiteSetting.invalidate_inactive_admin_email_after_days.days.ago)
+        .each do |user|
+
+        user.email_tokens.update_all(confirmed: false, expired: true)
+
+        Discourse.authenticators.each do |authenticator|
+          if authenticator.can_revoke? && authenticator.description_for_user(user).present?
+            authenticator.revoke(user)
+          end
+        end
+      end
+    end
+  end
+
+end
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 44ea490..4b1d90e 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1299,6 +1299,7 @@ en:
     content_security_policy_report_only: EXPERIMENTAL - Turn on Content-Security-Policy-Report-Only
     content_security_policy_collect_reports: Enable CSP violation report collection at /csp_reports
     content_security_policy_script_src: Additional whitelisted script sources. The current host and CDN are included by default.
+    invalidate_inactive_admin_email_after_days: "Admin accounts that have not visited the site in this number of days will need to re-validate their email address before logging in. Set to 0 to disable."
     top_menu: "Determine which items appear in the homepage navigation, and in what order. Example latest|new|unread|categories|top|read|posted|bookmarks"
     post_menu: "Determine which items appear on the post menu, and in what order. Example like|edit|flag|delete|share|bookmark|reply"
     post_menu_hidden_items: "The menu items to hide by default in the post menu unless an expansion ellipsis is clicked on."
diff --git a/config/site_settings.yml b/config/site_settings.yml
index ebb1930..7be81be 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -1260,6 +1260,10 @@ security:
   content_security_policy_script_src:
     type: list
     default: ''
+  invalidate_inactive_admin_email_after_days:
+    default: 365
+    min: 0
+    max: 2000
 
 onebox:
   enable_flash_video_onebox: false
diff --git a/spec/jobs/invalidate_inactive_admins_spec.rb b/spec/jobs/invalidate_inactive_admins_spec.rb
new file mode 100644
index 0000000..1e8bbae
--- /dev/null
+++ b/spec/jobs/invalidate_inactive_admins_spec.rb
@@ -0,0 +1,69 @@
+require 'rails_helper'
+
+require_dependency 'jobs/scheduled/invalidate_inactive_admins'
+
+describe Jobs::InvalidateInactiveAdmins do
+  let!(:active_admin) { Fabricate(:admin, last_seen_at: 1.hour.ago) }
+  before { active_admin.email_tokens.update_all(confirmed: true) }
+
+  subject { Jobs::InvalidateInactiveAdmins.new.execute({}) }
+
+  it "does nothing when all admins have been seen recently" do
+    SiteSetting.invalidate_inactive_admin_email_after_days = 365
+    subject
+    expect(active_admin.active).to eq(true)
+    expect(active_admin.email_tokens.where(confirmed: true).exists?).to eq(true)
+  end
+
+  context "with an admin who hasn't been seen recently" do
+    let!(:not_seen_admin) { Fabricate(:admin, last_seen_at: 370.days.ago) }
+    before { not_seen_admin.email_tokens.update_all(confirmed: true) }
+
+    context 'invalidate_inactive_admin_email_after_days = 365' do
+      before do
+        SiteSetting.invalidate_inactive_admin_email_after_days = 365
+      end
+
+      it 'marks email tokens as unconfirmed and keeps user as active' do
+        subject
+        expect(not_seen_admin.email_tokens.where(confirmed: true).exists?).to eq(false)
+      end
+
+      it 'keeps the user active' do
+        subject
+        expect(not_seen_admin.active).to eq(true)
+      end
+
+      context 'with social logins' do
+        before do
+          GithubUserInfo.create!(user_id: not_seen_admin.id, screen_name: 'bob', github_user_id: 100)
+          InstagramUserInfo.create!(user_id: not_seen_admin.id, screen_name: 'bob', instagram_user_id: 'examplel123123')
+          UserOpenId.create!(url: 'https://me.yahoo.com/id/123' , user_id: not_seen_admin.id, email: 'bob@example.com', active: true)
+          GoogleUserInfo.create!(user_id: not_seen_admin.id, google_user_id: 100, email: 'bob@example.com')
+        end
+
+        it 'removes the social logins' do
+          subject
+          expect(GithubUserInfo.where(user_id: not_seen_admin.id).exists?).to eq(false)
+          expect(InstagramUserInfo.where(user_id: not_seen_admin.id).exists?).to eq(false)
+          expect(GoogleUserInfo.where(user_id: not_seen_admin.id).exists?).to eq(false)
+          expect(UserOpenId.where(user_id: not_seen_admin.id).exists?).to eq(false)
+        end
+      end
+    end
+
+    context 'invalidate_inactive_admin_email_after_days = 0 to disable this feature' do
+      before do
+        SiteSetting.invalidate_inactive_admin_email_after_days = 0
+      end
+
+      it 'does nothing' do
+        subject
+        expect(active_admin.active).to eq(true)
+        expect(active_admin.email_tokens.where(confirmed: true).exists?).to eq(true)
+        expect(not_seen_admin.active).to eq(true)
+        expect(not_seen_admin.email_tokens.where(confirmed: true).exists?).to eq(true)
+      end
+    end
+  end
+end

GitHub

1 Like

I don’t feel this is quite strong enough. Why not update the admin as active = false? This change only means they will need to re-connect social logins, they will still be able to login using username/password without email confirmation as far as I can tell.

It was my understanding that Neil was taking the other bit?

On Wed, Dec 12, 2018 at 12:49 PM Sam notifications@github.com wrote:

I don’t feel this is quite strong enough. Why not update the admin as active
= false? This change only means they will need to re-connect social
logins, they will still be able to login using username/password without
email confirmation as far as I can tell.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/discourse/discourse/commit/a1db15feadc7951d8a2b4ae63384babd6c568ae0#commitcomment-31657133,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABc7VauTew2jCsJ7NkzIgkxtnrbP9xEjks5u4WvogaJpZM4ZQbAU
.

What other bit?

That’s not the case in my testing. The behaviour is identical to deactivating. (Both cases show the same modal as if you had signed up but not clicked the activation link yet, which is not great.)

Followed up here: FIX: invalidating inactive admin emails should mark them as not active

looks good to me!

2 Likes