Skip to content

feat: Add OpenID Connect (OIDC) support for Single Sign-On (SSO) integration#201

Open
clementmouchet wants to merge 3 commits into
basecamp:mainfrom
clementmouchet:sso
Open

feat: Add OpenID Connect (OIDC) support for Single Sign-On (SSO) integration#201
clementmouchet wants to merge 3 commits into
basecamp:mainfrom
clementmouchet:sso

Conversation

@clementmouchet

@clementmouchet clementmouchet commented Apr 26, 2026

Copy link
Copy Markdown

Summary

This PR adds OpenID Connect (OIDC) Single Sign-On (SSO) support to Campfire, including multi-provider configuration, SSO login/callback handling, automatic user linking/creation, and OIDC-aware logout. It also introduces an SSO-first onboarding mode when password registration is disabled.

What Changed

  • Added OIDC dependencies and OmniAuth setup:
    • omniauth
    • omniauth-rails_csrf_protection
    • omniauth_openid_connect
    • New initializer to register providers from environment config.
  • Added OIDC provider configuration service (OidcConfiguration):
    • Supports multiple providers via OIDC_PROVIDERS.
    • Validates required env vars, provider key format, and callback-derived strategy names.
  • Added SSO authentication/session flow:
    • New SsoSessionsController for /auth/:provider/callback and /auth/failure.
    • New SsoAuthentication service to:
      • Find existing users by sso_provider/sso_uid.
      • Link existing users by email on first SSO login.
      • Auto-create users when no match exists.
  • Added OIDC logout support:
    • New OidcLogout service to build RP-initiated logout URLs.
    • Uses configured END_SESSION_ENDPOINT or issuer discovery metadata.
    • SessionsController#destroy now redirects through provider logout when available.
  • Added SSO room invite onboarding:
    • New signed room invite token (Room#sso_invite_token) and lookup.
    • First SSO login from invite URL auto-grants membership to that room.
    • Added SSO invite partial and integrated it in account/room invitation UI.
  • Updated auth/account behavior:
    • Added DISABLE_PASSWORD_REGISTRATION handling to join flow.
    • SSO users cannot edit name/email in profile; password edit is constrained by registration mode.
    • Login page now supports SSO-first and mixed auth layouts.
  • Database changes:
    • users: add sso_provider, sso_uid, unique partial index on both.
    • sessions: add sso_provider, oidc_id_token.
  • Docs/dev setup:
    • README updated with full OIDC/SSO configuration and examples.
    • Added compose.yml for local Docker Compose flow.

Validation

Ran targeted SSO/auth test suite successfully:

PARALLEL_WORKERS=1 bin/rails test test/controllers/sso_sessions_controller_test.rb test/controllers/sessions_controller_test.rb test/controllers/users_controller_test.rb test/controllers/users/profiles_controller_test.rb test/models/sso_authentication_test.rb test/models/oidc_configuration_test.rb test/models/oidc_logout_test.rb

Result: 59 runs, 162 assertions, 0 failures, 0 errors, 0 skips.

@clementmouchet clementmouchet changed the title Add OpenID Connect (OIDC) support for Single Sign-On (SSO) integration feat: Add OpenID Connect (OIDC) support for Single Sign-On (SSO) integration Apr 26, 2026
@clementmouchet clementmouchet marked this pull request as ready for review April 26, 2026 17:41
Copilot AI review requested due to automatic review settings April 26, 2026 17:41

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@clementmouchet

Copy link
Copy Markdown
Author

@claude review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds OpenID Connect (OIDC) Single Sign-On (SSO) to Campfire, including multi-provider configuration, SSO session creation/callback handling, user auto-linking/creation, invite-based onboarding, and OIDC-aware logout behavior.

Changes:

  • Add OmniAuth OIDC middleware configuration and environment-driven provider configuration (OidcConfiguration).
  • Introduce SSO session flow (SsoSessionsController) + user linking/creation (SsoAuthentication) and RP-initiated logout URL generation (OidcLogout).
  • Update UI/routes/tests to support SSO-first onboarding (when password registration is disabled) and room invite links for SSO.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 38 out of 40 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
test/test_helpers/sso_test_helper.rb Adds OmniAuth mocking helpers for controller/integration tests.
test/test_helper.rb Includes the SSO test helper globally.
test/models/sso_authentication_test.rb Tests user lookup/linking/creation logic for SSO auth hashes.
test/models/oidc_logout_test.rb Tests logout URL generation via configured and discovered endpoints.
test/models/oidc_configuration_test.rb Tests env parsing/validation for multiple OIDC providers.
test/fixtures/users.yml Adds an SSO-linked user fixture.
test/controllers/users_controller_test.rb Tests join flow redirect behavior when password registration is disabled.
test/controllers/users/profiles_controller_test.rb Tests SSO profile field disabling and update restrictions.
test/controllers/sso_sessions_controller_test.rb Tests callback/failure flows and invite-based room membership grant.
test/controllers/sessions_controller_test.rb Tests login page layouts and OIDC-aware logout redirect behavior.
db/schema.rb Adds SSO/user + session OIDC columns and indexes to schema dump.
db/migrate/20260426170000_add_oidc_id_token_to_sessions.rb Adds oidc_id_token storage to sessions.
db/migrate/20260426163000_add_sso_provider_to_sessions.rb Adds sso_provider to sessions.
db/migrate/20260424120000_add_sso_fields_to_users.rb Adds SSO identity columns and uniqueness index to users.
config/routes.rb Adds /auth/:provider/callback + /auth/failure endpoints for SSO.
config/initializers/omniauth.rb Registers OIDC providers with OmniAuth from OidcConfiguration.
compose.yml Adds Docker Compose setup for local running/testing.
app/views/users/profiles/show.html.erb Disables profile edits for SSO identities and adjusts logout form behavior.
app/views/sessions/new.html.erb Adds SSO-first vs mixed-auth login layouts + provider buttons.
app/views/rooms/show/_sso_invite.html.erb Adds SSO invite link partial for room onboarding.
app/views/rooms/show/_invitation.html.erb Switches invitation UI based on password registration availability.
app/views/rooms/layouts/_edit.html.erb Adds SSO invite panel for room admins in SSO-only onboarding mode.
app/views/accounts/edit.html.erb Adjusts account invite area to show SSO invite when join links are hidden.
app/views/accounts/_help_contact.html.erb Tweaks help contact styling.
app/models/user.rb Adds sso? predicate.
app/models/sso_authentication.rb Adds SSO user linking/creation and email extraction/normalization logic.
app/models/session.rb Extends session creation to store sso_provider + oidc_id_token.
app/models/room.rb Adds signed SSO invite tokens and lookup helper.
app/models/oidc_logout.rb Adds logout URL generation with optional discovery.
app/models/oidc_configuration.rb Adds env-based provider configuration builder/validator.
app/helpers/sso_helper.rb Adds SSO enablement and password-registration mode helpers.
app/controllers/users_controller.rb Blocks join flow when password registration is disabled.
app/controllers/users/profiles_controller.rb Prevents SSO users from changing name/email; gates password changes by mode.
app/controllers/sso_sessions_controller.rb Implements SSO callback + failure handling + invite-room membership grant.
app/controllers/sessions_controller.rb Adds SSO-aware login error and OIDC-aware logout redirect.
app/controllers/concerns/authentication.rb Extends session start API to include SSO provider + id_token hint.
README.md Documents OIDC/SSO configuration and SSO-only onboarding mode.
Gemfile.lock Adds OmniAuth/OpenID Connect dependencies.
Gemfile Adds OmniAuth/OpenID Connect gems.
.gitignore Ignores .idea project files.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/controllers/users_controller_test.rb Outdated
Comment thread app/models/sso_authentication.rb
Comment thread app/models/session.rb
Comment thread app/controllers/sessions_controller.rb
Comment thread app/views/users/profiles/show.html.erb Outdated
Comment thread app/helpers/sso_helper.rb Outdated
Comment thread db/migrate/20260424120000_add_sso_fields_to_users.rb Outdated
Comment thread app/models/sso_authentication.rb Outdated
Comment thread app/models/oidc_logout.rb Outdated
Comment thread test/controllers/sessions_controller_test.rb
- app/models/sso_authentication.rb: case-insensitive email lookup, verified-email enforcement for link/create, and blocking relink when the matched account is already SSO-linked.
- app/models/oidc_logout.rb: HTTPS/host validation for logout endpoints and cached OIDC discovery metadata.
- app/models/session.rb + config/initializers/active_record_encryption.rb: oidc_id_token is now encrypted at rest, with key configuration fallback.
- app/helpers/sso_helper.rb: case-insensitive parsing for DISABLE_PASSWORD_REGISTRATION.
- app/views/users/profiles/show.html.erb: password field now remains visible for non-SSO users even in SSO-first mode.
- db/migrate/20260424120000_add_sso_fields_to_users.rb and db/schema.rb: tightened SSO uniqueness/index predicate and added provider/uid presence check constraint.
- Test isolation for ENV mutations via test/test_helper.rb, then refactored affected controller tests and expanded model/controller coverage.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@clementmouchet clementmouchet marked this pull request as draft April 26, 2026 20:41
@clementmouchet clementmouchet marked this pull request as ready for review April 26, 2026 20:42
@clementmouchet clementmouchet requested a review from Copilot April 26, 2026 20:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 41 out of 43 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +31 to +37
# Find by email (link existing user to SSO on first SSO login)
user = User.where("LOWER(email_address) = ?", email).first
if user
raise Error, "Email is already linked to another SSO account" if user.sso?

user.update!(sso_provider: provider, sso_uid: uid)
return user

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User.where("LOWER(email_address) = ?", email).first can return an arbitrary user if the database contains multiple accounts whose email_address differs only by case (the unique index on email_address is case-sensitive). In that situation, first-time SSO linking could attach the SSO identity to the wrong account. Consider normalizing emails on write (and/or adding a case-insensitive unique constraint), or at minimum detecting multiple matches here and raising an error instead of picking .first.

Copilot uses AI. Check for mistakes.
Comment thread app/models/room.rb
Comment on lines +69 to +71
def sso_invite_token
signed_id(purpose: SSO_INVITE_PURPOSE)
end

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sso_invite_token is generated from signed_id with no expiry and no rotation mechanism. That makes the room invite URL effectively permanent; if it’s ever leaked, anyone who gets the link can continue joining the room indefinitely, and there’s no way to invalidate it without rotating global secrets. Consider adding expires_in: and/or storing a per-room invite secret/nonce so admins can regenerate/revoke invite links.

Copilot uses AI. Check for mistakes.
Comment on lines +31 to +38
strategy = params[:strategy].to_s.presence&.upcase
detail = params[:detail].to_s.presence || params[:message].to_s.humanize
detail = detail.gsub(/\s+/, " ").strip.first(280)

message = [ "SSO authentication failed", strategy&.then { "(#{_1})" }, detail.presence&.then { "— #{_1}" } ].compact.join(" ") + "."

Rails.logger.error message
redirect_to new_session_url, alert: message

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In failure, strategy comes directly from query params and isn’t length-capped. Since the app uses cookie-based sessions, a very large strategy value can bloat the flash message and trigger ActionDispatch::Cookies::CookieOverflow (500) on /auth/failure. Consider truncating strategy (and/or enforcing a maximum total message length) before building the flash alert/log message.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants