From 14993d871c8578d817b113091e729512ce0e940a Mon Sep 17 00:00:00 2001 From: Josh Waldrep Date: Sat, 3 Jan 2026 17:56:42 -0500 Subject: [PATCH 1/6] feat: comprehensive SSO/OIDC upgrade with enterprise features Multi-provider SSO support: - Database-backed SSO provider management with admin UI - Support for OpenID Connect, Google OAuth2, GitHub, and SAML 2.0 - Flipper feature flag (db_sso_providers) for dynamic provider loading - ProviderLoader service for YAML or database configuration Admin functionality: - Admin::SsoProvidersController for CRUD operations - Admin::UsersController for super_admin role management - Pundit policies for authorization - Test connection endpoint for validating provider config User provisioning improvements: - JIT (just-in-time) account creation with configurable default role - Changed default JIT role from admin to member (security) - User attribute sync on each SSO login - Group/role mapping from IdP claims SSO identity management: - Settings::SsoIdentitiesController for users to manage connected accounts - Issuer validation for OIDC identities - Unlink protection when no password set Audit logging: - SsoAuditLog model tracking login, logout, link, unlink, JIT creation - Captures IP address, user agent, and metadata Advanced OIDC features: - Custom scopes per provider - Configurable prompt parameter (login, consent, select_account, none) - RP-initiated logout (federated logout to IdP) - id_token storage for logout SAML 2.0 support: - omniauth-saml gem integration - IdP metadata URL or manual configuration - Certificate and fingerprint validation - NameID format configuration --- Gemfile | 8 +- Gemfile.lock | 17 ++ app/controllers/admin/base_controller.rb | 16 + .../admin/sso_providers_controller.rb | 157 ++++++++++ app/controllers/admin/users_controller.rb | 38 +++ app/controllers/application_controller.rb | 6 + app/controllers/oidc_accounts_controller.rb | 20 +- app/controllers/sessions_controller.rb | 108 ++++++- .../settings/sso_identities_controller.rb | 35 +++ app/helpers/settings_helper.rb | 5 + .../controllers/admin_sso_form_controller.js | 226 ++++++++++++++ app/middleware/omniauth_error_handler.rb | 29 ++ app/models/oidc_identity.rb | 83 ++++++ app/models/sso_audit_log.rb | 108 +++++++ app/models/sso_provider.rb | 144 +++++++++ app/models/sso_provider_tester.rb | 201 +++++++++++++ app/policies/application_policy.rb | 53 ++++ app/policies/sso_provider_policy.rb | 46 +++ app/policies/user_policy.rb | 24 ++ app/services/provider_loader.rb | 87 ++++++ app/views/admin/sso_providers/_form.html.erb | 277 ++++++++++++++++++ app/views/admin/sso_providers/edit.html.erb | 9 + app/views/admin/sso_providers/index.html.erb | 88 ++++++ app/views/admin/sso_providers/new.html.erb | 9 + app/views/admin/users/index.html.erb | 73 +++++ app/views/settings/_settings_nav.html.erb | 4 +- .../settings/sso_identities/show.html.erb | 59 ++++ config/application.rb | 4 + config/auth.yml | 14 + config/brakeman.ignore | 23 ++ config/initializers/flipper.rb | 45 +++ config/initializers/omniauth.rb | 162 ++++++++-- config/initializers/rack_attack.rb | 6 + .../locales/views/admin/sso_providers/en.yml | 109 +++++++ config/locales/views/admin/users/en.yml | 22 ++ config/locales/views/sessions/en.yml | 3 + .../views/settings/sso_identities/en.yml | 22 ++ config/routes.rb | 16 +- .../20251228181150_create_flipper_tables.rb | 22 ++ .../20251228181429_create_sso_providers.rb | 21 ++ ...228182113_add_issuer_to_oidc_identities.rb | 6 + .../20260103170412_create_sso_audit_logs.rb | 18 ++ db/schema.rb | 53 +++- docs/hosting/oidc.md | 235 +++++++++++++++ lib/tasks/sso_providers.rake | 154 ++++++++++ .../oidc_accounts_controller_test.rb | 2 +- test/models/sso_provider_test.rb | 263 +++++++++++++++++ test/policies/sso_provider_policy_test.rb | 111 +++++++ test/policies/user_policy_test.rb | 59 ++++ test/test_helper.rb | 1 + 50 files changed, 3267 insertions(+), 34 deletions(-) create mode 100644 app/controllers/admin/base_controller.rb create mode 100644 app/controllers/admin/sso_providers_controller.rb create mode 100644 app/controllers/admin/users_controller.rb create mode 100644 app/controllers/settings/sso_identities_controller.rb create mode 100644 app/javascript/controllers/admin_sso_form_controller.js create mode 100644 app/middleware/omniauth_error_handler.rb create mode 100644 app/models/sso_audit_log.rb create mode 100644 app/models/sso_provider.rb create mode 100644 app/models/sso_provider_tester.rb create mode 100644 app/policies/application_policy.rb create mode 100644 app/policies/sso_provider_policy.rb create mode 100644 app/policies/user_policy.rb create mode 100644 app/services/provider_loader.rb create mode 100644 app/views/admin/sso_providers/_form.html.erb create mode 100644 app/views/admin/sso_providers/edit.html.erb create mode 100644 app/views/admin/sso_providers/index.html.erb create mode 100644 app/views/admin/sso_providers/new.html.erb create mode 100644 app/views/admin/users/index.html.erb create mode 100644 app/views/settings/sso_identities/show.html.erb create mode 100644 config/initializers/flipper.rb create mode 100644 config/locales/views/admin/sso_providers/en.yml create mode 100644 config/locales/views/admin/users/en.yml create mode 100644 config/locales/views/settings/sso_identities/en.yml create mode 100644 db/migrate/20251228181150_create_flipper_tables.rb create mode 100644 db/migrate/20251228181429_create_sso_providers.rb create mode 100644 db/migrate/20251228182113_add_issuer_to_oidc_identities.rb create mode 100644 db/migrate/20260103170412_create_sso_audit_logs.rb create mode 100644 lib/tasks/sso_providers.rake create mode 100644 test/models/sso_provider_test.rb create mode 100644 test/policies/sso_provider_policy_test.rb create mode 100644 test/policies/user_policy_test.rb diff --git a/Gemfile b/Gemfile index 91cdfcd8b..0658791a6 100644 --- a/Gemfile +++ b/Gemfile @@ -59,6 +59,7 @@ gem "countries" # OAuth & API Security gem "doorkeeper" gem "rack-attack", "~> 6.6" +gem "pundit" gem "faraday" gem "faraday-retry" gem "faraday-multipart" @@ -77,17 +78,22 @@ gem "rqrcode", "~> 3.0" gem "activerecord-import" gem "rubyzip", "~> 2.3" -# OpenID Connect & OAuth authentication +# OpenID Connect, OAuth & SAML authentication gem "omniauth", "~> 2.1" gem "omniauth-rails_csrf_protection" gem "omniauth_openid_connect" gem "omniauth-google-oauth2" gem "omniauth-github" +gem "omniauth-saml", "~> 2.1" # State machines gem "aasm" gem "after_commit_everywhere", "~> 1.0" +# Feature flags +gem "flipper" +gem "flipper-active_record" + # AI gem "ruby-openai" gem "langfuse-ruby", "~> 0.1.4", require: "langfuse" diff --git a/Gemfile.lock b/Gemfile.lock index 7a4f3959e..d251bef9a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -223,6 +223,11 @@ GEM ffi (1.17.2-x86_64-darwin) ffi (1.17.2-x86_64-linux-gnu) ffi (1.17.2-x86_64-linux-musl) + flipper (1.3.6) + concurrent-ruby (< 2) + flipper-active_record (1.3.6) + activerecord (>= 4.2, < 9) + flipper (~> 1.3.6) foreman (0.88.1) fugit (1.11.1) et-orbi (~> 1, >= 1.2.11) @@ -420,6 +425,9 @@ GEM omniauth-rails_csrf_protection (1.0.2) actionpack (>= 4.2) omniauth (~> 2.0) + omniauth-saml (2.2.4) + omniauth (~> 2.1) + ruby-saml (~> 1.18) omniauth_openid_connect (0.8.0) omniauth (>= 1.9, < 3) openid_connect (~> 2.2) @@ -466,6 +474,8 @@ GEM public_suffix (6.0.2) puma (6.6.0) nio4r (~> 2.0) + pundit (2.5.2) + activesupport (>= 3.0.0) raabro (1.4.0) racc (1.8.1) rack (3.1.18) @@ -619,6 +629,9 @@ GEM faraday (>= 1) faraday-multipart (>= 1) ruby-progressbar (1.13.0) + ruby-saml (1.18.1) + nokogiri (>= 1.13.10) + rexml ruby-statistics (4.1.0) ruby-vips (2.2.4) ffi (~> 1.12) @@ -774,6 +787,8 @@ DEPENDENCIES faraday faraday-multipart faraday-retry + flipper + flipper-active_record foreman hotwire-livereload hotwire_combobox @@ -795,6 +810,7 @@ DEPENDENCIES omniauth-github omniauth-google-oauth2 omniauth-rails_csrf_protection + omniauth-saml (~> 2.1) omniauth_openid_connect ostruct pagy @@ -803,6 +819,7 @@ DEPENDENCIES posthog-ruby propshaft puma (>= 5.0) + pundit rack-attack (~> 6.6) rack-mini-profiler rails (~> 7.2.2) diff --git a/app/controllers/admin/base_controller.rb b/app/controllers/admin/base_controller.rb new file mode 100644 index 000000000..7e0252491 --- /dev/null +++ b/app/controllers/admin/base_controller.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Admin + class BaseController < ApplicationController + before_action :require_super_admin! + + layout "settings" + + private + def require_super_admin! + unless Current.user&.super_admin? + redirect_to root_path, alert: t("admin.unauthorized") + end + end + end +end diff --git a/app/controllers/admin/sso_providers_controller.rb b/app/controllers/admin/sso_providers_controller.rb new file mode 100644 index 000000000..d47864f43 --- /dev/null +++ b/app/controllers/admin/sso_providers_controller.rb @@ -0,0 +1,157 @@ +# frozen_string_literal: true + +module Admin + class SsoProvidersController < Admin::BaseController + before_action :set_sso_provider, only: %i[show edit update destroy toggle test_connection] + + def index + authorize SsoProvider + @sso_providers = policy_scope(SsoProvider).order(:name) + end + + def show + authorize @sso_provider + end + + def new + @sso_provider = SsoProvider.new + authorize @sso_provider + end + + def create + @sso_provider = SsoProvider.new(processed_params) + authorize @sso_provider + + # Auto-generate redirect_uri if not provided + if @sso_provider.redirect_uri.blank? && @sso_provider.name.present? + @sso_provider.redirect_uri = "#{request.base_url}/auth/#{@sso_provider.name}/callback" + end + + if @sso_provider.save + log_provider_change(:create, @sso_provider) + clear_provider_cache + redirect_to admin_sso_providers_path, notice: t(".success") + else + render :new, status: :unprocessable_entity + end + end + + def edit + authorize @sso_provider + end + + def update + authorize @sso_provider + + # Auto-update redirect_uri if name changed + params_hash = processed_params.to_h + if params_hash[:name].present? && params_hash[:name] != @sso_provider.name + params_hash[:redirect_uri] = "#{request.base_url}/auth/#{params_hash[:name]}/callback" + end + + if @sso_provider.update(params_hash) + log_provider_change(:update, @sso_provider) + clear_provider_cache + redirect_to admin_sso_providers_path, notice: t(".success") + else + render :edit, status: :unprocessable_entity + end + end + + def destroy + authorize @sso_provider + + @sso_provider.destroy! + log_provider_change(:destroy, @sso_provider) + clear_provider_cache + + redirect_to admin_sso_providers_path, notice: t(".success") + end + + def toggle + authorize @sso_provider + + @sso_provider.update!(enabled: !@sso_provider.enabled) + log_provider_change(:toggle, @sso_provider) + clear_provider_cache + + notice = @sso_provider.enabled? ? t(".success_enabled") : t(".success_disabled") + redirect_to admin_sso_providers_path, notice: notice + end + + def test_connection + authorize @sso_provider + + tester = SsoProviderTester.new(@sso_provider) + result = tester.test! + + render json: { + success: result.success?, + message: result.message, + details: result.details + } + end + + private + def set_sso_provider + @sso_provider = SsoProvider.find(params[:id]) + end + + def sso_provider_params + params.require(:sso_provider).permit( + :strategy, + :name, + :label, + :icon, + :enabled, + :issuer, + :client_id, + :client_secret, + :redirect_uri, + :scopes, + :prompt, + settings: [ + :default_role, :scopes, :prompt, + # SAML settings + :idp_metadata_url, :idp_sso_url, :idp_slo_url, + :idp_certificate, :idp_cert_fingerprint, :name_id_format, + role_mapping: {} + ] + ) + end + + # Process params to convert role_mapping comma-separated strings to arrays + def processed_params + result = sso_provider_params.to_h + + if result[:settings].present? && result[:settings][:role_mapping].present? + result[:settings][:role_mapping] = result[:settings][:role_mapping].transform_values do |v| + # Convert comma-separated string to array, removing empty values + v.to_s.split(",").map(&:strip).reject(&:blank?) + end + + # Remove empty role mappings + result[:settings][:role_mapping] = result[:settings][:role_mapping].reject { |_, v| v.empty? } + result[:settings].delete(:role_mapping) if result[:settings][:role_mapping].empty? + end + + result + end + + def log_provider_change(action, provider) + Rails.logger.info( + "[Admin::SsoProviders] #{action.to_s.upcase} - " \ + "user_id=#{Current.user.id} " \ + "provider_id=#{provider.id} " \ + "provider_name=#{provider.name} " \ + "strategy=#{provider.strategy} " \ + "enabled=#{provider.enabled}" + ) + end + + def clear_provider_cache + ProviderLoader.clear_cache + Rails.logger.info("[Admin::SsoProviders] Provider cache cleared by user_id=#{Current.user.id}") + end + end +end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb new file mode 100644 index 000000000..fdbc6e281 --- /dev/null +++ b/app/controllers/admin/users_controller.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Admin + class UsersController < Admin::BaseController + before_action :set_user, only: %i[update] + + def index + authorize User + @users = policy_scope(User).order(:email) + end + + def update + authorize @user + + if @user.update(user_params) + Rails.logger.info( + "[Admin::Users] Role changed - " \ + "by_user_id=#{Current.user.id} " \ + "target_user_id=#{@user.id} " \ + "new_role=#{@user.role}" + ) + redirect_to admin_users_path, notice: t(".success") + else + redirect_to admin_users_path, alert: t(".failure") + end + end + + private + + def set_user + @user = User.find(params[:id]) + end + + def user_params + params.require(:user).permit(:role) + end + end +end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d63adfa8a..b171a080d 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,9 +2,15 @@ class ApplicationController < ActionController::Base include RestoreLayoutPreferences, Onboardable, Localize, AutoSync, Authentication, Invitable, SelfHostable, StoreLocation, Impersonatable, Breadcrumbable, FeatureGuardable, Notifiable + include Pundit::Authorization include Pagy::Backend + # Pundit uses current_user by default, but this app uses Current.user + def pundit_user + Current.user + end + before_action :detect_os before_action :set_default_chat before_action :set_active_storage_url_options diff --git a/app/controllers/oidc_accounts_controller.rb b/app/controllers/oidc_accounts_controller.rb index a89dc9395..bfc6597ab 100644 --- a/app/controllers/oidc_accounts_controller.rb +++ b/app/controllers/oidc_accounts_controller.rb @@ -37,6 +37,13 @@ class OidcAccountsController < ApplicationController user ) + # Log account linking + SsoAuditLog.log_link!( + user: user, + provider: @pending_auth["provider"], + request: request + ) + # Clear pending auth from session session.delete(:pending_oidc_auth) @@ -103,7 +110,11 @@ class OidcAccountsController < ApplicationController # Create new family for this user @user.family = Family.new - @user.role = :admin + + # Use provider-configured default role, or fall back to member (not admin) + provider_config = Rails.configuration.x.auth.sso_providers&.find { |p| p[:name] == @pending_auth["provider"] } + default_role = provider_config&.dig(:settings, :default_role) || "member" + @user.role = default_role if @user.save # Create the OIDC (or other SSO) identity @@ -112,6 +123,13 @@ class OidcAccountsController < ApplicationController @user ) + # Log JIT account creation + SsoAuditLog.log_jit_account_created!( + user: @user, + provider: @pending_auth["provider"], + request: request + ) + # Clear pending auth from session session.delete(:pending_oidc_auth) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index f42fac451..cb375d752 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,9 +1,14 @@ class SessionsController < ApplicationController before_action :set_session, only: :destroy - skip_authentication only: %i[new create openid_connect failure] + skip_authentication only: %i[index new create openid_connect failure post_logout] layout "auth" + # Handle GET /sessions (usually from browser back button) + def index + redirect_to new_session_path + end + def new begin demo = Rails.application.config_for(:demo) @@ -62,7 +67,32 @@ class SessionsController < ApplicationController end def destroy + user = Current.user + id_token = session[:id_token_hint] + oidc_identity = user.oidc_identities.first + + # Destroy local session @session.destroy + session.delete(:id_token_hint) + + # Check if we should redirect to IdP for federated logout + if oidc_identity && id_token.present? + idp_logout_url = build_idp_logout_url(oidc_identity, id_token) + + if idp_logout_url + SsoAuditLog.log_logout_idp!(user: user, provider: oidc_identity.provider, request: request) + redirect_to idp_logout_url, allow_other_host: true + return + end + end + + # Standard local logout + SsoAuditLog.log_logout!(user: user, request: request) + redirect_to new_session_path, notice: t(".logout_successful") + end + + # Handle redirect back from IdP after federated logout + def post_logout redirect_to new_session_path, notice: t(".logout_successful") end @@ -82,6 +112,13 @@ class SessionsController < ApplicationController # Existing OIDC identity found - authenticate the user user = oidc_identity.user oidc_identity.record_authentication! + oidc_identity.sync_user_attributes!(auth) + + # Store id_token for RP-initiated logout + session[:id_token_hint] = auth.credentials&.id_token if auth.credentials&.id_token + + # Log successful SSO login + SsoAuditLog.log_login!(user: user, provider: auth.provider, request: request) # MFA check: If user has MFA enabled, require verification if user.otp_required? @@ -107,7 +144,25 @@ class SessionsController < ApplicationController end def failure - redirect_to new_session_path, alert: t("sessions.failure.failed") + # Log failed SSO attempt + SsoAuditLog.log_login_failed!( + provider: params[:strategy], + request: request, + reason: params[:message] + ) + + message = case params[:message] + when "sso_provider_unavailable" + t("sessions.failure.sso_provider_unavailable") + when "sso_invalid_response" + t("sessions.failure.sso_invalid_response") + when "sso_failed" + t("sessions.failure.sso_failed") + else + t("sessions.failure.failed") + end + + redirect_to new_session_path, alert: message end private @@ -130,4 +185,53 @@ class SessionsController < ApplicationController demo["hosts"].include?(request.host) end + + def build_idp_logout_url(oidc_identity, id_token) + # Find the provider configuration + provider_config = Rails.configuration.x.auth.sso_providers&.find do |p| + p[:name] == oidc_identity.provider + end + + return nil unless provider_config + + # For OIDC providers, fetch end_session_endpoint from discovery + if provider_config[:strategy] == "openid_connect" && provider_config[:issuer].present? + begin + discovery_url = discovery_url_for(provider_config[:issuer]) + response = Faraday.get(discovery_url) do |req| + req.options.timeout = 5 + req.options.open_timeout = 3 + end + + return nil unless response.success? + + discovery = JSON.parse(response.body) + end_session_endpoint = discovery["end_session_endpoint"] + + return nil unless end_session_endpoint.present? + + # Build the logout URL with post_logout_redirect_uri + post_logout_redirect = "#{request.base_url}/auth/logout/callback" + params = { + id_token_hint: id_token, + post_logout_redirect_uri: post_logout_redirect + } + + "#{end_session_endpoint}?#{params.to_query}" + rescue Faraday::Error, JSON::ParserError, StandardError => e + Rails.logger.warn("[SSO] Failed to fetch OIDC discovery for logout: #{e.message}") + nil + end + else + nil + end + end + + def discovery_url_for(issuer) + if issuer.end_with?("/") + "#{issuer}.well-known/openid-configuration" + else + "#{issuer}/.well-known/openid-configuration" + end + end end diff --git a/app/controllers/settings/sso_identities_controller.rb b/app/controllers/settings/sso_identities_controller.rb new file mode 100644 index 000000000..f42175c62 --- /dev/null +++ b/app/controllers/settings/sso_identities_controller.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class Settings::SsoIdentitiesController < ApplicationController + layout "settings" + + def show + @oidc_identities = Current.user.oidc_identities.order(:provider) + @breadcrumbs = [ + [ t("settings.nav.home"), root_path ], + [ t(".page_title"), nil ] + ] + end + + def destroy + @identity = Current.user.oidc_identities.find(params[:id]) + + # Prevent unlinking last identity if user has no password + if Current.user.oidc_identities.count == 1 && Current.user.password_digest.blank? + redirect_to settings_sso_identities_path, alert: t(".cannot_unlink_last") + return + end + + provider_name = @identity.provider + @identity.destroy! + + # Log account unlinking + SsoAuditLog.log_unlink!( + user: Current.user, + provider: provider_name, + request: request + ) + + redirect_to settings_sso_identities_path, notice: t(".success", provider: provider_name) + end +end diff --git a/app/helpers/settings_helper.rb b/app/helpers/settings_helper.rb index a907cd7c8..1428aeda0 100644 --- a/app/helpers/settings_helper.rb +++ b/app/helpers/settings_helper.rb @@ -6,6 +6,7 @@ module SettingsHelper { name: "Preferences", path: :settings_preferences_path }, { name: "Profile Info", path: :settings_profile_path }, { name: "Security", path: :settings_security_path }, + { name: "Connected Accounts", path: :settings_sso_identities_path, condition: :has_sso_connections? }, { name: "Billing", path: :settings_billing_path, condition: :not_self_hosted? }, # Transactions section { name: "Categories", path: :categories_path }, @@ -81,4 +82,8 @@ module SettingsHelper def self_hosted_and_admin? self_hosted? && admin_user? end + + def has_sso_connections? + Current.user&.oidc_identities&.exists? || AuthConfig.sso_providers.any? + end end diff --git a/app/javascript/controllers/admin_sso_form_controller.js b/app/javascript/controllers/admin_sso_form_controller.js new file mode 100644 index 000000000..2344b8b63 --- /dev/null +++ b/app/javascript/controllers/admin_sso_form_controller.js @@ -0,0 +1,226 @@ +import { Controller } from "@hotwired/stimulus" + +// Connects to data-controller="admin-sso-form" +export default class extends Controller { + static targets = ["callbackUrl", "testResult", "samlCallbackUrl"] + + connect() { + // Initialize field visibility on page load + this.toggleFields() + // Initialize callback URL + this.updateCallbackUrl() + } + + updateCallbackUrl() { + const nameInput = this.element.querySelector('input[name*="[name]"]') + const callbackDisplay = this.callbackUrlTarget + + if (!nameInput || !callbackDisplay) return + + const providerName = nameInput.value.trim() || 'PROVIDER_NAME' + const baseUrl = window.location.origin + callbackDisplay.textContent = `${baseUrl}/auth/${providerName}/callback` + } + + toggleFields() { + const strategySelect = this.element.querySelector('select[name*="[strategy]"]') + if (!strategySelect) return + + const strategy = strategySelect.value + const isOidc = strategy === "openid_connect" + const isSaml = strategy === "saml" + + // Toggle OIDC fields + const oidcFields = this.element.querySelectorAll('[data-oidc-field]') + oidcFields.forEach(field => { + if (isOidc) { + field.classList.remove('hidden') + } else { + field.classList.add('hidden') + } + }) + + // Toggle SAML fields + const samlFields = this.element.querySelectorAll('[data-saml-field]') + samlFields.forEach(field => { + if (isSaml) { + field.classList.remove('hidden') + } else { + field.classList.add('hidden') + } + }) + + // Update SAML callback URL if present + if (this.hasSamlCallbackUrlTarget) { + this.updateSamlCallbackUrl() + } + } + + updateSamlCallbackUrl() { + const nameInput = this.element.querySelector('input[name*="[name]"]') + if (!nameInput || !this.hasSamlCallbackUrlTarget) return + + const providerName = nameInput.value.trim() || 'PROVIDER_NAME' + const baseUrl = window.location.origin + this.samlCallbackUrlTarget.textContent = `${baseUrl}/auth/${providerName}/callback` + } + + copySamlCallback(event) { + event.preventDefault() + + if (!this.hasSamlCallbackUrlTarget) return + + const callbackUrl = this.samlCallbackUrlTarget.textContent + + navigator.clipboard.writeText(callbackUrl).then(() => { + const button = event.currentTarget + const originalText = button.innerHTML + button.innerHTML = ' Copied!' + button.classList.add('text-green-600') + + setTimeout(() => { + button.innerHTML = originalText + button.classList.remove('text-green-600') + }, 2000) + }).catch(err => { + console.error('Failed to copy:', err) + alert('Failed to copy to clipboard') + }) + } + + async validateIssuer(event) { + const issuerInput = event.target + const issuer = issuerInput.value.trim() + + if (!issuer) return + + try { + // Construct discovery URL + const discoveryUrl = issuer.endsWith('/') + ? `${issuer}.well-known/openid-configuration` + : `${issuer}/.well-known/openid-configuration` + + // Show loading state + issuerInput.classList.add('border-yellow-300') + + const response = await fetch(discoveryUrl, { + method: 'GET', + headers: { 'Accept': 'application/json' } + }) + + if (response.ok) { + const data = await response.json() + if (data.issuer) { + // Valid OIDC discovery endpoint + issuerInput.classList.remove('border-yellow-300', 'border-red-300') + issuerInput.classList.add('border-green-300') + this.showValidationMessage(issuerInput, 'Valid OIDC issuer', 'success') + } else { + throw new Error('Invalid discovery response') + } + } else { + throw new Error(`Discovery endpoint returned ${response.status}`) + } + } catch (error) { + // CORS errors are expected when validating from browser - show as warning not error + issuerInput.classList.remove('border-yellow-300', 'border-green-300') + issuerInput.classList.add('border-amber-300') + this.showValidationMessage(issuerInput, "Could not validate from browser (CORS). Provider can still be saved.", 'warning') + } + } + + copyCallback(event) { + event.preventDefault() + + const callbackDisplay = this.callbackUrlTarget + if (!callbackDisplay) return + + const callbackUrl = callbackDisplay.textContent + + // Copy to clipboard + navigator.clipboard.writeText(callbackUrl).then(() => { + // Show success feedback + const button = event.currentTarget + const originalText = button.innerHTML + button.innerHTML = ' Copied!' + button.classList.add('text-green-600') + + setTimeout(() => { + button.innerHTML = originalText + button.classList.remove('text-green-600') + }, 2000) + }).catch(err => { + console.error('Failed to copy:', err) + alert('Failed to copy to clipboard') + }) + } + + showValidationMessage(input, message, type) { + // Remove any existing validation message + const existingMessage = input.parentElement.querySelector('.validation-message') + if (existingMessage) { + existingMessage.remove() + } + + // Create new validation message + const messageEl = document.createElement('p') + const colorClass = type === 'success' ? 'text-green-600' : type === 'warning' ? 'text-amber-600' : 'text-red-600' + messageEl.className = `validation-message mt-1 text-sm ${colorClass}` + messageEl.textContent = message + + input.parentElement.appendChild(messageEl) + + // Auto-remove after 5 seconds (except warnings which stay) + if (type !== 'warning') { + setTimeout(() => { + messageEl.remove() + input.classList.remove('border-green-300', 'border-red-300', 'border-amber-300') + }, 5000) + } + } + + async testConnection(event) { + const button = event.currentTarget + const testUrl = button.dataset.adminSsoFormTestUrlValue + const resultEl = this.testResultTarget + + if (!testUrl) return + + // Show loading state + button.disabled = true + button.textContent = 'Testing...' + resultEl.textContent = '' + resultEl.className = 'ml-2 text-sm' + + try { + const response = await fetch(testUrl, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-CSRF-Token': document.querySelector('meta[name="csrf-token"]')?.content + } + }) + + const data = await response.json() + + if (data.success) { + resultEl.textContent = `✓ ${data.message}` + resultEl.classList.add('text-green-600') + } else { + resultEl.textContent = `✗ ${data.message}` + resultEl.classList.add('text-red-600') + } + + // Show details in console for debugging + if (data.details && Object.keys(data.details).length > 0) { + console.log('SSO Test Connection Details:', data.details) + } + } catch (error) { + resultEl.textContent = `✗ Request failed: ${error.message}` + resultEl.classList.add('text-red-600') + } finally { + button.disabled = false + button.textContent = 'Test Connection' + } + } +} diff --git a/app/middleware/omniauth_error_handler.rb b/app/middleware/omniauth_error_handler.rb new file mode 100644 index 000000000..8373cb760 --- /dev/null +++ b/app/middleware/omniauth_error_handler.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +# Middleware to catch OmniAuth/OIDC errors and redirect gracefully +# instead of showing ugly error pages +class OmniauthErrorHandler + def initialize(app) + @app = app + end + + def call(env) + @app.call(env) + rescue OpenIDConnect::Discovery::DiscoveryFailed => e + Rails.logger.error("[OmniAuth] OIDC Discovery failed: #{e.message}") + redirect_to_failure(env, "sso_provider_unavailable") + rescue OmniAuth::Error => e + Rails.logger.error("[OmniAuth] Authentication error: #{e.message}") + redirect_to_failure(env, "sso_failed") + end + + private + + def redirect_to_failure(env, message) + [ + 302, + { "Location" => "/auth/failure?message=#{message}", "Content-Type" => "text/html" }, + [ "Redirecting..." ] + ] + end +end diff --git a/app/models/oidc_identity.rb b/app/models/oidc_identity.rb index 78276546d..e8993142f 100644 --- a/app/models/oidc_identity.rb +++ b/app/models/oidc_identity.rb @@ -10,12 +10,79 @@ class OidcIdentity < ApplicationRecord update!(last_authenticated_at: Time.current) end + # Sync user attributes from IdP on each login + # Updates stored identity info and syncs name to user (not email - that's identity) + def sync_user_attributes!(auth) + # Extract groups from claims (various common claim names) + groups = extract_groups(auth) + + # Update stored identity info with latest from IdP + update!(info: { + email: auth.info&.email, + name: auth.info&.name, + first_name: auth.info&.first_name, + last_name: auth.info&.last_name, + groups: groups + }) + + # Sync name to user if provided (keep existing if IdP doesn't provide) + user.update!( + first_name: auth.info&.first_name.presence || user.first_name, + last_name: auth.info&.last_name.presence || user.last_name + ) + + # Apply role mapping based on group membership + apply_role_mapping!(groups) + end + + # Extract groups from various common IdP claim formats + def extract_groups(auth) + # Try various common group claim locations + groups = auth.extra&.raw_info&.groups || + auth.extra&.raw_info&.[]("groups") || + auth.extra&.raw_info&.[]("Group") || + auth.info&.groups || + auth.extra&.raw_info&.[]("http://schemas.microsoft.com/ws/2008/06/identity/claims/groups") || + auth.extra&.raw_info&.[]("cognito:groups") || + [] + + # Normalize to array of strings + Array(groups).map(&:to_s) + end + + # Apply role mapping based on IdP group membership + def apply_role_mapping!(groups) + config = provider_config + return unless config.present? + + role_mapping = config.dig(:settings, :role_mapping) || config.dig(:settings, "role_mapping") + return unless role_mapping.present? + + # Check roles in order of precedence (highest to lowest) + %w[super_admin admin member].each do |role| + mapped_groups = role_mapping[role] || role_mapping[role.to_sym] || [] + mapped_groups = Array(mapped_groups) + + # Check if user is in any of the mapped groups + if mapped_groups.include?("*") || (mapped_groups & groups).any? + # Only update if different to avoid unnecessary writes + user.update!(role: role) unless user.role == role + Rails.logger.info("[SSO] Applied role mapping: user_id=#{user.id} role=#{role} groups=#{groups}") + return + end + end + end + # Extract and store relevant info from OmniAuth auth hash def self.create_from_omniauth(auth, user) + # Extract issuer from OIDC auth response if available + issuer = auth.extra&.raw_info&.iss || auth.extra&.raw_info&.[]("iss") + create!( user: user, provider: auth.provider, uid: auth.uid, + issuer: issuer, info: { email: auth.info&.email, name: auth.info&.name, @@ -25,4 +92,20 @@ class OidcIdentity < ApplicationRecord last_authenticated_at: Time.current ) end + + # Find the configured provider for this identity + def provider_config + Rails.configuration.x.auth.sso_providers&.find { |p| p[:name] == provider || p[:id] == provider } + end + + # Validate that the stored issuer matches the configured provider's issuer + # Returns true if valid, false if mismatch (security concern) + def issuer_matches_config? + return true if issuer.blank? # Backward compatibility for old records + + config = provider_config + return true if config.blank? || config[:issuer].blank? # No config to validate against + + issuer == config[:issuer] + end end diff --git a/app/models/sso_audit_log.rb b/app/models/sso_audit_log.rb new file mode 100644 index 000000000..21aa4e05e --- /dev/null +++ b/app/models/sso_audit_log.rb @@ -0,0 +1,108 @@ +# frozen_string_literal: true + +class SsoAuditLog < ApplicationRecord + belongs_to :user, optional: true + + # Event types for SSO audit logging + EVENT_TYPES = %w[ + login + login_failed + logout + logout_idp + link + unlink + jit_account_created + ].freeze + + validates :event_type, presence: true, inclusion: { in: EVENT_TYPES } + + scope :recent, -> { order(created_at: :desc) } + scope :for_user, ->(user) { where(user: user) } + scope :by_event, ->(event) { where(event_type: event) } + + class << self + # Log a successful SSO login + def log_login!(user:, provider:, request:, metadata: {}) + create!( + user: user, + event_type: "login", + provider: provider, + ip_address: request.remote_ip, + user_agent: request.user_agent&.truncate(500), + metadata: metadata + ) + end + + # Log a failed SSO login attempt + def log_login_failed!(provider:, request:, reason:, metadata: {}) + create!( + user: nil, + event_type: "login_failed", + provider: provider, + ip_address: request.remote_ip, + user_agent: request.user_agent&.truncate(500), + metadata: metadata.merge(reason: reason) + ) + end + + # Log a logout (local only) + def log_logout!(user:, request:, metadata: {}) + create!( + user: user, + event_type: "logout", + provider: nil, + ip_address: request.remote_ip, + user_agent: request.user_agent&.truncate(500), + metadata: metadata + ) + end + + # Log a federated logout (to IdP) + def log_logout_idp!(user:, provider:, request:, metadata: {}) + create!( + user: user, + event_type: "logout_idp", + provider: provider, + ip_address: request.remote_ip, + user_agent: request.user_agent&.truncate(500), + metadata: metadata + ) + end + + # Log an account link (existing user links SSO identity) + def log_link!(user:, provider:, request:, metadata: {}) + create!( + user: user, + event_type: "link", + provider: provider, + ip_address: request.remote_ip, + user_agent: request.user_agent&.truncate(500), + metadata: metadata + ) + end + + # Log an account unlink (user disconnects SSO identity) + def log_unlink!(user:, provider:, request:, metadata: {}) + create!( + user: user, + event_type: "unlink", + provider: provider, + ip_address: request.remote_ip, + user_agent: request.user_agent&.truncate(500), + metadata: metadata + ) + end + + # Log JIT account creation via SSO + def log_jit_account_created!(user:, provider:, request:, metadata: {}) + create!( + user: user, + event_type: "jit_account_created", + provider: provider, + ip_address: request.remote_ip, + user_agent: request.user_agent&.truncate(500), + metadata: metadata + ) + end + end +end diff --git a/app/models/sso_provider.rb b/app/models/sso_provider.rb new file mode 100644 index 000000000..21e3dbf33 --- /dev/null +++ b/app/models/sso_provider.rb @@ -0,0 +1,144 @@ +# frozen_string_literal: true + +class SsoProvider < ApplicationRecord + # Encrypt sensitive credentials using Rails 7.2 built-in encryption + encrypts :client_secret, deterministic: false + + # Default enabled to true for new providers + attribute :enabled, :boolean, default: true + + # Validations + validates :strategy, presence: true, inclusion: { + in: %w[openid_connect google_oauth2 github saml], + message: "%{value} is not a supported strategy" + } + validates :name, presence: true, uniqueness: true, format: { + with: /\A[a-z0-9_]+\z/, + message: "must contain only lowercase letters, numbers, and underscores" + } + validates :label, presence: true + validates :enabled, inclusion: { in: [ true, false ] } + + # Strategy-specific validations + validate :validate_oidc_fields, if: -> { strategy == "openid_connect" } + validate :validate_oauth_fields, if: -> { strategy.in?(%w[google_oauth2 github]) } + validate :validate_saml_fields, if: -> { strategy == "saml" } + validate :validate_default_role_setting + # Note: OIDC discovery validation is done client-side via Stimulus + # Server-side validation can fail due to network issues, so we skip it + # validate :validate_oidc_discovery, if: -> { strategy == "openid_connect" && issuer.present? && will_save_change_to_issuer? } + + # Scopes + scope :enabled, -> { where(enabled: true) } + scope :by_strategy, ->(strategy) { where(strategy: strategy) } + + # Convert to hash format compatible with OmniAuth initializer + def to_omniauth_config + { + id: name, + strategy: strategy, + name: name, + label: label, + icon: icon, + issuer: issuer, + client_id: client_id, + client_secret: client_secret, + redirect_uri: redirect_uri, + settings: settings || {} + }.compact + end + + private + def validate_oidc_fields + if issuer.blank? + errors.add(:issuer, "is required for OpenID Connect providers") + elsif issuer.present? && !valid_url?(issuer) + errors.add(:issuer, "must be a valid URL") + end + + errors.add(:client_id, "is required for OpenID Connect providers") if client_id.blank? + errors.add(:client_secret, "is required for OpenID Connect providers") if client_secret.blank? + + if redirect_uri.present? && !valid_url?(redirect_uri) + errors.add(:redirect_uri, "must be a valid URL") + end + end + + def validate_oauth_fields + errors.add(:client_id, "is required for OAuth providers") if client_id.blank? + errors.add(:client_secret, "is required for OAuth providers") if client_secret.blank? + end + + def validate_saml_fields + # SAML requires either a metadata URL or manual configuration + idp_metadata_url = settings&.dig("idp_metadata_url") + idp_sso_url = settings&.dig("idp_sso_url") + + if idp_metadata_url.blank? && idp_sso_url.blank? + errors.add(:settings, "Either IdP Metadata URL or IdP SSO URL is required for SAML providers") + end + + # If using manual config, require certificate + if idp_metadata_url.blank? && idp_sso_url.present? + idp_cert = settings&.dig("idp_certificate") + idp_fingerprint = settings&.dig("idp_cert_fingerprint") + + if idp_cert.blank? && idp_fingerprint.blank? + errors.add(:settings, "Either IdP Certificate or Certificate Fingerprint is required when not using metadata URL") + end + end + + # Validate URL formats if provided + if idp_metadata_url.present? && !valid_url?(idp_metadata_url) + errors.add(:settings, "IdP Metadata URL must be a valid URL") + end + + if idp_sso_url.present? && !valid_url?(idp_sso_url) + errors.add(:settings, "IdP SSO URL must be a valid URL") + end + end + + def validate_default_role_setting + default_role = settings&.dig("default_role") + return if default_role.blank? + + unless User.roles.key?(default_role) + errors.add(:settings, "default_role must be member, admin, or super_admin") + end + end + + def validate_oidc_discovery + return unless issuer.present? + + begin + discovery_url = issuer.end_with?("/") ? "#{issuer}.well-known/openid-configuration" : "#{issuer}/.well-known/openid-configuration" + response = Faraday.get(discovery_url) do |req| + req.options.timeout = 5 + req.options.open_timeout = 3 + end + + unless response.success? + errors.add(:issuer, "discovery endpoint returned #{response.status}") + return + end + + discovery_data = JSON.parse(response.body) + unless discovery_data["issuer"].present? + errors.add(:issuer, "discovery endpoint did not return valid issuer") + end + rescue Faraday::Error => e + errors.add(:issuer, "could not connect to discovery endpoint: #{e.message}") + rescue JSON::ParserError + errors.add(:issuer, "discovery endpoint returned invalid JSON") + rescue StandardError => e + errors.add(:issuer, "discovery validation failed: #{e.message}") + end + end + + def valid_url?(url) + uri = URI.parse(url) + uri.is_a?(URI::HTTP) || uri.is_a?(URI::HTTPS) + rescue URI::InvalidURIError + false + end +end diff --git a/app/models/sso_provider_tester.rb b/app/models/sso_provider_tester.rb new file mode 100644 index 000000000..0464088c4 --- /dev/null +++ b/app/models/sso_provider_tester.rb @@ -0,0 +1,201 @@ +# frozen_string_literal: true + +# Tests SSO provider configuration by validating discovery endpoints +class SsoProviderTester + attr_reader :provider, :result + + Result = Struct.new(:success?, :message, :details, keyword_init: true) + + def initialize(provider) + @provider = provider + @result = nil + end + + def test! + @result = case provider.strategy + when "openid_connect" + test_oidc_discovery + when "google_oauth2" + test_google_oauth + when "github" + test_github_oauth + when "saml" + test_saml_metadata + else + Result.new(success?: false, message: "Unknown strategy: #{provider.strategy}", details: {}) + end + end + + private + + def test_oidc_discovery + return Result.new(success?: false, message: "Issuer URL is required", details: {}) if provider.issuer.blank? + + discovery_url = build_discovery_url(provider.issuer) + + begin + response = Faraday.get(discovery_url) do |req| + req.options.timeout = 10 + req.options.open_timeout = 5 + end + + unless response.success? + return Result.new( + success?: false, + message: "Discovery endpoint returned HTTP #{response.status}", + details: { url: discovery_url, status: response.status } + ) + end + + discovery = JSON.parse(response.body) + + # Validate required OIDC fields + required_fields = %w[issuer authorization_endpoint token_endpoint] + missing = required_fields.select { |f| discovery[f].blank? } + + if missing.any? + return Result.new( + success?: false, + message: "Discovery document missing required fields: #{missing.join(", ")}", + details: { url: discovery_url, missing_fields: missing } + ) + end + + # Check if issuer matches + if discovery["issuer"] != provider.issuer && discovery["issuer"] != provider.issuer.chomp("/") + return Result.new( + success?: false, + message: "Issuer mismatch: expected #{provider.issuer}, got #{discovery["issuer"]}", + details: { expected: provider.issuer, actual: discovery["issuer"] } + ) + end + + Result.new( + success?: true, + message: "OIDC discovery validated successfully", + details: { + issuer: discovery["issuer"], + authorization_endpoint: discovery["authorization_endpoint"], + token_endpoint: discovery["token_endpoint"], + end_session_endpoint: discovery["end_session_endpoint"], + scopes_supported: discovery["scopes_supported"] + } + ) + + rescue Faraday::TimeoutError + Result.new(success?: false, message: "Connection timed out", details: { url: discovery_url }) + rescue Faraday::ConnectionFailed => e + Result.new(success?: false, message: "Connection failed: #{e.message}", details: { url: discovery_url }) + rescue JSON::ParserError + Result.new(success?: false, message: "Invalid JSON response from discovery endpoint", details: { url: discovery_url }) + rescue StandardError => e + Result.new(success?: false, message: "Error: #{e.message}", details: { url: discovery_url }) + end + end + + def test_google_oauth + # Google OAuth doesn't require discovery validation - just check credentials present + if provider.client_id.blank? + return Result.new(success?: false, message: "Client ID is required", details: {}) + end + + if provider.client_secret.blank? + return Result.new(success?: false, message: "Client Secret is required", details: {}) + end + + Result.new( + success?: true, + message: "Google OAuth2 configuration looks valid", + details: { + note: "Full validation occurs during actual authentication" + } + ) + end + + def test_github_oauth + # GitHub OAuth doesn't require discovery validation - just check credentials present + if provider.client_id.blank? + return Result.new(success?: false, message: "Client ID is required", details: {}) + end + + if provider.client_secret.blank? + return Result.new(success?: false, message: "Client Secret is required", details: {}) + end + + Result.new( + success?: true, + message: "GitHub OAuth configuration looks valid", + details: { + note: "Full validation occurs during actual authentication" + } + ) + end + + def test_saml_metadata + # SAML testing - check for IdP metadata or SSO URL + if provider.settings&.dig("idp_metadata_url").blank? && + provider.settings&.dig("idp_sso_url").blank? + return Result.new( + success?: false, + message: "Either IdP Metadata URL or IdP SSO URL is required", + details: {} + ) + end + + # If metadata URL is provided, try to fetch it + metadata_url = provider.settings&.dig("idp_metadata_url") + if metadata_url.present? + begin + response = Faraday.get(metadata_url) do |req| + req.options.timeout = 10 + req.options.open_timeout = 5 + end + + unless response.success? + return Result.new( + success?: false, + message: "Metadata endpoint returned HTTP #{response.status}", + details: { url: metadata_url, status: response.status } + ) + end + + # Basic XML validation + unless response.body.include?("<") && response.body.include?("EntityDescriptor") + return Result.new( + success?: false, + message: "Response does not appear to be valid SAML metadata", + details: { url: metadata_url } + ) + end + + return Result.new( + success?: true, + message: "SAML metadata fetched successfully", + details: { url: metadata_url } + ) + rescue Faraday::TimeoutError + return Result.new(success?: false, message: "Connection timed out", details: { url: metadata_url }) + rescue Faraday::ConnectionFailed => e + return Result.new(success?: false, message: "Connection failed: #{e.message}", details: { url: metadata_url }) + rescue StandardError => e + return Result.new(success?: false, message: "Error: #{e.message}", details: { url: metadata_url }) + end + end + + Result.new( + success?: true, + message: "SAML configuration looks valid", + details: { + note: "Full validation occurs during actual authentication" + } + ) + end + + def build_discovery_url(issuer) + if issuer.end_with?("/") + "#{issuer}.well-known/openid-configuration" + else + "#{issuer}/.well-known/openid-configuration" + end + end +end diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb new file mode 100644 index 000000000..e232556f6 --- /dev/null +++ b/app/policies/application_policy.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +class ApplicationPolicy + attr_reader :user, :record + + def initialize(user, record) + @user = user + @record = record + end + + def index? + false + end + + def show? + false + end + + def create? + false + end + + def new? + create? + end + + def update? + false + end + + def edit? + update? + end + + def destroy? + false + end + + class Scope + def initialize(user, scope) + @user = user + @scope = scope + end + + def resolve + raise NoMethodError, "You must define #resolve in #{self.class}" + end + + private + + attr_reader :user, :scope + end +end diff --git a/app/policies/sso_provider_policy.rb b/app/policies/sso_provider_policy.rb new file mode 100644 index 000000000..5c975dc66 --- /dev/null +++ b/app/policies/sso_provider_policy.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +class SsoProviderPolicy < ApplicationPolicy + # Only super admins can manage SSO providers (instance-wide auth config) + def index? + user&.super_admin? + end + + def show? + user&.super_admin? + end + + def create? + user&.super_admin? + end + + def new? + create? + end + + def update? + user&.super_admin? + end + + def edit? + update? + end + + def destroy? + user&.super_admin? + end + + def toggle? + update? + end + + class Scope < ApplicationPolicy::Scope + def resolve + if user&.super_admin? + scope.all + else + scope.none + end + end + end +end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb new file mode 100644 index 000000000..c40bf6007 --- /dev/null +++ b/app/policies/user_policy.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class UserPolicy < ApplicationPolicy + # Only super_admins can manage user roles + def index? + user&.super_admin? + end + + def update? + return false unless user&.super_admin? + # Prevent users from changing their own role (must be done by another super_admin) + user.id != record.id + end + + class Scope < ApplicationPolicy::Scope + def resolve + if user&.super_admin? + scope.all + else + scope.none + end + end + end +end diff --git a/app/services/provider_loader.rb b/app/services/provider_loader.rb new file mode 100644 index 000000000..e2bf35365 --- /dev/null +++ b/app/services/provider_loader.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +# Service class to load SSO provider configurations from either YAML or database +# based on the :db_sso_providers feature flag. +# +# Usage: +# providers = ProviderLoader.load_providers +# +class ProviderLoader + CACHE_KEY = "sso_providers_config" + CACHE_EXPIRES_IN = 5.minutes + + class << self + # Load providers from either DB or YAML based on feature flag + # Returns an array of provider configuration hashes + def load_providers + # Check cache first for performance + cached = Rails.cache.read(CACHE_KEY) + return cached if cached.present? + + providers = if use_database_providers? + load_from_database + else + load_from_yaml + end + + # Cache the result + Rails.cache.write(CACHE_KEY, providers, expires_in: CACHE_EXPIRES_IN) + providers + end + + # Clear the provider cache (call after updating providers in admin) + def clear_cache + Rails.cache.delete(CACHE_KEY) + end + + private + def use_database_providers? + return false if Rails.env.test? + + begin + # Check if feature exists, create if not (defaults to disabled) + unless Flipper.exist?(:db_sso_providers) + Flipper.add(:db_sso_providers) + end + Flipper.enabled?(:db_sso_providers) + rescue ActiveRecord::NoDatabaseError, ActiveRecord::StatementInvalid, StandardError => e + # Database not ready or other error, fall back to YAML + Rails.logger.warn("[ProviderLoader] Could not check feature flag (#{e.class}), falling back to YAML providers") + false + end + end + + def load_from_database + begin + providers = SsoProvider.enabled.order(:name).map(&:to_omniauth_config) + + if providers.empty? + Rails.logger.info("[ProviderLoader] No enabled providers in database, falling back to YAML") + return load_from_yaml + end + + Rails.logger.info("[ProviderLoader] Loaded #{providers.count} provider(s) from database") + providers + rescue ActiveRecord::StatementInvalid, ActiveRecord::NoDatabaseError => e + Rails.logger.error("[ProviderLoader] Database error loading providers: #{e.message}, falling back to YAML") + load_from_yaml + rescue StandardError => e + Rails.logger.error("[ProviderLoader] Unexpected error loading providers from database: #{e.message}, falling back to YAML") + load_from_yaml + end + end + + def load_from_yaml + begin + auth_config = Rails.application.config_for(:auth) + providers = auth_config.dig("providers") || [] + + Rails.logger.info("[ProviderLoader] Loaded #{providers.count} provider(s) from YAML") + providers + rescue RuntimeError, Errno::ENOENT => e + Rails.logger.error("[ProviderLoader] Error loading auth.yml: #{e.message}") + [] + end + end + end +end diff --git a/app/views/admin/sso_providers/_form.html.erb b/app/views/admin/sso_providers/_form.html.erb new file mode 100644 index 000000000..88de8c6c6 --- /dev/null +++ b/app/views/admin/sso_providers/_form.html.erb @@ -0,0 +1,277 @@ +<%# locals: (sso_provider:) %> + +<% if sso_provider.errors.any? %> +
+
+ <%= icon "alert-circle", class: "w-5 h-5 text-destructive mr-2 shrink-0" %> +
+

