diff --git a/README.md b/README.md index a29594f..923d75b 100644 --- a/README.md +++ b/README.md @@ -122,10 +122,54 @@ Send emails for: - **Session revocation** - Users and admins can revoke individual sessions ### Access Control -- **Group-based allowlists** - Restrict applications to specific user groups -- **Per-application access** - Each app defines which groups can access it -- **Automatic enforcement** - Access checks during OIDC authorization and ForwardAuth -- **Custom claims** - Add arbitrary claims to OIDC tokens via groups and users (perfect for app-specific roles) + +#### Group-Based Application Access +Clinch uses groups to control which users can access which applications: + +- **Create groups** - Organize users into logical groups (readers, editors, family, developers, etc.) +- **Assign groups to applications** - Each app defines which groups are allowed to access it + - Example: Kavita app allows the "readers" group → only users in the "readers" group can sign in + - If no groups are assigned to an app → all active users can access it +- **Automatic enforcement** - Access checks happen automatically: + - During OIDC authorization flow (before consent) + - During ForwardAuth verification (before proxying requests) + - Users not in allowed groups receive a "You do not have permission" error + +#### Group Claims in Tokens +- **OIDC tokens include group membership** - ID tokens contain a `groups` claim with all user's groups +- **Custom claims** - Add arbitrary key-value pairs to tokens via groups and users + - Group claims apply to all members (e.g., `{"role": "viewer"}`) + - User claims override group claims for fine-grained control + - Perfect for app-specific authorization (e.g., admin vs. read-only roles) + +#### Custom Claims Merging +Custom claims from groups and users are merged into OIDC ID tokens with the following precedence: + +1. **Default OIDC claims** - Standard claims (`iss`, `sub`, `aud`, `exp`, `email`, etc.) +2. **Standard Clinch claims** - `groups` array (list of user's group names) +3. **Group custom claims** - Merged in order; later groups override earlier ones +4. **User custom claims** - Override all group claims +5. **Application-specific claims** - Highest priority; override all other claims + +**Example:** +- Group "readers" has `{"role": "viewer", "max_items": 10}` +- Group "premium" has `{"role": "subscriber", "max_items": 100}` +- User (in both groups) has `{"max_items": 500}` +- **Result:** `{"role": "subscriber", "max_items": 500}` (user overrides max_items, premium overrides role) + +#### Application-Specific Claims +Configure different claims for different applications on a per-user basis: + +- **Per-app customization** - Each application can have unique claims for each user +- **Highest precedence** - App-specific claims override group and user global claims +- **Use case** - Different roles in different apps (e.g., admin in Kavita, user in Audiobookshelf) +- **Admin UI** - Configure via Admin → Users → Edit User → App-Specific Claim Overrides + +**Example:** +- User Alice, global claims: `{"theme": "dark"}` +- Kavita app-specific: `{"kavita_groups": ["admin"]}` +- Audiobookshelf app-specific: `{"abs_groups": ["user"]}` +- **Result:** Kavita receives `{"theme": "dark", "kavita_groups": ["admin"]}`, Audiobookshelf receives `{"theme": "dark", "abs_groups": ["user"]}` --- diff --git a/VERSION b/VERSION index 5c27e65..919d666 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2025.02 +2025.03 diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index d27842d..49e82c0 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -18,7 +18,25 @@ module Admin end def create - @group = Group.new(group_params) + create_params = group_params + + # Parse custom_claims JSON if provided + if create_params[:custom_claims].present? + begin + create_params[:custom_claims] = JSON.parse(create_params[:custom_claims]) + rescue JSON::ParserError + @group = Group.new + @group.errors.add(:custom_claims, "must be valid JSON") + @available_users = User.order(:email_address) + render :new, status: :unprocessable_entity + return + end + else + # If empty or blank, set to empty hash (NOT NULL constraint) + create_params[:custom_claims] = {} + end + + @group = Group.new(create_params) if @group.save # Handle user assignments @@ -39,7 +57,24 @@ module Admin end def update - if @group.update(group_params) + update_params = group_params + + # Parse custom_claims JSON if provided + if update_params[:custom_claims].present? + begin + update_params[:custom_claims] = JSON.parse(update_params[:custom_claims]) + rescue JSON::ParserError + @group.errors.add(:custom_claims, "must be valid JSON") + @available_users = User.order(:email_address) + render :edit, status: :unprocessable_entity + return + end + else + # If empty or blank, set to empty hash (NOT NULL constraint) + update_params[:custom_claims] = {} + end + + if @group.update(update_params) # Handle user assignments if params[:group][:user_ids].present? user_ids = params[:group][:user_ids].reject(&:blank?) @@ -67,7 +102,7 @@ module Admin end def group_params - params.require(:group).permit(:name, :description, custom_claims: {}) + params.require(:group).permit(:name, :description, :custom_claims) end end end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index a74d4a9..7c86216 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -1,6 +1,6 @@ module Admin class UsersController < BaseController - before_action :set_user, only: [:show, :edit, :update, :destroy, :resend_invitation] + before_action :set_user, only: [:show, :edit, :update, :destroy, :resend_invitation, :update_application_claims, :delete_application_claims] def index @users = User.order(created_at: :desc) @@ -27,6 +27,7 @@ module Admin end def edit + @applications = Application.active.order(:name) end def update @@ -35,9 +36,25 @@ module Admin # Only update password if provided update_params.delete(:password) if update_params[:password].blank? + # Parse custom_claims JSON if provided + if update_params[:custom_claims].present? + begin + update_params[:custom_claims] = JSON.parse(update_params[:custom_claims]) + rescue JSON::ParserError + @user.errors.add(:custom_claims, "must be valid JSON") + @applications = Application.active.order(:name) + render :edit, status: :unprocessable_entity + return + end + else + # If empty or blank, set to empty hash (NOT NULL constraint) + update_params[:custom_claims] = {} + end + if @user.update(update_params) redirect_to admin_users_path, notice: "User updated successfully." else + @applications = Application.active.order(:name) render :edit, status: :unprocessable_entity end end @@ -63,6 +80,41 @@ module Admin redirect_to admin_users_path, notice: "User deleted successfully." end + # POST /admin/users/:id/update_application_claims + def update_application_claims + application = Application.find(params[:application_id]) + + claims_json = params[:custom_claims].presence || "{}" + begin + claims = JSON.parse(claims_json) + rescue JSON::ParserError + redirect_to edit_admin_user_path(@user), alert: "Invalid JSON format for claims." + return + end + + app_claim = @user.application_user_claims.find_or_initialize_by(application: application) + app_claim.custom_claims = claims + + if app_claim.save + redirect_to edit_admin_user_path(@user), notice: "App-specific claims updated for #{application.name}." + else + error_message = app_claim.errors.full_messages.join(", ") + redirect_to edit_admin_user_path(@user), alert: "Failed to update claims: #{error_message}" + end + end + + # DELETE /admin/users/:id/delete_application_claims + def delete_application_claims + application = Application.find(params[:application_id]) + app_claim = @user.application_user_claims.find_by(application: application) + + if app_claim&.destroy + redirect_to edit_admin_user_path(@user), notice: "App-specific claims removed for #{application.name}." + else + redirect_to edit_admin_user_path(@user), alert: "No claims found to remove." + end + end + private def set_user @@ -71,7 +123,7 @@ module Admin def user_params # Base attributes that all admins can modify - base_params = params.require(:user).permit(:email_address, :name, :password, :status, :totp_required, custom_claims: {}) + base_params = params.require(:user).permit(:email_address, :username, :name, :password, :status, :totp_required, :custom_claims) # Only allow modifying admin status when editing other users (prevent self-demotion) if params[:id] != Current.session.user.id.to_s diff --git a/app/controllers/oidc_controller.rb b/app/controllers/oidc_controller.rb index 7b424b7..605cad9 100644 --- a/app/controllers/oidc_controller.rb +++ b/app/controllers/oidc_controller.rb @@ -534,9 +534,6 @@ class OidcController < ApplicationController claims[:groups] = user.groups.pluck(:name) end - # Add admin claim if user is admin - claims[:admin] = true if user.admin? - # Merge custom claims from groups user.groups.each do |group| claims.merge!(group.parsed_custom_claims) @@ -545,6 +542,10 @@ class OidcController < ApplicationController # Merge custom claims from user (overrides group claims) claims.merge!(user.parsed_custom_claims) + # Merge app-specific custom claims (highest priority) + application = access_token.application + claims.merge!(application.custom_claims_for_user(user)) + render json: claims end diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index 5b97998..b0f2df5 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -11,7 +11,7 @@ class PasswordsController < ApplicationController PasswordsMailer.reset(user).deliver_later end - redirect_to new_session_path, notice: "Password reset instructions sent (if user with that email address exists)." + redirect_to signin_path, notice: "Password reset instructions sent (if user with that email address exists)." end def edit @@ -20,7 +20,7 @@ class PasswordsController < ApplicationController def update if @user.update(params.permit(:password, :password_confirmation)) @user.sessions.destroy_all - redirect_to new_session_path, notice: "Password has been reset." + redirect_to signin_path, notice: "Password has been reset." else redirect_to edit_password_path(params[:token]), alert: "Passwords did not match." end @@ -29,6 +29,7 @@ class PasswordsController < ApplicationController private def set_user_by_token @user = User.find_by_token_for(:password_reset, params[:token]) + redirect_to new_password_path, alert: "Password reset link is invalid or has expired." if @user.nil? rescue ActiveSupport::MessageVerifier::InvalidSignature redirect_to new_password_path, alert: "Password reset link is invalid or has expired." end diff --git a/app/helpers/claims_helper.rb b/app/helpers/claims_helper.rb new file mode 100644 index 0000000..3ddbd74 --- /dev/null +++ b/app/helpers/claims_helper.rb @@ -0,0 +1,69 @@ +module ClaimsHelper + include ClaimsMerger + + # Preview final merged claims for a user accessing an application + def preview_user_claims(user, application) + claims = { + # Standard OIDC claims + email: user.email_address, + email_verified: true, + preferred_username: user.username.presence || user.email_address, + name: user.name.presence || user.email_address + } + + # Add groups + if user.groups.any? + claims[:groups] = user.groups.pluck(:name) + end + + # Merge group custom claims (arrays are combined, not overwritten) + user.groups.each do |group| + claims = deep_merge_claims(claims, group.parsed_custom_claims) + end + + # Merge user custom claims (arrays are combined, other values override) + claims = deep_merge_claims(claims, user.parsed_custom_claims) + + # Merge app-specific claims (arrays are combined) + claims = deep_merge_claims(claims, application.custom_claims_for_user(user)) + + claims + end + + # Get claim sources breakdown for display + def claim_sources(user, application) + sources = [] + + # Group claims + user.groups.each do |group| + if group.parsed_custom_claims.any? + sources << { + type: :group, + name: group.name, + claims: group.parsed_custom_claims + } + end + end + + # User claims + if user.parsed_custom_claims.any? + sources << { + type: :user, + name: "User Override", + claims: user.parsed_custom_claims + } + end + + # App-specific claims + app_claims = application.custom_claims_for_user(user) + if app_claims.any? + sources << { + type: :application, + name: "App-Specific (#{application.name})", + claims: app_claims + } + end + + sources + end +end diff --git a/app/models/application.rb b/app/models/application.rb index 6f6c336..21a54a0 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -3,6 +3,7 @@ class Application < ApplicationRecord has_many :application_groups, dependent: :destroy has_many :allowed_groups, through: :application_groups, source: :group + has_many :application_user_claims, dependent: :destroy has_many :oidc_authorization_codes, dependent: :destroy has_many :oidc_access_tokens, dependent: :destroy has_many :oidc_refresh_tokens, dependent: :destroy @@ -186,6 +187,12 @@ class Application < ApplicationRecord duration_to_human(id_token_ttl || 3600) end + # Get app-specific custom claims for a user + def custom_claims_for_user(user) + app_claim = application_user_claims.find_by(user: user) + app_claim&.parsed_custom_claims || {} + end + private def duration_to_human(seconds) diff --git a/app/models/application_user_claim.rb b/app/models/application_user_claim.rb new file mode 100644 index 0000000..2e67073 --- /dev/null +++ b/app/models/application_user_claim.rb @@ -0,0 +1,31 @@ +class ApplicationUserClaim < ApplicationRecord + belongs_to :application + belongs_to :user + + # Reserved OIDC claim names that should not be overridden + RESERVED_CLAIMS = %w[ + iss sub aud exp iat nbf jti nonce azp + email email_verified preferred_username name + groups + ].freeze + + validates :user_id, uniqueness: { scope: :application_id } + validate :no_reserved_claim_names + + # Parse custom_claims JSON field + def parsed_custom_claims + return {} if custom_claims.blank? + custom_claims.is_a?(Hash) ? custom_claims : {} + end + + private + + def no_reserved_claim_names + return if custom_claims.blank? + + reserved_used = parsed_custom_claims.keys.map(&:to_s) & RESERVED_CLAIMS + if reserved_used.any? + errors.add(:custom_claims, "cannot override reserved OIDC claims: #{reserved_used.join(', ')}") + end + end +end diff --git a/app/models/group.rb b/app/models/group.rb index 778b494..4a1c77d 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -4,11 +4,31 @@ class Group < ApplicationRecord has_many :application_groups, dependent: :destroy has_many :applications, through: :application_groups + # Reserved OIDC claim names that should not be overridden + RESERVED_CLAIMS = %w[ + iss sub aud exp iat nbf jti nonce azp + email email_verified preferred_username name + groups + ].freeze + validates :name, presence: true, uniqueness: { case_sensitive: false } normalizes :name, with: ->(name) { name.strip.downcase } + validate :no_reserved_claim_names # Parse custom_claims JSON field def parsed_custom_claims - custom_claims || {} + return {} if custom_claims.blank? + custom_claims.is_a?(Hash) ? custom_claims : {} + end + + private + + def no_reserved_claim_names + return if custom_claims.blank? + + reserved_used = parsed_custom_claims.keys.map(&:to_s) & RESERVED_CLAIMS + if reserved_used.any? + errors.add(:custom_claims, "cannot override reserved OIDC claims: #{reserved_used.join(', ')}") + end end end diff --git a/app/models/user.rb b/app/models/user.rb index fd3753f..703a574 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,6 +3,7 @@ class User < ApplicationRecord has_many :sessions, dependent: :destroy has_many :user_groups, dependent: :destroy has_many :groups, through: :user_groups + has_many :application_user_claims, dependent: :destroy has_many :oidc_user_consents, dependent: :destroy has_many :webauthn_credentials, dependent: :destroy @@ -20,10 +21,22 @@ class User < ApplicationRecord end normalizes :email_address, with: ->(e) { e.strip.downcase } + normalizes :username, with: ->(u) { u.strip.downcase if u.present? } + + # Reserved OIDC claim names that should not be overridden + RESERVED_CLAIMS = %w[ + iss sub aud exp iat nbf jti nonce azp + email email_verified preferred_username name + groups + ].freeze validates :email_address, presence: true, uniqueness: { case_sensitive: false }, format: { with: URI::MailTo::EMAIL_REGEXP } + validates :username, uniqueness: { case_sensitive: false }, allow_nil: true, + format: { with: /\A[a-zA-Z0-9_-]+\z/, message: "can only contain letters, numbers, underscores, and hyphens" }, + length: { minimum: 2, maximum: 30 } validates :password, length: { minimum: 8 }, allow_nil: true + validate :no_reserved_claim_names # Enum - automatically creates scopes (User.active, User.disabled, etc.) enum :status, { active: 0, disabled: 1, pending_invitation: 2 } @@ -182,11 +195,39 @@ class User < ApplicationRecord # Parse custom_claims JSON field def parsed_custom_claims - custom_claims || {} + return {} if custom_claims.blank? + custom_claims.is_a?(Hash) ? custom_claims : {} + end + + # Get fully merged claims for a specific application + def merged_claims_for_application(application) + merged = {} + + # Start with group claims (in order) + groups.each do |group| + merged.merge!(group.parsed_custom_claims) + end + + # Merge user global claims + merged.merge!(parsed_custom_claims) + + # Merge app-specific claims (highest priority) + merged.merge!(application.custom_claims_for_user(self)) + + merged end private + def no_reserved_claim_names + return if custom_claims.blank? + + reserved_used = parsed_custom_claims.keys.map(&:to_s) & RESERVED_CLAIMS + if reserved_used.any? + errors.add(:custom_claims, "cannot override reserved OIDC claims: #{reserved_used.join(', ')}") + end + end + def generate_backup_codes # Generate plain codes for user to see/save plain_codes = Array.new(10) { SecureRandom.alphanumeric(8).upcase } diff --git a/app/services/concerns/claims_merger.rb b/app/services/concerns/claims_merger.rb new file mode 100644 index 0000000..e58c0e8 --- /dev/null +++ b/app/services/concerns/claims_merger.rb @@ -0,0 +1,35 @@ +module ClaimsMerger + extend ActiveSupport::Concern + + # Deep merge claims, combining arrays instead of overwriting them + # This ensures that array values (like roles) are combined across group/user/app claims + # + # Example: + # base = { "roles" => ["user"], "level" => 1 } + # incoming = { "roles" => ["admin"], "department" => "IT" } + # deep_merge_claims(base, incoming) + # # => { "roles" => ["user", "admin"], "level" => 1, "department" => "IT" } + def deep_merge_claims(base, incoming) + result = base.dup + + incoming.each do |key, value| + if result.key?(key) + # If both values are arrays, combine them (union to avoid duplicates) + if result[key].is_a?(Array) && value.is_a?(Array) + result[key] = (result[key] + value).uniq + # If both values are hashes, recursively merge them + elsif result[key].is_a?(Hash) && value.is_a?(Hash) + result[key] = deep_merge_claims(result[key], value) + else + # Otherwise, incoming value wins (override) + result[key] = value + end + else + # New key, just add it + result[key] = value + end + end + + result + end +end diff --git a/app/services/oidc_jwt_service.rb b/app/services/oidc_jwt_service.rb index 712cf4a..57ea186 100644 --- a/app/services/oidc_jwt_service.rb +++ b/app/services/oidc_jwt_service.rb @@ -1,4 +1,6 @@ class OidcJwtService + extend ClaimsMerger + class << self # Generate an ID token (JWT) for the user def generate_id_token(user, application, consent: nil, nonce: nil) @@ -17,7 +19,7 @@ class OidcJwtService iat: now, email: user.email_address, email_verified: true, - preferred_username: user.email_address, + preferred_username: user.username.presence || user.email_address, name: user.name.presence || user.email_address } @@ -29,16 +31,16 @@ class OidcJwtService payload[:groups] = user.groups.pluck(:name) end - # Add admin claim if user is admin - payload[:admin] = true if user.admin? - - # Merge custom claims from groups + # Merge custom claims from groups (arrays are combined, not overwritten) user.groups.each do |group| - payload.merge!(group.parsed_custom_claims) + payload = deep_merge_claims(payload, group.parsed_custom_claims) end - # Merge custom claims from user (overrides group claims) - payload.merge!(user.parsed_custom_claims) + # Merge custom claims from user (arrays are combined, other values override) + payload = deep_merge_claims(payload, user.parsed_custom_claims) + + # Merge app-specific custom claims (highest priority, arrays are combined) + payload = deep_merge_claims(payload, application.custom_claims_for_user(user)) JWT.encode(payload, private_key, "RS256", { kid: key_id, typ: "JWT" }) end diff --git a/app/views/admin/users/_application_claims.html.erb b/app/views/admin/users/_application_claims.html.erb new file mode 100644 index 0000000..f477ee0 --- /dev/null +++ b/app/views/admin/users/_application_claims.html.erb @@ -0,0 +1,185 @@ +<% oidc_apps = applications.select(&:oidc?) %> +<% forward_auth_apps = applications.select(&:forward_auth?) %> + + +<% if oidc_apps.any? %> +
+

