QT/sure
1
0
Fork 0
mirror of https://github.com/we-promise/sure.git synced 2026-04-07 06:21:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: improve SSO provider management and logging

Browse Source 
- Simplified `name_id_format` selection logic in SSO provider form.
- Switched raw database query to sanitized SQL in client secret tests.
- Added condition to log JIT account creation only when identity persists.
- Sanitized failure reasons in SSO login failure handling.
- Added SSO provider connection test policy tests for super admin and regular users.
This commit is contained in:
Josh Waldrep
2026-01-03 21:13:24 -05:00
parent d3055b2e0b
commit b2ecc6bc67
5 changed files with 28 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
app/controllers/oidc_accounts_controller.rb
View File

@@ -118,17 +118,19 @@ class OidcAccountsController < ApplicationController
if @user.save
# Create the OIDC (or other SSO) identity
OidcIdentity.create_from_omniauth(
identity = OidcIdentity.create_from_omniauth(
build_auth_hash(@pending_auth),
@user
)
# Log JIT account creation
SsoAuditLog.log_jit_account_created!(
user: @user,
provider: @pending_auth["provider"],
request: request
)
# Only log JIT account creation if identity was successfully created
if identity.persisted?
SsoAuditLog.log_jit_account_created!(
user: @user,
provider: @pending_auth["provider"],
request: request
)
end
# Clear pending auth from session
session.delete(:pending_oidc_auth)

6
app/controllers/sessions_controller.rb
View File

@@ -144,11 +144,15 @@ class SessionsController < ApplicationController
end
def failure
# Sanitize reason to known values only
known_reasons = %w[sso_provider_unavailable sso_invalid_response sso_failed]
sanitized_reason = known_reasons.include?(params[:message]) ? params[:message] : "sso_failed"
# Log failed SSO attempt
SsoAuditLog.log_login_failed!(
provider: params[:strategy],
request: request,
reason: params[:message]
reason: sanitized_reason
)
message = case params[:message]

3
app/views/admin/sso_providers/_form.html.erb
View File

@@ -158,8 +158,7 @@
<label class="block text-sm font-medium text-primary mb-1"><%= t("admin.sso_providers.form.name_id_format") %></label>
<select name="sso_provider[settings][name_id_format]"
class="w-full px-3 py-2 border border-primary rounded-lg text-sm">
<option value="" <%= "selected" if sso_provider.settings&.dig("name_id_format").blank? %>><%= t("admin.sso_providers.form.name_id_email") %></option>
<option value="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" <%= "selected" if sso_provider.settings&.dig("name_id_format") == "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" %>><%= t("admin.sso_providers.form.name_id_email") %></option>
<option value="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" <%= "selected" if sso_provider.settings&.dig("name_id_format").blank? || sso_provider.settings&.dig("name_id_format") == "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" %>><%= t("admin.sso_providers.form.name_id_email") %></option>
<option value="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" <%= "selected" if sso_provider.settings&.dig("name_id_format") == "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" %>><%= t("admin.sso_providers.form.name_id_persistent") %></option>
<option value="urn:oasis:names:tc:SAML:2.0:nameid-format:transient" <%= "selected" if sso_provider.settings&.dig("name_id_format") == "urn:oasis:names:tc:SAML:2.0:nameid-format:transient" %>><%= t("admin.sso_providers.form.name_id_transient") %></option>
<option value="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified" <%= "selected" if sso_provider.settings&.dig("name_id_format") == "urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified" %>><%= t("admin.sso_providers.form.name_id_unspecified") %></option>

8
test/models/sso_provider_test.rb
View File

@@ -96,9 +96,11 @@ class SsoProviderTest < ActiveSupport::TestCase
assert_equal "super_secret_value", provider.client_secret
# Raw database value should be encrypted (not plain text)
raw_value = ActiveRecord::Base.connection.execute(
"SELECT client_secret FROM sso_providers WHERE id = '#{provider.id}'"
).first["client_secret"]
raw_value = ActiveRecord::Base.connection.select_value(
ActiveRecord::Base.sanitize_sql_array(
[ "SELECT client_secret FROM sso_providers WHERE id = ?", provider.id ]
)
)
assert_not_equal "super_secret_value", raw_value
end

8
test/policies/sso_provider_policy_test.rb
View File

@@ -86,6 +86,14 @@ class SsoProviderPolicyTest < ActiveSupport::TestCase
assert_not SsoProviderPolicy.new(@regular_user, @provider).toggle?
end
test "super admin can test connection" do
assert SsoProviderPolicy.new(@super_admin, @provider).test_connection?
end
test "regular user cannot test connection" do
assert_not SsoProviderPolicy.new(@regular_user, @provider).test_connection?
end
test "scope returns all providers for super admin" do
SsoProvider.create!(
strategy: "google_oauth2",