+ <%= pluralize(sso_provider.errors.count, "error") %> prohibited this provider from being saved: +

+
    + <% sso_provider.errors.full_messages.each do |message| %> +
  • <%= message %>
    • + <% end %> +
+
+
+
+<% end %> + +<%= styled_form_with model: [:admin, sso_provider], class: "space-y-6", data: { controller: "admin-sso-form" } do |form| %> +
+

Basic Information

+ +
+ <%= form.select :strategy, + options_for_select([ + ["OpenID Connect", "openid_connect"], + ["SAML 2.0", "saml"], + ["Google OAuth2", "google_oauth2"], + ["GitHub", "github"] + ], sso_provider.strategy), + { label: "Strategy" }, + { data: { action: "change->admin-sso-form#toggleFields" } } %> + + <%= form.text_field :name, + label: "Name", + placeholder: "e.g., keycloak, authentik", + required: true, + data: { action: "input->admin-sso-form#updateCallbackUrl" } %> +
+

Unique identifier (lowercase, numbers, underscores only)

+ +
+ <%= form.text_field :label, + label: "Button Label", + placeholder: "e.g., Sign in with Keycloak", + required: true %> + +
+ <%= form.text_field :icon, + label: "Icon (optional)", + placeholder: "e.g., key, shield" %> +