OIDC App-Specific Claims

+

+ Configure custom claims that apply only to specific OIDC applications. These override both group and user global claims and are included in ID tokens. +

+ +
+ <% oidc_apps.each do |app| %> + <% app_claim = user.application_user_claims.find_by(application: app) %> +
> + +
+ <%= app.name %> + + OIDC + + <% if app_claim&.custom_claims&.any? %> + + <%= app_claim.custom_claims.keys.count %> claim(s) + + <% end %> +
+ + + + + +
+ <%= form_with url: update_application_claims_admin_user_path(user), method: :post, class: "space-y-4", data: { controller: "json-validator" } do |form| %> + <%= hidden_field_tag :application_id, app.id %> + +
+ + <%= text_area_tag :custom_claims, + (app_claim&.custom_claims.present? ? JSON.pretty_generate(app_claim.custom_claims) : ""), + rows: 8, + class: "w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 sm:text-sm font-mono", + placeholder: '{"kavita_groups": ["admin"], "library_access": "all"}', + data: { + action: "input->json-validator#validate blur->json-validator#format", + json_validator_target: "textarea" + } %> +
+

+ Example for <%= app.name %>: Add claims that this app specifically needs to read. +

+

+ Note: Do not use reserved claim names (groups, email, name, etc.). Use app-specific names like kavita_groups instead. +

