Compare commits
7 Commits
a6480b0860
...
main
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
444ae6291c | ||
|
|
233fb723d5 | ||
|
|
cc6d4fcc65 | ||
|
|
5268f10eb3 | ||
|
|
5c5662eaab | ||
|
|
27d77ebf47 | ||
|
|
ba08158c85 |
@@ -76,7 +76,7 @@ Apps that only need "who is it?", or you want available from the internet behind
|
||||
|
||||
#### OpenID Connect (OIDC)
|
||||
|
||||
**[OpenID Certified](https://www.certification.openid.net/plan-detail.html?plan=FbQNTJuYVzrzs&public=true)** - Clinch passes the official OpenID Connect conformance tests (valid as of [v0.8.6](https://github.com/dkam/clinch/releases/tag/0.8.6)).
|
||||
**[OpenID Connect Conformance](https://www.certification.openid.net/plan-detail.html?plan=FbQNTJuYVzrzs&public=true)** - Clinch passes the official OpenID Connect conformance tests (valid as of [v0.8.6](https://github.com/dkam/clinch/releases/tag/0.8.6)).
|
||||
|
||||
Standard OAuth2/OIDC provider with endpoints:
|
||||
- `/.well-known/openid-configuration` - Discovery endpoint
|
||||
|
||||
@@ -88,6 +88,8 @@ module Api
|
||||
case key
|
||||
when :user, :email, :name
|
||||
[header_name, user.email_address]
|
||||
when :username
|
||||
[header_name, user.username] if user.username.present?
|
||||
when :groups
|
||||
user.groups.any? ? [header_name, user.groups.pluck(:name).join(",")] : nil
|
||||
when :admin
|
||||
|
||||
@@ -422,7 +422,11 @@ class OidcController < ApplicationController
|
||||
|
||||
# Record user consent
|
||||
requested_scopes = oauth_params["scope"].split(" ")
|
||||
parsed_claims = JSON.parse(oauth_params["claims_requests"]) rescue {}
|
||||
parsed_claims = begin
|
||||
JSON.parse(oauth_params["claims_requests"])
|
||||
rescue
|
||||
{}
|
||||
end
|
||||
|
||||
consent = OidcUserConsent.find_or_initialize_by(user: user, application: application)
|
||||
consent.scopes_granted = requested_scopes.join(" ")
|
||||
@@ -457,6 +461,16 @@ class OidcController < ApplicationController
|
||||
|
||||
# POST /oauth/token
|
||||
def token
|
||||
# Reject claims parameter - per OIDC security, claims parameter is only valid
|
||||
# in authorization requests, not at the token endpoint
|
||||
if params[:claims].present?
|
||||
render json: {
|
||||
error: "invalid_request",
|
||||
error_description: "claims parameter is not allowed at the token endpoint"
|
||||
}, status: :bad_request
|
||||
return
|
||||
end
|
||||
|
||||
grant_type = params[:grant_type]
|
||||
|
||||
case grant_type
|
||||
@@ -770,10 +784,10 @@ class OidcController < ApplicationController
|
||||
# Extract access token from Authorization header or POST body
|
||||
# RFC 6750: Bearer token can be in Authorization header, request body, or query string
|
||||
token = if request.headers["Authorization"]&.start_with?("Bearer ")
|
||||
request.headers["Authorization"].sub("Bearer ", "")
|
||||
elsif request.params["access_token"].present?
|
||||
request.params["access_token"]
|
||||
end
|
||||
request.headers["Authorization"].sub("Bearer ", "")
|
||||
elsif request.params["access_token"].present?
|
||||
request.params["access_token"]
|
||||
end
|
||||
|
||||
unless token
|
||||
head :unauthorized
|
||||
@@ -1016,7 +1030,7 @@ class OidcController < ApplicationController
|
||||
end
|
||||
|
||||
# Validate code verifier format (per RFC 7636: [A-Za-z0-9\-._~], 43-128 characters)
|
||||
unless code_verifier.match?(/\A[A-Za-z0-9\.\-_~]{43,128}\z/)
|
||||
unless code_verifier.match?(/\A[A-Za-z0-9.\-_~]{43,128}\z/)
|
||||
return {
|
||||
valid: false,
|
||||
error: "invalid_request",
|
||||
|
||||
45
app/lib/duration_parser.rb
Normal file
45
app/lib/duration_parser.rb
Normal file
@@ -0,0 +1,45 @@
|
||||
class DurationParser
|
||||
UNITS = {
|
||||
"s" => 1, # seconds
|
||||
"m" => 60, # minutes
|
||||
"h" => 3600, # hours
|
||||
"d" => 86400, # days
|
||||
"w" => 604800, # weeks
|
||||
"M" => 2592000, # months (30 days)
|
||||
"y" => 31536000 # years (365 days)
|
||||
}
|
||||
|
||||
# Parse a duration string into seconds
|
||||
# Accepts formats: "1h", "30m", "1d", "1M" (month), "3600" (plain number)
|
||||
# Returns integer seconds or nil if invalid
|
||||
# Case-sensitive: 1s, 1m, 1h, 1d, 1w, 1M (month), 1y
|
||||
def self.parse(input)
|
||||
# Handle integers directly
|
||||
return input if input.is_a?(Integer)
|
||||
|
||||
# Convert to string and strip whitespace
|
||||
str = input.to_s.strip
|
||||
|
||||
# Return nil for blank input
|
||||
return nil if str.blank?
|
||||
|
||||
# Try to parse as plain number (already in seconds)
|
||||
if str.match?(/^\d+$/)
|
||||
return str.to_i
|
||||
end
|
||||
|
||||
# Try to parse with unit (e.g., "1h", "30m", "1M")
|
||||
# Allow optional space between number and unit
|
||||
# Case-sensitive to avoid confusion (1m = minute, 1M = month)
|
||||
match = str.match(/^(\d+)\s*([smhdwMy])$/)
|
||||
return nil unless match
|
||||
|
||||
number = match[1].to_i
|
||||
unit = match[2]
|
||||
|
||||
multiplier = UNITS[unit]
|
||||
return nil unless multiplier
|
||||
|
||||
number * multiplier
|
||||
end
|
||||
end
|
||||
@@ -76,6 +76,7 @@ class Application < ApplicationRecord
|
||||
user: "X-Remote-User",
|
||||
email: "X-Remote-Email",
|
||||
name: "X-Remote-Name",
|
||||
username: "X-Remote-Username",
|
||||
groups: "X-Remote-Groups",
|
||||
admin: "X-Remote-Admin"
|
||||
}.freeze
|
||||
@@ -195,6 +196,8 @@ class Application < ApplicationRecord
|
||||
headers[header_name] = user.email_address
|
||||
when :name
|
||||
headers[header_name] = user.name.presence || user.email_address
|
||||
when :username
|
||||
headers[header_name] = user.username if user.username.present?
|
||||
when :groups
|
||||
headers[header_name] = user.groups.pluck(:name).join(",") if user.groups.any?
|
||||
when :admin
|
||||
|
||||
@@ -330,10 +330,10 @@
|
||||
<p class="font-medium">Optional: Customize header names sent to your application.</p>
|
||||
<div class="flex items-center gap-2">
|
||||
<button type="button" data-action="json-validator#format" class="text-xs bg-gray-100 hover:bg-gray-200 px-2 py-1 rounded">Format JSON</button>
|
||||
<button type="button" data-action="json-validator#insertSample" data-json-sample='{"user": "Remote-User", "groups": "Remote-Groups", "email": "Remote-Email", "name": "Remote-Name", "admin": "Remote-Admin"}' class="text-xs bg-blue-100 hover:bg-blue-200 text-blue-700 px-2 py-1 rounded">Insert Example</button>
|
||||
<button type="button" data-action="json-validator#insertSample" data-json-sample='{"user": "Remote-User", "groups": "Remote-Groups", "email": "Remote-Email", "name": "Remote-Name", "username": "Remote-Username", "admin": "Remote-Admin"}' class="text-xs bg-blue-100 hover:bg-blue-200 text-blue-700 px-2 py-1 rounded">Insert Example</button>
|
||||
</div>
|
||||
</div>
|
||||
<p><strong>Default headers:</strong> X-Remote-User, X-Remote-Email, X-Remote-Name, X-Remote-Groups, X-Remote-Admin</p>
|
||||
<p><strong>Default headers:</strong> X-Remote-User, X-Remote-Email, X-Remote-Name, X-Remote-Username, X-Remote-Groups, X-Remote-Admin</p>
|
||||
<div data-json-validator-target="status" class="text-xs font-medium"></div>
|
||||
<details class="mt-2">
|
||||
<summary class="cursor-pointer text-blue-600 hover:text-blue-800">Show available header keys and what data they send</summary>
|
||||
@@ -341,9 +341,10 @@
|
||||
<p><code class="bg-gray-100 px-1 rounded">user</code> - User's email address</p>
|
||||
<p><code class="bg-gray-100 px-1 rounded">email</code> - User's email address</p>
|
||||
<p><code class="bg-gray-100 px-1 rounded">name</code> - User's display name (falls back to email if not set)</p>
|
||||
<p><code class="bg-gray-100 px-1 rounded">username</code> - User's login username (only sent if set)</p>
|
||||
<p><code class="bg-gray-100 px-1 rounded">groups</code> - Comma-separated list of group names (e.g., "admin,developers")</p>
|
||||
<p><code class="bg-gray-100 px-1 rounded">admin</code> - "true" or "false" indicating admin status</p>
|
||||
<p class="mt-2 italic">Example: <code class="bg-gray-100 px-1 rounded">{"user": "Remote-User", "groups": "Remote-Groups"}</code></p>
|
||||
<p class="mt-2 italic">Example: <code class="bg-gray-100 px-1 rounded">{"user": "Remote-User", "groups": "Remote-Groups", "username": "Remote-Username"}</code></p>
|
||||
<p class="italic">Need custom user fields? Add them to user's custom_claims for OIDC tokens</p>
|
||||
</div>
|
||||
</details>
|
||||
|
||||
@@ -215,7 +215,7 @@
|
||||
<code class="block bg-gray-100 px-3 py-2 rounded font-mono text-xs whitespace-pre-wrap"><%= JSON.pretty_generate(@application.headers_config) %></code>
|
||||
<% else %>
|
||||
<div class="bg-gray-100 px-3 py-2 rounded text-xs text-gray-500">
|
||||
Using default headers: X-Remote-User, X-Remote-Email, X-Remote-Name, X-Remote-Groups, X-Remote-Admin
|
||||
Using default headers: X-Remote-User, X-Remote-Email, X-Remote-Name, X-Remote-Username, X-Remote-Groups, X-Remote-Admin
|
||||
</div>
|
||||
<% end %>
|
||||
</dd>
|
||||
|
||||
@@ -59,6 +59,7 @@ Rails.application.configure do
|
||||
|
||||
# Use Solid Queue for background jobs
|
||||
config.active_job.queue_adapter = :solid_queue
|
||||
config.solid_queue.connects_to = {database: {writing: :queue}}
|
||||
|
||||
# Ignore bad email addresses and do not raise email delivery errors.
|
||||
# Set this to true and configure the email server for immediate delivery to raise delivery errors.
|
||||
|
||||
@@ -56,7 +56,8 @@ This checklist ensures Clinch meets security, quality, and documentation standar
|
||||
- [x] Authorization code flow with PKCE support
|
||||
- [x] Refresh token rotation
|
||||
- [x] Token family tracking (detects replay attacks)
|
||||
- [x] All tokens HMAC-SHA256 hashed in database
|
||||
- [x] All tokens and authorization codes HMAC-SHA256 hashed in database
|
||||
- [x] TOTP secrets AES-256-GCM encrypted at rest (Rails credentials)
|
||||
- [x] Configurable token expiry (access, refresh, ID)
|
||||
- [x] One-time use authorization codes
|
||||
- [x] Pairwise subject identifiers (privacy)
|
||||
@@ -130,8 +131,7 @@ This checklist ensures Clinch meets security, quality, and documentation standar
|
||||
|
||||
## Code Quality
|
||||
|
||||
- [x] **RuboCop** - Code style and linting
|
||||
- Configuration: Rails Omakase
|
||||
- [x] **StandardRB** - Code style and linting
|
||||
- CI: Runs on every PR and push to main
|
||||
|
||||
- [x] **Documentation** - Comprehensive README
|
||||
@@ -158,7 +158,7 @@ This checklist ensures Clinch meets security, quality, and documentation standar
|
||||
|
||||
### Performance
|
||||
- [ ] Review N+1 queries
|
||||
- [ ] Add database indexes where needed
|
||||
- [x] Add database indexes where needed
|
||||
- [ ] Test with realistic data volumes
|
||||
- [ ] Review token cleanup job performance
|
||||
|
||||
|
||||
394
test/controllers/oidc_claims_security_test.rb
Normal file
394
test/controllers/oidc_claims_security_test.rb
Normal file
@@ -0,0 +1,394 @@
|
||||
require "test_helper"
|
||||
|
||||
class OidcClaimsSecurityTest < ActionDispatch::IntegrationTest
|
||||
setup do
|
||||
@user = User.create!(email_address: "claims_security_test@example.com", password: "password123")
|
||||
@application = Application.create!(
|
||||
name: "Claims Security Test App",
|
||||
slug: "claims-security-test-app",
|
||||
app_type: "oidc",
|
||||
redirect_uris: ["http://localhost:4000/callback"].to_json,
|
||||
active: true,
|
||||
require_pkce: false
|
||||
)
|
||||
|
||||
# Store the plain text client secret for testing
|
||||
@application.generate_new_client_secret!
|
||||
@plain_client_secret = @application.client_secret
|
||||
@application.save!
|
||||
end
|
||||
|
||||
def teardown
|
||||
# Delete in correct order to avoid foreign key constraints
|
||||
OidcRefreshToken.where(application: @application).delete_all
|
||||
OidcAccessToken.where(application: @application).delete_all
|
||||
OidcAuthorizationCode.where(application: @application).delete_all
|
||||
OidcUserConsent.where(application: @application).delete_all
|
||||
@user.destroy
|
||||
@application.destroy
|
||||
end
|
||||
|
||||
# ====================
|
||||
# CLAIMS PARAMETER ESCALATION ATTACKS
|
||||
# ====================
|
||||
|
||||
test "rejects claims parameter during authorization code exchange" do
|
||||
# Create consent with minimal scopes (no profile, email, or admin access)
|
||||
OidcUserConsent.create!(
|
||||
user: @user,
|
||||
application: @application,
|
||||
scopes_granted: "openid",
|
||||
granted_at: Time.current,
|
||||
sid: "test-sid-123"
|
||||
)
|
||||
|
||||
auth_code = OidcAuthorizationCode.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
redirect_uri: "http://localhost:4000/callback",
|
||||
scope: "openid",
|
||||
expires_at: 10.minutes.from_now
|
||||
)
|
||||
|
||||
# ATTEMPT: Inject claims parameter during token exchange (ATTACK!)
|
||||
# The client is trying to request 'admin' claim that they never got consent for
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "authorization_code",
|
||||
code: auth_code.plaintext_code,
|
||||
redirect_uri: "http://localhost:4000/callback",
|
||||
claims: '{"id_token":{"admin":{"essential":true}}}' # ← ATTACK!
|
||||
}, headers: {
|
||||
"Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@plain_client_secret}")
|
||||
}
|
||||
|
||||
# SHOULD: Reject the claims parameter - it's only allowed in authorization requests
|
||||
assert_response :bad_request
|
||||
error = JSON.parse(response.body)
|
||||
assert_equal "invalid_request", error["error"], "Should reject claims parameter at token endpoint"
|
||||
assert_match(/claims.*not allowed|unsupported parameter/i, error["error_description"], "Error should mention claims parameter not allowed")
|
||||
end
|
||||
|
||||
test "rejects claims parameter during authorization code exchange with profile escalation" do
|
||||
# Create consent with ONLY openid scope (no profile scope)
|
||||
OidcUserConsent.create!(
|
||||
user: @user,
|
||||
application: @application,
|
||||
scopes_granted: "openid",
|
||||
granted_at: Time.current,
|
||||
sid: "test-sid-123"
|
||||
)
|
||||
|
||||
auth_code = OidcAuthorizationCode.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
redirect_uri: "http://localhost:4000/callback",
|
||||
scope: "openid",
|
||||
expires_at: 10.minutes.from_now
|
||||
)
|
||||
|
||||
# ATTEMPT: Try to get profile claims via claims parameter without profile scope
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "authorization_code",
|
||||
code: auth_code.plaintext_code,
|
||||
redirect_uri: "http://localhost:4000/callback",
|
||||
claims: '{"id_token":{"name":null,"email":{"essential":true}}}'
|
||||
}, headers: {
|
||||
"Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@plain_client_secret}")
|
||||
}
|
||||
|
||||
# SHOULD: Reject the claims parameter
|
||||
assert_response :bad_request
|
||||
error = JSON.parse(response.body)
|
||||
assert_equal "invalid_request", error["error"]
|
||||
end
|
||||
|
||||
test "rejects claims parameter during refresh token grant" do
|
||||
access_token = OidcAccessToken.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
scope: "openid"
|
||||
)
|
||||
|
||||
refresh_token = OidcRefreshToken.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
oidc_access_token: access_token,
|
||||
scope: "openid"
|
||||
)
|
||||
|
||||
plaintext_refresh_token = refresh_token.token
|
||||
|
||||
# ATTEMPT: Inject claims parameter during refresh (ATTACK!)
|
||||
# Trying to escalate to admin claims during refresh
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "refresh_token",
|
||||
refresh_token: plaintext_refresh_token,
|
||||
claims: '{"id_token":{"admin":true,"role":{"essential":true}}}' # ← ATTACK!
|
||||
}, headers: {
|
||||
"Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@plain_client_secret}")
|
||||
}
|
||||
|
||||
# SHOULD: Reject the claims parameter
|
||||
assert_response :bad_request
|
||||
error = JSON.parse(response.body)
|
||||
assert_equal "invalid_request", error["error"], "Should reject claims parameter at refresh token endpoint"
|
||||
assert_match(/claims.*not allowed|unsupported parameter/i, error["error_description"])
|
||||
end
|
||||
|
||||
test "rejects claims parameter during refresh token grant with custom claims escalation" do
|
||||
# Setup: User has a custom claim at user level
|
||||
@user.update!(custom_claims: {"role" => "user"})
|
||||
|
||||
access_token = OidcAccessToken.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
scope: "openid"
|
||||
)
|
||||
|
||||
refresh_token = OidcRefreshToken.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
oidc_access_token: access_token,
|
||||
scope: "openid"
|
||||
)
|
||||
|
||||
plaintext_refresh_token = refresh_token.token
|
||||
|
||||
# ATTEMPT: Try to escalate role to admin via claims parameter
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "refresh_token",
|
||||
refresh_token: plaintext_refresh_token,
|
||||
claims: '{"id_token":{"role":{"value":"admin"}}}' # ← ATTACK! Trying to override role value
|
||||
}, headers: {
|
||||
"Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@plain_client_secret}")
|
||||
}
|
||||
|
||||
# SHOULD: Reject the claims parameter
|
||||
assert_response :bad_request
|
||||
error = JSON.parse(response.body)
|
||||
assert_equal "invalid_request", error["error"]
|
||||
end
|
||||
|
||||
test "allows token exchange without claims parameter" do
|
||||
# Create consent
|
||||
OidcUserConsent.create!(
|
||||
user: @user,
|
||||
application: @application,
|
||||
scopes_granted: "openid profile",
|
||||
granted_at: Time.current,
|
||||
sid: "test-sid-123"
|
||||
)
|
||||
|
||||
auth_code = OidcAuthorizationCode.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
redirect_uri: "http://localhost:4000/callback",
|
||||
scope: "openid profile",
|
||||
expires_at: 10.minutes.from_now
|
||||
)
|
||||
|
||||
# Normal token exchange WITHOUT claims parameter should work fine
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "authorization_code",
|
||||
code: auth_code.plaintext_code,
|
||||
redirect_uri: "http://localhost:4000/callback"
|
||||
}, headers: {
|
||||
"Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@plain_client_secret}")
|
||||
}
|
||||
|
||||
assert_response :success
|
||||
response_body = JSON.parse(response.body)
|
||||
assert response_body.key?("access_token")
|
||||
assert response_body.key?("id_token")
|
||||
end
|
||||
|
||||
test "allows refresh without claims parameter" do
|
||||
# Create consent for this application
|
||||
OidcUserConsent.create!(
|
||||
user: @user,
|
||||
application: @application,
|
||||
scopes_granted: "openid profile",
|
||||
granted_at: Time.current,
|
||||
sid: "test-sid-refresh-456"
|
||||
)
|
||||
|
||||
access_token = OidcAccessToken.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
scope: "openid profile"
|
||||
)
|
||||
|
||||
refresh_token = OidcRefreshToken.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
oidc_access_token: access_token,
|
||||
scope: "openid profile"
|
||||
)
|
||||
|
||||
plaintext_refresh_token = refresh_token.token
|
||||
|
||||
# Normal refresh WITHOUT claims parameter should work fine
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "refresh_token",
|
||||
refresh_token: plaintext_refresh_token
|
||||
}, headers: {
|
||||
"Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@plain_client_secret}")
|
||||
}
|
||||
|
||||
assert_response :success
|
||||
response_body = JSON.parse(response.body)
|
||||
assert response_body.key?("access_token")
|
||||
assert response_body.key?("id_token")
|
||||
end
|
||||
|
||||
# ====================
|
||||
# CLAIMS PARAMETER IS AUTHORIZATION-ONLY
|
||||
# ====================
|
||||
|
||||
test "claims parameter is only valid in authorization request per OIDC spec" do
|
||||
# Per OIDC Core spec section 18.2.1, claims parameter usage location is "Authorization Request"
|
||||
# This test verifies that claims parameter cannot be used at token endpoint
|
||||
|
||||
OidcUserConsent.create!(
|
||||
user: @user,
|
||||
application: @application,
|
||||
scopes_granted: "openid",
|
||||
granted_at: Time.current,
|
||||
sid: "test-sid-123"
|
||||
)
|
||||
|
||||
auth_code = OidcAuthorizationCode.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
redirect_uri: "http://localhost:4000/callback",
|
||||
scope: "openid",
|
||||
expires_at: 10.minutes.from_now
|
||||
)
|
||||
|
||||
# Test various attempts to inject claims parameter
|
||||
malicious_claims = [
|
||||
'{"id_token":{"admin":true}}',
|
||||
'{"id_token":{"email":{"essential":true}}}',
|
||||
'{"userinfo":{"groups":{"values":["admin"]}}}',
|
||||
'{"id_token":{"custom_claim":"custom_value"}}',
|
||||
"invalid-json"
|
||||
]
|
||||
|
||||
malicious_claims.each do |claims_value|
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "authorization_code",
|
||||
code: auth_code.plaintext_code,
|
||||
redirect_uri: "http://localhost:4000/callback",
|
||||
claims: claims_value
|
||||
}, headers: {
|
||||
"Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@plain_client_secret}")
|
||||
}
|
||||
|
||||
# All should be rejected
|
||||
assert_response :bad_request, "Claims parameter '#{claims_value}' should be rejected"
|
||||
error = JSON.parse(response.body)
|
||||
assert_equal "invalid_request", error["error"]
|
||||
end
|
||||
end
|
||||
|
||||
# ====================
|
||||
# VERIFY CONSENT-BASED ACCESS IS ENFORCED
|
||||
# ====================
|
||||
|
||||
test "token endpoint respects scopes granted during authorization" do
|
||||
# Create consent with ONLY openid scope (no email, profile, etc.)
|
||||
OidcUserConsent.create!(
|
||||
user: @user,
|
||||
application: @application,
|
||||
scopes_granted: "openid",
|
||||
granted_at: Time.current,
|
||||
sid: "test-sid-123"
|
||||
)
|
||||
|
||||
auth_code = OidcAuthorizationCode.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
redirect_uri: "http://localhost:4000/callback",
|
||||
scope: "openid",
|
||||
expires_at: 10.minutes.from_now
|
||||
)
|
||||
|
||||
# Exchange code for tokens
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "authorization_code",
|
||||
code: auth_code.plaintext_code,
|
||||
redirect_uri: "http://localhost:4000/callback"
|
||||
}, headers: {
|
||||
"Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@plain_client_secret}")
|
||||
}
|
||||
|
||||
assert_response :success
|
||||
response_body = JSON.parse(response.body)
|
||||
id_token = response_body["id_token"]
|
||||
|
||||
# Decode ID token to check claims
|
||||
decoded = JWT.decode(id_token, nil, false).first
|
||||
|
||||
# Should only have required claims, not email/profile
|
||||
assert_includes decoded.keys, "iss"
|
||||
assert_includes decoded.keys, "sub"
|
||||
assert_includes decoded.keys, "aud"
|
||||
assert_includes decoded.keys, "exp"
|
||||
assert_includes decoded.keys, "iat"
|
||||
|
||||
# Should NOT have claims that weren't consented to
|
||||
refute_includes decoded.keys, "email", "Should not include email without email scope"
|
||||
refute_includes decoded.keys, "email_verified", "Should not include email_verified without email scope"
|
||||
refute_includes decoded.keys, "name", "Should not include name without profile scope"
|
||||
refute_includes decoded.keys, "preferred_username", "Should not include preferred_username without profile scope"
|
||||
end
|
||||
|
||||
test "refresh token preserves original scopes granted during authorization" do
|
||||
# Create consent with specific scopes
|
||||
OidcUserConsent.create!(
|
||||
user: @user,
|
||||
application: @application,
|
||||
scopes_granted: "openid email",
|
||||
granted_at: Time.current,
|
||||
sid: "test-sid-refresh-123"
|
||||
)
|
||||
|
||||
access_token = OidcAccessToken.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
scope: "openid email"
|
||||
)
|
||||
|
||||
refresh_token = OidcRefreshToken.create!(
|
||||
application: @application,
|
||||
user: @user,
|
||||
oidc_access_token: access_token,
|
||||
scope: "openid email"
|
||||
)
|
||||
|
||||
plaintext_refresh_token = refresh_token.token
|
||||
|
||||
# Refresh the token
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "refresh_token",
|
||||
refresh_token: plaintext_refresh_token
|
||||
}, headers: {
|
||||
"Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@plain_client_secret}")
|
||||
}
|
||||
|
||||
assert_response :success
|
||||
response_body = JSON.parse(response.body)
|
||||
id_token = response_body["id_token"]
|
||||
|
||||
# Decode ID token to verify scopes are preserved
|
||||
decoded = JWT.decode(id_token, nil, false).first
|
||||
|
||||
# Should have email claims (from original consent)
|
||||
assert_includes decoded.keys, "email", "Should preserve email scope from original consent"
|
||||
assert_includes decoded.keys, "email_verified", "Should preserve email_verified scope from original consent"
|
||||
|
||||
# Should NOT have profile claims (not in original consent)
|
||||
refute_includes decoded.keys, "name", "Should not add profile claims that weren't consented to"
|
||||
refute_includes decoded.keys, "preferred_username", "Should not add preferred_username that wasn't consented to"
|
||||
end
|
||||
end
|
||||
236
test/controllers/oidc_prompt_login_test.rb
Normal file
236
test/controllers/oidc_prompt_login_test.rb
Normal file
@@ -0,0 +1,236 @@
|
||||
require "test_helper"
|
||||
|
||||
class OidcPromptLoginTest < ActionDispatch::IntegrationTest
|
||||
setup do
|
||||
@user = users(:alice)
|
||||
@application = applications(:kavita_app)
|
||||
@client_secret = SecureRandom.urlsafe_base64(48)
|
||||
@application.client_secret = @client_secret
|
||||
@application.save!
|
||||
|
||||
# Pre-authorize the application so we skip consent screen
|
||||
consent = OidcUserConsent.find_or_initialize_by(
|
||||
user: @user,
|
||||
application: @application
|
||||
)
|
||||
consent.scopes_granted ||= "openid profile email"
|
||||
consent.save!
|
||||
end
|
||||
|
||||
teardown do
|
||||
# Clean up
|
||||
OidcAccessToken.where(user: @user, application: @application).destroy_all
|
||||
OidcAuthorizationCode.where(user: @user, application: @application).destroy_all
|
||||
end
|
||||
|
||||
test "max_age requires re-authentication when session is too old" do
|
||||
# Sign in to create a session
|
||||
post "/signin", params: {
|
||||
email_address: @user.email_address,
|
||||
password: "password"
|
||||
}
|
||||
|
||||
assert_response :redirect
|
||||
follow_redirect!
|
||||
assert_response :success
|
||||
|
||||
# Get first auth_time
|
||||
get "/oauth/authorize", params: {
|
||||
client_id: @application.client_id,
|
||||
redirect_uri: @application.parsed_redirect_uris.first,
|
||||
response_type: "code",
|
||||
scope: "openid",
|
||||
state: "first-state",
|
||||
nonce: "first-nonce"
|
||||
}
|
||||
|
||||
assert_response :redirect
|
||||
first_redirect_url = response.location
|
||||
first_code = CGI.parse(URI(first_redirect_url).query)["code"].first
|
||||
|
||||
# Exchange for tokens and extract auth_time
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "authorization_code",
|
||||
code: first_code,
|
||||
redirect_uri: @application.parsed_redirect_uris.first,
|
||||
client_id: @application.client_id,
|
||||
client_secret: @client_secret
|
||||
}
|
||||
|
||||
assert_response :success
|
||||
first_tokens = JSON.parse(response.body)
|
||||
first_id_token = OidcJwtService.decode_id_token(first_tokens["id_token"])
|
||||
first_auth_time = first_id_token[0]["auth_time"]
|
||||
|
||||
# Wait a bit (simulate time passing - in real scenario this would be actual seconds)
|
||||
# Then request with max_age=0 (means session must be brand new)
|
||||
get "/oauth/authorize", params: {
|
||||
client_id: @application.client_id,
|
||||
redirect_uri: @application.parsed_redirect_uris.first,
|
||||
response_type: "code",
|
||||
scope: "openid",
|
||||
state: "second-state",
|
||||
nonce: "second-nonce",
|
||||
max_age: "0" # Requires session to be 0 seconds old (i.e., brand new)
|
||||
}
|
||||
|
||||
# Should redirect to sign in because session is too old
|
||||
assert_response :redirect
|
||||
assert_redirected_to(/signin/)
|
||||
|
||||
# Sign in again
|
||||
post "/signin", params: {
|
||||
email_address: @user.email_address,
|
||||
password: "password"
|
||||
}
|
||||
|
||||
assert_response :redirect
|
||||
follow_redirect!
|
||||
|
||||
# Should receive authorization code
|
||||
assert_response :redirect
|
||||
second_redirect_url = response.location
|
||||
second_code = CGI.parse(URI(second_redirect_url).query)["code"].first
|
||||
|
||||
assert second_code.present?, "Should receive authorization code after re-authentication"
|
||||
|
||||
# Exchange second authorization code for tokens
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "authorization_code",
|
||||
code: second_code,
|
||||
redirect_uri: @application.parsed_redirect_uris.first,
|
||||
client_id: @application.client_id,
|
||||
client_secret: @client_secret
|
||||
}
|
||||
|
||||
assert_response :success
|
||||
second_tokens = JSON.parse(response.body)
|
||||
second_id_token = OidcJwtService.decode_id_token(second_tokens["id_token"])
|
||||
second_auth_time = second_id_token[0]["auth_time"]
|
||||
|
||||
# The second auth_time should be >= the first (re-authentication occurred)
|
||||
# Note: May be equal if both occur in the same second (test timing edge case)
|
||||
assert second_auth_time >= first_auth_time,
|
||||
"max_age=0 should result in a re-authentication. " \
|
||||
"First: #{first_auth_time}, Second: #{second_auth_time}"
|
||||
end
|
||||
|
||||
test "prompt=none returns login_required error when not authenticated" do
|
||||
# Don't sign in - user is not authenticated
|
||||
|
||||
# Request authorization with prompt=none
|
||||
get "/oauth/authorize", params: {
|
||||
client_id: @application.client_id,
|
||||
redirect_uri: @application.parsed_redirect_uris.first,
|
||||
response_type: "code",
|
||||
scope: "openid",
|
||||
state: "test-state",
|
||||
prompt: "none"
|
||||
}
|
||||
|
||||
# Should redirect with error=login_required (NOT to sign-in page)
|
||||
assert_response :redirect
|
||||
redirect_url = response.location
|
||||
|
||||
# Parse the redirect URL
|
||||
uri = URI.parse(redirect_url)
|
||||
query_params = uri.query ? CGI.parse(uri.query) : {}
|
||||
|
||||
assert_equal "login_required", query_params["error"]&.first,
|
||||
"Should return login_required error for prompt=none when not authenticated"
|
||||
assert_equal "test-state", query_params["state"]&.first,
|
||||
"Should return state parameter"
|
||||
end
|
||||
|
||||
test "prompt=login forces re-authentication with new auth_time" do
|
||||
# First authentication
|
||||
post "/signin", params: {
|
||||
email_address: @user.email_address,
|
||||
password: "password"
|
||||
}
|
||||
|
||||
assert_response :redirect
|
||||
follow_redirect!
|
||||
assert_response :success
|
||||
|
||||
# Get first authorization code
|
||||
get "/oauth/authorize", params: {
|
||||
client_id: @application.client_id,
|
||||
redirect_uri: @application.parsed_redirect_uris.first,
|
||||
response_type: "code",
|
||||
scope: "openid",
|
||||
state: "first-state",
|
||||
nonce: "first-nonce"
|
||||
}
|
||||
|
||||
assert_response :redirect
|
||||
first_redirect_url = response.location
|
||||
first_code = CGI.parse(URI(first_redirect_url).query)["code"].first
|
||||
|
||||
# Exchange for tokens and extract auth_time from ID token
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "authorization_code",
|
||||
code: first_code,
|
||||
redirect_uri: @application.parsed_redirect_uris.first,
|
||||
client_id: @application.client_id,
|
||||
client_secret: @client_secret
|
||||
}
|
||||
|
||||
assert_response :success
|
||||
first_tokens = JSON.parse(response.body)
|
||||
first_id_token = OidcJwtService.decode_id_token(first_tokens["id_token"])
|
||||
first_auth_time = first_id_token[0]["auth_time"]
|
||||
|
||||
# Now request authorization again with prompt=login
|
||||
get "/oauth/authorize", params: {
|
||||
client_id: @application.client_id,
|
||||
redirect_uri: @application.parsed_redirect_uris.first,
|
||||
response_type: "code",
|
||||
scope: "openid",
|
||||
state: "second-state",
|
||||
nonce: "second-nonce",
|
||||
prompt: "login"
|
||||
}
|
||||
|
||||
# Should redirect to sign in
|
||||
assert_response :redirect
|
||||
assert_redirected_to(/signin/)
|
||||
|
||||
# Sign in again (simulating user re-authentication)
|
||||
post "/signin", params: {
|
||||
email_address: @user.email_address,
|
||||
password: "password"
|
||||
}
|
||||
|
||||
assert_response :redirect
|
||||
# Follow redirect to after_authentication_url (which is /oauth/authorize without prompt=login)
|
||||
follow_redirect!
|
||||
|
||||
# Should receive authorization code redirect
|
||||
assert_response :redirect
|
||||
second_redirect_url = response.location
|
||||
second_code = CGI.parse(URI(second_redirect_url).query)["code"].first
|
||||
|
||||
assert second_code.present?, "Should receive authorization code after re-authentication"
|
||||
|
||||
# Exchange second authorization code for tokens
|
||||
post "/oauth/token", params: {
|
||||
grant_type: "authorization_code",
|
||||
code: second_code,
|
||||
redirect_uri: @application.parsed_redirect_uris.first,
|
||||
client_id: @application.client_id,
|
||||
client_secret: @client_secret
|
||||
}
|
||||
|
||||
assert_response :success
|
||||
second_tokens = JSON.parse(response.body)
|
||||
second_id_token = OidcJwtService.decode_id_token(second_tokens["id_token"])
|
||||
second_auth_time = second_id_token[0]["auth_time"]
|
||||
|
||||
# The second auth_time should be >= the first (re-authentication occurred)
|
||||
# Note: May be equal if both occur in the same second (test timing edge case)
|
||||
assert second_auth_time >= first_auth_time,
|
||||
"prompt=login should result in a later auth_time. " \
|
||||
"First: #{first_auth_time}, Second: #{second_auth_time}"
|
||||
end
|
||||
end
|
||||
136
test/lib/duration_parser_test.rb
Normal file
136
test/lib/duration_parser_test.rb
Normal file
@@ -0,0 +1,136 @@
|
||||
require "test_helper"
|
||||
|
||||
class DurationParserTest < ActiveSupport::TestCase
|
||||
# Valid formats
|
||||
test "parses seconds" do
|
||||
assert_equal 1, DurationParser.parse("1s")
|
||||
assert_equal 30, DurationParser.parse("30s")
|
||||
assert_equal 3600, DurationParser.parse("3600s")
|
||||
end
|
||||
|
||||
test "parses minutes" do
|
||||
assert_equal 60, DurationParser.parse("1m")
|
||||
assert_equal 300, DurationParser.parse("5m")
|
||||
assert_equal 1800, DurationParser.parse("30m")
|
||||
end
|
||||
|
||||
test "parses hours" do
|
||||
assert_equal 3600, DurationParser.parse("1h")
|
||||
assert_equal 7200, DurationParser.parse("2h")
|
||||
assert_equal 86400, DurationParser.parse("24h")
|
||||
end
|
||||
|
||||
test "parses days" do
|
||||
assert_equal 86400, DurationParser.parse("1d")
|
||||
assert_equal 172800, DurationParser.parse("2d")
|
||||
assert_equal 2592000, DurationParser.parse("30d")
|
||||
end
|
||||
|
||||
test "parses weeks" do
|
||||
assert_equal 604800, DurationParser.parse("1w")
|
||||
assert_equal 1209600, DurationParser.parse("2w")
|
||||
end
|
||||
|
||||
test "parses months (30 days)" do
|
||||
assert_equal 2592000, DurationParser.parse("1M")
|
||||
assert_equal 5184000, DurationParser.parse("2M")
|
||||
end
|
||||
|
||||
test "parses years (365 days)" do
|
||||
assert_equal 31536000, DurationParser.parse("1y")
|
||||
assert_equal 63072000, DurationParser.parse("2y")
|
||||
end
|
||||
|
||||
# Plain numbers
|
||||
test "parses plain integer as seconds" do
|
||||
assert_equal 3600, DurationParser.parse(3600)
|
||||
assert_equal 300, DurationParser.parse(300)
|
||||
assert_equal 0, DurationParser.parse(0)
|
||||
end
|
||||
|
||||
test "parses plain numeric string as seconds" do
|
||||
assert_equal 3600, DurationParser.parse("3600")
|
||||
assert_equal 300, DurationParser.parse("300")
|
||||
assert_equal 0, DurationParser.parse("0")
|
||||
end
|
||||
|
||||
# Whitespace handling
|
||||
test "handles leading and trailing whitespace" do
|
||||
assert_equal 3600, DurationParser.parse(" 1h ")
|
||||
assert_equal 300, DurationParser.parse(" 5m ")
|
||||
assert_equal 86400, DurationParser.parse("\t1d\n")
|
||||
end
|
||||
|
||||
test "handles space between number and unit" do
|
||||
assert_equal 3600, DurationParser.parse("1 h")
|
||||
assert_equal 300, DurationParser.parse("5 m")
|
||||
assert_equal 86400, DurationParser.parse("1 d")
|
||||
end
|
||||
|
||||
# Case sensitivity - only lowercase units work (except M for months)
|
||||
test "lowercase units work" do
|
||||
assert_equal 1, DurationParser.parse("1s")
|
||||
assert_equal 60, DurationParser.parse("1m") # minute (lowercase)
|
||||
assert_equal 3600, DurationParser.parse("1h")
|
||||
assert_equal 86400, DurationParser.parse("1d")
|
||||
assert_equal 604800, DurationParser.parse("1w")
|
||||
assert_equal 31536000, DurationParser.parse("1y")
|
||||
end
|
||||
|
||||
test "uppercase M for months works" do
|
||||
assert_equal 2592000, DurationParser.parse("1M") # month (uppercase)
|
||||
end
|
||||
|
||||
test "returns nil for wrong case" do
|
||||
assert_nil DurationParser.parse("1S") # Should be 1s
|
||||
assert_nil DurationParser.parse("1H") # Should be 1h
|
||||
assert_nil DurationParser.parse("1D") # Should be 1d
|
||||
assert_nil DurationParser.parse("1W") # Should be 1w
|
||||
assert_nil DurationParser.parse("1Y") # Should be 1y
|
||||
end
|
||||
|
||||
# Edge cases
|
||||
test "handles zero duration" do
|
||||
assert_equal 0, DurationParser.parse("0s")
|
||||
assert_equal 0, DurationParser.parse("0m")
|
||||
assert_equal 0, DurationParser.parse("0h")
|
||||
end
|
||||
|
||||
test "handles large numbers" do
|
||||
assert_equal 86400000, DurationParser.parse("1000d")
|
||||
assert_equal 360000, DurationParser.parse("100h")
|
||||
end
|
||||
|
||||
# Invalid formats - should return nil (not raise)
|
||||
test "returns nil for invalid format" do
|
||||
assert_nil DurationParser.parse("invalid")
|
||||
assert_nil DurationParser.parse("1x")
|
||||
assert_nil DurationParser.parse("abc")
|
||||
assert_nil DurationParser.parse("1.5h") # No decimals
|
||||
assert_nil DurationParser.parse("-1h") # No negatives
|
||||
assert_nil DurationParser.parse("h1") # Wrong order
|
||||
end
|
||||
|
||||
test "returns nil for blank input" do
|
||||
assert_nil DurationParser.parse("")
|
||||
assert_nil DurationParser.parse(nil)
|
||||
assert_nil DurationParser.parse(" ")
|
||||
end
|
||||
|
||||
test "returns nil for multiple units" do
|
||||
assert_nil DurationParser.parse("1h30m") # Keep it simple, don't support this
|
||||
assert_nil DurationParser.parse("1d2h")
|
||||
end
|
||||
|
||||
# String coercion
|
||||
test "handles string input" do
|
||||
assert_equal 3600, DurationParser.parse("1h")
|
||||
assert_equal 3600, DurationParser.parse(:"1h") # Symbol
|
||||
end
|
||||
|
||||
# Boundary validation (not parser's job, but good to know)
|
||||
test "parses values outside typical TTL ranges without error" do
|
||||
assert_equal 1, DurationParser.parse("1s") # Below min access_token_ttl
|
||||
assert_equal 315360000, DurationParser.parse("10y") # Above max refresh_token_ttl
|
||||
end
|
||||
end
|
||||
109
test/models/application_duration_parser_test.rb
Normal file
109
test/models/application_duration_parser_test.rb
Normal file
@@ -0,0 +1,109 @@
|
||||
require "test_helper"
|
||||
|
||||
class ApplicationDurationParserTest < ActiveSupport::TestCase
|
||||
test "access_token_ttl accepts human-friendly durations" do
|
||||
app = Application.new(access_token_ttl: "1h")
|
||||
assert_equal 3600, app.access_token_ttl
|
||||
|
||||
app.access_token_ttl = "30m"
|
||||
assert_equal 1800, app.access_token_ttl
|
||||
|
||||
app.access_token_ttl = "5m"
|
||||
assert_equal 300, app.access_token_ttl
|
||||
end
|
||||
|
||||
test "refresh_token_ttl accepts human-friendly durations" do
|
||||
app = Application.new(refresh_token_ttl: "30d")
|
||||
assert_equal 2592000, app.refresh_token_ttl
|
||||
|
||||
app.refresh_token_ttl = "1M"
|
||||
assert_equal 2592000, app.refresh_token_ttl
|
||||
|
||||
app.refresh_token_ttl = "7d"
|
||||
assert_equal 604800, app.refresh_token_ttl
|
||||
end
|
||||
|
||||
test "id_token_ttl accepts human-friendly durations" do
|
||||
app = Application.new(id_token_ttl: "1h")
|
||||
assert_equal 3600, app.id_token_ttl
|
||||
|
||||
app.id_token_ttl = "2h"
|
||||
assert_equal 7200, app.id_token_ttl
|
||||
end
|
||||
|
||||
test "TTL fields still accept plain numbers" do
|
||||
app = Application.new(
|
||||
access_token_ttl: 3600,
|
||||
refresh_token_ttl: 2592000,
|
||||
id_token_ttl: 3600
|
||||
)
|
||||
|
||||
assert_equal 3600, app.access_token_ttl
|
||||
assert_equal 2592000, app.refresh_token_ttl
|
||||
assert_equal 3600, app.id_token_ttl
|
||||
end
|
||||
|
||||
test "TTL fields accept plain number strings" do
|
||||
app = Application.new(
|
||||
access_token_ttl: "3600",
|
||||
refresh_token_ttl: "2592000",
|
||||
id_token_ttl: "3600"
|
||||
)
|
||||
|
||||
assert_equal 3600, app.access_token_ttl
|
||||
assert_equal 2592000, app.refresh_token_ttl
|
||||
assert_equal 3600, app.id_token_ttl
|
||||
end
|
||||
|
||||
test "invalid TTL values are set to nil" do
|
||||
app = Application.new(
|
||||
access_token_ttl: "invalid",
|
||||
refresh_token_ttl: "bad",
|
||||
id_token_ttl: "nope"
|
||||
)
|
||||
|
||||
assert_nil app.access_token_ttl
|
||||
assert_nil app.refresh_token_ttl
|
||||
assert_nil app.id_token_ttl
|
||||
end
|
||||
|
||||
test "validation still works with parsed values" do
|
||||
app = Application.new(
|
||||
name: "Test",
|
||||
slug: "test",
|
||||
app_type: "oidc",
|
||||
redirect_uris: "https://example.com/callback"
|
||||
)
|
||||
|
||||
# Too short (below 5 minutes)
|
||||
app.access_token_ttl = "1m"
|
||||
assert_not app.valid?
|
||||
assert_includes app.errors[:access_token_ttl], "must be greater than or equal to 300"
|
||||
|
||||
# Too long (above 24 hours for access token)
|
||||
app.access_token_ttl = "2d"
|
||||
assert_not app.valid?
|
||||
assert_includes app.errors[:access_token_ttl], "must be less than or equal to 86400"
|
||||
|
||||
# Just right
|
||||
app.access_token_ttl = "1h"
|
||||
app.valid? # Revalidate
|
||||
assert app.errors[:access_token_ttl].blank?
|
||||
end
|
||||
|
||||
test "can create OIDC app with human-friendly TTL values" do
|
||||
app = Application.create!(
|
||||
name: "Test App",
|
||||
slug: "test-app",
|
||||
app_type: "oidc",
|
||||
redirect_uris: "https://example.com/callback",
|
||||
access_token_ttl: "1h",
|
||||
refresh_token_ttl: "30d",
|
||||
id_token_ttl: "2h"
|
||||
)
|
||||
|
||||
assert_equal 3600, app.access_token_ttl
|
||||
assert_equal 2592000, app.refresh_token_ttl
|
||||
assert_equal 7200, app.id_token_ttl
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user