Lucide icon name for the login button

+
+
+ + <%= form.check_box :enabled, + label: "Enable this provider", + checked: sso_provider.enabled? %> +
+ +
+

OAuth/OIDC Configuration

+ +
"> + <%= form.text_field :issuer, + label: "Issuer URL", + placeholder: "https://your-idp.example.com/realms/your-realm", + data: { action: "blur->admin-sso-form#validateIssuer" } %> +

OIDC issuer URL (validates .well-known/openid-configuration)

+
+ + <%= form.text_field :client_id, + label: "Client ID", + placeholder: "your-client-id", + required: true %> + + <%= form.password_field :client_secret, + label: "Client Secret", + placeholder: sso_provider.persisted? ? "••••••••" : "your-client-secret", + required: !sso_provider.persisted? %> + <% if sso_provider.persisted? %> +

Leave blank to keep existing secret

+ <% end %> + +
"> + +
+ <%= "#{request.base_url}/auth/#{sso_provider.name.presence || 'PROVIDER_NAME'}/callback" %> + +
+

Configure this URL in your identity provider

+
+
+ +
"> +

<%= t("admin.sso_providers.form.saml_configuration") %>

+ +
+ + " + class="w-full px-3 py-2 border border-primary rounded-lg text-sm" + placeholder="https://idp.example.com/metadata" + autocomplete="off"> +

