2 Commits

Author SHA1 Message Date
Dan Milne
abbb11a41d Return only scopes requested, add tests ( OpenID conformance test )
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
2026-01-02 14:55:06 +11:00
Dan Milne
b2030df8c2 Return only scopes requested ( OpenID conformance test. Update README 2026-01-02 14:05:54 +11:00
6 changed files with 196 additions and 38 deletions

View File

@@ -347,27 +347,39 @@ services:
Create a `.env` file in the same directory: Create a `.env` file in the same directory:
```bash **Generate required secrets first:**
# Generate with: openssl rand -hex 64
SECRET_KEY_BASE=your-secret-key-here
# Application URLs ```bash
# Generate SECRET_KEY_BASE (required)
openssl rand -hex 64
# Generate OIDC private key (optional - auto-generated if not provided)
openssl genpkey -algorithm RSA -out private_key.pem -pkeyopt rsa_keygen_bits:2048
cat private_key.pem # Copy the output into OIDC_PRIVATE_KEY below
```
**Then create `.env`:**
```bash
# Rails Secret (REQUIRED)
SECRET_KEY_BASE=paste-output-from-openssl-rand-hex-64-here
# Application URLs (REQUIRED)
CLINCH_HOST=https://auth.yourdomain.com CLINCH_HOST=https://auth.yourdomain.com
CLINCH_FROM_EMAIL=noreply@yourdomain.com CLINCH_FROM_EMAIL=noreply@yourdomain.com
# SMTP Settings # SMTP Settings (REQUIRED for invitations and password resets)
SMTP_ADDRESS=smtp.example.com SMTP_ADDRESS=smtp.example.com
SMTP_PORT=587 SMTP_PORT=587
SMTP_DOMAIN=yourdomain.com SMTP_DOMAIN=yourdomain.com
SMTP_USERNAME=your-smtp-username SMTP_USERNAME=your-smtp-username
SMTP_PASSWORD=your-smtp-password SMTP_PASSWORD=your-smtp-password
# OIDC (optional - generates temporary key if not set) # OIDC Private Key (OPTIONAL - generates temporary key if not provided)
# Generate with: openssl genpkey -algorithm RSA -out private_key.pem -pkeyopt rsa_keygen_bits:2048 # For production, generate a persistent key and paste the ENTIRE contents here
# Then: OIDC_PRIVATE_KEY=$(cat private_key.pem)
OIDC_PRIVATE_KEY= OIDC_PRIVATE_KEY=
# Optional: Force SSL redirects (if not behind a reverse proxy handling SSL) # Optional: Force SSL redirects (only if NOT behind a reverse proxy handling SSL)
FORCE_SSL=false FORCE_SSL=false
``` ```

View File

@@ -419,6 +419,7 @@ class OidcController < ApplicationController
# Generate ID token (JWT) with pairwise SID, at_hash, auth_time, and acr # Generate ID token (JWT) with pairwise SID, at_hash, auth_time, and acr
# auth_time and acr come from the authorization code (captured at /authorize time) # auth_time and acr come from the authorization code (captured at /authorize time)
# scopes determine which claims are included (per OIDC Core spec)
id_token = OidcJwtService.generate_id_token( id_token = OidcJwtService.generate_id_token(
user, user,
application, application,
@@ -426,7 +427,8 @@ class OidcController < ApplicationController
nonce: auth_code.nonce, nonce: auth_code.nonce,
access_token: access_token_record.plaintext_token, access_token: access_token_record.plaintext_token,
auth_time: auth_code.auth_time, auth_time: auth_code.auth_time,
acr: auth_code.acr acr: auth_code.acr,
scopes: auth_code.scope
) )
# Return tokens # Return tokens
@@ -547,13 +549,15 @@ class OidcController < ApplicationController
# Generate new ID token (JWT with pairwise SID, at_hash, auth_time, acr; no nonce for refresh grants) # Generate new ID token (JWT with pairwise SID, at_hash, auth_time, acr; no nonce for refresh grants)
# auth_time and acr come from the original refresh token (carried over from initial auth) # auth_time and acr come from the original refresh token (carried over from initial auth)
# scopes determine which claims are included (per OIDC Core spec)
id_token = OidcJwtService.generate_id_token( id_token = OidcJwtService.generate_id_token(
user, user,
application, application,
consent: consent, consent: consent,
access_token: new_access_token.plaintext_token, access_token: new_access_token.plaintext_token,
auth_time: refresh_token_record.auth_time, auth_time: refresh_token_record.auth_time,
acr: refresh_token_record.acr acr: refresh_token_record.acr,
scopes: refresh_token_record.scope
) )
# Return new tokens # Return new tokens

View File

@@ -3,7 +3,7 @@ class OidcJwtService
class << self class << self
# Generate an ID token (JWT) for the user # Generate an ID token (JWT) for the user
def generate_id_token(user, application, consent: nil, nonce: nil, access_token: nil, auth_time: nil, acr: nil) def generate_id_token(user, application, consent: nil, nonce: nil, access_token: nil, auth_time: nil, acr: nil, scopes: "openid")
now = Time.current.to_i now = Time.current.to_i
# Use application's configured ID token TTL (defaults to 1 hour) # Use application's configured ID token TTL (defaults to 1 hour)
ttl = application.id_token_expiry_seconds ttl = application.id_token_expiry_seconds
@@ -11,18 +11,30 @@ class OidcJwtService
# Use pairwise SID from consent if available, fallback to user ID # Use pairwise SID from consent if available, fallback to user ID
subject = consent&.sid || user.id.to_s subject = consent&.sid || user.id.to_s
# Parse scopes (space-separated string)
requested_scopes = scopes.to_s.split
# Required claims (always included per OIDC Core spec)
payload = { payload = {
iss: issuer_url, iss: issuer_url,
sub: subject, sub: subject,
aud: application.client_id, aud: application.client_id,
exp: now + ttl, exp: now + ttl,
iat: now, iat: now
email: user.email_address,
email_verified: true,
preferred_username: user.username.presence || user.email_address,
name: user.name.presence || user.email_address
} }
# Email claims (only if 'email' scope requested)
if requested_scopes.include?("email")
payload[:email] = user.email_address
payload[:email_verified] = true
end
# Profile claims (only if 'profile' scope requested)
if requested_scopes.include?("profile")
payload[:preferred_username] = user.username.presence || user.email_address
payload[:name] = user.name.presence || user.email_address
end
# Add nonce if provided (OIDC requires this for implicit flow) # Add nonce if provided (OIDC requires this for implicit flow)
payload[:nonce] = nonce if nonce.present? payload[:nonce] = nonce if nonce.present?
@@ -44,12 +56,13 @@ class OidcJwtService
payload[:at_hash] = at_hash payload[:at_hash] = at_hash
end end
# Add groups if user has any # Groups claims (only if 'groups' scope requested)
if user.groups.any? if requested_scopes.include?("groups") && user.groups.any?
payload[:groups] = user.groups.pluck(:name) payload[:groups] = user.groups.pluck(:name)
end end
# Merge custom claims from groups (arrays are combined, not overwritten) # Merge custom claims from groups (arrays are combined, not overwritten)
# Note: Custom claims from groups are always merged (not scope-dependent)
user.groups.each do |group| user.groups.each do |group|
payload = deep_merge_claims(payload, group.parsed_custom_claims) payload = deep_merge_claims(payload, group.parsed_custom_claims)
end end

View File

@@ -1,5 +1,5 @@
# frozen_string_literal: true # frozen_string_literal: true
module Clinch module Clinch
VERSION = "0.8.3" VERSION = "0.8.4"
end end

View File

@@ -204,13 +204,13 @@ This checklist ensures Clinch meets security, quality, and documentation standar
- [ ] Document backup code security (single-use, store securely) - [ ] Document backup code security (single-use, store securely)
- [ ] Document admin password security requirements - [ ] Document admin password security requirements
### Future Security Enhancements ### Future Security Enhancements (Post-Beta)
- [ ] Rate limiting on authentication endpoints - [x] Rate limiting on authentication endpoints (comprehensive coverage implemented)
- [ ] Account lockout after N failed attempts - [ ] Account lockout after N failed attempts (rate limiting provides similar protection)
- [ ] Admin audit logging - [ ] Admin audit logging
- [ ] Security event notifications - [ ] Security event notifications (email/webhook alerts for suspicious activity)
- [ ] Brute force detection - [ ] Advanced brute force detection (pattern analysis beyond rate limiting)
- [ ] Suspicious login detection - [ ] Suspicious login detection (geolocation, device fingerprinting)
- [ ] IP allowlist/blocklist - [ ] IP allowlist/blocklist
## External Security Review ## External Security Review

View File

@@ -57,7 +57,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
end end
test "should generate id token with required claims" do test "should generate id token with required claims" do
token = @service.generate_id_token(@user, @application) token = @service.generate_id_token(@user, @application, scopes: "openid email profile")
assert_not_nil token, "Should generate token" assert_not_nil token, "Should generate token"
assert token.length > 100, "Token should be substantial" assert token.length > 100, "Token should be substantial"
@@ -88,7 +88,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
admin_group = groups(:admin_group) admin_group = groups(:admin_group)
@user.groups << admin_group unless @user.groups.include?(admin_group) @user.groups << admin_group unless @user.groups.include?(admin_group)
token = @service.generate_id_token(@user, @application) token = @service.generate_id_token(@user, @application, scopes: "openid groups")
decoded = JWT.decode(token, nil, false).first decoded = JWT.decode(token, nil, false).first
assert_includes decoded["groups"], "Administrators", "Should include user's groups" assert_includes decoded["groups"], "Administrators", "Should include user's groups"
@@ -248,10 +248,10 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
end end
test "should handle access token generation" do test "should handle access token generation" do
token = @service.generate_id_token(@user, @application) token = @service.generate_id_token(@user, @application, scopes: "openid email")
decoded = JWT.decode(token, nil, false).first decoded = JWT.decode(token, nil, false).first
# ID tokens always include email_verified # ID tokens include email_verified when email scope is requested
assert_includes decoded.keys, "email_verified" assert_includes decoded.keys, "email_verified"
assert_equal @user.id.to_s, decoded["sub"], "Should decode subject correctly" assert_equal @user.id.to_s, decoded["sub"], "Should decode subject correctly"
assert_equal @application.client_id, decoded["aud"], "Should decode audience correctly" assert_equal @application.client_id, decoded["aud"], "Should decode audience correctly"
@@ -278,7 +278,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
custom_claims: {app_groups: ["admin"], library_access: "all"} custom_claims: {app_groups: ["admin"], library_access: "all"}
) )
token = @service.generate_id_token(user, app) token = @service.generate_id_token(user, app, scopes: "openid email profile groups")
decoded = JWT.decode(token, nil, false).first decoded = JWT.decode(token, nil, false).first
assert_equal ["admin"], decoded["app_groups"] assert_equal ["admin"], decoded["app_groups"]
@@ -305,7 +305,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
custom_claims: {role: "admin", app_specific: true} custom_claims: {role: "admin", app_specific: true}
) )
token = @service.generate_id_token(user, app) token = @service.generate_id_token(user, app, scopes: "openid email profile groups")
decoded = JWT.decode(token, nil, false).first decoded = JWT.decode(token, nil, false).first
# App-specific claim should win # App-specific claim should win
@@ -330,7 +330,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
# User adds roles: ["admin"] # User adds roles: ["admin"]
user.update!(custom_claims: {"roles" => ["admin"], "permissions" => ["write"]}) user.update!(custom_claims: {"roles" => ["admin"], "permissions" => ["write"]})
token = @service.generate_id_token(user, app) token = @service.generate_id_token(user, app, scopes: "openid email profile groups")
decoded = JWT.decode(token, nil, false).first decoded = JWT.decode(token, nil, false).first
# Roles should be combined (not overwritten) # Roles should be combined (not overwritten)
@@ -360,7 +360,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
# User adds roles: ["admin"] # User adds roles: ["admin"]
user.update!(custom_claims: {"roles" => ["admin"]}) user.update!(custom_claims: {"roles" => ["admin"]})
token = @service.generate_id_token(user, app) token = @service.generate_id_token(user, app, scopes: "openid email profile groups")
decoded = JWT.decode(token, nil, false).first decoded = JWT.decode(token, nil, false).first
# All roles should be combined # All roles should be combined
@@ -382,7 +382,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
# User also has "user" role (duplicate) # User also has "user" role (duplicate)
user.update!(custom_claims: {"roles" => ["user", "admin"]}) user.update!(custom_claims: {"roles" => ["user", "admin"]})
token = @service.generate_id_token(user, app) token = @service.generate_id_token(user, app, scopes: "openid email profile groups")
decoded = JWT.decode(token, nil, false).first decoded = JWT.decode(token, nil, false).first
# "user" should only appear once # "user" should only appear once
@@ -404,7 +404,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
# User overrides max_items and theme, adds to roles # User overrides max_items and theme, adds to roles
user.update!(custom_claims: {"roles" => ["admin"], "max_items" => 100, "theme" => "dark"}) user.update!(custom_claims: {"roles" => ["admin"], "max_items" => 100, "theme" => "dark"})
token = @service.generate_id_token(user, app) token = @service.generate_id_token(user, app, scopes: "openid email profile groups")
decoded = JWT.decode(token, nil, false).first decoded = JWT.decode(token, nil, false).first
# Arrays should be combined # Arrays should be combined
@@ -438,7 +438,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
} }
}) })
token = @service.generate_id_token(user, app) token = @service.generate_id_token(user, app, scopes: "openid email profile groups")
decoded = JWT.decode(token, nil, false).first decoded = JWT.decode(token, nil, false).first
# Nested hashes should be deep merged # Nested hashes should be deep merged
@@ -467,7 +467,7 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
custom_claims: {"roles" => ["app_admin"]} custom_claims: {"roles" => ["app_admin"]}
) )
token = @service.generate_id_token(user, app) token = @service.generate_id_token(user, app, scopes: "openid email profile groups")
decoded = JWT.decode(token, nil, false).first decoded = JWT.decode(token, nil, false).first
# All three sources should be combined # All three sources should be combined
@@ -562,4 +562,133 @@ class OidcJwtServiceTest < ActiveSupport::TestCase
assert_includes decoded.keys, "azp", "Should include azp claim" assert_includes decoded.keys, "azp", "Should include azp claim"
assert_equal @application.client_id, decoded["azp"], "azp should be the application's client_id" assert_equal @application.client_id, decoded["azp"], "azp should be the application's client_id"
end end
# Scope-based claim filtering tests (OIDC Core compliance)
test "openid scope only should include minimal required claims" do
token = @service.generate_id_token(@user, @application, scopes: "openid")
decoded = JWT.decode(token, nil, false).first
# Required claims should always be present
assert_includes decoded.keys, "iss", "Should include issuer"
assert_includes decoded.keys, "sub", "Should include subject"
assert_includes decoded.keys, "aud", "Should include audience"
assert_includes decoded.keys, "exp", "Should include expiration"
assert_includes decoded.keys, "iat", "Should include issued at"
assert_includes decoded.keys, "azp", "Should include authorized party"
# Scope-dependent claims should NOT be present
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"
refute_includes decoded.keys, "groups", "Should not include groups without groups scope"
end
test "email scope should include email claims" do
token = @service.generate_id_token(@user, @application, scopes: "openid email")
decoded = JWT.decode(token, nil, false).first
# Email claims should be present
assert_includes decoded.keys, "email", "Should include email with email scope"
assert_includes decoded.keys, "email_verified", "Should include email_verified with email scope"
assert_equal @user.email_address, decoded["email"]
assert_equal true, decoded["email_verified"]
# Profile claims should NOT be present
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 "profile scope should include profile claims" do
token = @service.generate_id_token(@user, @application, scopes: "openid profile")
decoded = JWT.decode(token, nil, false).first
# Profile claims should be present
assert_includes decoded.keys, "name", "Should include name with profile scope"
assert_includes decoded.keys, "preferred_username", "Should include preferred_username with profile scope"
assert_equal @user.email_address, decoded["name"]
assert_equal @user.email_address, decoded["preferred_username"]
# Email claims should NOT be present
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"
end
test "groups scope should include groups claim" do
admin_group = groups(:admin_group)
@user.groups << admin_group unless @user.groups.include?(admin_group)
token = @service.generate_id_token(@user, @application, scopes: "openid groups")
decoded = JWT.decode(token, nil, false).first
# Groups claim should be present
assert_includes decoded.keys, "groups", "Should include groups with groups scope"
assert_includes decoded["groups"], "Administrators"
# Email and profile claims should NOT be present
refute_includes decoded.keys, "email", "Should not include email without email scope"
refute_includes decoded.keys, "name", "Should not include name without profile scope"
end
test "groups scope should not include groups claim when user has no groups" do
# Ensure user has no groups
@user.groups.clear
token = @service.generate_id_token(@user, @application, scopes: "openid groups")
decoded = JWT.decode(token, nil, false).first
# Groups claim should not be present when user has no groups
refute_includes decoded.keys, "groups", "Should not include empty groups claim"
end
test "multiple scopes should include all requested claims" do
admin_group = groups(:admin_group)
@user.groups << admin_group unless @user.groups.include?(admin_group)
token = @service.generate_id_token(@user, @application, scopes: "openid email profile groups")
decoded = JWT.decode(token, nil, false).first
# All scope-based claims should be present
assert_includes decoded.keys, "email", "Should include email"
assert_includes decoded.keys, "email_verified", "Should include email_verified"
assert_includes decoded.keys, "name", "Should include name"
assert_includes decoded.keys, "preferred_username", "Should include preferred_username"
assert_includes decoded.keys, "groups", "Should include groups"
end
test "scope parameter should handle space-separated string" do
token = @service.generate_id_token(@user, @application, scopes: "openid email profile")
decoded = JWT.decode(token, nil, false).first
assert_includes decoded.keys, "email", "Should parse space-separated scopes"
assert_includes decoded.keys, "name", "Should parse space-separated scopes"
end
test "custom claims should always be merged regardless of scopes" do
user = users(:bob)
app = applications(:another_app)
# Add user custom claim
user.update!(custom_claims: {"custom_field" => "custom_value"})
# Request only openid scope (no email, profile, or groups)
token = @service.generate_id_token(user, app, scopes: "openid")
decoded = JWT.decode(token, nil, false).first
# Custom claims should be present even with minimal scopes
assert_equal "custom_value", decoded["custom_field"], "Custom claims should be included regardless of scopes"
# Standard claims should be filtered
refute_includes decoded.keys, "email", "Should not include email without email scope"
refute_includes decoded.keys, "name", "Should not include name without profile scope"
end
end end