Default-deny access control with group flags and access enumeration
Some checks failed
CI / scan_ruby (push) Has been cancelled
CI / scan_js (push) Has been cancelled
CI / scan_container (push) Has been cancelled
CI / lint (push) Has been cancelled
CI / test (push) Has been cancelled
CI / system-test (push) Has been cancelled

Replaces the implicit "empty allowed_groups means public" rule with
explicit default-deny across both OIDC and ForwardAuth. Adds two boolean
flags on Group — auto_assign (Keycloak-style auto-join on user create)
and admin (members can reach the admin panel) — and drops the
users.admin column entirely. Adds "Users with access" and "Accessible
applications" panels with via-group badges on the application/user show
pages.

BEHAVIOR CHANGE: a ForwardAuth app with no allowed_groups previously
bypassed authentication entirely; it now returns 403 like any other
unauthorized request. The data migration seeds an "everyone" group and
attaches it to all previously group-less apps to preserve behavior on
existing installs. An "admins" group is seeded and backfilled from any
user with the old admin column.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Dan Milne
2026-06-07 15:53:27 +10:00
parent 6b58b685c4
commit 03dfdbd83a
32 changed files with 530 additions and 88 deletions

View File

@@ -48,5 +48,23 @@ module Admin
assert_equal [app], Group.find_by(name: "new group").applications
end
test "can mark a group as auto_assign and admin" do
patch admin_group_path(@group), params: {
group: {name: @group.name, auto_assign: "1", admin: "1"}
}
@group.reload
assert @group.auto_assign?
assert @group.admin?
end
test "cannot delete the last admin group" do
admins = groups(:admin_group)
delete admin_group_path(admins)
# Destroy was aborted by the before_destroy guard
assert Group.exists?(admins.id), "admin group should not have been deleted"
end
end
end

View File

@@ -0,0 +1,69 @@
require "test_helper"
module Admin
class UsersControllerTest < ActionDispatch::IntegrationTest
setup do
@admin = users(:two) # in admin_group via fixtures
sign_in_as(@admin)
end
test "show loads accessible applications via the user's groups" do
kavita = applications(:kavita_app)
# alice is in admin_group via fixtures; kavita is attached to admin_group via app_groups
get admin_user_path(users(:alice))
assert_response :success
assert_match kavita.name, response.body
# The "via" badge mentions the granting group name
assert_match groups(:admin_group).name, response.body
end
test "update assigns group memberships from group_ids" do
target = users(:bob)
editors = groups(:editor_group)
one = groups(:one)
patch admin_user_path(target), params: {
user: {email_address: target.email_address, group_ids: [editors.id, one.id]}
}
assert_redirected_to admin_users_path
assert_equal [editors, one].sort, target.reload.groups.sort
end
test "cannot remove yourself from the last admin group" do
# @admin (users:two) is in admin_group. Removing them via the user form
# while no other admin exists is blocked.
sole_admin = users(:two)
# Strip alice (the other admin) so @admin is the last one.
users(:alice).groups.delete(groups(:admin_group))
patch admin_user_path(sole_admin), params: {
user: {email_address: sole_admin.email_address, group_ids: []}
}
assert_response :unprocessable_entity
assert sole_admin.reload.admin?, "should still be admin"
end
test "create with auto_assign=0 skips the auto-assign callback" do
post admin_users_path, params: {
user: {email_address: "restricted@example.com"},
auto_assign: "0"
}
assert_response :redirect
created = User.find_by(email_address: "restricted@example.com")
assert_not_includes created.groups, groups(:everyone)
end
test "create without auto_assign param auto-joins everyone" do
post admin_users_path, params: {
user: {email_address: "newbie@example.com"}
}
assert_response :redirect
created = User.find_by(email_address: "newbie@example.com")
assert_includes created.groups, groups(:everyone)
end
end
end

View File

@@ -11,6 +11,7 @@ module Api
domain_pattern: "webdav.example.com",
active: true
)
grant_everyone_access(@app)
@api_key = @user.api_keys.create!(name: "Test Key", application: @app)
@token = @api_key.plaintext_token
end