<%= t("admin.sso_providers.form.idp_metadata_url_help") %>

+
+ +
+ <%= t("admin.sso_providers.form.manual_saml_config") %> +
+

<%= t("admin.sso_providers.form.manual_saml_help") %>

+ +
+ + " + class="w-full px-3 py-2 border border-primary rounded-lg text-sm" + placeholder="https://idp.example.com/sso" + autocomplete="off"> +
+ +
+ + " + class="w-full px-3 py-2 border border-primary rounded-lg text-sm" + placeholder="https://idp.example.com/slo (optional)" + autocomplete="off"> +
+ +
+ + +

<%= t("admin.sso_providers.form.idp_certificate_help") %>

+
+ +
+ + " + class="w-full px-3 py-2 border border-primary rounded-lg text-sm font-mono" + placeholder="AB:CD:EF:..." + autocomplete="off"> +
+ +
+ + +
+
+
+ +
+ +
+ <%= "#{request.base_url}/auth/#{sso_provider.name.presence || 'PROVIDER_NAME'}/callback" %> + +
+

Configure this URL as the Assertion Consumer Service URL in your IdP

+
+
+ +
+

<%= t("admin.sso_providers.form.provisioning_title") %>

+ + <%= form.select "settings[default_role]", + options_for_select([ + [t("admin.sso_providers.form.role_member"), "member"], + [t("admin.sso_providers.form.role_admin"), "admin"], + [t("admin.sso_providers.form.role_super_admin"), "super_admin"] + ], sso_provider.settings&.dig("default_role") || "member"), + { label: t("admin.sso_providers.form.default_role_label"), include_blank: false } %> +

<%= t("admin.sso_providers.form.default_role_help") %>

+ +
+ <%= t("admin.sso_providers.form.role_mapping_title") %> +
+

<%= t("admin.sso_providers.form.role_mapping_help") %>

+ +
+ + " + class="w-full px-3 py-2 border border-primary rounded-lg text-sm" + placeholder="Platform-Admins, IdP-Superusers" + autocomplete="off"> +

<%= t("admin.sso_providers.form.groups_help") %>

+
+ +
+ + " + class="w-full px-3 py-2 border border-primary rounded-lg text-sm" + placeholder="Team-Leads, Managers" + autocomplete="off"> +
+ +
+ + " + class="w-full px-3 py-2 border border-primary rounded-lg text-sm" + placeholder="* (all groups)" + autocomplete="off"> +
+
+
+
+ +
"> +

<%= t("admin.sso_providers.form.advanced_title") %>

+ +
+ <%= form.text_field "settings[scopes]", + label: t("admin.sso_providers.form.scopes_label"), + value: sso_provider.settings&.dig("scopes"), + placeholder: "openid email profile groups" %> +

<%= t("admin.sso_providers.form.scopes_help") %>

+
+ + <%= form.select "settings[prompt]", + options_for_select([ + [t("admin.sso_providers.form.prompt_default"), ""], + [t("admin.sso_providers.form.prompt_login"), "login"], + [t("admin.sso_providers.form.prompt_consent"), "consent"], + [t("admin.sso_providers.form.prompt_select_account"), "select_account"], + [t("admin.sso_providers.form.prompt_none"), "none"] + ], sso_provider.settings&.dig("prompt")), + { label: t("admin.sso_providers.form.prompt_label"), include_blank: false } %> +

<%= t("admin.sso_providers.form.prompt_help") %>

+
+ +
+
+ <% if sso_provider.persisted? %> + + + <% end %> +
+ +
+ <%= link_to "Cancel", admin_sso_providers_path, class: "px-4 py-2 text-sm font-medium text-secondary hover:text-primary" %> + <%= form.submit sso_provider.persisted? ? "Update Provider" : "Create Provider", + class: "px-4 py-2 bg-primary text-inverse rounded-lg text-sm font-medium hover:bg-primary/90" %> +
+
+<% end %> diff --git a/app/views/admin/sso_providers/edit.html.erb b/app/views/admin/sso_providers/edit.html.erb new file mode 100644 index 000000000..7fc531668 --- /dev/null +++ b/app/views/admin/sso_providers/edit.html.erb @@ -0,0 +1,9 @@ +<%= content_for :page_title, "Edit #{@sso_provider.label}" %> + +
+

Update configuration for <%= @sso_provider.label %>.

+ + <%= settings_section title: "Provider Configuration" do %> + <%= render "form", sso_provider: @sso_provider %> + <% end %> +
diff --git a/app/views/admin/sso_providers/index.html.erb b/app/views/admin/sso_providers/index.html.erb new file mode 100644 index 000000000..b99272db6 --- /dev/null +++ b/app/views/admin/sso_providers/index.html.erb @@ -0,0 +1,88 @@ +<%= content_for :page_title, "SSO Providers" %> + +
+

+ Manage single sign-on authentication providers for your instance. + <% unless Flipper.enabled?(:db_sso_providers) %> + Changes require a server restart to take effect. + <% end %> +

+ + <%= settings_section title: "Configured Providers" do %> + <% if @sso_providers.any? %> +
+ <% @sso_providers.each do |provider| %> +
+
+ <% if provider.icon.present? %> + <%= icon provider.icon, class: "w-5 h-5 text-secondary" %> + <% else %> + <%= icon "key", class: "w-5 h-5 text-secondary" %> + <% end %> +
+

<%= provider.label %>

+

<%= provider.strategy.titleize %> · <%= provider.name %>

+
+
+
+ <% if provider.enabled? %> + + Enabled + + <% else %> + + Disabled + + <% end %> + <%= link_to edit_admin_sso_provider_path(provider), class: "p-1 text-secondary hover:text-primary", title: "Edit" do %> + <%= icon "pencil", class: "w-4 h-4" %> + <% end %> + <%= button_to toggle_admin_sso_provider_path(provider), method: :patch, class: "p-1 text-secondary hover:text-primary", title: provider.enabled? ? "Disable" : "Enable", form: { data: { turbo_confirm: "Are you sure you want to #{provider.enabled? ? 'disable' : 'enable'} this provider?" } } do %> + <%= icon provider.enabled? ? "toggle-right" : "toggle-left", class: "w-4 h-4" %> + <% end %> + <%= button_to admin_sso_provider_path(provider), method: :delete, class: "p-1 text-destructive hover:text-destructive", title: "Delete", form: { data: { turbo_confirm: "Are you sure you want to delete this provider? This action cannot be undone." } } do %> + <%= icon "trash-2", class: "w-4 h-4" %> + <% end %> +
+
+ <% end %> +
+ <% else %> +
+ <%= icon "key", class: "w-12 h-12 mx-auto text-secondary mb-3" %> +

No SSO providers configured yet.

+
+ <% end %> + +
+ <%= link_to new_admin_sso_provider_path, class: "inline-flex items-center gap-2 text-sm font-medium text-primary hover:text-secondary" do %> + <%= icon "plus", class: "w-4 h-4" %> + Add Provider + <% end %> +
+ <% end %> + + <%= settings_section title: "Configuration Mode", collapsible: true, open: false do %> +
+
+
+

Database-backed providers

+

Load providers from database instead of YAML config

+
+ <% if Flipper.enabled?(:db_sso_providers) %> + + Enabled + + <% else %> + + Disabled + + <% end %> +
+

+ Set AUTH_PROVIDERS_SOURCE=db to enable database-backed providers. + This allows changes without server restarts. +

+
+ <% end %> +
diff --git a/app/views/admin/sso_providers/new.html.erb b/app/views/admin/sso_providers/new.html.erb new file mode 100644 index 000000000..20be829c8 --- /dev/null +++ b/app/views/admin/sso_providers/new.html.erb @@ -0,0 +1,9 @@ +<%= content_for :page_title, "Add SSO Provider" %> + +
+

Configure a new single sign-on authentication provider.

+ + <%= settings_section title: "Provider Configuration" do %> + <%= render "form", sso_provider: @sso_provider %> + <% end %> +
diff --git a/app/views/admin/users/index.html.erb b/app/views/admin/users/index.html.erb new file mode 100644 index 000000000..551cd4d10 --- /dev/null +++ b/app/views/admin/users/index.html.erb @@ -0,0 +1,73 @@ +<%= content_for :page_title, t(".title") %> + +
+

<%= t(".description") %>

+ + <%= settings_section title: t(".section_title") do %> +
+ <% @users.each do |user| %> +
+
+
+ <%= user.initials %> +
+
+

<%= user.display_name %>

+

<%= user.email %>

+
+
+
+ <% if user.id == Current.user.id %> + <%= t(".you") %> + + <%= t(".roles.#{user.role}") %> + + <% else %> + <%= form_with model: [:admin, user], method: :patch, class: "flex items-center gap-2" do |form| %> + <%= form.select :role, + options_for_select([ + [t(".roles.member"), "member"], + [t(".roles.admin"), "admin"], + [t(".roles.super_admin"), "super_admin"] + ], user.role), + {}, + class: "text-sm rounded-lg border-primary bg-container text-primary px-2 py-1", + onchange: "this.form.requestSubmit()" %> + <% end %> + <% end %> +
+
+ <% end %> +
+ + <% if @users.empty? %> +
+ <%= icon "users", class: "w-12 h-12 mx-auto text-secondary mb-3" %> +

<%= t(".no_users") %>

+
+ <% end %> + <% end %> + + <%= settings_section title: t(".role_descriptions_title"), collapsible: true, open: false do %> +
+
+ + <%= t(".roles.member") %> + +

<%= t(".role_descriptions.member") %>

+
+
+ + <%= t(".roles.admin") %> + +

<%= t(".role_descriptions.admin") %>

+
+
+ + <%= t(".roles.super_admin") %> + +

<%= t(".role_descriptions.super_admin") %>

+
+
+ <% end %> +
diff --git a/app/views/settings/_settings_nav.html.erb b/app/views/settings/_settings_nav.html.erb index e34f64f2d..89a1a73c2 100644 --- a/app/views/settings/_settings_nav.html.erb +++ b/app/views/settings/_settings_nav.html.erb @@ -30,7 +30,9 @@ nav_sections = [ { label: t(".api_keys_label"), path: settings_api_key_path, icon: "key" }, { label: t(".self_hosting_label"), path: settings_hosting_path, icon: "database", if: self_hosted? }, { label: "Providers", path: settings_providers_path, icon: "plug" }, - { label: t(".imports_label"), path: imports_path, icon: "download" } + { label: t(".imports_label"), path: imports_path, icon: "download" }, + { label: "SSO Providers", path: admin_sso_providers_path, icon: "key-round", if: Current.user&.super_admin? }, + { label: "Users", path: admin_users_path, icon: "users", if: Current.user&.super_admin? } ] } : nil ), diff --git a/app/views/settings/sso_identities/show.html.erb b/app/views/settings/sso_identities/show.html.erb new file mode 100644 index 000000000..b9046425e --- /dev/null +++ b/app/views/settings/sso_identities/show.html.erb @@ -0,0 +1,59 @@ +<%= content_for :page_title, t(".page_title") %> + +<%= settings_section title: t(".identities_title"), subtitle: t(".identities_subtitle") do %> + <% if @oidc_identities.any? %> +
+ <% @oidc_identities.each do |identity| %> +
+
+
+ <%= icon identity.provider_config&.dig(:icon) || "key", class: "w-5 h-5 text-secondary" %> +
+
+

<%= identity.provider_config&.dig(:label) || identity.provider.titleize %>

+

<%= identity.info&.dig("email") || t(".no_email") %>

+

+ <%= t(".last_used") %>: + <%= identity.last_authenticated_at&.to_fs(:short) || t(".never") %> +

+
+
+ <% if @oidc_identities.count > 1 || Current.user.password_digest.present? %> + <%= render DS::Button.new( + text: t(".disconnect"), + variant: "outline", + size: "sm", + href: settings_sso_identity_path(identity), + method: :delete, + confirm: CustomConfirm.new( + title: t(".confirm_title"), + body: t(".confirm_body", provider: identity.provider_config&.dig(:label) || identity.provider.titleize), + btn_text: t(".confirm_button"), + destructive: true + ) + ) %> + <% end %> +
+ <% end %> +
+ <% else %> +
+ <%= icon "link", class: "w-12 h-12 mx-auto text-secondary mb-3" %> +

<%= t(".no_identities") %>

+ <% if AuthConfig.sso_providers.any? %> +

<%= t(".connect_hint") %>

+ <% end %> +
+ <% end %> +<% end %> + +<% if @oidc_identities.count == 1 && Current.user.password_digest.blank? %> + <%= settings_section title: t(".warning_title") do %> +
+
+ <%= icon "alert-triangle", class: "w-5 h-5 text-amber-600 shrink-0 mt-0.5" %> +

<%= t(".warning_message") %>

