Fix/OIDC provider conflict (#711)

* fix: resolve OIDC provider conflict and add auto-link

OmniAuth returns auth.provider as symbol (:oidc) but DB
stores string ("oidc"). Comparison always triggered
ProviderConflictError. rescue_from ordering hid the
actionable message behind generic "unexpected error."

- Provider+uid-first lookup in User.from_omniauth
- .to_s coercion for type-safe comparison
- Fix rescue_from ordering (StandardError first)
- Add VULCAN_AUTO_LINK_USER global setting
- email_verified check for auto-link security
- just_auto_linked? flag (no duplicate lookup)
- Genericize oauth_error flash message
- Replace gitlab_omniauth-ldap with omniauth-ldap
- Remove nkf gem, lazy-load LDAP
- 75 backend auth specs

Authored by: Aaron Lippold<lippold@gmail.com>

* feat: session auth tracking, profile UX, unlink

- session[:auth_method] tracks HOW user signed in
- Profile shows "Signed in via X" + "Linked to Y"
- POST /users/unlink_identity with password check
- Unlink button with confirmation modal
- 15 backend + 29 frontend tests

Authored by: Aaron Lippold<lippold@gmail.com>

* fix: pre-existing bugs + infrastructure hardening

Bug fixes found during auth code review:
- vulcan_audit.rb: bitwise & vs && crash on nil rule
- users_controller: Slack fires on every update
  (now gated on saved_change_to_admin?)
- History.vue: suppress raw field changes when
  audit has a comment (link/unlink UX)
- registrations: polymorphic audit + user_type

Infrastructure:
- Ruby 3.4.8 -> 3.4.9
- parallel_sync.rake: recursion guard
- docker-compose.dev.yml: trust auth for VPN

Authored by: Aaron Lippold<lippold@gmail.com>

* docs: add what's-left tracking and beads export

- WHATS-LEFT.md: pending bug fixes, future features
- beads-export.jsonl: full task board for handoff

Authored by: Aaron Lippold<lippold@gmail.com>

* chore: fix Gemfile extra blank line (rubocop)

Authored by: Aaron Lippold<lippold@gmail.com>

* test: add TDD tests for Slack notification gate and polymorphic audit filter

71q.1: Verify Slack notification only fires on admin flag changes,
not on name/email updates. Tests promotion, demotion, and no-op cases.

71q.2: Verify registrations#edit filters audits by user_type: 'User',
excluding rogue audits with matching user_id but different user_type.


Signed-off-by: Will Dower <will@dower.dev>

* fix: restore prev_unconfirmed_email for reconfirmation flash (71q.3)

The update action read resource.unconfirmed_email but discarded the
value (no assignment). After email change, users got generic "Profile
updated" instead of "confirmation link sent to new address".

Fix: capture prev_unconfirmed_email, compare after update, show
appropriate flash message per Devise stock behavior.

TDD: red (email-change test failed with generic flash) → green.


Signed-off-by: Will Dower <will@dower.dev>

* fix: use update_columns for reset token to bypass validations (71q.4)

generate_reset_url used update! which runs full validations. If a
user has pre-existing validation failures (e.g., name exceeds a
tightened limit), the admin's reset link generation fails with
RecordInvalid — blocking an unrelated recovery flow.

Fix: use update_columns (matches Devise's save(validate: false)
pattern). Token writes carry no business logic.

TDD: red (user with 500-char name → validation error) → green.


Signed-off-by: Will Dower <will@dower.dev>

* fix: stop leaking exception.message in users_controller rescues (71q.5)

Both send_password_reset and set_password rescue blocks echoed
e.message directly to the client, leaking SMTP hostnames, DB
connection strings, or other internal details.

Fix: log full exception with backtrace server-side via
Rails.logger.error, return generic message to client.

TDD:
- send_password_reset: red (response contained 'smtp.internal.corp')
  → green (generic message returned)
- set_password: source inspection test verifies rescue block does
  not interpolate e.message (Rails test mode re-raises before
  controller rescue runs, preventing request-level testing)


Signed-off-by: Will Dower <will@dower.dev>

* fix: remove dead authProvider computed and add visibility guard (71q.6, 71q.7)

71q.6: Add source-inspection test ensuring no bare 'public' keyword
exists between private and protected sections in registrations
controller (issue was already fixed, test prevents regression).

71q.7: Remove dead authProvider computed property from UserProfile.vue.
Nothing in the template or script references it — the refactored
linkedProvider + currentSessionMethod properties cover all use cases.

TDD: red (authProvider still in computed options) → green (removed).


Signed-off-by: Will Dower <will@dower.dev>

* fix: P3/P4 hardening — backtraces, email_verified, null guard, constants, docs

71q.8: Log OmniAuth exception backtraces in all environments (was
dev-only). Use error level with first 10 frames.

71q.9: Cast email_verified OIDC claim via ActiveModel::Type::Boolean
to catch providers sending "false" (string) instead of false.

71q.10: Use falsy check in UsersTable typeColumn to handle both
null and undefined provider gracefully.

71q.11: Normalize PROJECT_MEMBER_ADMINS to array for consistency
with VIEWERS/AUTHORS/REVIEWERS. Add ROLE_ADMIN scalar constant
for attribute assignment. Update call sites.

71q.12: Document valid_password? hidden bcrypt→PBKDF2 rehash
side-effect at the unlink call site.


Signed-off-by: Will Dower <will@dower.dev>

* fix: use fake ID in vulcan_audit destroy action test

Test referenced undefined `rule` variable. Use a fake ID like the
adjacent nil-rule test — the destroy action guard skips before any
DB lookup so the ID value doesn't matter.

Signed-off-by: Will Dower <will@dower.dev>

* fix: address Copilot review — v-for key, remove dev artifacts

- Fix History.vue v-for key: changes.id is undefined, use
  changes.field with index fallback for stable Vue diffing
- Remove WHATS-LEFT.md and beads-export.jsonl (branch-specific
  tracking docs, not for the repo)

Signed-off-by: Will Dower <will@dower.dev>

* fix: properly exercise RecordNotUnique retry path in race condition test

Test claimed to verify retry-on-RecordNotUnique but never stubbed
save! to raise. Now stubs save! to raise once, pre-creates the user
with matching provider+uid so retry lookup succeeds, and asserts
save! was called exactly once before the retry found the existing user.

Signed-off-by: Will Dower <will@dower.dev>

---------

Signed-off-by: Will Dower <will@dower.dev>
Co-authored-by: Aaron Lippold <lippold@gmail.com>

Author: wdower

.ruby-version

@@ -1 +1 @@
-3.4.8
+3.4.9

.tool-versions

@@ -1,2 +1,2 @@
-ruby 3.3.9
+ruby 3.4.9
 nodejs 22.13.1

ENVIRONMENT_VARIABLES.md

@@ -59,6 +59,11 @@ DB_SUFFIX=_v3    # → vulcan_vue_development_v3, vulcan_vue_test_v3
 | `VULCAN_ENABLE_REMEMBER_ME` | Show "Remember Me" checkbox on login forms | `true` | `false` for DoD |
 | `VULCAN_REMEMBER_ME_DURATION` | How long Remember Me keeps session alive. Same format as session timeout. | `8h` | `1d`, `28800` |
 
+### Account Linking
+| Variable | Description | Default | Example |
+|----------|-------------|---------|---------|
+| `VULCAN_AUTO_LINK_USER` | Automatically link external identities (OIDC, LDAP, GitHub) to existing local accounts with the same email. Only enable when all configured identity providers verify email ownership (e.g., enterprise Okta, corporate LDAP). When `false`, users see a clear error directing them to sign in with their existing method or contact an administrator. | `false` | `true` or `false` |
+
 ### User Registration
 | Variable | Description | Default | Example |
 |----------|-------------|---------|---------|

Gemfile

@@ -2,7 +2,7 @@
 
 source 'https://rubygems.org'
 
-ruby '3.4.8'
+ruby '3.4.9'
 
 # Bundle edge Rails instead: gem 'rails', github: 'rails/rails'
 gem 'rails', '~> 8.0.0'
@@ -29,10 +29,10 @@ gem 'activerecord-session_store'
 gem 'devise-security', github: 'mitre/devise-security', ref: '9f560ed125c096205ba9434de8feea532ce97b4c'
 # Use Omniauth to support additional login providers
 gem 'omniauth', '~> 2.1'
-# LDAP Auth
-# GitLab fork with several improvements to original library. For full list of changes
-# see https://github.com/intridea/omniauth-ldap/compare/master...gitlabhq:master
-gem 'gitlab_omniauth-ldap', '~> 2.2.0', require: 'omniauth-ldap'
+# LDAP Auth — original omniauth-ldap (actively maintained, no nkf/kconv dependency).
+# Replaces gitlab_omniauth-ldap which had a dead `require 'kconv'` that triggered
+# Ruby VM crash bugs.ruby-lang.org/issues/21967 on parallel_tests forking.
+gem 'omniauth-ldap', '~> 2.3', require: false
 # Allow users to sign in with GitHub
 gem 'omniauth-github'
 # https://github.com/omniauth/omniauth/wiki/Resolving-CVE-2015-9284
@@ -81,9 +81,6 @@ gem 'caxlsx'
 # For reading excel files
 gem 'ruh-roo', '~> 3.0.0', require: 'roo'
 
-# NKF/kconv - removed from default gems in Ruby 3.4+, needed by gitlab_omniauth-ldap
-gem 'nkf'
-
 # REXML - required explicitly in Ruby 3.0+
 gem 'rexml'
 

Gemfile.lock

@@ -223,11 +223,6 @@ GEM
     foreman (0.90.0)
       thor (~> 1.4)
     fuzzyurl (0.9.0)
-    gitlab_omniauth-ldap (2.2.0)
-      net-ldap (~> 0.16)
-      omniauth (>= 1.3, < 3)
-      pyu-ruby-sasl (>= 0.0.3.3, < 0.1)
-      rubyntlm (~> 0.5)
     gli (2.22.2)
       ostruct
     globalid (1.3.0)
@@ -352,7 +347,9 @@ GEM
     net-imap (0.6.3)
       date
       net-protocol
-    net-ldap (0.19.0)
+    net-ldap (0.20.0)
+      base64
+      ostruct
     net-pop (0.1.2)
       net-protocol
     net-protocol (0.2.2)
@@ -364,7 +361,6 @@ GEM
     net-ssh (7.3.0)
     netrc (0.11.0)
     nio4r (2.7.5)
-    nkf (0.2.0)
     nokogiri (1.19.2)
       mini_portile2 (~> 2.8.2)
       racc (~> 1.4)
@@ -378,23 +374,30 @@ GEM
       racc (~> 1.4)
     nokogiri-happymapper (0.10.0)
       nokogiri (~> 1.5)
-    oauth2 (2.0.12)
+    oauth2 (2.0.18)
       faraday (>= 0.17.3, < 4.0)
       jwt (>= 1.0, < 4.0)
       logger (~> 1.2)
       multi_xml (~> 0.5)
       rack (>= 1.2, < 4)
       snaky_hash (~> 2.0, >= 2.0.3)
-      version_gem (>= 1.1.8, < 3)
+      version_gem (~> 1.1, >= 1.1.9)
     omniauth (2.1.3)
       hashie (>= 3.4.6)
       rack (>= 2.2.3)
       rack-protection
     omniauth-github (2.0.1)
       omniauth (~> 2.0)
       omniauth-oauth2 (~> 1.8)
-    omniauth-oauth2 (1.8.0)
-      oauth2 (>= 1.4, < 3)
+    omniauth-ldap (2.3.3)
+      net-ldap (~> 0.16, < 1)
+      omniauth (>= 1.2, < 3)
+      pyu-ruby-sasl (>= 0.0.3.3, < 0.1)
+      rack (>= 1, < 4)
+      rubyntlm (~> 0.6.2, < 1)
+      version_gem (~> 1.1, >= 1.1.9)
+    omniauth-oauth2 (1.9.0)
+      oauth2 (>= 2.0.2, < 3)
       omniauth (~> 2.0)
     omniauth-rails_csrf_protection (1.0.2)
       actionpack (>= 4.2)
@@ -426,11 +429,11 @@ GEM
     parslet (2.0.0)
     pastel (0.8.0)
       tty-color (~> 0.5)
-    pg (1.6.1)
-    pg (1.6.1-aarch64-linux)
-    pg (1.6.1-arm64-darwin)
-    pg (1.6.1-x86_64-linux)
-    pg (1.6.1-x86_64-linux-musl)
+    pg (1.6.3)
+    pg (1.6.3-aarch64-linux)
+    pg (1.6.3-arm64-darwin)
+    pg (1.6.3-x86_64-linux)
+    pg (1.6.3-x86_64-linux-musl)
     pg_search (2.3.7)
       activerecord (>= 6.1)
       activesupport (>= 6.1)
@@ -681,7 +684,7 @@ GEM
     validate_url (1.0.15)
       activemodel (>= 3.0.0)
       public_suffix
-    version_gem (1.1.8)
+    version_gem (1.1.9)
     warden (1.2.9)
       rack (>= 2.0.9)
     web-console (4.2.1)
@@ -739,7 +742,6 @@ DEPENDENCIES
   factory_bot_rails (~> 6.5.0)
   ffaker (~> 2.10)
   foreman
-  gitlab_omniauth-ldap (~> 2.2.0)
   haml-rails (~> 2.0)
   health_check (~> 3.1)
   highline (~> 2.0)
@@ -749,11 +751,11 @@ DEPENDENCIES
   listen (~> 3.7)
   mitre-inspec-objects
   mitre-settingslogic (~> 3.0)
-  nkf
   nokogiri
   nokogiri-happymapper
   omniauth (~> 2.1)
   omniauth-github
+  omniauth-ldap (~> 2.3)
   omniauth-rails_csrf_protection (~> 1.0)
   omniauth_openid_connect (~> 0.6.0)
   ox
@@ -790,7 +792,7 @@ DEPENDENCIES
   with_advisory_lock (~> 5.1)
 
 RUBY VERSION
-   ruby 3.4.8p72
+   ruby 3.4.9p82
 
 BUNDLED WITH
    2.3.27

app.json

@@ -82,6 +82,9 @@
         },
         "VULCAN_SEED_DEMO_DATA": {
           "value": "true"
+        },
+        "VULCAN_AUTO_LINK_USER": {
+          "value": "true"
         }
       },
       "scripts": {

app/constants/project_member_constants.rb

@@ -6,5 +6,6 @@ module ProjectMemberConstants
   PROJECT_MEMBER_VIEWERS = %w[viewer author reviewer admin].freeze
   PROJECT_MEMBER_AUTHORS = %w[author reviewer admin].freeze
   PROJECT_MEMBER_REVIEWERS = %w[reviewer admin].freeze
-  PROJECT_MEMBER_ADMINS = 'admin'
+  PROJECT_MEMBER_ADMINS = %w[admin].freeze
+  ROLE_ADMIN = 'admin'
 end

app/controllers/projects_controller.rb

@@ -24,7 +24,7 @@ def index
     @projects = current_user.available_projects.eager_load(:memberships).alphabetical.as_json(methods: %i[memberships])
     @projects.each do |project|
       project['admin'] = project['memberships'].any? do |m|
-        m['role'] == PROJECT_MEMBER_ADMINS && m['user_id'] == current_user.id
+        PROJECT_MEMBER_ADMINS.include?(m['role']) && m['user_id'] == current_user.id
       end
       project['is_member'] = project['memberships'].any? do |m|
         m['user_id'] == current_user.id
@@ -76,7 +76,7 @@ def create
     project = Project.new(
       name: new_project_params[:name],
       description: new_project_params[:description],
-      memberships_attributes: [{ user: current_user, role: PROJECT_MEMBER_ADMINS }],
+      memberships_attributes: [{ user: current_user, role: ROLE_ADMIN }],
       visibility: new_project_params[:visibility]
     )
     project.project_metadata_attributes = { data: { 'Slack Channel ID' => new_project_params[:slack_channel_id] } } if new_project_params[:slack_channel_id].present?
@@ -368,7 +368,7 @@ def perform_create_from_backup(file, include_reviews, include_memberships,
         name: project_name,
         description: project_description,
         visibility: project_visibility,
-        memberships_attributes: [{ user: current_user, role: PROJECT_MEMBER_ADMINS }]
+        memberships_attributes: [{ user: current_user, role: ROLE_ADMIN }]
       )
 
       result = Import::JsonArchiveImporter.new(

app/controllers/sessions_controller.rb

@@ -11,10 +11,15 @@ class SessionsController < Devise::SessionsController
   # Devise calls reset_session on login (session fixation protection).
   # We save the consent timestamp before and restore it after so the
   # user doesn't have to acknowledge twice (once on login page, once after).
+  #
+  # We also record session[:auth_method] AFTER super so it survives the reset.
+  # This lets the profile show "Signed in via email and password" vs "Signed in
+  # via Okta" — distinct from user.provider (which tracks linked identities).
   def create
     consent_at = session[:consent_acknowledged_at]
     super
     session[:consent_acknowledged_at] = consent_at if consent_at.present?
+    session[:auth_method] = :local if user_signed_in?
   end
 
   def destroy

app/controllers/users/omniauth_callbacks_controller.rb

@@ -5,15 +5,17 @@ module Users
   # that is hit. Currently we don't have any provider-specific code, since
   # both LDAP and Github return data in a similar enough manner.
   class OmniauthCallbacksController < Devise::OmniauthCallbacksController
-    # Comprehensive error handling for various OmniAuth failure scenarios
+    # Error handling for OmniAuth failure scenarios.
+    # Rails checks rescue_from in REVERSE order (last-defined = first-checked).
+    # StandardError must be FIRST so specific subclasses defined after it take priority.
+    rescue_from StandardError, with: :omniauth_generic_error
     rescue_from Rack::OAuth2::Client::Error, with: :oauth_error
     rescue_from OmniAuth::Strategies::OAuth2::CallbackError, with: :omniauth_callback_error
     rescue_from Timeout::Error, with: :omniauth_timeout_error
     rescue_from Faraday::TimeoutError, with: :omniauth_timeout_error
     rescue_from User::ProviderConflictError, with: :omniauth_provider_conflict
     rescue_from ArgumentError, with: :omniauth_validation_error
     rescue_from ActiveRecord::RecordInvalid, with: :omniauth_record_error
-    rescue_from StandardError, with: :omniauth_generic_error
     def all
       auth = request.env['omniauth.auth']
 
@@ -29,6 +31,18 @@ def all
 
       user = User.from_omniauth(auth)
 
+      # Notify user when their local account was auto-linked to an external provider.
+      # `just_auto_linked?` is set by User.from_omniauth (no extra DB query needed).
+      if user.just_auto_linked?
+        provider_name = auth.provider.to_s == 'oidc' ? (Settings.oidc&.title || 'OIDC') : auth.provider.to_s.upcase
+        flash.notice = "Your account has been linked to #{provider_name}. You can now sign in with either method."
+      end
+
+      # Record the auth method for this session so the profile can distinguish
+      # "Signed in via Okta" from "Signed in via email and password" — this is
+      # distinct from user.provider which records linked identities.
+      session[:auth_method] = auth.provider.to_s.to_sym
+
       # Store ID token in session for OIDC logout
       if auth.credentials&.id_token
         session[:id_token] = auth.credentials.id_token
@@ -44,7 +58,7 @@ def all
       should_remember = params[:remember_me] == '1' || omniauth_params['remember_me'] == '1'
       remember_me(user) if should_remember
 
-      flash.notice = I18n.t('devise.sessions.signed_in')
+      flash.notice ||= I18n.t('devise.sessions.signed_in')
       sign_in_and_redirect(user) && return
     end
 
@@ -53,54 +67,59 @@ def all
     alias oidc all
 
     def oauth_error(exception)
+      # Log full details server-side for debugging.
       Rails.logger.error "OAuth authentication error: #{exception.class} - #{exception.message}"
-      Rails.logger.debug exception.backtrace.join("\n") if Rails.env.development?
+      Rails.logger.error exception.backtrace&.first(10)&.join("\n")
 
-      flash.alert = "OAuth error: #{exception.message}"
-      redirect_to root_path
+      # Do NOT include exception.message in the user-facing flash — Rack::OAuth2 errors
+      # can include sensitive details (token hints, client config, redirect URIs).
+      flash.alert = 'Authentication failed. Please try again or contact your administrator.'
+      redirect_to new_user_session_path
     end
 
     def omniauth_callback_error(exception)
       Rails.logger.error "OmniAuth callback error: #{exception.class} - #{exception.message}"
-      Rails.logger.debug exception.backtrace.join("\n") if Rails.env.development?
+      Rails.logger.error exception.backtrace&.first(10)&.join("\n")
 
       flash.alert = 'Authentication failed. Please try again or contact your administrator.'
       redirect_to new_user_session_path
     end
 
     def omniauth_timeout_error(exception)
       Rails.logger.error "OmniAuth timeout error: #{exception.class} - #{exception.message}"
-      Rails.logger.debug exception.backtrace.join("\n") if Rails.env.development?
+      Rails.logger.error exception.backtrace&.first(10)&.join("\n")
 
       flash.alert = 'Authentication timed out. Please try again.'
       redirect_to new_user_session_path
     end
 
     def omniauth_provider_conflict(exception)
       Rails.logger.warn "Provider conflict: #{exception.message}"
-      flash.alert = exception.message
+      flash.alert = "#{exception.message} " \
+                    'Please sign in using your existing account, ' \
+                    'or contact an administrator to link your accounts.'
       redirect_to new_user_session_path
     end
 
     def omniauth_validation_error(exception)
       Rails.logger.error "OmniAuth validation error: #{exception.class} - #{exception.message}"
-      Rails.logger.debug exception.backtrace.join("\n") if Rails.env.development?
+      Rails.logger.error exception.backtrace&.first(10)&.join("\n")
 
       flash.alert = "Authentication failed: #{exception.message}"
       redirect_to new_user_session_path
     end
 
     def omniauth_record_error(exception)
       Rails.logger.error "OmniAuth database error: #{exception.class} - #{exception.message}"
-      Rails.logger.debug exception.backtrace.join("\n") if Rails.env.development?
+      Rails.logger.error exception.backtrace&.first(10)&.join("\n")
 
       flash.alert = 'Account creation failed. Please contact your administrator.'
       redirect_to new_user_session_path
     end
 
     def omniauth_generic_error(exception)
       Rails.logger.error "Unexpected OmniAuth error: #{exception.class} - #{exception.message}"
-      Rails.logger.debug exception.backtrace.join("\n") if Rails.env.development?
+      Rails.logger.error exception.backtrace&.first(10)&.join("\n")
 
       flash.alert = 'An unexpected error occurred during authentication. Please try again.'
       redirect_to new_user_session_path