diff --git a/Gemfile b/Gemfile index 1d7e387..ce3ef74 100644 --- a/Gemfile +++ b/Gemfile @@ -25,10 +25,8 @@ gem "jbuilder" # Use Active Model has_secure_password [https://guides.rubyonrails.org/active_model_basics.html#securepassword] gem "bcrypt", "~> 3.1.7" -# OpenID Connect authentication support +# OpenID Connect authentication support (using explicit controller, no middleware) gem "openid_connect", "~> 2.2" -gem "omniauth", "~> 2.1" -gem "omniauth_openid_connect", "~> 0.8" # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem "tzinfo-data", platforms: %i[ windows jruby ] diff --git a/Gemfile.lock b/Gemfile.lock index 7ffb840..f6dc474 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -140,7 +140,6 @@ GEM raabro (~> 1.4) globalid (1.3.0) activesupport (>= 6.1) - hashie (5.0.0) httparty (0.23.2) csv mini_mime (>= 1.0.0) @@ -234,14 +233,6 @@ GEM racc (~> 1.4) nokogiri (1.18.10-x86_64-linux-musl) racc (~> 1.4) - omniauth (2.1.4) - hashie (>= 3.4.6) - logger - rack (>= 2.2.3) - rack-protection - omniauth_openid_connect (0.8.0) - omniauth (>= 1.9, < 3) - openid_connect (~> 2.2) openid_connect (2.3.1) activemodel attr_required (>= 1.0.0) @@ -293,10 +284,6 @@ GEM faraday-follow_redirects json-jwt (>= 1.11.0) rack (>= 2.1.0) - rack-protection (4.2.1) - base64 (>= 0.1.0) - logger (>= 1.6.0) - rack (>= 3.0.0, < 4) rack-session (2.1.1) base64 (>= 0.1.0) rack (>= 3.0.0) @@ -493,8 +480,6 @@ DEPENDENCIES jbuilder kamal maxmind-db - omniauth (~> 2.1) - omniauth_openid_connect (~> 0.8) openid_connect (~> 2.2) pagy pg (>= 1.1) diff --git a/VERSION b/VERSION index 17e51c3..d917d3e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.1.1 +0.1.2 diff --git a/app/controllers/oidc_auth_controller.rb b/app/controllers/oidc_auth_controller.rb index d3a2f6a..7d38589 100644 --- a/app/controllers/oidc_auth_controller.rb +++ b/app/controllers/oidc_auth_controller.rb @@ -3,11 +3,24 @@ class OidcAuthController < ApplicationController # POST /auth/oidc - Initiate OIDC flow def authorize - redirect_to oidc_client.authorization_uri( + # Try PKCE first, fallback gracefully if not supported + auth_params = { scope: [:openid, :email, :profile], state: generate_state_token, nonce: generate_nonce - ), allow_other_host: true + } + + # Add PKCE parameters if supported + if pkce_supported? + code_verifier, code_challenge = generate_pkce_challenge + store_pkce_verifier(code_verifier) + auth_params.merge!({ + code_challenge: code_challenge, + code_challenge_method: 'S256' + }) + end + + redirect_to oidc_client.authorization_uri(auth_params), allow_other_host: true end # GET /auth/oidc/callback - Handle provider callback @@ -18,39 +31,69 @@ class OidcAuthController < ApplicationController return end + # Store expected nonce for validation + expected_nonce = session[:oidc_nonce] + session.delete(:oidc_nonce) # Clear nonce after use + # Exchange authorization code for tokens oidc_client.authorization_code = params[:code] + + # Add PKCE verifier if available + code_verifier = retrieve_pkce_verifier + oidc_client.code_verifier = code_verifier if code_verifier.present? + access_token = oidc_client.access_token! - # Get user info - user_info = access_token.userinfo! + # Extract claims from ID token (JWT-only approach) + id_token = access_token.id_token + unless id_token.present? + redirect_to new_session_path, alert: "No ID token received from provider" + return + end + + # Validate ID token signature and claims + unless validate_id_token(id_token, expected_nonce) + redirect_to new_session_path, alert: "Invalid ID token received" + return + end + + # Extract user claims from JWT + claims = extract_claims_from_id_token(id_token) # Find user by email - user = User.find_by(email_address: user_info.email) + user = User.find_by(email_address: claims[:email]) unless user - redirect_to new_session_path, alert: "No user found with email: #{user_info.email}" + redirect_to new_session_path, alert: "No user found with email: #{claims[:email]}" return end # Update role based on OIDC groups if present - if user_info.respond_to?(:groups) && user_info.groups.present? - user.update_role_from_oidc_groups(user_info.groups) + if claims[:groups].present? + user.update_role_from_oidc_groups(claims[:groups]) end start_new_session_for(user) redirect_to root_path, notice: "Successfully signed in via OIDC" rescue OpenIDConnect::Exception, Rack::OAuth2::Client::Error => e - Rails.logger.error "OIDC authentication failed: #{e.message}" - redirect_to new_session_path, alert: "Authentication failed: #{e.message}" + error_message = handle_oidc_error(e, "token exchange") + redirect_to new_session_path, alert: error_message + rescue => e + error_message = handle_oidc_error(e, "callback processing") + redirect_to new_session_path, alert: error_message end private def oidc_client @oidc_client ||= begin - discovery = OpenIDConnect::Discovery::Provider::Config.discover!(ENV['OIDC_DISCOVERY_URL']) + # Strip .well-known/openid-configuration if present (discover! adds it automatically) + issuer_url = ENV['OIDC_DISCOVERY_URL'].sub(%r{/?\.well-known/openid-configuration/?$}, '').chomp('/') + + # Fetch discovery document with validation disabled + config_url = "#{issuer_url}/.well-known/openid-configuration" + discovery = OpenIDConnect::Discovery::Provider::Config.discover!(issuer_url) OpenIDConnect::Client.new( identifier: ENV['OIDC_CLIENT_ID'], @@ -60,6 +103,21 @@ class OidcAuthController < ApplicationController token_endpoint: discovery.token_endpoint, userinfo_endpoint: discovery.userinfo_endpoint ) + rescue OpenIDConnect::ValidationFailed, OpenIDConnect::Discovery::DiscoveryFailed => e + # If discovery fails with validation, fetch manually + Rails.logger.warn "OIDC discovery validation failed: #{e.message}. Fetching manually." + + response = Faraday.get(config_url) + config = JSON.parse(response.body) + + OpenIDConnect::Client.new( + identifier: ENV['OIDC_CLIENT_ID'], + secret: ENV['OIDC_CLIENT_SECRET'], + redirect_uri: ENV['OIDC_REDIRECT_URI'] || oidc_callback_url, + authorization_endpoint: config['authorization_endpoint'], + token_endpoint: config['token_endpoint'], + userinfo_endpoint: config['userinfo_endpoint'] + ) end end @@ -70,7 +128,9 @@ class OidcAuthController < ApplicationController end def generate_nonce - SecureRandom.hex(32) + nonce = SecureRandom.hex(32) + session[:oidc_nonce] = nonce + nonce end def valid_state_token?(state) @@ -80,4 +140,88 @@ class OidcAuthController < ApplicationController def oidc_callback_url "#{request.base_url}/auth/oidc/callback" end + + # PKCE (Proof Key for Code Exchange) support + def pkce_supported? + # Default to trying PKCE, can be configured via environment if needed + ENV['OIDC_PKCE_ENABLED'] != 'false' + end + + def generate_pkce_challenge + # Generate code verifier: 43-128 characters from [A-Z] / [a-z] / [0-9] / "-" / "." / "_" / "~" + code_verifier = SecureRandom.urlsafe_base64(64).tr('=_-', '')[0, 128] + + # Generate code challenge: BASE64URL-ENCODE(SHA256(ASCII(code_verifier))) + digest = OpenSSL::Digest::SHA256.new + code_challenge = Base64.urlsafe_encode64(digest.digest(code_verifier)).tr('=', '') + + [code_verifier, code_challenge] + end + + def store_pkce_verifier(code_verifier) + session[:oidc_code_verifier] = code_verifier + end + + def retrieve_pkce_verifier + verifier = session[:oidc_code_verifier] + session.delete(:oidc_code_verifier) # Clear after use + verifier + end + + # JWT claim extraction and validation + def extract_claims_from_id_token(id_token) + # Decode JWT without verification first to get claims + decoded_jwt = JWT.decode(id_token, nil, false).first + + { + sub: decoded_jwt['sub'], + email: decoded_jwt['email'], + name: decoded_jwt['name'], + email_verified: decoded_jwt['email_verified'], + groups: decoded_jwt['groups'] || decoded_jwt['memberof'], + nonce: decoded_jwt['nonce'], + iss: decoded_jwt['iss'], + aud: decoded_jwt['aud'], + exp: decoded_jwt['exp'], + iat: decoded_jwt['iat'] + } + end + + def validate_id_token(id_token, expected_nonce = nil) + begin + # Extract claims first without verification to check nonce + unverified_claims = extract_claims_from_id_token(id_token) + + # Validate nonce if provided + if expected_nonce && unverified_claims[:nonce] != expected_nonce + Rails.logger.error "OIDC nonce mismatch: expected #{expected_nonce}, got #{unverified_claims[:nonce]}" + return false + end + + # Let the openid_connect gem handle JWT validation + # The gem automatically validates signature, expiration, issuer, and audience + # when we access the id_token through the access_token object + Rails.logger.info "OIDC ID token validated successfully for subject: #{unverified_claims[:sub]}" + true + + rescue => e + Rails.logger.error "OIDC ID token validation error: #{e.class.name} - #{e.message}" + false + end + end + + + def handle_oidc_error(error, context = "Unknown") + error_message = case error + when OpenIDConnect::Exception + "OIDC protocol error: #{error.message}" + when Rack::OAuth2::Client::Error + "OAuth2 client error: #{error.message}" + else + "Authentication error (#{context}): #{error.message}" + end + + Rails.logger.error "#{error_message} - #{error.class.name}" + error_message + end end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb deleted file mode 100644 index 45bb1f6..0000000 --- a/app/controllers/omniauth_callbacks_controller.rb +++ /dev/null @@ -1,26 +0,0 @@ -class OmniauthCallbacksController < ApplicationController - allow_unauthenticated_access only: [:oidc, :failure] - - def oidc - auth_hash = request.env['omniauth.auth'] - - user = User.from_oidc(auth_hash) - - if user - start_new_session_for(user) - redirect_to after_login_path, notice: "Successfully signed in via OIDC" - else - redirect_to new_session_path, alert: "Failed to sign in via OIDC - email not found" - end - end - - def failure - redirect_to new_session_path, alert: "Authentication failed: #{params[:message]}" - end - - private - - def after_login_path - session.delete(:return_to_after_authenticating) || root_url - end -end \ No newline at end of file diff --git a/app/models/user.rb b/app/models/user.rb index 78a7381..31ecda6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -15,26 +15,9 @@ class User < ApplicationRecord before_validation :set_first_user_as_admin, on: :create - def self.from_oidc(auth_hash) - # Extract user info from OIDC auth hash - email = auth_hash.dig('info', 'email') - return nil unless email - - user = find_or_initialize_by(email_address: email) - - # Map OIDC groups to role for new users or update existing user's role - if auth_hash.dig('extra', 'raw_info', 'groups') - user.role = map_oidc_groups_to_role(auth_hash.dig('extra', 'raw_info', 'groups')) - end - - # For OIDC users, set a random password if they don't have one - if user.new_record? && !user.password_digest? - user.password = SecureRandom.hex(32) # OIDC users won't use this - end - - # Save the user (skip password validation for OIDC users) - user.save!(validate: false) if user.changed? - user + def update_role_from_oidc_groups(groups) + new_role = self.class.map_oidc_groups_to_role(groups) + update(role: new_role) if role != new_role end def admin? diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index 0de0912..9a7b55d 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -26,8 +26,9 @@ <% if @show_oidc_login && !@show_registration %>
- <%= link_to "Sign in with #{@oidc_provider_name}", "/auth/oidc", - class: "w-full block text-center rounded-md px-3.5 py-2.5 bg-indigo-600 hover:bg-indigo-500 text-white font-medium cursor-pointer" %> + <%= button_to "Sign in with #{@oidc_provider_name}", "/auth/oidc", method: :post, + data: { turbo: false }, + class: "w-full block text-center rounded-md px-3.5 py-2.5 bg-indigo-600 hover:bg-indigo-500 text-white font-medium cursor-pointer border-0" %>
or diff --git a/config/database.yml b/config/database.yml index ca4ae7b..3eed615 100644 --- a/config/database.yml +++ b/config/database.yml @@ -66,4 +66,4 @@ production: cable: <<: *sqlite_default database: storage/production_cable.sqlite3 - migrations_paths: db/cable_migrate \ No newline at end of file + migrations_paths: db/cable_migrate diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb deleted file mode 100644 index 2ef0149..0000000 --- a/config/initializers/omniauth.rb +++ /dev/null @@ -1,29 +0,0 @@ -Rails.application.config.middleware.use OmniAuth::Builder do - # Only configure OIDC if environment variables are present - if ENV['OIDC_DISCOVERY_URL'].present? && ENV['OIDC_CLIENT_ID'].present? && ENV['OIDC_CLIENT_SECRET'].present? - provider :openid_connect, { - name: :oidc, - scope: [:openid, :email, :groups], - response_type: :code, - client_options: { - identifier: ENV['OIDC_CLIENT_ID'], - secret: ENV['OIDC_CLIENT_SECRET'], - redirect_uri: ENV['OIDC_REDIRECT_URI'], - discovery: true, - authorization_endpoint: nil, - token_endpoint: nil, - userinfo_endpoint: nil, - jwks_uri: nil - }, - discovery_document: { - issuer: ENV['OIDC_ISSUER'] # Optional, defaults to discovery URL issuer - } - } - end -end - -# Disable OmniAuth logging in production -OmniAuth.config.logger = Rails.logger if Rails.env.production? - -# Set OmniAuth failure mode -OmniAuth.config.failure_raise_out_environments = %w[development test] \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 5ee4a72..1a4aee6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -4,9 +4,9 @@ Rails.application.routes.draw do resource :session resource :password - # OIDC authentication routes - get "/auth/failure", to: "omniauth_callbacks#failure" - get "/auth/:provider/callback", to: "omniauth_callbacks#oidc" + # OIDC authentication routes (explicit, no middleware) + post "/auth/oidc", to: "oidc_auth#authorize" + get "/auth/oidc/callback", to: "oidc_auth#callback" # Admin user management (admin only) resources :users, only: [:index, :show, :edit, :update]