+
+
+
+ +
+ <%= button_tag type: :submit, class: "rounded-md bg-blue-600 px-3 py-2 text-sm font-semibold text-white shadow-sm hover:bg-blue-500" do %> + <%= app_claim ? "Update" : "Add" %> Claims + <% end %> + + <% if app_claim %> + <%= button_to "Remove Override", + delete_application_claims_admin_user_path(user, application_id: app.id), + method: :delete, + data: { turbo_confirm: "Remove app-specific claims for #{app.name}?" }, + class: "rounded-md bg-white px-3 py-2 text-sm font-semibold text-gray-900 shadow-sm ring-1 ring-inset ring-gray-300 hover:bg-gray-50" %> + <% end %> +
+ <% end %> + + +
+

Preview: Final ID Token Claims for <%= app.name %>

+
+ 
<%= JSON.pretty_generate(preview_user_claims(user, app)) %>
+
+ +
+ Show claim sources +
+ <% claim_sources(user, app).each do |source| %> +
+ + <%= source[:name] %> + + <%= source[:claims].to_json %> +
+ <% end %> +
+
+
+
+
+ <% end %> +
+
+<% end %> + + +<% if forward_auth_apps.any? %> +
+

ForwardAuth Headers Preview

+

+ ForwardAuth applications receive HTTP headers (not OIDC tokens). Headers are based on user's email, name, groups, and admin status. +