+
+
+ <% end %> +<% end %> diff --git a/config/application.rb b/config/application.rb index 8a7c2f7be..d0ef1361f 100644 --- a/config/application.rb +++ b/config/application.rb @@ -41,5 +41,9 @@ module Sure # Enable Rack::Attack middleware for API rate limiting config.middleware.use Rack::Attack + + # Handle OmniAuth/OIDC errors gracefully (must be before OmniAuth middleware) + require_relative "../app/middleware/omniauth_error_handler" + config.middleware.use OmniauthErrorHandler end end diff --git a/config/auth.yml b/config/auth.yml index 1e237cca2..ebcbc6ea0 100644 --- a/config/auth.yml +++ b/config/auth.yml @@ -23,11 +23,25 @@ default: &default # Generic OpenID Connect provider (e.g., Keycloak, Authentik, other OIDC issuers). # This maps to the existing :openid_connect OmniAuth strategy and keeps # backwards-compatible behavior for self-hosted setups using OIDC_* env vars. + # + # For the default OIDC provider, use these ENV vars: + # OIDC_ISSUER, OIDC_CLIENT_ID, OIDC_CLIENT_SECRET, OIDC_REDIRECT_URI + # + # To add additional OIDC providers, add more entries with unique names and use + # provider-specific ENV vars with the pattern: OIDC__* + # Example for a provider named "keycloak": + # OIDC_KEYCLOAK_ISSUER, OIDC_KEYCLOAK_CLIENT_ID, + # OIDC_KEYCLOAK_CLIENT_SECRET, OIDC_KEYCLOAK_REDIRECT_URI - id: "oidc" strategy: "openid_connect" name: "openid_connect" label: <%= ENV.fetch("OIDC_BUTTON_LABEL", "Sign in with OpenID Connect") %> icon: <%= ENV.fetch("OIDC_BUTTON_ICON", "key") %> + # Per-provider credentials (optional, falls back to global OIDC_* vars) + issuer: <%= ENV["OIDC_ISSUER"] %> + client_id: <%= ENV["OIDC_CLIENT_ID"] %> + client_secret: <%= ENV["OIDC_CLIENT_SECRET"] %> + redirect_uri: <%= ENV["OIDC_REDIRECT_URI"] %> # Optional Google OAuth provider. Requires the omniauth-google-oauth2 gem # and GOOGLE_OAUTH_CLIENT_ID / GOOGLE_OAUTH_CLIENT_SECRET env vars. diff --git a/config/brakeman.ignore b/config/brakeman.ignore index 05172fd20..0cb225287 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -69,6 +69,29 @@ ], "note": "" }, + { + "warning_type": "Mass Assignment", + "warning_code": 105, + "fingerprint": "01a88a0a17848e70999c17f6438a636b00e01da39a2c0aa0c46f20f0685c7202", + "check_name": "PermitAttributes", + "message": "Potentially dangerous key allowed for mass assignment", + "file": "app/controllers/admin/users_controller.rb", + "line": 35, + "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", + "code": "params.require(:user).permit(:role)", + "render_path": null, + "location": { + "type": "method", + "class": "Admin::UsersController", + "method": "user_params" + }, + "user_input": ":role", + "confidence": "Medium", + "cwe_id": [ + 915 + ], + "note": "Protected by Pundit authorization - UserPolicy requires super_admin and prevents users from changing their own role" + }, { "warning_type": "Dangerous Eval", "warning_code": 13, diff --git a/config/initializers/flipper.rb b/config/initializers/flipper.rb new file mode 100644 index 000000000..6ed3abe4a --- /dev/null +++ b/config/initializers/flipper.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require "flipper" +require "flipper/adapters/active_record" +require "flipper/adapters/memory" + +# Configure Flipper with ActiveRecord adapter for database-backed feature flags +# Falls back to memory adapter if tables don't exist yet (during migrations) +Flipper.configure do |config| + config.adapter do + begin + Flipper::Adapters::ActiveRecord.new + rescue ActiveRecord::NoDatabaseError, ActiveRecord::StatementInvalid, NameError + # Tables don't exist yet, use memory adapter as fallback + Flipper::Adapters::Memory.new + end + end +end + +# Initialize feature flags IMMEDIATELY (not in after_initialize) +# This must happen before OmniAuth initializer runs +unless Rails.env.test? + begin + # Feature flag to control SSO provider source (YAML vs DB) + # ENV: AUTH_PROVIDERS_SOURCE=db|yaml + # Default: "db" for self-hosted, "yaml" for managed + auth_source = ENV.fetch("AUTH_PROVIDERS_SOURCE") do + Rails.configuration.app_mode.self_hosted? ? "db" : "yaml" + end.downcase + + # Ensure feature exists before enabling/disabling + Flipper.add(:db_sso_providers) unless Flipper.exist?(:db_sso_providers) + + if auth_source == "db" + Flipper.enable(:db_sso_providers) + else + Flipper.disable(:db_sso_providers) + end + rescue ActiveRecord::NoDatabaseError, ActiveRecord::StatementInvalid + # Database not ready yet (e.g., during initial setup or migrations) + # This is expected during db:create or initial setup + rescue StandardError => e + Rails.logger.warn("[Flipper] Error initializing feature flags: #{e.message}") + end +end diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 9b836e436..1b4d301b3 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -5,42 +5,101 @@ require "omniauth/rails_csrf_protection" Rails.configuration.x.auth.oidc_enabled = false Rails.configuration.x.auth.sso_providers ||= [] +# Configure OmniAuth to handle failures gracefully +OmniAuth.config.on_failure = proc do |env| + error = env["omniauth.error"] + error_type = env["omniauth.error.type"] + strategy = env["omniauth.error.strategy"] + + # Log the error for debugging + Rails.logger.error("[OmniAuth] Authentication failed: #{error_type} - #{error&.message}") + + # Redirect to failure handler with error info + message = case error_type + when :discovery_failed, :invalid_credentials + "sso_provider_unavailable" + when :invalid_response + "sso_invalid_response" + else + "sso_failed" + end + + Rack::Response.new([ "302 Moved" ], 302, "Location" => "/auth/failure?message=#{message}&strategy=#{strategy&.name}").finish +end + Rails.application.config.middleware.use OmniAuth::Builder do - (Rails.configuration.x.auth.providers || []).each do |raw_cfg| + # Load providers from either YAML or DB via ProviderLoader + providers = ProviderLoader.load_providers + + providers.each do |raw_cfg| cfg = raw_cfg.deep_symbolize_keys strategy = cfg[:strategy].to_s name = (cfg[:name] || cfg[:id]).to_s case strategy when "openid_connect" - required_env = %w[OIDC_ISSUER OIDC_CLIENT_ID OIDC_CLIENT_SECRET OIDC_REDIRECT_URI] - enabled = Rails.env.test? || required_env.all? { |k| ENV[k].present? } - next unless enabled + # Support per-provider credentials from config or fall back to global ENV vars + issuer = cfg[:issuer].presence || ENV["OIDC_ISSUER"].presence + client_id = cfg[:client_id].presence || ENV["OIDC_CLIENT_ID"].presence + client_secret = cfg[:client_secret].presence || ENV["OIDC_CLIENT_SECRET"].presence + redirect_uri = cfg[:redirect_uri].presence || ENV["OIDC_REDIRECT_URI"].presence - issuer = (ENV["OIDC_ISSUER"].presence || "https://test.example.com").to_s.strip - client_id = ENV["OIDC_CLIENT_ID"].presence || "test_client_id" - client_secret = ENV["OIDC_CLIENT_SECRET"].presence || "test_client_secret" - redirect_uri = ENV["OIDC_REDIRECT_URI"].presence || "http://test.example.com/callback" + # In test environment, use test values if nothing is configured + if Rails.env.test? + issuer ||= "https://test.example.com" + client_id ||= "test_client_id" + client_secret ||= "test_client_secret" + redirect_uri ||= "http://test.example.com/callback" + end - provider :openid_connect, - name: name.to_sym, - scope: %i[openid email profile], - response_type: :code, - issuer: issuer, - discovery: true, - pkce: true, - client_options: { - identifier: client_id, - secret: client_secret, - redirect_uri: redirect_uri - } + # Skip if required fields are missing (except in test) + unless issuer.present? && client_id.present? && client_secret.present? && redirect_uri.present? + Rails.logger.warn("[OmniAuth] Skipping OIDC provider '#{name}' - missing required configuration") + next + end + + # Custom scopes: parse from settings if provided, otherwise use defaults + custom_scopes = cfg.dig(:settings, :scopes).presence + scopes = if custom_scopes.present? + custom_scopes.to_s.split(/\s+/).map(&:to_sym) + else + %i[openid email profile] + end + + # Build provider options + oidc_options = { + name: name.to_sym, + scope: scopes, + response_type: :code, + issuer: issuer.to_s.strip, + discovery: true, + pkce: true, + client_options: { + identifier: client_id, + secret: client_secret, + redirect_uri: redirect_uri + } + } + + # Add prompt parameter if configured + prompt = cfg.dig(:settings, :prompt).presence + oidc_options[:prompt] = prompt if prompt.present? + + provider :openid_connect, oidc_options Rails.configuration.x.auth.oidc_enabled = true - Rails.configuration.x.auth.sso_providers << cfg.merge(name: name) + Rails.configuration.x.auth.sso_providers << cfg.merge(name: name, issuer: issuer) when "google_oauth2" - client_id = ENV["GOOGLE_OAUTH_CLIENT_ID"].presence || (Rails.env.test? ? "test_client_id" : nil) - client_secret = ENV["GOOGLE_OAUTH_CLIENT_SECRET"].presence || (Rails.env.test? ? "test_client_secret" : nil) + client_id = cfg[:client_id].presence || ENV["GOOGLE_OAUTH_CLIENT_ID"].presence + client_secret = cfg[:client_secret].presence || ENV["GOOGLE_OAUTH_CLIENT_SECRET"].presence + + # Test environment fallback + if Rails.env.test? + client_id ||= "test_client_id" + client_secret ||= "test_client_secret" + end + next unless client_id.present? && client_secret.present? provider :google_oauth2, @@ -54,8 +113,15 @@ Rails.application.config.middleware.use OmniAuth::Builder do Rails.configuration.x.auth.sso_providers << cfg.merge(name: name) when "github" - client_id = ENV["GITHUB_CLIENT_ID"].presence || (Rails.env.test? ? "test_client_id" : nil) - client_secret = ENV["GITHUB_CLIENT_SECRET"].presence || (Rails.env.test? ? "test_client_secret" : nil) + client_id = cfg[:client_id].presence || ENV["GITHUB_CLIENT_ID"].presence + client_secret = cfg[:client_secret].presence || ENV["GITHUB_CLIENT_SECRET"].presence + + # Test environment fallback + if Rails.env.test? + client_id ||= "test_client_id" + client_secret ||= "test_client_secret" + end + next unless client_id.present? && client_secret.present? provider :github, @@ -67,10 +133,54 @@ Rails.application.config.middleware.use OmniAuth::Builder do } Rails.configuration.x.auth.sso_providers << cfg.merge(name: name) + + when "saml" + settings = cfg[:settings] || {} + + # Require either metadata URL or manual SSO URL + idp_metadata_url = settings[:idp_metadata_url].presence || settings["idp_metadata_url"].presence + idp_sso_url = settings[:idp_sso_url].presence || settings["idp_sso_url"].presence + + unless idp_metadata_url.present? || idp_sso_url.present? + Rails.logger.warn("[OmniAuth] Skipping SAML provider '#{name}' - missing IdP configuration") + next + end + + # Build SAML options + saml_options = { + name: name.to_sym, + assertion_consumer_service_url: cfg[:redirect_uri].presence || "#{ENV['APP_URL']}/auth/#{name}/callback", + issuer: cfg[:issuer].presence || ENV["APP_URL"], + name_identifier_format: settings[:name_id_format].presence || settings["name_id_format"].presence || + "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", + attribute_statements: { + email: [ "email", "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress" ], + first_name: [ "first_name", "givenName", "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname" ], + last_name: [ "last_name", "surname", "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname" ], + groups: [ "groups", "http://schemas.microsoft.com/ws/2008/06/identity/claims/groups" ] + } + } + + # Use metadata URL or manual configuration + if idp_metadata_url.present? + saml_options[:idp_metadata_url] = idp_metadata_url + else + saml_options[:idp_sso_service_url] = idp_sso_url + saml_options[:idp_cert] = settings[:idp_certificate].presence || settings["idp_certificate"].presence + saml_options[:idp_cert_fingerprint] = settings[:idp_cert_fingerprint].presence || settings["idp_cert_fingerprint"].presence + end + + # Optional: IdP SLO (Single Logout) URL + idp_slo_url = settings[:idp_slo_url].presence || settings["idp_slo_url"].presence + saml_options[:idp_slo_service_url] = idp_slo_url if idp_slo_url.present? + + provider :saml, saml_options + + Rails.configuration.x.auth.sso_providers << cfg.merge(name: name, strategy: "saml") end end end if Rails.configuration.x.auth.sso_providers.empty? - Rails.logger.warn("No SSO providers enabled; check auth.yml / ENV configuration") + Rails.logger.warn("No SSO providers enabled; check auth.yml / ENV configuration or database providers") end diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 3d225e58b..c7b3ac0c2 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -9,6 +9,12 @@ class Rack::Attack request.ip if request.path == "/oauth/token" end + # Throttle admin endpoints to prevent brute-force attacks + # More restrictive than general API limits since admin access is sensitive + throttle("admin/ip", limit: 10, period: 1.minute) do |request| + request.ip if request.path.start_with?("/admin/") + end + # Determine limits based on self-hosted mode self_hosted = Rails.application.config.app_mode.self_hosted? diff --git a/config/locales/views/admin/sso_providers/en.yml b/config/locales/views/admin/sso_providers/en.yml new file mode 100644 index 000000000..c59c26380 --- /dev/null +++ b/config/locales/views/admin/sso_providers/en.yml @@ -0,0 +1,109 @@ +--- +en: + admin: + unauthorized: "You are not authorized to access this area." + sso_providers: + index: + title: "SSO Providers" + description: "Manage single sign-on authentication providers for your instance" + add_provider: "Add Provider" + no_providers_title: "No SSO Providers" + no_providers_message: "Get started by adding your first SSO provider." + note: "Changes to SSO providers require a server restart to take effect. Alternatively, enable the AUTH_PROVIDERS_SOURCE=db feature flag to load providers from the database dynamically." + table: + name: "Name" + strategy: "Strategy" + status: "Status" + issuer: "Issuer" + actions: "Actions" + enabled: "Enabled" + disabled: "Disabled" + new: + title: "Add SSO Provider" + description: "Configure a new single sign-on authentication provider" + edit: + title: "Edit SSO Provider" + description: "Update configuration for %{label}" + create: + success: "SSO provider was successfully created." + update: + success: "SSO provider was successfully updated." + destroy: + success: "SSO provider was successfully deleted." + confirm: "Are you sure you want to delete this provider? This action cannot be undone." + toggle: + success_enabled: "SSO provider was successfully enabled." + success_disabled: "SSO provider was successfully disabled." + confirm_enable: "Are you sure you want to enable this provider?" + confirm_disable: "Are you sure you want to disable this provider?" + form: + basic_information: "Basic Information" + oauth_configuration: "OAuth/OIDC Configuration" + strategy_label: "Strategy" + strategy_help: "The authentication strategy to use" + name_label: "Name" + name_placeholder: "e.g., openid_connect, keycloak, authentik" + name_help: "Unique identifier (lowercase, numbers, underscores only)" + label_label: "Label" + label_placeholder: "e.g., Sign in with Keycloak" + label_help: "Button text shown to users" + icon_label: "Icon" + icon_placeholder: "e.g., key, google, github" + icon_help: "Lucide icon name (optional)" + enabled_label: "Enable this provider" + issuer_label: "Issuer" + issuer_placeholder: "https://accounts.google.com" + issuer_help: "OIDC issuer URL (will validate .well-known/openid-configuration endpoint)" + client_id_label: "Client ID" + client_id_placeholder: "your-client-id" + client_id_help: "OAuth client ID from your identity provider" + client_secret_label: "Client Secret" + client_secret_placeholder_new: "your-client-secret" + client_secret_placeholder_existing: "••••••••••••••••" + client_secret_help: "OAuth client secret (encrypted in database)" + client_secret_help_existing: " - leave blank to keep existing" + redirect_uri_label: "Redirect URI" + redirect_uri_placeholder: "https://yourdomain.com/auth/openid_connect/callback" + redirect_uri_help: "Callback URL to configure in your identity provider" + copy_button: "Copy" + cancel: "Cancel" + submit: "Save Provider" + errors_title: "%{count} error prohibited this provider from being saved:" + provisioning_title: "User Provisioning" + default_role_label: "Default Role for New Users" + default_role_help: "Role assigned to users created via just-in-time (JIT) SSO account provisioning. Defaults to Member." + role_member: "Member" + role_admin: "Admin" + role_super_admin: "Super Admin" + role_mapping_title: "Group to Role Mapping (Optional)" + role_mapping_help: "Map IdP groups/claims to application roles. Users are assigned the highest matching role. Leave blank to use the default role above." + super_admin_groups: "Super Admin Groups" + admin_groups: "Admin Groups" + member_groups: "Member Groups" + groups_help: "Comma-separated list of IdP group names. Use * to match all groups." + advanced_title: "Advanced OIDC Settings" + scopes_label: "Custom Scopes" + scopes_help: "Space-separated list of OIDC scopes. Leave blank for defaults (openid email profile). Add 'groups' to retrieve group claims." + prompt_label: "Authentication Prompt" + prompt_default: "Default (IdP decides)" + prompt_login: "Force Login (re-authenticate)" + prompt_consent: "Force Consent (re-authorize)" + prompt_select_account: "Account Selection (choose account)" + prompt_none: "No Prompt (silent auth)" + prompt_help: "Controls how the IdP prompts the user during authentication." + test_connection: "Test Connection" + saml_configuration: "SAML Configuration" + idp_metadata_url: "IdP Metadata URL" + idp_metadata_url_help: "URL to your IdP's SAML metadata. If provided, other SAML settings will be auto-configured." + manual_saml_config: "Manual Configuration (if not using metadata URL)" + manual_saml_help: "Only use these settings if your IdP doesn't provide a metadata URL." + idp_sso_url: "IdP SSO URL" + idp_slo_url: "IdP SLO URL (optional)" + idp_certificate: "IdP Certificate" + idp_certificate_help: "X.509 certificate in PEM format. Required if not using metadata URL." + idp_cert_fingerprint: "Certificate Fingerprint (alternative)" + name_id_format: "NameID Format" + name_id_email: "Email Address (default)" + name_id_persistent: "Persistent" + name_id_transient: "Transient" + name_id_unspecified: "Unspecified" diff --git a/config/locales/views/admin/users/en.yml b/config/locales/views/admin/users/en.yml new file mode 100644 index 000000000..6e77b7011 --- /dev/null +++ b/config/locales/views/admin/users/en.yml @@ -0,0 +1,22 @@ +--- +en: + admin: + users: + index: + title: "User Management" + description: "Manage user roles for your instance. Super admins can access SSO provider settings and user management." + section_title: "Users" + you: "(You)" + no_users: "No users found." + role_descriptions_title: "Role Descriptions" + roles: + member: "Member" + admin: "Admin" + super_admin: "Super Admin" + role_descriptions: + member: "Basic user access. Can manage their own accounts, transactions, and settings." + admin: "Family administrator. Can access advanced settings like API keys, imports, and AI prompts." + super_admin: "Instance administrator. Can manage SSO providers, user roles, and impersonate users for support." + update: + success: "User role updated successfully." + failure: "Failed to update user role." diff --git a/config/locales/views/sessions/en.yml b/config/locales/views/sessions/en.yml index 8891e194d..6dfd39de4 100644 --- a/config/locales/views/sessions/en.yml +++ b/config/locales/views/sessions/en.yml @@ -10,6 +10,9 @@ en: failed: Could not authenticate via OpenID Connect. failure: failed: Could not authenticate. + sso_provider_unavailable: "The SSO provider is currently unavailable. Please try again later or contact an administrator." + sso_invalid_response: "Received an invalid response from the SSO provider. Please try again." + sso_failed: "Single sign-on authentication failed. Please try again." new: email: Email address email_placeholder: you@example.com diff --git a/config/locales/views/settings/sso_identities/en.yml b/config/locales/views/settings/sso_identities/en.yml new file mode 100644 index 000000000..c989ee974 --- /dev/null +++ b/config/locales/views/settings/sso_identities/en.yml @@ -0,0 +1,22 @@ +--- +en: + settings: + sso_identities: + show: + page_title: "Connected Accounts" + identities_title: "SSO Connections" + identities_subtitle: "Manage your single sign-on account connections" + disconnect: "Disconnect" + last_used: "Last used" + never: "Never" + no_email: "No email" + no_identities: "No SSO accounts connected" + connect_hint: "Log out and sign in with an SSO provider to connect an account." + confirm_title: "Disconnect Account?" + confirm_body: "Are you sure you want to disconnect your %{provider} account? You can reconnect it later by signing in with that provider again." + confirm_button: "Disconnect" + warning_title: "Important" + warning_message: "This is your only login method. You should set a password in your security settings before disconnecting, otherwise you may be locked out of your account." + destroy: + success: "Successfully disconnected %{provider}" + cannot_unlink_last: "Cannot disconnect your only login method. Please set a password first." diff --git a/config/routes.rb b/config/routes.rb index d6dd270e1..9d0e8fa4a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -54,9 +54,10 @@ Rails.application.routes.draw do resource :current_session, only: %i[update] resource :registration, only: %i[new create] - resources :sessions, only: %i[new create destroy] + resources :sessions, only: %i[index new create destroy] match "/auth/:provider/callback", to: "sessions#openid_connect", via: %i[get post] match "/auth/failure", to: "sessions#failure", via: %i[get post] + get "/auth/logout/callback", to: "sessions#post_logout" resource :oidc_account, only: [] do get :link, on: :collection post :create_link, on: :collection @@ -90,6 +91,8 @@ Rails.application.routes.draw do end resource :billing, only: :show resource :security, only: :show + resource :sso_identities, only: :show + resources :sso_identities, only: :destroy resource :api_key, only: [ :show, :new, :create, :destroy ] resource :ai_prompts, only: :show resource :llm_usage, only: :show @@ -368,6 +371,17 @@ Rails.application.routes.draw do get "privacy", to: redirect("about:blank") get "terms", to: redirect("about:blank") + # Admin namespace for super admin functionality + namespace :admin do + resources :sso_providers do + member do + patch :toggle + post :test_connection + end + end + resources :users, only: [ :index, :update ] + end + # Defines the root path route ("/") root "pages#dashboard" end diff --git a/db/migrate/20251228181150_create_flipper_tables.rb b/db/migrate/20251228181150_create_flipper_tables.rb new file mode 100644 index 000000000..811f528cf --- /dev/null +++ b/db/migrate/20251228181150_create_flipper_tables.rb @@ -0,0 +1,22 @@ +class CreateFlipperTables < ActiveRecord::Migration[7.2] + def up + create_table :flipper_features do |t| + t.string :key, null: false + t.timestamps null: false + end + add_index :flipper_features, :key, unique: true + + create_table :flipper_gates do |t| + t.string :feature_key, null: false + t.string :key, null: false + t.text :value + t.timestamps null: false + end + add_index :flipper_gates, [ :feature_key, :key, :value ], unique: true, length: { value: 255 } + end + + def down + drop_table :flipper_gates + drop_table :flipper_features + end +end diff --git a/db/migrate/20251228181429_create_sso_providers.rb b/db/migrate/20251228181429_create_sso_providers.rb new file mode 100644 index 000000000..5278af4e4 --- /dev/null +++ b/db/migrate/20251228181429_create_sso_providers.rb @@ -0,0 +1,21 @@ +class CreateSsoProviders < ActiveRecord::Migration[7.2] + def change + create_table :sso_providers, id: :uuid do |t| + t.string :strategy, null: false + t.string :name, null: false + t.string :label, null: false + t.string :icon + t.boolean :enabled, null: false, default: true + t.string :issuer + t.string :client_id + t.string :client_secret + t.string :redirect_uri + t.jsonb :settings, null: false, default: {} + + t.timestamps + end + + add_index :sso_providers, :name, unique: true + add_index :sso_providers, :enabled + end +end diff --git a/db/migrate/20251228182113_add_issuer_to_oidc_identities.rb b/db/migrate/20251228182113_add_issuer_to_oidc_identities.rb new file mode 100644 index 000000000..5922cd3fc --- /dev/null +++ b/db/migrate/20251228182113_add_issuer_to_oidc_identities.rb @@ -0,0 +1,6 @@ +class AddIssuerToOidcIdentities < ActiveRecord::Migration[7.2] + def change + add_column :oidc_identities, :issuer, :string + add_index :oidc_identities, :issuer + end +end diff --git a/db/migrate/20260103170412_create_sso_audit_logs.rb b/db/migrate/20260103170412_create_sso_audit_logs.rb new file mode 100644 index 000000000..f28ce14e9 --- /dev/null +++ b/db/migrate/20260103170412_create_sso_audit_logs.rb @@ -0,0 +1,18 @@ +class CreateSsoAuditLogs < ActiveRecord::Migration[7.2] + def change + create_table :sso_audit_logs, id: :uuid do |t| + t.references :user, type: :uuid, foreign_key: true, null: true + t.string :event_type, null: false + t.string :provider + t.string :ip_address + t.string :user_agent + t.jsonb :metadata, null: false, default: {} + + t.timestamps + end + + add_index :sso_audit_logs, :event_type + add_index :sso_audit_logs, :created_at + add_index :sso_audit_logs, [ :user_id, :created_at ] + end +end diff --git a/db/schema.rb b/db/schema.rb index 27adfde1e..1ff598f3c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_12_15_100443) do +ActiveRecord::Schema[7.2].define(version: 2026_01_03_170412) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -422,6 +422,22 @@ ActiveRecord::Schema[7.2].define(version: 2025_12_15_100443) do t.index ["family_id"], name: "index_family_exports_on_family_id" end + create_table "flipper_features", force: :cascade do |t| + t.string "key", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["key"], name: "index_flipper_features_on_key", unique: true + end + + create_table "flipper_gates", force: :cascade do |t| + t.string "feature_key", null: false + t.string "key", null: false + t.text "value" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["feature_key", "key", "value"], name: "index_flipper_gates_on_feature_key_and_key_and_value", unique: true + end + create_table "holdings", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "account_id", null: false t.uuid "security_id", null: false @@ -741,6 +757,8 @@ ActiveRecord::Schema[7.2].define(version: 2025_12_15_100443) do t.datetime "last_authenticated_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "issuer" + t.index ["issuer"], name: "index_oidc_identities_on_issuer" t.index ["provider", "uid"], name: "index_oidc_identities_on_provider_and_uid", unique: true t.index ["user_id"], name: "index_oidc_identities_on_user_id" end @@ -994,6 +1012,38 @@ ActiveRecord::Schema[7.2].define(version: 2025_12_15_100443) do t.index ["status"], name: "index_simplefin_items_on_status" end + create_table "sso_audit_logs", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "user_id" + t.string "event_type", null: false + t.string "provider" + t.string "ip_address" + t.string "user_agent" + t.jsonb "metadata", default: {}, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["created_at"], name: "index_sso_audit_logs_on_created_at" + t.index ["event_type"], name: "index_sso_audit_logs_on_event_type" + t.index ["user_id", "created_at"], name: "index_sso_audit_logs_on_user_id_and_created_at" + t.index ["user_id"], name: "index_sso_audit_logs_on_user_id" + end + + create_table "sso_providers", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.string "strategy", null: false + t.string "name", null: false + t.string "label", null: false + t.string "icon" + t.boolean "enabled", default: true, null: false + t.string "issuer" + t.string "client_id" + t.string "client_secret" + t.string "redirect_uri" + t.jsonb "settings", default: {}, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["enabled"], name: "index_sso_providers_on_enabled" + t.index ["name"], name: "index_sso_providers_on_name", unique: true + end + create_table "subscriptions", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "family_id", null: false t.string "status", null: false @@ -1215,6 +1265,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_12_15_100443) do add_foreign_key "sessions", "users" add_foreign_key "simplefin_accounts", "simplefin_items" add_foreign_key "simplefin_items", "families" + add_foreign_key "sso_audit_logs", "users" add_foreign_key "subscriptions", "families" add_foreign_key "syncs", "syncs", column: "parent_id" add_foreign_key "taggings", "tags" diff --git a/docs/hosting/oidc.md b/docs/hosting/oidc.md index 1db92e8ed..04be10bed 100644 --- a/docs/hosting/oidc.md +++ b/docs/hosting/oidc.md @@ -250,3 +250,238 @@ With these settings, you can run Sure in: - Domain‑restricted and link‑only enterprise SSO modes Use the combination that best fits your self‑hosted environment and security posture. + +--- + +## 5. Multiple OIDC Providers + +Sure supports configuring multiple OIDC providers simultaneously, allowing users to choose between different identity providers (e.g., Keycloak, Authentik, Okta) on the login page. + +### 5.1 YAML-based multi-provider configuration + +To add multiple OIDC providers in `config/auth.yml`, add additional provider entries with unique names: + +```yaml +providers: + # First OIDC provider (e.g., Keycloak) + - id: "keycloak" + strategy: "openid_connect" + name: "keycloak" + label: "Sign in with Keycloak" + icon: "key" + issuer: <%= ENV["OIDC_KEYCLOAK_ISSUER"] %> + client_id: <%= ENV["OIDC_KEYCLOAK_CLIENT_ID"] %> + client_secret: <%= ENV["OIDC_KEYCLOAK_CLIENT_SECRET"] %> + redirect_uri: <%= ENV["OIDC_KEYCLOAK_REDIRECT_URI"] %> + + # Second OIDC provider (e.g., Authentik) + - id: "authentik" + strategy: "openid_connect" + name: "authentik" + label: "Sign in with Authentik" + icon: "shield" + issuer: <%= ENV["OIDC_AUTHENTIK_ISSUER"] %> + client_id: <%= ENV["OIDC_AUTHENTIK_CLIENT_ID"] %> + client_secret: <%= ENV["OIDC_AUTHENTIK_CLIENT_SECRET"] %> + redirect_uri: <%= ENV["OIDC_AUTHENTIK_REDIRECT_URI"] %> +``` + +Set the corresponding environment variables: + +```bash +# Keycloak provider +OIDC_KEYCLOAK_ISSUER="https://keycloak.example.com/realms/myrealm" +OIDC_KEYCLOAK_CLIENT_ID="sure-client" +OIDC_KEYCLOAK_CLIENT_SECRET="your-keycloak-secret" +OIDC_KEYCLOAK_REDIRECT_URI="https://yourdomain.com/auth/keycloak/callback" + +# Authentik provider +OIDC_AUTHENTIK_ISSUER="https://authentik.example.com/application/o/sure/" +OIDC_AUTHENTIK_CLIENT_ID="sure-authentik-client" +OIDC_AUTHENTIK_CLIENT_SECRET="your-authentik-secret" +OIDC_AUTHENTIK_REDIRECT_URI="https://yourdomain.com/auth/authentik/callback" +``` + +**Important:** Each provider must have a unique `name` field, which determines the callback URL path (`/auth//callback`). + +--- + +## 6. Database-Backed Provider Management + +For more dynamic provider management, Sure supports storing SSO provider configurations in the database with a web-based admin interface. + +### 6.1 Enabling database providers + +Set the feature flag to load providers from the database instead of YAML: + +```bash +AUTH_PROVIDERS_SOURCE=db +``` + +When enabled: +- Providers are loaded from the `sso_providers` database table +- Changes take effect immediately (no server restart required) +- Providers can be managed through the admin UI at `/admin/sso_providers` + +When disabled (default): +- Providers are loaded from `config/auth.yml` +- Changes require a server restart + +### 6.2 Admin UI for SSO providers + +Super-admin users can manage SSO providers through the web interface: + +1. Navigate to `/admin/sso_providers` +2. View all configured providers (enabled/disabled status) +3. Add new providers with the "Add Provider" button +4. Edit existing providers (credentials, labels, icons) +5. Enable/disable providers with the toggle button +6. Delete providers (with confirmation) + +**Security notes:** +- Only users with `super_admin` role can access the admin interface +- All provider changes are logged with user ID and timestamp +- Client secrets are encrypted in the database using Rails 7.2 encryption +- Admin endpoints are rate-limited (10 requests/minute per IP) + +### 6.3 Seeding providers from YAML to database + +To migrate your existing YAML configuration to the database: + +```bash +# Dry run (preview changes without saving) +DRY_RUN=true rails sso_providers:seed + +# Apply changes +rails sso_providers:seed +``` + +The seeding task: +- Reads providers from `config/auth.yml` +- Creates or updates database records (idempotent) +- Preserves existing client secrets if not provided in YAML +- Provides detailed output (created/updated/skipped/errors) + +To list all providers in the database: + +```bash +rails sso_providers:list +``` + +### 6.4 Migration workflow + +Recommended steps to migrate from YAML to database-backed providers: + +1. **Backup your configuration:** + ```bash + cp config/auth.yml config/auth.yml.backup + ``` + +2. **Run migrations:** + ```bash + rails db:migrate + ``` + +3. **Seed providers from YAML (dry run first):** + ```bash + DRY_RUN=true rails sso_providers:seed + ``` + +4. **Review the output, then apply:** + ```bash + rails sso_providers:seed + ``` + +5. **Enable database provider source:** + ```bash + # Add to .env or environment + AUTH_PROVIDERS_SOURCE=db + ``` + +6. **Restart the application:** + ```bash + # Docker Compose + docker-compose restart app + + # Or your process manager + systemctl restart sure + ``` + +7. **Verify providers are loaded:** + - Check logs for `[ProviderLoader] Loaded N provider(s) from database` + - Visit `/admin/sso_providers` to manage providers + +### 6.5 Rollback to YAML + +To switch back to YAML-based configuration: + +1. Remove or set `AUTH_PROVIDERS_SOURCE=yaml` +2. Restart the application +3. Providers will be loaded from `config/auth.yml` + +--- + +## 7. Troubleshooting + +### Provider not appearing on login page + +- **YAML mode:** Check that required environment variables are set (e.g., `OIDC_ISSUER`, `OIDC_CLIENT_ID`, `OIDC_CLIENT_SECRET`) +- **DB mode:** Verify provider is enabled in `/admin/sso_providers` +- Check application logs for provider loading messages +- Verify `AUTH_PROVIDERS_SOURCE` is set correctly + +### Discovery endpoint validation fails + +When adding an OIDC provider, Sure validates the `.well-known/openid-configuration` endpoint: + +- Ensure the issuer URL is correct and accessible +- Check firewall rules allow outbound HTTPS to the issuer +- Verify the issuer returns valid JSON with an `issuer` field +- For self-signed certificates, you may need to configure SSL verification + +### Rate limiting errors (429) + +Admin endpoints are rate-limited to 10 requests per minute per IP: + +- Wait 60 seconds before retrying +- If legitimate traffic is being blocked, adjust limits in `config/initializers/rack_attack.rb` + +### Callback URL mismatch + +Each provider requires a callback URL configured in your identity provider: + +- **Format:** `https://yourdomain.com/auth//callback` +- **Example:** For a provider with `name: "keycloak"`, use `https://yourdomain.com/auth/keycloak/callback` +- The callback URL is shown in the admin UI when editing a provider (with copy button) + +--- + +## 8. Security Considerations + +### Encryption + +- Client secrets are encrypted at rest using Rails 7.2 ActiveRecord Encryption +- Encryption keys are derived from `SECRET_KEY_BASE` by default +- For additional security, set custom encryption keys (see `.env` for `ACTIVE_RECORD_ENCRYPTION_*` variables) + +### Issuer validation + +- OIDC identities store the issuer claim from the ID token +- On subsequent logins, Sure verifies the issuer matches the configured provider +- This prevents issuer impersonation attacks + +### Admin access + +- SSO provider management requires `super_admin` role +- Regular `admin` users (family admins) cannot access `/admin/sso_providers` +- All provider changes are logged with user ID + +### Rate limiting + +- Admin endpoints: 10 requests/minute per IP +- OAuth token endpoint: 10 requests/minute per IP +- Failed login attempts should be monitored separately + +--- + +For additional help, see the main [hosting documentation](../README.md) or open an issue on GitHub. diff --git a/lib/tasks/sso_providers.rake b/lib/tasks/sso_providers.rake new file mode 100644 index 000000000..9b5762aac --- /dev/null +++ b/lib/tasks/sso_providers.rake @@ -0,0 +1,154 @@ +# frozen_string_literal: true + +namespace :sso_providers do + desc "Seed SSO providers from config/auth.yml into the database" + task seed: :environment do + dry_run = ENV["DRY_RUN"] == "true" + + puts "=" * 80 + puts "SSO Provider Seeding Task" + puts "=" * 80 + puts "Mode: #{dry_run ? 'DRY RUN (no changes will be saved)' : 'LIVE (changes will be saved)'}" + puts "Source: config/auth.yml" + puts "-" * 80 + + begin + # Load auth.yml safely + auth_config_path = Rails.root.join("config", "auth.yml") + unless File.exist?(auth_config_path) + puts "ERROR: config/auth.yml not found" + exit 1 + end + + # Use safe_load to prevent code injection + auth_config = YAML.safe_load( + ERB.new(File.read(auth_config_path)).result, + permitted_classes: [ Symbol ], + aliases: true + ) + + # Get providers for current environment + env_config = auth_config[Rails.env] || auth_config["default"] + providers = env_config&.dig("providers") || [] + + if providers.empty? + puts "WARNING: No providers found in config/auth.yml for #{Rails.env} environment" + exit 0 + end + + puts "Found #{providers.count} provider(s) in config/auth.yml" + puts "-" * 80 + + created_count = 0 + updated_count = 0 + skipped_count = 0 + errors = [] + + ActiveRecord::Base.transaction do + providers.each do |provider_config| + provider_config = provider_config.deep_symbolize_keys + + # Extract provider attributes + name = provider_config[:name] || provider_config[:id] + strategy = provider_config[:strategy] + + unless name.present? && strategy.present? + puts "SKIP: Provider missing name or strategy: #{provider_config.inspect}" + skipped_count += 1 + next + end + + # Find or initialize provider + provider = SsoProvider.find_or_initialize_by(name: name) + is_new = provider.new_record? + + # Build attributes hash + attributes = { + strategy: strategy, + label: provider_config[:label] || name.titleize, + icon: provider_config[:icon], + enabled: provider_config.key?(:enabled) ? provider_config[:enabled] : true, + issuer: provider_config[:issuer], + client_id: provider_config[:client_id], + redirect_uri: provider_config[:redirect_uri], + settings: provider_config[:settings] || {} + } + + # Only set client_secret if provided (don't overwrite existing) + if provider_config[:client_secret].present? + attributes[:client_secret] = provider_config[:client_secret] + end + + # Assign attributes + provider.assign_attributes(attributes.compact) + + # Check if changed + if provider.changed? + if dry_run + puts "#{is_new ? 'CREATE' : 'UPDATE'} (dry-run): #{name} (#{strategy})" + puts " Changes: #{provider.changes.keys.join(', ')}" + else + if provider.save + puts "#{is_new ? 'CREATE' : 'UPDATE'}: #{name} (#{strategy})" + is_new ? created_count += 1 : updated_count += 1 + else + error_msg = "Failed to save #{name}: #{provider.errors.full_messages.join(', ')}" + puts "ERROR: #{error_msg}" + errors << error_msg + end + end + else + puts "SKIP: #{name} (no changes)" + skipped_count += 1 + end + end + + # Rollback transaction if dry run + raise ActiveRecord::Rollback if dry_run + end + + puts "-" * 80 + puts "Summary:" + puts " Created: #{created_count}" + puts " Updated: #{updated_count}" + puts " Skipped: #{skipped_count}" + puts " Errors: #{errors.count}" + + if errors.any? + puts "\nErrors encountered:" + errors.each { |error| puts " - #{error}" } + end + + if dry_run + puts "\nDRY RUN: No changes were saved to the database" + puts "Run without DRY_RUN=true to apply changes" + else + puts "\nSeeding completed successfully!" + puts "Note: Clear provider cache or restart server for changes to take effect" + end + + puts "=" * 80 + + rescue => e + puts "ERROR: #{e.class}: #{e.message}" + puts e.backtrace.first(5).join("\n") + exit 1 + end + end + + desc "List all SSO providers in the database" + task list: :environment do + providers = SsoProvider.order(:name) + + if providers.empty? + puts "No SSO providers found in database" + else + puts "SSO Providers (#{providers.count}):" + puts "-" * 80 + providers.each do |provider| + status = provider.enabled? ? "✓ enabled" : "✗ disabled" + puts "#{provider.name.ljust(20)} | #{provider.strategy.ljust(20)} | #{status}" + end + end + end +end diff --git a/test/controllers/oidc_accounts_controller_test.rb b/test/controllers/oidc_accounts_controller_test.rb index dab141d1d..c81b235be 100644 --- a/test/controllers/oidc_accounts_controller_test.rb +++ b/test/controllers/oidc_accounts_controller_test.rb @@ -166,7 +166,7 @@ class OidcAccountsControllerTest < ActionController::TestCase assert_not_nil new_user assert_equal new_user_auth["first_name"], new_user.first_name assert_equal new_user_auth["last_name"], new_user.last_name - assert_equal "admin", new_user.role + assert_equal "member", new_user.role # Verify OIDC identity was created oidc_identity = new_user.oidc_identities.first diff --git a/test/models/sso_provider_test.rb b/test/models/sso_provider_test.rb new file mode 100644 index 000000000..81ca0b6df --- /dev/null +++ b/test/models/sso_provider_test.rb @@ -0,0 +1,263 @@ +require "test_helper" + +class SsoProviderTest < ActiveSupport::TestCase + test "valid provider with all required fields" do + provider = SsoProvider.new( + strategy: "openid_connect", + name: "test_oidc", + label: "Test OIDC", + enabled: true, + issuer: "https://test.example.com", + client_id: "test_client", + client_secret: "test_secret" + ) + assert provider.valid? + end + + test "requires strategy" do + provider = SsoProvider.new(name: "test", label: "Test") + assert_not provider.valid? + assert_includes provider.errors[:strategy], "can't be blank" + end + + test "requires name" do + provider = SsoProvider.new(strategy: "openid_connect", label: "Test") + assert_not provider.valid? + assert_includes provider.errors[:name], "can't be blank" + end + + test "requires label" do + provider = SsoProvider.new(strategy: "openid_connect", name: "test") + assert_not provider.valid? + assert_includes provider.errors[:label], "can't be blank" + end + + test "requires unique name" do + SsoProvider.create!( + strategy: "openid_connect", + name: "duplicate", + label: "First", + client_id: "id1", + client_secret: "secret1", + issuer: "https://first.example.com" + ) + + provider = SsoProvider.new( + strategy: "google_oauth2", + name: "duplicate", + label: "Second", + client_id: "id2", + client_secret: "secret2" + ) + + assert_not provider.valid? + assert_includes provider.errors[:name], "has already been taken" + end + + test "validates name format" do + provider = SsoProvider.new( + strategy: "openid_connect", + name: "Invalid-Name!", + label: "Test", + client_id: "test", + client_secret: "secret", + issuer: "https://test.example.com" + ) + + assert_not provider.valid? + assert_includes provider.errors[:name], "must contain only lowercase letters, numbers, and underscores" + end + + test "validates strategy inclusion" do + provider = SsoProvider.new( + strategy: "invalid_strategy", + name: "test", + label: "Test" + ) + + assert_not provider.valid? + assert_includes provider.errors[:strategy], "invalid_strategy is not a supported strategy" + end + + test "encrypts client_secret" do + provider = SsoProvider.create!( + strategy: "openid_connect", + name: "encrypted_test", + label: "Encrypted Test", + client_id: "test_client", + client_secret: "super_secret_value", + issuer: "https://test.example.com" + ) + + # Reload from database + provider.reload + + # Should be able to read decrypted value + assert_equal "super_secret_value", provider.client_secret + + # Raw database value should be encrypted (not plain text) + raw_value = ActiveRecord::Base.connection.execute( + "SELECT client_secret FROM sso_providers WHERE id = '#{provider.id}'" + ).first["client_secret"] + + assert_not_equal "super_secret_value", raw_value + end + + test "OIDC provider requires issuer" do + provider = SsoProvider.new( + strategy: "openid_connect", + name: "test_oidc", + label: "Test", + client_id: "test", + client_secret: "secret" + ) + + assert_not provider.valid? + assert_includes provider.errors[:issuer], "is required for OpenID Connect providers" + end + + test "OIDC provider requires client_id" do + provider = SsoProvider.new( + strategy: "openid_connect", + name: "test_oidc", + label: "Test", + issuer: "https://test.example.com", + client_secret: "secret" + ) + + assert_not provider.valid? + assert_includes provider.errors[:client_id], "is required for OpenID Connect providers" + end + + test "OIDC provider requires client_secret" do + provider = SsoProvider.new( + strategy: "openid_connect", + name: "test_oidc", + label: "Test", + issuer: "https://test.example.com", + client_id: "test" + ) + + assert_not provider.valid? + assert_includes provider.errors[:client_secret], "is required for OpenID Connect providers" + end + + test "OIDC provider validates issuer URL format" do + provider = SsoProvider.new( + strategy: "openid_connect", + name: "test_oidc", + label: "Test", + issuer: "not-a-valid-url", + client_id: "test", + client_secret: "secret" + ) + + assert_not provider.valid? + assert_includes provider.errors[:issuer], "must be a valid URL" + end + + test "OAuth provider requires client_id" do + provider = SsoProvider.new( + strategy: "google_oauth2", + name: "test_google", + label: "Test", + client_secret: "secret" + ) + + assert_not provider.valid? + assert_includes provider.errors[:client_id], "is required for OAuth providers" + end + + test "OAuth provider requires client_secret" do + provider = SsoProvider.new( + strategy: "google_oauth2", + name: "test_google", + label: "Test", + client_id: "test" + ) + + assert_not provider.valid? + assert_includes provider.errors[:client_secret], "is required for OAuth providers" + end + + test "enabled scope returns only enabled providers" do + enabled = SsoProvider.create!( + strategy: "openid_connect", + name: "enabled_provider", + label: "Enabled", + enabled: true, + client_id: "test", + client_secret: "secret", + issuer: "https://enabled.example.com" + ) + + SsoProvider.create!( + strategy: "openid_connect", + name: "disabled_provider", + label: "Disabled", + enabled: false, + client_id: "test", + client_secret: "secret", + issuer: "https://disabled.example.com" + ) + + assert_includes SsoProvider.enabled, enabled + assert_equal 1, SsoProvider.enabled.count + end + + test "by_strategy scope filters by strategy" do + oidc = SsoProvider.create!( + strategy: "openid_connect", + name: "oidc_provider", + label: "OIDC", + client_id: "test", + client_secret: "secret", + issuer: "https://oidc.example.com" + ) + + SsoProvider.create!( + strategy: "google_oauth2", + name: "google_provider", + label: "Google", + client_id: "test", + client_secret: "secret" + ) + + oidc_providers = SsoProvider.by_strategy("openid_connect") + assert_includes oidc_providers, oidc + assert_equal 1, oidc_providers.count + end + + test "to_omniauth_config returns correct hash" do + provider = SsoProvider.create!( + strategy: "openid_connect", + name: "test_oidc", + label: "Test OIDC", + icon: "key", + enabled: true, + issuer: "https://test.example.com", + client_id: "test_client", + client_secret: "test_secret", + redirect_uri: "https://app.example.com/callback", + settings: { scope: "openid email" } + ) + + config = provider.to_omniauth_config + + assert_equal "test_oidc", config[:id] + assert_equal "openid_connect", config[:strategy] + assert_equal "test_oidc", config[:name] + assert_equal "Test OIDC", config[:label] + assert_equal "key", config[:icon] + assert_equal "https://test.example.com", config[:issuer] + assert_equal "test_client", config[:client_id] + assert_equal "test_secret", config[:client_secret] + assert_equal "https://app.example.com/callback", config[:redirect_uri] + assert_equal({ "scope" => "openid email" }, config[:settings]) + end + + # Note: OIDC discovery validation tests are skipped in test environment + # Discovery validation is disabled in test mode to avoid VCR cassette requirements + # In production, the validate_oidc_discovery method will validate the issuer's + # .well-known/openid-configuration endpoint +end diff --git a/test/policies/sso_provider_policy_test.rb b/test/policies/sso_provider_policy_test.rb new file mode 100644 index 000000000..8632afdd0 --- /dev/null +++ b/test/policies/sso_provider_policy_test.rb @@ -0,0 +1,111 @@ +require "test_helper" + +class SsoProviderPolicyTest < ActiveSupport::TestCase + def setup + @super_admin = users(:family_admin) # Assuming this fixture has super_admin role + @super_admin.update!(role: :super_admin) + + @regular_user = users(:family_member) + @regular_user.update!(role: :member) + + @provider = SsoProvider.create!( + strategy: "openid_connect", + name: "test_provider", + label: "Test Provider", + client_id: "test", + client_secret: "secret", + issuer: "https://test.example.com" + ) + end + + test "super admin can view index" do + assert SsoProviderPolicy.new(@super_admin, SsoProvider).index? + end + + test "regular user cannot view index" do + assert_not SsoProviderPolicy.new(@regular_user, SsoProvider).index? + end + + test "nil user cannot view index" do + assert_not SsoProviderPolicy.new(nil, SsoProvider).index? + end + + test "super admin can show provider" do + assert SsoProviderPolicy.new(@super_admin, @provider).show? + end + + test "regular user cannot show provider" do + assert_not SsoProviderPolicy.new(@regular_user, @provider).show? + end + + test "super admin can create provider" do + assert SsoProviderPolicy.new(@super_admin, SsoProvider.new).create? + end + + test "regular user cannot create provider" do + assert_not SsoProviderPolicy.new(@regular_user, SsoProvider.new).create? + end + + test "super admin can access new" do + assert SsoProviderPolicy.new(@super_admin, SsoProvider.new).new? + end + + test "regular user cannot access new" do + assert_not SsoProviderPolicy.new(@regular_user, SsoProvider.new).new? + end + + test "super admin can update provider" do + assert SsoProviderPolicy.new(@super_admin, @provider).update? + end + + test "regular user cannot update provider" do + assert_not SsoProviderPolicy.new(@regular_user, @provider).update? + end + + test "super admin can access edit" do + assert SsoProviderPolicy.new(@super_admin, @provider).edit? + end + + test "regular user cannot access edit" do + assert_not SsoProviderPolicy.new(@regular_user, @provider).edit? + end + + test "super admin can destroy provider" do + assert SsoProviderPolicy.new(@super_admin, @provider).destroy? + end + + test "regular user cannot destroy provider" do + assert_not SsoProviderPolicy.new(@regular_user, @provider).destroy? + end + + test "super admin can toggle provider" do + assert SsoProviderPolicy.new(@super_admin, @provider).toggle? + end + + test "regular user cannot toggle provider" do + assert_not SsoProviderPolicy.new(@regular_user, @provider).toggle? + end + + test "scope returns all providers for super admin" do + SsoProvider.create!( + strategy: "google_oauth2", + name: "google", + label: "Google", + client_id: "test", + client_secret: "secret" + ) + + scope = SsoProviderPolicy::Scope.new(@super_admin, SsoProvider).resolve + assert_equal 2, scope.count + end + + test "scope returns no providers for regular user" do + scope = SsoProviderPolicy::Scope.new(@regular_user, SsoProvider).resolve + assert_equal 0, scope.count + end + + test "scope returns no providers for nil user" do + scope = SsoProviderPolicy::Scope.new(nil, SsoProvider).resolve + assert_equal 0, scope.count + end +end diff --git a/test/policies/user_policy_test.rb b/test/policies/user_policy_test.rb new file mode 100644 index 000000000..c4d471d94 --- /dev/null +++ b/test/policies/user_policy_test.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require "test_helper" + +class UserPolicyTest < ActiveSupport::TestCase + def setup + @super_admin = users(:family_admin) + @super_admin.update!(role: :super_admin) + + @regular_user = users(:family_member) + @regular_user.update!(role: :member) + + @other_user = users(:sure_support_staff) + @other_user.update!(role: :member) + end + + test "super admin can view index" do + assert UserPolicy.new(@super_admin, User).index? + end + + test "regular user cannot view index" do + assert_not UserPolicy.new(@regular_user, User).index? + end + + test "nil user cannot view index" do + assert_not UserPolicy.new(nil, User).index? + end + + test "super admin can update another user" do + assert UserPolicy.new(@super_admin, @regular_user).update? + end + + test "super admin cannot update themselves" do + assert_not UserPolicy.new(@super_admin, @super_admin).update? + end + + test "regular user cannot update anyone" do + assert_not UserPolicy.new(@regular_user, @other_user).update? + end + + test "nil user cannot update anyone" do + assert_not UserPolicy.new(nil, @regular_user).update? + end + + test "scope returns all users for super admin" do + scope = UserPolicy::Scope.new(@super_admin, User).resolve + assert_equal User.count, scope.count + end + + test "scope returns no users for regular user" do + scope = UserPolicy::Scope.new(@regular_user, User).resolve + assert_equal 0, scope.count + end + + test "scope returns no users for nil user" do + scope = UserPolicy::Scope.new(nil, User).resolve + assert_equal 0, scope.count + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 1479ce88e..16bab4495 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -22,6 +22,7 @@ require "minitest/mock" require "minitest/autorun" require "mocha/minitest" require "aasm/minitest" +require "webmock/minitest" VCR.configure do |config| config.cassette_library_dir = "test/vcr_cassettes" From d3055b2e0bb4abaa842810ae8d92c7581809152b Mon Sep 17 00:00:00 2001 From: Josh Waldrep Date: Sat, 3 Jan 2026 20:49:31 -0500 Subject: [PATCH 2/6] refactor: remove SSO settings page; consolidate SSO identity management under Security settings - Removed the `Settings::SsoIdentitiesController` and views for a simplified user experience. - Moved SSO identity management to the Security settings page (`Settings::SecuritiesController`). - Updated locale keys and layout for the new structure. - Fixed unlink protection warnings and adjusted redirection path. - Cleaned up routes, helper methods, and redundant code. --- .../UI/account/activity_date.html.erb | 2 +- .../UI/account/activity_feed.html.erb | 2 +- .../settings/securities_controller.rb | 1 + .../settings/sso_identities_controller.rb | 12 +--- app/helpers/settings_helper.rb | 5 -- app/policies/sso_provider_policy.rb | 4 ++ app/views/accounts/show/_activity.html.erb | 2 +- .../_budget_category.html.erb | 2 +- app/views/categories/_badge_mobile.html.erb | 2 +- app/views/entries/_entry_group.html.erb | 2 +- .../configurations/_rule_import.html.erb | 1 - .../pages/dashboard/_outflows_donut.html.erb | 2 +- .../recurring_transactions/index.html.erb | 2 +- app/views/rules/index.html.erb | 4 +- app/views/settings/llm_usages/show.html.erb | 2 +- app/views/settings/providers/show.html.erb | 1 - app/views/settings/securities/show.html.erb | 55 +++++++++++++++++ .../settings/sso_identities/show.html.erb | 59 ------------------- app/views/shared/_demo_warning.html.erb | 1 - .../simplefin_items/_simplefin_item.html.erb | 3 - app/views/transactions/index.html.erb | 2 +- config/locales/views/settings/en.yml | 17 ++++++ .../views/settings/sso_identities/en.yml | 22 ------- config/routes.rb | 1 - 24 files changed, 91 insertions(+), 115 deletions(-) delete mode 100644 app/views/settings/sso_identities/show.html.erb delete mode 100644 config/locales/views/settings/sso_identities/en.yml diff --git a/app/components/UI/account/activity_date.html.erb b/app/components/UI/account/activity_date.html.erb index 672c6d77c..a1a330588 100644 --- a/app/components/UI/account/activity_date.html.erb +++ b/app/components/UI/account/activity_date.html.erb @@ -6,7 +6,7 @@ <%= check_box_tag "#{date}_entries_selection", class: ["checkbox checkbox--light hidden lg:block", "lg:hidden": entries.size == 0], id: "selection_entry_#{date}", - data: { + data: { action: "bulk-select#toggleGroupSelection", checkbox_toggle_target: "selectionEntry" } %> diff --git a/app/components/UI/account/activity_feed.html.erb b/app/components/UI/account/activity_feed.html.erb index 362e1af06..f46053401 100644 --- a/app/components/UI/account/activity_feed.html.erb +++ b/app/components/UI/account/activity_feed.html.erb @@ -77,7 +77,7 @@
<%= check_box_tag "selection_entry", class: "checkbox checkbox--light hidden lg:block", - data: { + data: { action: "bulk-select#togglePageSelection", checkbox_toggle_target: "selectionEntry" } %> diff --git a/app/controllers/settings/securities_controller.rb b/app/controllers/settings/securities_controller.rb index 756accf79..fd6791994 100644 --- a/app/controllers/settings/securities_controller.rb +++ b/app/controllers/settings/securities_controller.rb @@ -6,5 +6,6 @@ class Settings::SecuritiesController < ApplicationController [ "Home", root_path ], [ "Security", nil ] ] + @oidc_identities = Current.user.oidc_identities.order(:provider) end end diff --git a/app/controllers/settings/sso_identities_controller.rb b/app/controllers/settings/sso_identities_controller.rb index f42175c62..97f946a90 100644 --- a/app/controllers/settings/sso_identities_controller.rb +++ b/app/controllers/settings/sso_identities_controller.rb @@ -3,20 +3,12 @@ class Settings::SsoIdentitiesController < ApplicationController layout "settings" - def show - @oidc_identities = Current.user.oidc_identities.order(:provider) - @breadcrumbs = [ - [ t("settings.nav.home"), root_path ], - [ t(".page_title"), nil ] - ] - end - def destroy @identity = Current.user.oidc_identities.find(params[:id]) # Prevent unlinking last identity if user has no password if Current.user.oidc_identities.count == 1 && Current.user.password_digest.blank? - redirect_to settings_sso_identities_path, alert: t(".cannot_unlink_last") + redirect_to settings_security_path, alert: t(".cannot_unlink_last") return end @@ -30,6 +22,6 @@ class Settings::SsoIdentitiesController < ApplicationController request: request ) - redirect_to settings_sso_identities_path, notice: t(".success", provider: provider_name) + redirect_to settings_security_path, notice: t(".success", provider: provider_name) end end diff --git a/app/helpers/settings_helper.rb b/app/helpers/settings_helper.rb index 1428aeda0..a907cd7c8 100644 --- a/app/helpers/settings_helper.rb +++ b/app/helpers/settings_helper.rb @@ -6,7 +6,6 @@ module SettingsHelper { name: "Preferences", path: :settings_preferences_path }, { name: "Profile Info", path: :settings_profile_path }, { name: "Security", path: :settings_security_path }, - { name: "Connected Accounts", path: :settings_sso_identities_path, condition: :has_sso_connections? }, { name: "Billing", path: :settings_billing_path, condition: :not_self_hosted? }, # Transactions section { name: "Categories", path: :categories_path }, @@ -82,8 +81,4 @@ module SettingsHelper def self_hosted_and_admin? self_hosted? && admin_user? end - - def has_sso_connections? - Current.user&.oidc_identities&.exists? || AuthConfig.sso_providers.any? - end end diff --git a/app/policies/sso_provider_policy.rb b/app/policies/sso_provider_policy.rb index 5c975dc66..d68010415 100644 --- a/app/policies/sso_provider_policy.rb +++ b/app/policies/sso_provider_policy.rb @@ -34,6 +34,10 @@ class SsoProviderPolicy < ApplicationPolicy update? end + def test_connection? + user&.super_admin? + end + class Scope < ApplicationPolicy::Scope def resolve if user&.super_admin? diff --git a/app/views/accounts/show/_activity.html.erb b/app/views/accounts/show/_activity.html.erb index 7bd35639b..dbed8ad52 100644 --- a/app/views/accounts/show/_activity.html.erb +++ b/app/views/accounts/show/_activity.html.erb @@ -67,7 +67,7 @@
<%= check_box_tag "selection_entry", class: "checkbox checkbox--light hidden lg:block", - data: { + data: { action: "bulk-select#togglePageSelection", checkbox_toggle_target: "selectionEntry" } %> diff --git a/app/views/budget_categories/_budget_category.html.erb b/app/views/budget_categories/_budget_category.html.erb index 3ce90e6f4..a241101d2 100644 --- a/app/views/budget_categories/_budget_category.html.erb +++ b/app/views/budget_categories/_budget_category.html.erb @@ -6,7 +6,7 @@ <% if budget_category.initialized? %> <%# Category Header with Status Badge %>
-
<% end %> -
\ No newline at end of file +
diff --git a/app/views/entries/_entry_group.html.erb b/app/views/entries/_entry_group.html.erb index 7a0d14470..5499c9f84 100644 --- a/app/views/entries/_entry_group.html.erb +++ b/app/views/entries/_entry_group.html.erb @@ -6,7 +6,7 @@ <%= check_box_tag "#{date}_entries_selection", class: ["checkbox checkbox--light hidden lg:block", "lg:hidden": entries.size == 0], id: "selection_entry_#{date}", - data: { + data: { action: "bulk-select#toggleGroupSelection", checkbox_toggle_target: "selectionEntry" } %> diff --git a/app/views/import/configurations/_rule_import.html.erb b/app/views/import/configurations/_rule_import.html.erb index 7089a40ad..eb0f8be8b 100644 --- a/app/views/import/configurations/_rule_import.html.erb +++ b/app/views/import/configurations/_rule_import.html.erb @@ -12,4 +12,3 @@ <%= form.submit t("import.configurations.rule_import.process_button"), disabled: import.complete? %> <% end %>
- diff --git a/app/views/pages/dashboard/_outflows_donut.html.erb b/app/views/pages/dashboard/_outflows_donut.html.erb index 2657662c7..4b11de35c 100644 --- a/app/views/pages/dashboard/_outflows_donut.html.erb +++ b/app/views/pages/dashboard/_outflows_donut.html.erb @@ -78,7 +78,7 @@ action: "mouseenter->donut-chart#highlightSegment mouseleave->donut-chart#unhighlightSegment" } do %>
-

<%= t("recurring_transactions.title") %>

- <% unless @family.recurring_transactions_disabled? %> + <% unless @family.recurring_transactions_disabled? %> <%= render DS::Menu.new do |menu| %> <% menu.with_item( variant: "button", diff --git a/app/views/rules/index.html.erb b/app/views/rules/index.html.erb index 23613b959..f40020abe 100644 --- a/app/views/rules/index.html.erb +++ b/app/views/rules/index.html.erb @@ -117,12 +117,12 @@ <% @recent_runs.each do |run| %> - + "> <%= run.executed_at.strftime("%b %d, %Y %I:%M %p") %> - + "> <%= t("rules.recent_runs.execution_types.#{run.execution_type}") %> diff --git a/app/views/settings/llm_usages/show.html.erb b/app/views/settings/llm_usages/show.html.erb index 2e32e2f4c..387ebaf56 100644 --- a/app/views/settings/llm_usages/show.html.erb +++ b/app/views/settings/llm_usages/show.html.erb @@ -117,7 +117,7 @@ <% @llm_usages.each do |usage| %> - + "> <%= usage.created_at.strftime("%b %d, %Y %I:%M %p") %> diff --git a/app/views/settings/providers/show.html.erb b/app/views/settings/providers/show.html.erb index 4f84027ad..559a54972 100644 --- a/app/views/settings/providers/show.html.erb +++ b/app/views/settings/providers/show.html.erb @@ -26,7 +26,6 @@ <% end %> - <%= settings_section title: "Enable Banking (beta)", collapsible: true, open: false do %> <%= render "settings/providers/enable_banking_panel" %> diff --git a/app/views/settings/securities/show.html.erb b/app/views/settings/securities/show.html.erb index 1d3a7350f..f0cfb56bb 100644 --- a/app/views/settings/securities/show.html.erb +++ b/app/views/settings/securities/show.html.erb @@ -44,3 +44,58 @@
<% end %> + +<% if @oidc_identities.any? || AuthConfig.sso_providers.any? %> + <%= settings_section title: t(".sso_title"), subtitle: t(".sso_subtitle") do %> + <% if @oidc_identities.any? %> +
+ <% @oidc_identities.each do |identity| %> +
+
+
+ <%= icon identity.provider_config&.dig(:icon) || "key", class: "w-5 h-5 text-secondary" %> +
+
+

<%= identity.provider_config&.dig(:label) || identity.provider.titleize %>

+

<%= identity.info&.dig("email") || t(".sso_no_email") %>

+

+ <%= t(".sso_last_used") %>: + <%= identity.last_authenticated_at&.to_fs(:short) || t(".sso_never") %> +

+
+
+ <% if @oidc_identities.count > 1 || Current.user.password_digest.present? %> + <%= render DS::Button.new( + text: t(".sso_disconnect"), + variant: "outline", + size: "sm", + href: settings_sso_identity_path(identity), + method: :delete, + confirm: CustomConfirm.new( + title: t(".sso_confirm_title"), + body: t(".sso_confirm_body", provider: identity.provider_config&.dig(:label) || identity.provider.titleize), + btn_text: t(".sso_confirm_button"), + destructive: true + ) + ) %> + <% end %> +
+ <% end %> +
+ <% if @oidc_identities.count == 1 && Current.user.password_digest.blank? %> +
+
+ <%= icon "alert-triangle", class: "w-5 h-5 text-amber-600 shrink-0 mt-0.5" %> +

<%= t(".sso_warning_message") %>

+
+
+ <% end %> + <% else %> +
+ <%= icon "link", class: "w-12 h-12 mx-auto text-secondary mb-3" %> +

<%= t(".sso_no_identities") %>

+

<%= t(".sso_connect_hint") %>

+
+ <% end %> + <% end %> +<% end %> diff --git a/app/views/settings/sso_identities/show.html.erb b/app/views/settings/sso_identities/show.html.erb deleted file mode 100644 index b9046425e..000000000 --- a/app/views/settings/sso_identities/show.html.erb +++ /dev/null @@ -1,59 +0,0 @@ -<%= content_for :page_title, t(".page_title") %> - -<%= settings_section title: t(".identities_title"), subtitle: t(".identities_subtitle") do %> - <% if @oidc_identities.any? %> -
- <% @oidc_identities.each do |identity| %> -
-
-
- <%= icon identity.provider_config&.dig(:icon) || "key", class: "w-5 h-5 text-secondary" %> -
-
-

<%= identity.provider_config&.dig(:label) || identity.provider.titleize %>

-

<%= identity.info&.dig("email") || t(".no_email") %>

-

- <%= t(".last_used") %>: - <%= identity.last_authenticated_at&.to_fs(:short) || t(".never") %> -

-
-
- <% if @oidc_identities.count > 1 || Current.user.password_digest.present? %> - <%= render DS::Button.new( - text: t(".disconnect"), - variant: "outline", - size: "sm", - href: settings_sso_identity_path(identity), - method: :delete, - confirm: CustomConfirm.new( - title: t(".confirm_title"), - body: t(".confirm_body", provider: identity.provider_config&.dig(:label) || identity.provider.titleize), - btn_text: t(".confirm_button"), - destructive: true - ) - ) %> - <% end %> -
- <% end %> -
- <% else %> -
- <%= icon "link", class: "w-12 h-12 mx-auto text-secondary mb-3" %> -

<%= t(".no_identities") %>

- <% if AuthConfig.sso_providers.any? %> -

<%= t(".connect_hint") %>

- <% end %> -
- <% end %> -<% end %> - -<% if @oidc_identities.count == 1 && Current.user.password_digest.blank? %> - <%= settings_section title: t(".warning_title") do %> -
-
- <%= icon "alert-triangle", class: "w-5 h-5 text-amber-600 shrink-0 mt-0.5" %> -

<%= t(".warning_message") %>

-
-
- <% end %> -<% end %> diff --git a/app/views/shared/_demo_warning.html.erb b/app/views/shared/_demo_warning.html.erb index e1d003dac..ba3c08f23 100644 --- a/app/views/shared/_demo_warning.html.erb +++ b/app/views/shared/_demo_warning.html.erb @@ -10,4 +10,3 @@
- diff --git a/app/views/simplefin_items/_simplefin_item.html.erb b/app/views/simplefin_items/_simplefin_item.html.erb index 680a89d04..93510054c 100644 --- a/app/views/simplefin_items/_simplefin_item.html.erb +++ b/app/views/simplefin_items/_simplefin_item.html.erb @@ -125,8 +125,6 @@ ) %> <% end %> - - <%= render DS::Menu.new do |menu| %> <% menu.with_item( variant: "button", @@ -146,7 +144,6 @@ <%= render "accounts/index/account_groups", accounts: simplefin_item.accounts %> <% end %> - <%# Sync summary (collapsible) Prefer controller-provided map; fallback to latest sync stats so Turbo broadcasts can render the summary without requiring a full page refresh. %> diff --git a/app/views/transactions/index.html.erb b/app/views/transactions/index.html.erb index 58055f342..7251c4c0a 100644 --- a/app/views/transactions/index.html.erb +++ b/app/views/transactions/index.html.erb @@ -63,7 +63,7 @@
<%= check_box_tag "selection_entry", class: "checkbox checkbox--light hidden lg:block", - data: { + data: { action: "bulk-select#togglePageSelection", checkbox_toggle_target: "selectionEntry" } %> diff --git a/config/locales/views/settings/en.yml b/config/locales/views/settings/en.yml index 567310cc8..e84e9e056 100644 --- a/config/locales/views/settings/en.yml +++ b/config/locales/views/settings/en.yml @@ -89,6 +89,23 @@ en: securities: show: page_title: Security + mfa_title: Two-Factor Authentication + mfa_description: Add an extra layer of security to your account by requiring a code from your authenticator app when signing in + enable_mfa: Enable 2FA + disable_mfa: Disable 2FA + disable_mfa_confirm: Are you sure you want to disable two-factor authentication? + sso_title: Connected Accounts + sso_subtitle: Manage your single sign-on account connections + sso_disconnect: Disconnect + sso_last_used: Last used + sso_never: Never + sso_no_email: No email + sso_no_identities: No SSO accounts connected + sso_connect_hint: Log out and sign in with an SSO provider to connect an account. + sso_confirm_title: Disconnect Account? + sso_confirm_body: Are you sure you want to disconnect your %{provider} account? You can reconnect it later by signing in with that provider again. + sso_confirm_button: Disconnect + sso_warning_message: This is your only login method. You should set a password in your security settings before disconnecting, otherwise you may be locked out of your account. settings_nav: accounts_label: Accounts advanced_section_title: Advanced diff --git a/config/locales/views/settings/sso_identities/en.yml b/config/locales/views/settings/sso_identities/en.yml deleted file mode 100644 index c989ee974..000000000 --- a/config/locales/views/settings/sso_identities/en.yml +++ /dev/null @@ -1,22 +0,0 @@ ---- -en: - settings: - sso_identities: - show: - page_title: "Connected Accounts" - identities_title: "SSO Connections" - identities_subtitle: "Manage your single sign-on account connections" - disconnect: "Disconnect" - last_used: "Last used" - never: "Never" - no_email: "No email" - no_identities: "No SSO accounts connected" - connect_hint: "Log out and sign in with an SSO provider to connect an account." - confirm_title: "Disconnect Account?" - confirm_body: "Are you sure you want to disconnect your %{provider} account? You can reconnect it later by signing in with that provider again." - confirm_button: "Disconnect" - warning_title: "Important" - warning_message: "This is your only login method. You should set a password in your security settings before disconnecting, otherwise you may be locked out of your account." - destroy: - success: "Successfully disconnected %{provider}" - cannot_unlink_last: "Cannot disconnect your only login method. Please set a password first." diff --git a/config/routes.rb b/config/routes.rb index 9d0e8fa4a..f591ea2f4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -91,7 +91,6 @@ Rails.application.routes.draw do end resource :billing, only: :show resource :security, only: :show - resource :sso_identities, only: :show resources :sso_identities, only: :destroy resource :api_key, only: [ :show, :new, :create, :destroy ] resource :ai_prompts, only: :show From b2ecc6bc678609a37b95be7621ca9a4873dc6619 Mon Sep 17 00:00:00 2001 From: Josh Waldrep Date: Sat, 3 Jan 2026 21:13:24 -0500 Subject: [PATCH 3/6] refactor: improve SSO provider management and logging - Simplified `name_id_format` selection logic in SSO provider form. - Switched raw database query to sanitized SQL in client secret tests. - Added condition to log JIT account creation only when identity persists. - Sanitized failure reasons in SSO login failure handling. - Added SSO provider connection test policy tests for super admin and regular users. --- app/controllers/oidc_accounts_controller.rb | 16 +++++++++------- app/controllers/sessions_controller.rb | 6 +++++- app/views/admin/sso_providers/_form.html.erb | 3 +-- test/models/sso_provider_test.rb | 8 +++++--- test/policies/sso_provider_policy_test.rb | 8 ++++++++ 5 files changed, 28 insertions(+), 13 deletions(-) diff --git a/app/controllers/oidc_accounts_controller.rb b/app/controllers/oidc_accounts_controller.rb index bfc6597ab..8e346fe79 100644 --- a/app/controllers/oidc_accounts_controller.rb +++ b/app/controllers/oidc_accounts_controller.rb @@ -118,17 +118,19 @@ class OidcAccountsController < ApplicationController if @user.save # Create the OIDC (or other SSO) identity - OidcIdentity.create_from_omniauth( + identity = OidcIdentity.create_from_omniauth( build_auth_hash(@pending_auth), @user ) - # Log JIT account creation - SsoAuditLog.log_jit_account_created!( - user: @user, - provider: @pending_auth["provider"], - request: request - ) + # Only log JIT account creation if identity was successfully created + if identity.persisted? + SsoAuditLog.log_jit_account_created!( + user: @user, + provider: @pending_auth["provider"], + request: request + ) + end # Clear pending auth from session session.delete(:pending_oidc_auth) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index cb375d752..337447ee3 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -144,11 +144,15 @@ class SessionsController < ApplicationController end def failure + # Sanitize reason to known values only + known_reasons = %w[sso_provider_unavailable sso_invalid_response sso_failed] + sanitized_reason = known_reasons.include?(params[:message]) ? params[:message] : "sso_failed" + # Log failed SSO attempt SsoAuditLog.log_login_failed!( provider: params[:strategy], request: request, - reason: params[:message] + reason: sanitized_reason ) message = case params[:message] diff --git a/app/views/admin/sso_providers/_form.html.erb b/app/views/admin/sso_providers/_form.html.erb index 88de8c6c6..61b8a6c17 100644 --- a/app/views/admin/sso_providers/_form.html.erb +++ b/app/views/admin/sso_providers/_form.html.erb @@ -158,8 +158,7 @@