View File

@@ -7,8 +7,8 @@ module Api
@admin_user = users(:alice)
@inactive_user = User.create!(email_address: "inactive@example.com", password: "password", status: :disabled)
@group = groups(:admin_group)
@rule = Application.create!(name: "Test App", slug: "test-app", app_type: "forward_auth", domain_pattern: "test.example.com", active: true)
@inactive_rule = Application.create!(name: "Inactive App", slug: "inactive-app", app_type: "forward_auth", domain_pattern: "inactive.example.com", active: false)
@rule = grant_everyone_access(Application.create!(name: "Test App", slug: "test-app", app_type: "forward_auth", domain_pattern: "test.example.com", active: true))
@inactive_rule = grant_everyone_access(Application.create!(name: "Inactive App", slug: "inactive-app", app_type: "forward_auth", domain_pattern: "inactive.example.com", active: false))
end
# Authentication Tests
@@ -65,7 +65,7 @@ module Api
end
test "should return 403 when rule exists but user not in allowed groups" do
@rule.allowed_groups << @group
@rule.allowed_groups = [@group]
sign_in_as(@user) # User not in group
get "/api/verify", headers: {"X-Forwarded-Host" => "test.example.com"}
@@ -75,7 +75,7 @@ module Api
end
test "should return 200 when user is in allowed groups" do
@rule.allowed_groups << @group
@rule.allowed_groups = [@group]
@user.groups << @group
sign_in_as(@user)
@@ -86,7 +86,7 @@ module Api
# Domain Pattern Tests
test "should match wildcard domains correctly" do
Application.create!(name: "Wildcard App", slug: "wildcard-app", app_type: "forward_auth", domain_pattern: "*.example.com", active: true)
grant_everyone_access Application.create!(name: "Wildcard App", slug: "wildcard-app", app_type: "forward_auth", domain_pattern: "*.example.com", active: true)
sign_in_as(@user)
get "/api/verify", headers: {"X-Forwarded-Host" => "app.example.com"}
@@ -101,7 +101,7 @@ module Api
end
test "should match exact domains correctly" do
Application.create!(name: "Exact App", slug: "exact-app", app_type: "forward_auth", domain_pattern: "api.example.com", active: true)
grant_everyone_access Application.create!(name: "Exact App", slug: "exact-app", app_type: "forward_auth", domain_pattern: "api.example.com", active: true)
sign_in_as(@user)
get "/api/verify", headers: {"X-Forwarded-Host" => "api.example.com"}
@@ -126,7 +126,7 @@ module Api
end
test "should return custom headers when configured" do
Application.create!(
grant_everyone_access Application.create!(
name: "Custom App",
slug: "custom-app",
app_type: "forward_auth",
@@ -151,7 +151,7 @@ module Api
end
test "should return no headers when all headers disabled" do
Application.create!(
grant_everyone_access Application.create!(
name: "No Headers App",
slug: "no-headers-app",
app_type: "forward_auth",
@@ -182,11 +182,19 @@ module Api
assert_includes groups_header, "Editors"
end
test "should not include groups header when user has no groups" do
@user.groups.clear # Remove fixture groups
test "should not include groups header when user has no groups beyond the granting one and groups header empty" do
# Under default-deny the user must be in at least one group to access the app.
# This rewritten test verifies that when an app's headers_config disables the
# groups header, no x-remote-groups is sent regardless of memberships.
app = grant_everyone_access Application.create!(
name: "Headers Hidden", slug: "headers-hidden", app_type: "forward_auth",
domain_pattern: "hidden.example.com",
active: true,
headers_config: {groups: ""}
)
sign_in_as(@user)
get "/api/verify", headers: {"X-Forwarded-Host" => "test.example.com"}
get "/api/verify", headers: {"X-Forwarded-Host" => "hidden.example.com"}
assert_response 200
assert_nil response.headers["x-remote-groups"]
@@ -705,7 +713,7 @@ module Api
class FaTokenHostBindingTest < ActionDispatch::IntegrationTest
setup do
@user = users(:bob)
Application.create!(name: "Bound App", slug: "bound-app", app_type: "forward_auth", domain_pattern: "app.example.com", active: true)
grant_everyone_access Application.create!(name: "Bound App", slug: "bound-app", app_type: "forward_auth", domain_pattern: "app.example.com", active: true)
@original_cache = Rails.cache
Rails.cache = ActiveSupport::Cache::MemoryStore.new

View File

@@ -17,6 +17,7 @@ class OidcAuthorizationCodeSecurityTest < ActionDispatch::IntegrationTest
@application.generate_new_client_secret!
@plain_client_secret = @application.client_secret
@application.save!
grant_everyone_access(@application)
end
def teardown

View File

@@ -10,6 +10,7 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
redirect_uris: ["http://localhost:4000/callback"].to_json,
active: true
)
grant_everyone_access(@application)
# Sign in the user using the test helper
sign_in_as(@user)

View File

@@ -11,7 +11,13 @@ two:
admin_group:
name: Administrators
description: System administrators with full access
admin: true
editor_group:
name: Editors
description: Content editors with limited access
everyone:
name: everyone
description: Auto-assigned to new users.
auto_assign: true

View File

@@ -1,9 +1,28 @@
# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
# All users belong to "everyone" so existing tests that create group-less apps
# can be made compatible by attaching that group.
one_everyone:
user: one
group: everyone
two_everyone:
user: two
group: everyone
alice_everyone:
user: alice
group: everyone
bob_everyone:
user: bob
group: everyone
alice_admin_group:
user: alice
group: admin_group
two_admin_group:
user: two
group: admin_group
bob_editor_group:
user: bob
group: editor_group

View File

@@ -3,23 +3,19 @@
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 %>
admin: true
status: 0 # active
bob:
email_address: bob@example.com
password_digest: <%= password_digest %>
admin: false
status: 0 # active

View File

@@ -12,7 +12,7 @@ class ForwardAuthAdvancedTest < ActionDispatch::IntegrationTest
# End-to-End Authentication Flow Tests
test "complete forward auth flow with default headers" do
# Create an application with default headers
Application.create!(name: "App", slug: "app-system-test", app_type: "forward_auth", domain_pattern: "app.example.com", active: true)
grant_everyone_access Application.create!(name: "App", slug: "app-system-test", app_type: "forward_auth", domain_pattern: "app.example.com", active: true)
# Step 1: Unauthenticated request to protected resource
get "/api/verify", headers: {
@@ -48,14 +48,14 @@ class ForwardAuthAdvancedTest < ActionDispatch::IntegrationTest
test "multiple domain access with single session" do
# Create applications for different domains
Application.create!(name: "App Domain", slug: "app-domain", app_type: "forward_auth", domain_pattern: "app.example.com", active: true)
Application.create!(
grant_everyone_access Application.create!(name: "App Domain", slug: "app-domain", app_type: "forward_auth", domain_pattern: "app.example.com", active: true)
grant_everyone_access Application.create!(
name: "Grafana", slug: "grafana-system-test", app_type: "forward_auth",
domain_pattern: "grafana.example.com",
active: true,
headers_config: {user: "X-WEBAUTH-USER", email: "X-WEBAUTH-EMAIL"}
)
Application.create!(
grant_everyone_access Application.create!(
name: "Metube", slug: "metube-system-test", app_type: "forward_auth",
domain_pattern: "metube.example.com",
active: true,
@@ -106,7 +106,7 @@ class ForwardAuthAdvancedTest < ActionDispatch::IntegrationTest
# Should have access (in allowed group)
get "/api/verify", headers: {"X-Forwarded-Host" => "admin.example.com"}
assert_response 200
assert_equal @group.name, response.headers["x-remote-groups"]
assert_includes response.headers["x-remote-groups"], @group.name
# Add user to second group
@user.groups << @group2
@@ -126,31 +126,27 @@ class ForwardAuthAdvancedTest < ActionDispatch::IntegrationTest
assert_response 403
end
test "bypass mode when no groups assigned to rule" do
# Create bypass application (no groups)
test "default deny when no groups assigned to rule" do
# An app with no allowed_groups now denies all users (was: bypass mode).
Application.create!(
name: "Public", slug: "public-system-test", app_type: "forward_auth",
domain_pattern: "public.example.com",
name: "No Groups", slug: "nogroups-system-test", app_type: "forward_auth",
domain_pattern: "nogroups.example.com",
active: true
)
# Create user with no groups
@user.groups.clear
# Sign in
post "/signin", params: {email_address: @user.email_address, password: "password"}
assert_response 303
# Should have access (bypass mode)
get "/api/verify", headers: {"X-Forwarded-Host" => "public.example.com"}
assert_response 200
assert_equal @user.email_address, response.headers["x-remote-user"]
get "/api/verify", headers: {"X-Forwarded-Host" => "nogroups.example.com"}
assert_response 403
end
# Security System Tests
test "session expiration and cleanup" do
# Create test application
Application.create!(
grant_everyone_access Application.create!(
name: "Test", slug: "test-system-test", app_type: "forward_auth",
domain_pattern: "test.example.com",
active: true
@@ -179,7 +175,7 @@ class ForwardAuthAdvancedTest < ActionDispatch::IntegrationTest
test "concurrent access with rate limiting considerations" do
# Create wildcard application
Application.create!(
grant_everyone_access Application.create!(
name: "Wildcard", slug: "wildcard-test", app_type: "forward_auth",
domain_pattern: "*.example.com",
active: true
@@ -246,7 +242,11 @@ class ForwardAuthAdvancedTest < ActionDispatch::IntegrationTest
active: true,
headers_config: app[:headers_config]
)
app[:groups].each { |group| rule.allowed_groups << group }
if app[:groups].any?
app[:groups].each { |group| rule.allowed_groups << group }
else
grant_everyone_access(rule)
end
rule
end
@@ -286,7 +286,7 @@ class ForwardAuthAdvancedTest < ActionDispatch::IntegrationTest
]
patterns.each_with_index do |pattern_config, idx|
Application.create!(
grant_everyone_access Application.create!(
name: "Pattern Test #{idx}", slug: "pattern-test-#{idx}", app_type: "forward_auth",
domain_pattern: pattern_config[:pattern],
active: true
@@ -310,7 +310,7 @@ class ForwardAuthAdvancedTest < ActionDispatch::IntegrationTest
# Performance System Tests
test "system performance under load" do
# Create test application with wildcard pattern
Application.create!(name: "Load Test", slug: "loadtest", app_type: "forward_auth", domain_pattern: "*.loadtest.example.com", active: true)
grant_everyone_access Application.create!(name: "Load Test", slug: "loadtest", app_type: "forward_auth", domain_pattern: "*.loadtest.example.com", active: true)
# Sign in
post "/signin", params: {email_address: @user.email_address, password: "password"}

View File

@@ -15,6 +15,7 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest
domain_pattern: "test.example.com",
active: true
)
grant_everyone_access(@test_app)
end
# Basic Authentication Flow Tests
@@ -56,8 +57,8 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest
# Domain and Rule Integration Tests
test "different domain patterns with same session" do
# Create test rules
Application.create!(name: "Wildcard App", slug: "wildcard-app", app_type: "forward_auth", domain_pattern: "*.example.com", active: true)
Application.create!(name: "Exact App", slug: "exact-app", app_type: "forward_auth", domain_pattern: "api.example.com", active: true)
grant_everyone_access Application.create!(name: "Wildcard App", slug: "wildcard-app", app_type: "forward_auth", domain_pattern: "*.example.com", active: true)
grant_everyone_access Application.create!(name: "Exact App", slug: "exact-app", app_type: "forward_auth", domain_pattern: "api.example.com", active: true)
# Sign in
post "/signin", params: {email_address: @user.email_address, password: "password"}
@@ -103,14 +104,14 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest
# Header Configuration Integration Tests
test "different header configurations with same user" do
# Create applications with different configs
Application.create!(name: "Default App", slug: "default-app", app_type: "forward_auth", domain_pattern: "default.example.com", active: true)
Application.create!(
grant_everyone_access Application.create!(name: "Default App", slug: "default-app", app_type: "forward_auth", domain_pattern: "default.example.com", active: true)
grant_everyone_access 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"}
)
Application.create!(
grant_everyone_access Application.create!(
name: "No Headers App", slug: "no-headers-app", app_type: "forward_auth",
domain_pattern: "noheaders.example.com",
active: true,
@@ -196,7 +197,7 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest
admin_user = users(:two)
# Create restricted rule
Application.create!(
grant_everyone_access Application.create!(
name: "Admin App", slug: "admin-app", app_type: "forward_auth",
domain_pattern: "admin.example.com",
active: true,

View File

@@ -10,6 +10,7 @@ class ApiKeyTest < ActiveSupport::TestCase
domain_pattern: "webdav.example.com",
active: true
)
@app.allowed_groups << groups(:everyone)
end
test "generates clk_ prefixed token on create" do
@@ -78,9 +79,8 @@ class ApiKeyTest < ActiveSupport::TestCase
end
test "validates user must have access to application" do
group = groups(:admin_group)
@app.allowed_groups << group
# @user (bob) is not in admin_group
# Restrict the app to admin_group only — bob is not in admin_group.
@app.allowed_groups = [groups(:admin_group)]
key = @user.api_keys.build(name: "No Access", application: @app)
assert_not key.valid?
assert_includes key.errors[:user], "does not have access to this application"

View File

@@ -135,23 +135,36 @@ class UserTest < ActiveSupport::TestCase
assert_equal user, found_user
end
test "admin scope" do
admin_user = User.create!(
email_address: "admin@example.com",
password: "password123",
admin: true
)
regular_user = User.create!(
email_address: "user@example.com",
password: "password123",
admin: false
)
test "admin scope returns users in admin groups" do
admin_group = groups(:admin_group)
admin_user = User.create!(email_address: "admin@example.com", password: "password123")
admin_user.groups << admin_group
regular_user = User.create!(email_address: "user@example.com", password: "password123")
admins = User.admins
assert_includes admins, admin_user
assert_not_includes admins, regular_user
end
test "admin? reflects membership in any admin: true group" do
user = User.create!(email_address: "promoted@example.com", password: "password123")
assert_not user.admin?
user.groups << groups(:admin_group)
assert user.reload.admin?
end
test "after_create auto-joins all auto_assign groups" do
user = User.create!(email_address: "newbie@example.com", password: "password123")
assert_includes user.groups, groups(:everyone)
end
test "skip_auto_assign disables the after_create callback" do
user = User.new(email_address: "skipper@example.com", password: "password123")
user.skip_auto_assign = true
user.save!
assert_not_includes user.groups, groups(:everyone)
end
test "validates email address format" do
user = User.new(email_address: "invalid-email", password: "password123")
assert_not user.valid?

View File

@@ -95,7 +95,8 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
end
test "admin claim should not be included in token" do
@user.update!(admin: true)
# alice is already in admin_group via fixtures, so admin? is true here
assert @user.admin?
token = @service.generate_id_token(@user, @application)

View File

@@ -12,6 +12,15 @@ module SessionTestHelper
Current.session&.destroy!
cookies.delete("session_id")
end
# Attach the auto-assign "everyone" group to the given app so existing tests
# written under the old "empty allowed_groups = public" rule keep working.
# New tests should attach groups explicitly to model real access intent.
def grant_everyone_access(app)
everyone = (groups(:everyone) rescue Group.find_by(auto_assign: true))
app.allowed_groups << everyone unless app.allowed_groups.include?(everyone)
app
end
end
ActiveSupport.on_load(:action_dispatch_integration_test) do