+ +
+ <% forward_auth_apps.each do |app| %> +
+ +
+ <%= app.name %> + + FORWARD AUTH + + + <%= app.domain_pattern %> + +
+ + + + + +
+
+
+ + + +
+
+ +
+

Headers Sent to <%= app.name %>

+
+ <% headers = app.headers_for_user(user) %> + <% if headers.any? %> +
+ <% headers.each do |header_name, value| %> +
+
<%= header_name %>:
+
<%= value %>
+
+ <% end %> +
+ <% else %> +

All headers disabled for this application.

+ <% end %> +
+

+ These headers are configured in the application settings and sent by your reverse proxy (Caddy/Traefik) to the upstream application. +

+
+ + <% if user.groups.any? %> +
+

User's Groups

+
+ <% user.groups.each do |group| %> + + <%= group.name %> + + <% end %> +
+
+ <% end %> +
+
+ <% end %> +
+
+<% end %> + +<% if oidc_apps.empty? && forward_auth_apps.empty? %> +
+
+

No active applications found.

+

Create applications in the Admin panel first.

+
+
+<% end %> diff --git a/app/views/admin/users/_form.html.erb b/app/views/admin/users/_form.html.erb index 9811e0b..4cc066d 100644 --- a/app/views/admin/users/_form.html.erb +++ b/app/views/admin/users/_form.html.erb @@ -6,10 +6,16 @@ <%= form.email_field :email_address, required: true, class: "mt-1 block w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 sm:text-sm", placeholder: "user@example.com" %> +
+ <%= form.label :username, "Username (Optional)", class: "block text-sm font-medium text-gray-700" %> + <%= form.text_field :username, class: "mt-1 block w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 sm:text-sm", placeholder: "jsmith" %> +

Optional: Short username/handle for login. Can only contain letters, numbers, underscores, and hyphens.

+
+
<%= form.label :name, "Display Name (Optional)", class: "block text-sm font-medium text-gray-700" %> <%= form.text_field :name, class: "mt-1 block w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 sm:text-sm", placeholder: "John Smith" %> -

Optional: Name shown in applications. Defaults to email address if not set.

+

Optional: Full name shown in applications. Defaults to email address if not set.

diff --git a/app/views/admin/users/edit.html.erb b/app/views/admin/users/edit.html.erb index 1fe5b13..8c50c8e 100644 --- a/app/views/admin/users/edit.html.erb +++ b/app/views/admin/users/edit.html.erb @@ -1,5 +1,12 @@ -
+

Edit User

Editing: <%= @user.email_address %>

- <%= render "form", user: @user %> + +
+ <%= render "form", user: @user %> +
+ + <% if @user.persisted? %> + <%= render "application_claims", user: @user, applications: @applications %> + <% end %>
diff --git a/config/recurring.yml b/config/recurring.yml new file mode 100644 index 0000000..02cc038 --- /dev/null +++ b/config/recurring.yml @@ -0,0 +1,17 @@ +# Solid Queue Recurring Jobs Configuration +# This file defines scheduled/cron-like jobs that run periodically + +production: + oidc_token_cleanup: + class: OidcTokenCleanupJob + schedule: "0 3 * * *" # Run daily at 3:00 AM + queue: default + +development: + oidc_token_cleanup: + class: OidcTokenCleanupJob + schedule: "0 3 * * *" # Run daily at 3:00 AM + queue: default + +test: + # No recurring jobs in test environment diff --git a/config/routes.rb b/config/routes.rb index 19b45e7..c5377c9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -82,6 +82,8 @@ Rails.application.routes.draw do resources :users do member do post :resend_invitation + post :update_application_claims + delete :delete_application_claims end end resources :applications do diff --git a/db/migrate/20251122235519_add_sid_to_oidc_user_consent.rb b/db/migrate/20251122235519_add_sid_to_oidc_user_consent.rb new file mode 100644 index 0000000..f73f228 --- /dev/null +++ b/db/migrate/20251122235519_add_sid_to_oidc_user_consent.rb @@ -0,0 +1,15 @@ +class AddSidToOidcUserConsent < ActiveRecord::Migration[8.1] + def change + add_column :oidc_user_consents, :sid, :string + add_index :oidc_user_consents, :sid + + # Generate UUIDs for existing consent records + reversible do |dir| + dir.up do + OidcUserConsent.where(sid: nil).find_each do |consent| + consent.update_column(:sid, SecureRandom.uuid) + end + end + end + end +end diff --git a/db/migrate/20251123052026_create_application_user_claims.rb b/db/migrate/20251123052026_create_application_user_claims.rb new file mode 100644 index 0000000..1f380ec --- /dev/null +++ b/db/migrate/20251123052026_create_application_user_claims.rb @@ -0,0 +1,13 @@ +class CreateApplicationUserClaims < ActiveRecord::Migration[8.1] + def change + create_table :application_user_claims do |t| + t.references :application, null: false, foreign_key: { on_delete: :cascade } + t.references :user, null: false, foreign_key: { on_delete: :cascade } + t.json :custom_claims, default: {}, null: false + + t.timestamps + end + + add_index :application_user_claims, [:application_id, :user_id], unique: true, name: 'index_app_user_claims_unique' + end +end diff --git a/db/migrate/20251125012446_add_username_to_users.rb b/db/migrate/20251125012446_add_username_to_users.rb new file mode 100644 index 0000000..1bd5d05 --- /dev/null +++ b/db/migrate/20251125012446_add_username_to_users.rb @@ -0,0 +1,6 @@ +class AddUsernameToUsers < ActiveRecord::Migration[8.1] + def change + add_column :users, :username, :string + add_index :users, :username, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 79ed96a..de8269e 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[8.1].define(version: 2025_11_22_235519) do +ActiveRecord::Schema[8.1].define(version: 2025_11_25_012446) do create_table "application_groups", force: :cascade do |t| t.integer "application_id", null: false t.datetime "created_at", null: false @@ -21,6 +21,17 @@ ActiveRecord::Schema[8.1].define(version: 2025_11_22_235519) do t.index ["group_id"], name: "index_application_groups_on_group_id" end + create_table "application_user_claims", force: :cascade do |t| + t.integer "application_id", null: false + t.datetime "created_at", null: false + t.json "custom_claims", default: {}, null: false + t.datetime "updated_at", null: false + t.integer "user_id", null: false + t.index ["application_id", "user_id"], name: "index_app_user_claims_unique", unique: true + t.index ["application_id"], name: "index_application_user_claims_on_application_id" + t.index ["user_id"], name: "index_application_user_claims_on_user_id" + end + create_table "applications", force: :cascade do |t| t.integer "access_token_ttl", default: 3600 t.boolean "active", default: true, null: false @@ -169,10 +180,12 @@ ActiveRecord::Schema[8.1].define(version: 2025_11_22_235519) do t.boolean "totp_required", default: false, null: false t.string "totp_secret" t.datetime "updated_at", null: false + t.string "username" t.string "webauthn_id" t.boolean "webauthn_required", default: false, null: false t.index ["email_address"], name: "index_users_on_email_address", unique: true t.index ["status"], name: "index_users_on_status" + t.index ["username"], name: "index_users_on_username", unique: true t.index ["webauthn_id"], name: "index_users_on_webauthn_id", unique: true end @@ -200,6 +213,8 @@ ActiveRecord::Schema[8.1].define(version: 2025_11_22_235519) do add_foreign_key "application_groups", "applications" add_foreign_key "application_groups", "groups" + add_foreign_key "application_user_claims", "applications", on_delete: :cascade + add_foreign_key "application_user_claims", "users", on_delete: :cascade add_foreign_key "oidc_access_tokens", "applications" add_foreign_key "oidc_access_tokens", "users" add_foreign_key "oidc_authorization_codes", "applications" diff --git a/test/controllers/oidc_authorization_code_security_test.rb b/test/controllers/oidc_authorization_code_security_test.rb index 7aacd73..1a8e80e 100644 --- a/test/controllers/oidc_authorization_code_security_test.rb +++ b/test/controllers/oidc_authorization_code_security_test.rb @@ -19,8 +19,9 @@ class OidcAuthorizationCodeSecurityTest < ActionDispatch::IntegrationTest end def teardown - OidcAuthorizationCode.where(application: @application).destroy_all - OidcAccessToken.where(application: @application).destroy_all + OidcAuthorizationCode.where(application: @application).delete_all + # Use delete_all to avoid triggering callbacks that might have issues with the schema + OidcAccessToken.where(application: @application).delete_all @user.destroy @application.destroy end diff --git a/test/controllers/passwords_controller_test.rb b/test/controllers/passwords_controller_test.rb index e1a1b03..74cd926 100644 --- a/test/controllers/passwords_controller_test.rb +++ b/test/controllers/passwords_controller_test.rb @@ -11,7 +11,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest test "create" do post passwords_path, params: { email_address: @user.email_address } assert_enqueued_email_with PasswordsMailer, :reset, args: [ @user ] - assert_redirected_to new_session_path + assert_redirected_to signin_path follow_redirect! assert_notice "reset instructions sent" @@ -20,14 +20,14 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest test "create for an unknown user redirects but sends no mail" do post passwords_path, params: { email_address: "missing-user@example.com" } assert_enqueued_emails 0 - assert_redirected_to new_session_path + assert_redirected_to signin_path follow_redirect! assert_notice "reset instructions sent" end test "edit" do - get edit_password_path(@user.password_reset_token) + get edit_password_path(@user.generate_token_for(:password_reset)) assert_response :success end @@ -41,8 +41,8 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest test "update" do assert_changes -> { @user.reload.password_digest } do - put password_path(@user.password_reset_token), params: { password: "new", password_confirmation: "new" } - assert_redirected_to new_session_path + put password_path(@user.generate_token_for(:password_reset)), params: { password: "newpassword", password_confirmation: "newpassword" } + assert_redirected_to signin_path end follow_redirect! diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 07d72ef..9630ea3 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -18,7 +18,7 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest test "create with invalid credentials" do post session_path, params: { email_address: @user.email_address, password: "wrong" } - assert_redirected_to new_session_path + assert_redirected_to signin_path assert_nil cookies[:session_id] end @@ -27,7 +27,7 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest delete session_path - assert_redirected_to new_session_path + assert_redirected_to signin_path assert_empty cookies[:session_id] end end diff --git a/test/fixtures/application_user_claims.yml b/test/fixtures/application_user_claims.yml new file mode 100644 index 0000000..1e8d3f2 --- /dev/null +++ b/test/fixtures/application_user_claims.yml @@ -0,0 +1,11 @@ +# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html + +kavita_alice_claims: + application: kavita_app + user: alice + custom_claims: { "kavita_groups": ["admin"], "library_access": "all" } + +abs_alice_claims: + application: audiobookshelf_app + user: alice + custom_claims: { "abs_groups": ["user"], "abs_permissions": { "canDownload": true, "canUpload": false } } diff --git a/test/fixtures/applications.yml b/test/fixtures/applications.yml index b497f06..d821ffa 100644 --- a/test/fixtures/applications.yml +++ b/test/fixtures/applications.yml @@ -24,3 +24,14 @@ another_app: https://app.example.com/auth/callback metadata: "{}" active: true + +audiobookshelf_app: + name: Audiobookshelf + slug: audiobookshelf + app_type: oidc + client_id: <%= SecureRandom.urlsafe_base64(32) %> + client_secret_digest: <%= BCrypt::Password.create(SecureRandom.urlsafe_base64(48)) %> + redirect_uris: | + https://abs.example.com/auth/openid/callback + metadata: "{}" + active: true diff --git a/test/fixtures/groups.yml b/test/fixtures/groups.yml index 212aa2a..8e36c8a 100644 --- a/test/fixtures/groups.yml +++ b/test/fixtures/groups.yml @@ -1,5 +1,13 @@ # Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html +one: + name: Group One + description: First test group + +two: + name: Group Two + description: Second test group + admin_group: name: Administrators description: System administrators with full access diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 9413a38..738c183 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -1,5 +1,17 @@ <% password_digest = BCrypt::Password.create("password") %> +one: + email_address: one@example.com + password_digest: <%= password_digest %> + admin: false + status: 0 # active + +two: + email_address: two@example.com + password_digest: <%= password_digest %> + admin: true + status: 0 # active + alice: email_address: alice@example.com password_digest: <%= password_digest %> diff --git a/test/integration/forward_auth_integration_test.rb b/test/integration/forward_auth_integration_test.rb index cdb7716..582c4d3 100644 --- a/test/integration/forward_auth_integration_test.rb +++ b/test/integration/forward_auth_integration_test.rb @@ -58,8 +58,8 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest # Domain and Rule Integration Tests test "different domain patterns with same session" do # Create test rules - wildcard_rule = ForwardAuthRule.create!(domain_pattern: "*.example.com", active: true) - exact_rule = ForwardAuthRule.create!(domain_pattern: "api.example.com", active: true) + wildcard_rule = Application.create!(domain_pattern: "*.example.com", active: true) + exact_rule = Application.create!(domain_pattern: "api.example.com", active: true) # Sign in post "/signin", params: { email_address: @user.email_address, password: "password" } @@ -82,7 +82,7 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest test "group-based access control integration" do # Create restricted rule - restricted_rule = ForwardAuthRule.create!(domain_pattern: "restricted.example.com", active: true) + restricted_rule = Application.create!(domain_pattern: "restricted.example.com", active: true) restricted_rule.allowed_groups << @group # Sign in user without group @@ -104,17 +104,19 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest # Header Configuration Integration Tests test "different header configurations with same user" do - # Create rules with different header configs - default_rule = ForwardAuthRule.create!(domain_pattern: "default.example.com", active: true) - custom_rule = ForwardAuthRule.create!( + # Create applications with different configs + default_rule = Application.create!(name: "Default App", slug: "default-app", app_type: "forward_auth", domain_pattern: "default.example.com", active: true) + custom_rule = Application.create!( + name: "Custom App", slug: "custom-app", app_type: "forward_auth", domain_pattern: "custom.example.com", active: true, - headers_config: { user: "X-WEBAUTH-USER", groups: "X-WEBAUTH-ROLES" } + metadata: { headers: { user: "X-WEBAUTH-USER", groups: "X-WEBAUTH-ROLES" } }.to_json ) - no_headers_rule = ForwardAuthRule.create!( + no_headers_rule = Application.create!( + name: "No Headers App", slug: "no-headers-app", app_type: "forward_auth", domain_pattern: "noheaders.example.com", active: true, - headers_config: { user: "", email: "", name: "", groups: "", admin: "" } + metadata: { headers: { user: "", email: "", name: "", groups: "", admin: "" } }.to_json ) # Add user to groups @@ -191,7 +193,7 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest admin_user = users(:two) # Create restricted rule - admin_rule = ForwardAuthRule.create!( + admin_rule = Application.create!( domain_pattern: "admin.example.com", active: true, headers_config: { user: "X-Admin-User", admin: "X-Admin-Flag" } diff --git a/test/jobs/invitations_mailer_test.rb b/test/jobs/invitations_mailer_test.rb index 23982fc..c4571b9 100644 --- a/test/jobs/invitations_mailer_test.rb +++ b/test/jobs/invitations_mailer_test.rb @@ -25,8 +25,8 @@ class InvitationsMailerTest < ActionMailer::TestCase assert_equal "You're invited to join Clinch", email.subject assert_equal [@user.email_address], email.to - assert_equal [], email.cc - assert_equal [], email.bcc + assert_equal [], email.cc || [] + assert_equal [], email.bcc || [] # From address is configured in ApplicationMailer assert_not_nil email.from assert email.from.is_a?(Array) diff --git a/test/jobs/passwords_mailer_test.rb b/test/jobs/passwords_mailer_test.rb index ed9b35f..77d588d 100644 --- a/test/jobs/passwords_mailer_test.rb +++ b/test/jobs/passwords_mailer_test.rb @@ -25,8 +25,8 @@ class PasswordsMailerTest < ActionMailer::TestCase assert_equal "Reset your password", email.subject assert_equal [@user.email_address], email.to - assert_equal [], email.cc - assert_equal [], email.bcc + assert_equal [], email.cc || [] + assert_equal [], email.bcc || [] # From address is configured in ApplicationMailer assert_not_nil email.from assert email.from.is_a?(Array) diff --git a/test/models/application_user_claim_test.rb b/test/models/application_user_claim_test.rb new file mode 100644 index 0000000..1f7a651 --- /dev/null +++ b/test/models/application_user_claim_test.rb @@ -0,0 +1,78 @@ +require "test_helper" + +class ApplicationUserClaimTest < ActiveSupport::TestCase + def setup + @user = users(:bob) + @application = applications(:another_app) + end + + test "should create valid application user claim" do + claim = ApplicationUserClaim.new( + user: @user, + application: @application, + custom_claims: { "role": "admin" } + ) + assert claim.valid? + assert claim.save + end + + test "should enforce uniqueness of user per application" do + ApplicationUserClaim.create!( + user: @user, + application: @application, + custom_claims: { "role": "admin" } + ) + + duplicate = ApplicationUserClaim.new( + user: @user, + application: @application, + custom_claims: { "role": "user" } + ) + + assert_not duplicate.valid? + assert_includes duplicate.errors[:user_id], "has already been taken" + end + + test "parsed_custom_claims returns hash" do + claim = ApplicationUserClaim.new( + user: @user, + application: @application, + custom_claims: { "role": "admin", "level": 5 } + ) + + parsed = claim.parsed_custom_claims + assert_equal "admin", parsed["role"] + assert_equal 5, parsed["level"] + end + + test "parsed_custom_claims returns empty hash when nil" do + claim = ApplicationUserClaim.new( + user: @user, + application: @application, + custom_claims: nil + ) + + assert_equal({}, claim.parsed_custom_claims) + end + + test "should not allow reserved OIDC claim names" do + claim = ApplicationUserClaim.new( + user: @user, + application: @application, + custom_claims: { "groups": ["admin"], "role": "user" } + ) + + assert_not claim.valid? + assert_includes claim.errors[:custom_claims], "cannot override reserved OIDC claims: groups" + end + + test "should allow non-reserved claim names" do + claim = ApplicationUserClaim.new( + user: @user, + application: @application, + custom_claims: { "kavita_groups": ["admin"], "role": "user" } + ) + + assert claim.valid? + end +end diff --git a/test/services/oidc_jwt_service_test.rb b/test/services/oidc_jwt_service_test.rb index 14db9b8..d162ff2 100644 --- a/test/services/oidc_jwt_service_test.rb +++ b/test/services/oidc_jwt_service_test.rb @@ -14,7 +14,8 @@ class OidcJwtServiceTest < ActiveSupport::TestCase assert token.length > 100, "Token should be substantial" assert token.include?('.') - decoded = JWT.decode(token, nil, true) + # Decode without verification for testing the payload + decoded = JWT.decode(token, nil, false).first assert_equal @application.client_id, decoded['aud'], "Should have correct audience" assert_equal @user.id.to_s, decoded['sub'], "Should have correct subject" assert_equal @user.email_address, decoded['email'], "Should have correct email" @@ -22,16 +23,16 @@ class OidcJwtServiceTest < ActiveSupport::TestCase assert_equal @user.email_address, decoded['preferred_username'], "Should have preferred username" assert_equal @user.email_address, decoded['name'], "Should have name" assert_equal "https://localhost:3000", decoded['iss'], "Should have correct issuer" - assert_equal Time.now.to_i + 3600, decoded['exp'], "Should have correct expiration" + assert_in_delta Time.current.to_i + 3600, decoded['exp'], 5, "Should have correct expiration" end test "should handle nonce in id token" do nonce = "test-nonce-12345" token = @service.generate_id_token(@user, @application, nonce: nonce) - decoded = JWT.decode(token, nil, true) + decoded = JWT.decode(token, nil, false).first assert_equal nonce, decoded['nonce'], "Should preserve nonce in token" - assert_equal Time.now.to_i + 3600, decoded['exp'], "Should have correct expiration with nonce" + assert_in_delta Time.current.to_i + 3600, decoded['exp'], 5, "Should have correct expiration with nonce" end test "should include groups in token when user has groups" do @@ -39,17 +40,17 @@ class OidcJwtServiceTest < ActiveSupport::TestCase token = @service.generate_id_token(@user, @application) - decoded = JWT.decode(token, nil, true) + decoded = JWT.decode(token, nil, false).first assert_includes decoded['groups'], "admin", "Should include user's groups" end - test "should include admin claim for admin users" do + test "admin claim should not be included in token" do @user.update!(admin: true) token = @service.generate_id_token(@user, @application) - decoded = JWT.decode(token, nil, true) - assert_equal true, decoded['admin'], "Admin users should have admin claim" + decoded = JWT.decode(token, nil, false).first + refute decoded.key?('admin'), "Admin claim should not be included in ID tokens (use groups instead)" end test "should handle role-based claims when enabled" do @@ -63,7 +64,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase token = @service.generate_id_token(@user, @application) - decoded = JWT.decode(token, nil, true) + decoded = JWT.decode(token, nil, false).first assert_includes decoded['roles'], "editor", "Should include user's role" end @@ -96,7 +97,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase token = @service.generate_id_token(@user, @application) - decoded = JWT.decode(token, nil, true) + decoded = JWT.decode(token, nil, false).first assert_equal "Content Editor", decoded['role_display_name'], "Should include role display name" assert_includes decoded['role_permissions'], "read", "Should include read permission" assert_includes decoded['role_permissions'], "write", "Should include write permission" @@ -107,7 +108,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase test "should handle missing roles gracefully" do token = @service.generate_id_token(@user, @application) - decoded = JWT.decode(token, nil, true) + decoded = JWT.decode(token, nil, false).first refute_includes decoded, 'roles', "Should not have roles when not configured" end @@ -260,7 +261,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase test "should handle access token generation" do token = @service.generate_id_token(@user, @application) - decoded = JWT.decode(token, nil, true) + decoded = JWT.decode(token, nil, false).first refute_includes decoded.keys, 'email_verified' assert_equal @user.id.to_s, decoded['sub'], "Should decode subject correctly" assert_equal @application.client_id, decoded['aud'], "Should decode audience correctly" @@ -291,4 +292,215 @@ class OidcJwtServiceTest < ActiveSupport::TestCase end assert_match /no key found/, error.message, "Should warn about missing private key" end + + test "should include app-specific custom claims in token" do + # Use bob and another_app to avoid fixture conflicts + user = users(:bob) + app = applications(:another_app) + + # Create app-specific claim + ApplicationUserClaim.create!( + user: user, + application: app, + custom_claims: { "app_groups": ["admin"], "library_access": "all" } + ) + + token = @service.generate_id_token(user, app) + decoded = JWT.decode(token, nil, false).first + + assert_equal ["admin"], decoded["app_groups"] + assert_equal "all", decoded["library_access"] + end + + test "app-specific claims should override user and group claims" do + # Use bob and another_app to avoid fixture conflicts + user = users(:bob) + app = applications(:another_app) + + # Add user to group with claims + group = groups(:admin_group) + group.update!(custom_claims: { "role": "viewer", "max_items": 10 }) + user.groups << group + + # Add user custom claims + user.update!(custom_claims: { "role": "editor", "theme": "dark" }) + + # Add app-specific claims (should override both) + ApplicationUserClaim.create!( + user: user, + application: app, + custom_claims: { "role": "admin", "app_specific": true } + ) + + token = @service.generate_id_token(user, app) + decoded = JWT.decode(token, nil, false).first + + # App-specific claim should win + assert_equal "admin", decoded["role"] + # App-specific claim should be present + assert_equal true, decoded["app_specific"] + # User claim not overridden should still be present + assert_equal "dark", decoded["theme"] + # Group claim not overridden should still be present + assert_equal 10, decoded["max_items"] + end + + test "should deep merge array claims from group and user" do + user = users(:bob) + app = applications(:another_app) + + # Group has roles: ["user"] + group = groups(:admin_group) + group.update!(custom_claims: { "roles" => ["user"], "permissions" => ["read"] }) + user.groups << group + + # User adds roles: ["admin"] + user.update!(custom_claims: { "roles" => ["admin"], "permissions" => ["write"] }) + + token = @service.generate_id_token(user, app) + decoded = JWT.decode(token, nil, false).first + + # Roles should be combined (not overwritten) + assert_equal 2, decoded["roles"].length + assert_includes decoded["roles"], "user" + assert_includes decoded["roles"], "admin" + # Permissions should also be combined + assert_equal 2, decoded["permissions"].length + assert_includes decoded["permissions"], "read" + assert_includes decoded["permissions"], "write" + end + + test "should deep merge array claims from multiple groups" do + user = users(:bob) + app = applications(:another_app) + + # First group has roles: ["user"] + group1 = groups(:admin_group) + group1.update!(custom_claims: { "roles" => ["user"] }) + user.groups << group1 + + # Second group has roles: ["moderator"] + group2 = Group.create!(name: "moderators", description: "Moderators group") + group2.update!(custom_claims: { "roles" => ["moderator"] }) + user.groups << group2 + + # User adds roles: ["admin"] + user.update!(custom_claims: { "roles" => ["admin"] }) + + token = @service.generate_id_token(user, app) + decoded = JWT.decode(token, nil, false).first + + # All roles should be combined + assert_equal 3, decoded["roles"].length + assert_includes decoded["roles"], "user" + assert_includes decoded["roles"], "moderator" + assert_includes decoded["roles"], "admin" + end + + test "should remove duplicate values when merging arrays" do + user = users(:bob) + app = applications(:another_app) + + # Group has roles: ["user", "reader"] + group = groups(:admin_group) + group.update!(custom_claims: { "roles" => ["user", "reader"] }) + user.groups << group + + # User also has "user" role (duplicate) + user.update!(custom_claims: { "roles" => ["user", "admin"] }) + + token = @service.generate_id_token(user, app) + decoded = JWT.decode(token, nil, false).first + + # "user" should only appear once + assert_equal 3, decoded["roles"].length + assert_includes decoded["roles"], "user" + assert_includes decoded["roles"], "reader" + assert_includes decoded["roles"], "admin" + end + + test "should override non-array values while merging arrays" do + user = users(:bob) + app = applications(:another_app) + + # Group has roles array and max_items scalar + group = groups(:admin_group) + group.update!(custom_claims: { "roles" => ["user"], "max_items" => 10, "theme" => "light" }) + user.groups << group + + # User overrides max_items and theme, adds to roles + user.update!(custom_claims: { "roles" => ["admin"], "max_items" => 100, "theme" => "dark" }) + + token = @service.generate_id_token(user, app) + decoded = JWT.decode(token, nil, false).first + + # Arrays should be combined + assert_equal 2, decoded["roles"].length + assert_includes decoded["roles"], "user" + assert_includes decoded["roles"], "admin" + # Scalar values should be overridden (user wins) + assert_equal 100, decoded["max_items"] + assert_equal "dark", decoded["theme"] + end + + test "should deep merge nested hashes in claims" do + user = users(:bob) + app = applications(:another_app) + + # Group has nested config + group = groups(:admin_group) + group.update!(custom_claims: { + "config" => { + "theme" => "light", + "notifications" => { "email" => true } + } + }) + user.groups << group + + # User adds to nested config + user.update!(custom_claims: { + "config" => { + "language" => "en", + "notifications" => { "sms" => true } + } + }) + + token = @service.generate_id_token(user, app) + decoded = JWT.decode(token, nil, false).first + + # Nested hashes should be deep merged + assert_equal "light", decoded["config"]["theme"] + assert_equal "en", decoded["config"]["language"] + assert_equal true, decoded["config"]["notifications"]["email"] + assert_equal true, decoded["config"]["notifications"]["sms"] + end + + test "app-specific claims should combine arrays with group and user claims" do + user = users(:bob) + app = applications(:another_app) + + # Group has roles: ["user"] + group = groups(:admin_group) + group.update!(custom_claims: { "roles" => ["user"] }) + user.groups << group + + # User has roles: ["moderator"] + user.update!(custom_claims: { "roles" => ["moderator"] }) + + # App-specific has roles: ["app_admin"] + ApplicationUserClaim.create!( + user: user, + application: app, + custom_claims: { "roles" => ["app_admin"] } + ) + + token = @service.generate_id_token(user, app) + decoded = JWT.decode(token, nil, false).first + + # All three sources should be combined + assert_equal 3, decoded["roles"].length + assert_includes decoded["roles"], "user" + assert_includes decoded["roles"], "moderator" + assert_includes decoded["roles"], "app_admin" + end end \ No newline at end of file