Add OpenID Connect login support #77
Conversation
jjmata
added
the
codex
ChatGPT Codex Connector
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @jjmata, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've implemented OpenID Connect (OIDC) login support to enhance the application's authentication capabilities. This change integrates the omniauth_openid_connect gem, sets up the necessary callback routes, and documents the required environment variables for OIDC configuration, allowing users to authenticate via an OIDC provider.
Highlights
- OpenID Connect Integration: Integrated
omniauth_openid_connectgem along withomniauth-rails_csrf_protectionto enable secure OpenID Connect authentication. - Authentication Flow Enhancements: Introduced new routes and controller actions (
openid_connect,failure) inSessionsControllerto handle OIDC callbacks and authentication failures. - Configuration Updates: Updated
.env.example,.env.local.example, and.env.test.examplefiles to include new environment variables (OIDC_ISSUER,OIDC_CLIENT_ID,OIDC_CLIENT_SECRET,OIDC_REDIRECT_URI) required for OIDC configuration. - User Interface Changes: Added a new link on the login page (
app/views/sessions/new.html.erb) to initiate the OpenID Connect authentication flow.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces OpenID Connect (OIDC) login support, which is a great feature. The implementation correctly adds the necessary gems, configuration, and routes. However, I've found a critical security vulnerability in the SessionsController that allows for account takeover and bypasses MFA. I've also noted a couple of medium-severity issues: a bug in translation key usage and a misleading value in the example environment file. Please address the critical security issue with high priority.
|
This will obviously require a much more careful review. Any good volunteers with deep login flow expertise? |
✅ Actions performedReview triggered.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds OpenID Connect support: environment samples, OmniAuth gems and initializer, routes, controllers, model, views, styles, locales, tests, and a DB migration to create OIDC identities. No removals. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant B as Browser
participant M as OmniAuth
participant A as Rails App
participant IdP as OIDC Provider
U->>B: Click "Sign in with OpenID Connect"
B->>M: GET /auth/openid_connect
M->>IdP: Redirect to authorize (PKCE)
IdP-->>B: Redirect with code to /auth/openid_connect/callback
B->>M: GET /auth/:provider/callback
M->>A: env["omniauth.auth"] -> SessionsController#openid_connect
A->>A: Find OidcIdentity by provider+uid
alt Identity found
A->>A: record_authentication!, create session or start MFA
A-->>B: Redirect to root
else No identity
A->>A: store pending auth in session
A-->>B: Redirect to OidcAccountsController#link (or #new_user)
end
Note right of A: On middleware failure -> SessionsController#failure -> redirect to login with alert
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (7)app/**/*.rb📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.rb📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rb,js,jsx,ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/app/**/*.{rb,erb,js,css}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/app/controllers/**/*.rb📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{rb,erb,haml,slim}📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
Files:
app/{models,controllers,views}/**/*.{rb,erb}📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (3)
.env.example (1)
56-61: Do not hardcode a localhost callback in the production example; leave it blank and alphabetize keys.Keeping a localhost callback here is misleading for self-hosted/production usage and can cause deployment misconfiguration. Also addresses dotenv-linter ordering.
Apply this diff:
-# OpenID Connect configuration -OIDC_ISSUER= -OIDC_CLIENT_ID= -OIDC_CLIENT_SECRET= -OIDC_REDIRECT_URI=http://localhost:3000/auth/openid_connect/callback +# OpenID Connect configuration +# Set OIDC_REDIRECT_URI to your production callback URL (e.g., https://your-domain.com/auth/openid_connect/callback) +OIDC_CLIENT_ID= +OIDC_CLIENT_SECRET= +OIDC_ISSUER= +OIDC_REDIRECT_URI=Please verify docs/readme references (if any) to ensure examples consistently use an https production callback.
app/controllers/sessions_controller.rb (2)
30-38: Critical security vulnerabilities in OpenID Connect implementation.This implementation has multiple critical security issues that must be addressed:
Account Takeover Risk: Authentication is based solely on email matching without verifying the OIDC provider relationship, allowing potential account takeover if an attacker controls a matching email on the OIDC provider.
MFA Bypass: The implementation completely bypasses Multi-Factor Authentication, unlike the password-based login flow which properly checks
user.otp_required?.Null Pointer Risk:
auth.info.emailcould benil, potentially causing aNoMethodError.The previous review comment correctly identified these issues. A secure implementation should:
- Link OIDC identities to user accounts explicitly (e.g., store provider + UID)
- Respect MFA requirements
- Handle nil values safely
def openid_connect auth = request.env["omniauth.auth"] - if auth && (user = User.find_by(email: auth.info.email)) - @session = create_session_for(user) - redirect_to root_path + email = auth&.info&.email + + if email && (user = User.find_by(email: email)) + if user.otp_required? + session[:mfa_user_id] = user.id + redirect_to verify_mfa_path + else + @session = create_session_for(user) + redirect_to root_path + end else - redirect_to new_session_path, alert: t(".failed") + redirect_to new_session_path, alert: t("sessions.openid_connect.failed") end end
40-42: Fix translation key reference.The translation key
t(".failed")resolves tosessions.failure.failed, but the locale files definesessions.openid_connect.failed. This will result in a missing translation error.def failure - redirect_to new_session_path, alert: t(".failed") + redirect_to new_session_path, alert: t("sessions.openid_connect.failed") end
🧹 Nitpick comments (3)
.env.test.example (1)
3-8: Alphabetize OIDC keys to satisfy dotenv-linter and clarify test defaults.The current order triggers UnorderedKey warnings. Also, a brief note helps prevent accidentally enabling the provider in test.
Apply this diff:
-# OpenID Connect for tests -OIDC_ISSUER= -OIDC_CLIENT_ID= -OIDC_CLIENT_SECRET= -OIDC_REDIRECT_URI=http://localhost:3000/auth/openid_connect/callback +# OpenID Connect for tests +# Leave blank to disable OmniAuth in test (recommended unless explicitly testing the flow) +OIDC_CLIENT_ID= +OIDC_CLIENT_SECRET= +OIDC_ISSUER= +OIDC_REDIRECT_URI=http://localhost:3000/auth/openid_connect/callbackThis should resolve the dotenv-linter UnorderedKey warnings.
config/initializers/omniauth.rb (2)
3-3: Explicitly require the OpenID Connect strategy to avoid load-order surprisesWhile Bundler usually auto-requires gems, being explicit prevents subtle boot-order issues in some setups and makes the initializer self-contained.
Apply this diff:
require "omniauth/rails_csrf_protection" +require "omniauth_openid_connect"
15-17: Optional: consider credentials for secrets in productionEnvironment variables are fine, but for stronger secret management you could prefer Rails encrypted credentials (or a secrets manager) for OIDC_CLIENT_SECRET in production. No blocker.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.env.example(1 hunks).env.local.example(1 hunks).env.test.example(1 hunks)Gemfile(1 hunks)app/controllers/sessions_controller.rb(2 hunks)app/views/sessions/new.html.erb(1 hunks)config/initializers/omniauth.rb(1 hunks)config/locales/views/sessions/en.yml(1 hunks)config/locales/views/sessions/nb.yml(1 hunks)config/locales/views/sessions/tr.yml(1 hunks)config/routes.rb(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/controllers/sessions_controller.rb (3)
app/controllers/registrations_controller.rb (1)
skip_authentication(1-82)app/controllers/concerns/authentication.rb (2)
skip_authentication(11-14)create_session_for(40-44)app/models/current.rb (1)
user(8-10)
🪛 dotenv-linter (3.3.0)
.env.local.example
[warning] 9-9: [UnorderedKey] The OIDC_CLIENT_ID key should go before the OIDC_ISSUER key
[warning] 10-10: [UnorderedKey] The OIDC_CLIENT_SECRET key should go before the OIDC_ISSUER key
.env.example
[warning] 58-58: [UnorderedKey] The OIDC_CLIENT_ID key should go before the OIDC_ISSUER key
[warning] 59-59: [UnorderedKey] The OIDC_CLIENT_SECRET key should go before the OIDC_ISSUER key
.env.test.example
[warning] 5-5: [UnorderedKey] The OIDC_CLIENT_ID key should go before the OIDC_ISSUER key
[warning] 6-6: [UnorderedKey] The OIDC_CLIENT_SECRET key should go before the OIDC_ISSUER key
🔇 Additional comments (6)
config/routes.rb (1)
40-41: LGTM! Routes properly configured for OpenID Connect flow.The routes correctly handle both the callback and failure scenarios for OpenID Connect authentication, supporting both GET and POST methods as required by the OmniAuth specification.
config/locales/views/sessions/en.yml (1)
18-18: LGTM! Translation key correctly positioned.The
openid_connecttranslation key is properly positioned undersessions.newand will be correctly resolved by the login form.config/locales/views/sessions/tr.yml (2)
8-9: LGTM! Turkish translations properly structured.The translation keys are correctly positioned and consistent with the expected structure for OpenID Connect authentication messages.
18-18: LGTM! Login link translation correctly positioned.The Turkish translation for the OpenID Connect login link is properly placed under
sessions.new.app/controllers/sessions_controller.rb (1)
3-3: LGTM! Authentication properly bypassed for OIDC endpoints.The
skip_authenticationcorrectly includes the newopenid_connectandfailureactions, allowing unauthenticated access to these endpoints as required by the OIDC flow.config/initializers/omniauth.rb (1)
7-14: Good defaults: Authorization Code + PKCE with discovery and minimal scopesSolid choices that align with OIDC best practices. No changes requested here.
| # OpenID Connect authentication | ||
| gem "omniauth", "~> 2.1" | ||
| gem "omniauth-rails_csrf_protection" | ||
| gem "omniauth_openid_connect" | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Good call on required gems; consider pinning minor versions and validate latest releases/advisories.
To reduce surprise upgrades, pin minor versions for the new gems (adjust to the latest stable in your ecosystem):
gem "omniauth", "~> 2.1"
-gem "omniauth-rails_csrf_protection"
-gem "omniauth_openid_connect"
+gem "omniauth-rails_csrf_protection", "~> 1.0"
+gem "omniauth_openid_connect", "~> 0.6"Confirm the latest stable versions and any advisories before finalizing ranges.
🌐 Web query:
What are the latest stable versions of the Ruby gems "omniauth", "omniauth-rails_csrf_protection", and "omniauth_openid_connect" as of today? Include any recent security advisories for these gems.
💡 Result:
Latest stable versions as of today (2025-08-13 UTC):
- omniauth: 2.1.3 on RubyGems (released 2025-02-27).[7]
- omniauth-rails_csrf_protection: 1.0.2 on RubyGems (released 2024-05-10).[4][6]
- omniauth_openid_connect: The latest stable on RubyGems is not present in the provided results; please confirm on RubyGems. Based on typical ecosystem tracking, this gem is commonly published under the name omniauth_openid_connect; verify its current version directly on RubyGems for accuracy.
Recent security advisories and notes:
- omniauth:
- v1.9.2 backported a vulnerability fix from the 2.0 line; the release notes reference CVE-2020-36599 and clarify that security policy maintains v2.0 and v2.1 channels, with 1.9 generally not receiving patches.[1]
- omniauth-rails_csrf_protection:
- This gem specifically mitigates CVE-2015-9284 (CSRF during OmniAuth request phase in Rails apps) by enforcing POST and verifying Rails authenticity tokens.[4][6]
- Related OmniAuth ecosystem advisory (not directly for OpenID Connect but relevant context):
- omniauth-saml had critical issues via its ruby-saml dependency (CVE-2025-25291, CVE-2025-25292; plus CVE-2025-25293 DoS) with guidance to require ruby-saml 1.18.0 (2025-03-12). This does not affect OpenID Connect directly but is noteworthy for OmniAuth strategy gems.[3]
Notes on sources and verification:
- The omniauth latest version number (2.1.3) is from RubyGems’ all-versions listing and post-dates the GitHub releases page, which shows up to 2.1.1; prefer RubyGems for authoritative “latest” distribution status.[7][1]
- For omniauth-rails_csrf_protection, both RubyGems and the project’s GitHub releases confirm 1.0.2 as the latest.[4][6]
- I could not locate a current RubyGems page for omniauth_openid_connect in the provided results. If you want, I can look it up and confirm the exact version number.
Citations:
- 1: https://github.com/omniauth/omniauth/releases
- 2: https://gemwatch.debian.net/omniauth-rails_csrf_protection
- 3: https://advisories.gitlab.com/pkg/gem/omniauth-saml/GHSA-hw46-3hmr-x9xv/
- 4: https://rubygems.org/gems/omniauth-rails_csrf_protection/versions/1.0.2?locale=en
- 5: https://www.youtube.com/watch?v=c-sBNOOBhwM
- 6: https://github.com/cookpad/omniauth-rails_csrf_protection
- 7: https://rubygems.org/gems/omniauth/versions
- 8: https://my.diffend.io/gems/omniauth-rails_csrf_protection/prev/1.0.0
- 9: https://gorails.com/forum/rails-for-beginners-part-24-omniauth-urls-discussion
- 10: https://www.mintbit.com/blog/updating-omniauth-from-version-1-to-2/
🏁 Script executed:
#!/bin/bash
# Fetch the latest version number of omniauth_openid_connect from RubyGems API
curl -s https://rubygems.org/api/v1/versions/omniauth_openid_connect.json | jq '.[0].number'Length of output: 102
Pin OmniAuth strategy gems to current stable patch releases
To prevent unintended minor upgrades, lock each new gem to the latest published patch level:
• In Gemfile:
gem "omniauth", "~> 2.1"
-gem "omniauth-rails_csrf_protection"
-gem "omniauth_openid_connect"
+gem "omniauth", "~> 2.1.3"
+gem "omniauth-rails_csrf_protection", "~> 1.0.2"
+gem "omniauth_openid_connect", "~> 0.8.0"• These pins allow only patch-level updates (< 2.2.0, < 1.1.0, and < 0.9.0 respectively).
• Confirm no recent CVEs affect these versions before merging.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Gemfile around lines 77–81, the three OmniAuth gems are not pinned to
patch-level constraints; update their declarations to restrict upgrades to patch
releases only (i.e. set constraints that allow >= the current minor series but <
the next minor: < 2.2.0 for omniauth, < 1.1.0 for
omniauth-rails_csrf_protection, and < 0.9.0 for omniauth_openid_connect) so they
only receive patch updates, and then verify the exact latest patch numbers and
check for any recent CVEs before merging.
Signed-off-by: Juan José Mata <[email protected]>
Signed-off-by: Juan José Mata <[email protected]>
- Add support for linking existing account - Force users to sign-in with passoword first, when linking existing accounts - Add support to create new user when using OIDC - Add identities to user to prevent account take-ver
Signed-off-by: soky srm <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
.env.local.example (1)
16-20: Reorder OIDC variables alphabetically to satisfy dotenv-linter.The OIDC environment variables should be alphabetically ordered to comply with dotenv-linter rules.
Apply this diff:
# OpenID Connect for development -OIDC_ISSUER= OIDC_CLIENT_ID= OIDC_CLIENT_SECRET= +OIDC_ISSUER= OIDC_REDIRECT_URI=http://localhost:3000/auth/openid_connect/callback
🧹 Nitpick comments (3)
app/views/sessions/new.html.erb (2)
31-31: Remove the duplicate hidden span.Line 31 contains a hidden duplicate of the button text with no apparent purpose. This adds unnecessary DOM elements and translation calls.
<span class="gsi-material-button-contents"><%= t(".google_auth_connect") %></span> - <span style="display: none;"><%= t(".google_auth_connect") %></span>
17-34: Consider conditionally rendering the OIDC sign-in button.The OpenID Connect sign-in button is always rendered, even when OIDC is not configured (OIDC_ISSUER is blank). This could lead to broken authentication flows in environments where OIDC is not set up.
Consider wrapping the button in a conditional:
<% if ENV.fetch("OIDC_ISSUER", "").present? %> <div class="mt-6 text-center"> <%= button_to "/auth/openid_connect", method: :post, form: { data: { turbo: false } }, class: "gsi-material-button" do %> <!-- button contents --> <% end %> </div> <% end %>Alternatively, set a configuration flag in the OmniAuth initializer (e.g.,
Rails.configuration.x.auth.oidc_enabled = ENV['OIDC_ISSUER'].present?) and check that instead of reading ENV directly in the view.app/views/oidc_accounts/link.html.erb (1)
56-56: Ensure button styling is consistent with app patterns.The "Create Account" button only has
class: "w-full"but likely needs additional styling to match other primary action buttons in the application (e.g., the "Link Account" submit button on line 33 usesform.submitwhich may have default styling).Verify that
button_toreceives appropriate styling. Ifstyled_form_withprovides button styling viaform.submit, consider using a consistent approach here or applying the same button classes explicitly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
.env.example(1 hunks).env.local.example(1 hunks)Gemfile(1 hunks)app/assets/tailwind/application.css(1 hunks)app/assets/tailwind/google-sign-in.css(1 hunks)app/controllers/oidc_accounts_controller.rb(1 hunks)app/controllers/sessions_controller.rb(2 hunks)app/models/oidc_identity.rb(1 hunks)app/models/user.rb(1 hunks)app/views/oidc_accounts/link.html.erb(1 hunks)app/views/sessions/new.html.erb(1 hunks)config/locales/views/sessions/en.yml(1 hunks)config/routes.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- config/routes.rb
- Gemfile
🧰 Additional context used
📓 Path-based instructions (28)
app/views/**/*.erb
📄 CodeRabbit inference engine (AGENTS.md)
app/views/**/*.erb: Keep heavy logic out of ERB views; prefer helpers/components instead
ERB templates are linted by erb-lint per .erb_lint.ymlAlways use the icon helper (icon) for icons; never call lucide_icon directly
Files:
app/views/oidc_accounts/link.html.erbapp/views/sessions/new.html.erb
app/views/**/*.html.*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/views/**/*.html.*: Use partials only for simple, context-specific, mostly static HTML content.
Prefer semantic HTML; use Turbo Frames where possible instead of client-side solutions.
Use query params for state, not localStorage or sessionStorage.
Always perform server-side formatting for currencies, numbers, and dates.
Files:
app/views/oidc_accounts/link.html.erbapp/views/sessions/new.html.erb
**/*.{html,erb,slim,js,jsx,ts,tsx,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Always use functional design tokens (e.g., text-primary, bg-container) from the design system; do not use raw colors or ad-hoc classes.
Files:
app/views/oidc_accounts/link.html.erbapp/assets/tailwind/application.cssapp/views/sessions/new.html.erbapp/assets/tailwind/google-sign-in.css
**/app/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
Always use the
iconhelper for icons in templates; never uselucide_icondirectly.
Files:
app/views/oidc_accounts/link.html.erbapp/views/sessions/new.html.erb
**/app/views/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
**/app/views/**/*.erb: Keep domain logic OUT of view templates; logic belongs in component files, not templates.
Use partials only for primarily static, simple HTML with minimal logic, and only when not likely to be reused.
Prefer native HTML over JavaScript components for UI elements (e.g., use<dialog>,<details><summary>).
Leverage Turbo frames for page sections over client-side JavaScript solutions.
Use query params (not localStorage/sessions) for client state management.
Perform server-side formatting for currencies, numbers, and dates in templates.
Files:
app/views/oidc_accounts/link.html.erbapp/views/sessions/new.html.erb
**/app/**/*.{rb,erb,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Push Rails to its limits before adding new dependencies; a strong technical/business reason is required for new dependencies.
Files:
app/views/oidc_accounts/link.html.erbapp/models/user.rbapp/controllers/sessions_controller.rbapp/assets/tailwind/application.cssapp/controllers/oidc_accounts_controller.rbapp/models/oidc_identity.rbapp/views/sessions/new.html.erbapp/assets/tailwind/google-sign-in.css
**/*.{rb,erb,haml,slim}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb,haml,slim}: UseCurrent.userfor the current user; do not usecurrent_user
UseCurrent.familyfor the current family; do not usecurrent_family
Ignore i18n methods; hardcode strings in English for now (do not useI18n.t,t, or similar)
Files:
app/views/oidc_accounts/link.html.erbapp/models/user.rbapp/controllers/sessions_controller.rbapp/controllers/oidc_accounts_controller.rbapp/models/oidc_identity.rbapp/views/sessions/new.html.erb
app/views/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
app/views/**/*.html.erb: Prefer native HTML elements (e.g., ,) over JS-based components
Leverage Turbo frames to break up pages instead of JS-driven client-side solutions
Prefer native client-side form validation when possible
Files:
app/views/oidc_accounts/link.html.erbapp/views/sessions/new.html.erb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/views/oidc_accounts/link.html.erbapp/models/user.rbapp/controllers/sessions_controller.rbapp/controllers/oidc_accounts_controller.rbapp/models/oidc_identity.rbapp/views/sessions/new.html.erb
{app/javascript/controllers/**/*.{js,ts},app/views/**/*.erb,app/components/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Use declarative Stimulus actions in ERB (data-action) instead of imperative event listeners in controller lifecycle (e.g., avoid addEventListener in connect); controllers should just respond to actions
Files:
app/views/oidc_accounts/link.html.erbapp/views/sessions/new.html.erb
{app/components/**/*.{js,ts,erb},app/views/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Component-scoped Stimulus controllers in app/components must be used only within their component views, not in app/views
Files:
app/views/oidc_accounts/link.html.erbapp/views/sessions/new.html.erb
{app/views/**,app/helpers/**,app/javascript/controllers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
{app/views/**,app/helpers/**,app/javascript/controllers/**}: Style UI using TailwindCSS v4.x with the custom design system defined in app/assets/tailwind/maybe-design-system.css
Always start by referencing app/assets/tailwind/maybe-design-system.css to understand available primitives, functional tokens, and component tokens before styling
Prefer functional tokens from the design system over raw Tailwind values (e.g., use text-primary, bg-container, border border-primary instead of text-white, bg-white, border-gray-200)
Files:
app/views/oidc_accounts/link.html.erbapp/views/sessions/new.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/oidc_accounts/link.html.erbapp/views/sessions/new.html.erb
{app/views,app/components}/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
{app/views,app/components}/**/*.html.erb: Keep domain logic out of ERB templates; compute values in component/controller code and reference them in the template
Integrate Stimulus declaratively in ERB: templates declare data-controller/data-action; controllers respond to those declarations
Pass data from Rails to Stimulus via data-*-value attributes, not inline JavaScript
Files:
app/views/oidc_accounts/link.html.erbapp/views/sessions/new.html.erb
app/models/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-design.mdc)
Domain models should not call Provider::Registry directly; use a Provided concern within the model’s namespace to select providers and expose convenience methods
Use ActiveRecord validations for forms and complex domain constraints.
app/models/**/*.rb: Place business logic in POROs and model classes under app/models
Models should answer questions about themselves (e.g., prefer account.balance_series over service objects)
Implement complex validations and business logic with ActiveRecord validations
Model-level validations may mirror DB constraints but are not strictly required
Files:
app/models/user.rbapp/models/oidc_identity.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/
Files:
app/models/user.rbapp/controllers/sessions_controller.rbapp/controllers/oidc_accounts_controller.rbapp/models/oidc_identity.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
app/models/user.rbapp/controllers/sessions_controller.rbapp/controllers/oidc_accounts_controller.rbapp/models/oidc_identity.rb
**/*.{rb,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{rb,js,jsx,ts,tsx}: Make changes atomic, testable, and explain their impact briefly in code suggestions.
Respect existing tests and add tests when changing critical logic.
Files:
app/models/user.rbapp/controllers/sessions_controller.rbapp/controllers/oidc_accounts_controller.rbapp/models/oidc_identity.rb
**/app/models/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Business logic should primarily reside in models; use concerns and POROs for organization.
Files:
app/models/user.rbapp/models/oidc_identity.rb
app/{helpers,models}/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, and dates on the server side (Ruby) before sending to the client
Files:
app/models/user.rbapp/models/oidc_identity.rb
**/app/controllers/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Controllers should be skinny; business logic should go in models (
app/models/), not inapp/services/.
Files:
app/controllers/sessions_controller.rbapp/controllers/oidc_accounts_controller.rb
app/assets/**/*.{css,scss,sass,pcss,png,jpg,jpeg,gif,svg,webp,woff,woff2,ttf,otf}
📄 CodeRabbit inference engine (AGENTS.md)
Store styles and static assets (Tailwind, images, fonts) under app/assets/
Files:
app/assets/tailwind/application.cssapp/assets/tailwind/google-sign-in.css
app/assets/**/*.{css,scss,sass,pcss}
📄 CodeRabbit inference engine (AGENTS.md)
Let Biome format CSS/SCSS styles (npm run lint/format)
Files:
app/assets/tailwind/application.cssapp/assets/tailwind/google-sign-in.css
**/app/assets/tailwind/*.css
📄 CodeRabbit inference engine (CLAUDE.md)
**/app/assets/tailwind/*.css: NEVER create new styles in design system files without permission.
Use functional tokens (e.g.,text-primary,bg-container) as defined in the design system; do not use hardcoded color classes (e.g.,text-white,bg-white).
Files:
app/assets/tailwind/application.cssapp/assets/tailwind/google-sign-in.css
app/assets/tailwind/{maybe-design-system.css,application.css}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Do not create new styles in app/assets/tailwind/maybe-design-system.css or app/assets/tailwind/application.css without explicit permission
Files:
app/assets/tailwind/application.css
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Store application and environment configuration under config/
Files:
config/locales/views/sessions/en.yml
config/locales/**/*.{yml,yaml,json}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
Ignore i18n files; do not add or modify locale files
Files:
config/locales/views/sessions/en.yml
.env.{local,test}.example
📄 CodeRabbit inference engine (AGENTS.md)
Maintain environment example files as .env.local.example and .env.test.example
Files:
.env.local.example
🧠 Learnings (1)
📚 Learning: 2025-09-23T22:23:17.441Z
Learnt from: CR
PR: we-promise/sure#0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-09-23T22:23:17.441Z
Learning: Applies to app/assets/tailwind/{maybe-design-system.css,application.css} : Do not create new styles in app/assets/tailwind/maybe-design-system.css or app/assets/tailwind/application.css without explicit permission
Applied to files:
app/assets/tailwind/application.css
🧬 Code graph analysis (2)
app/controllers/sessions_controller.rb (4)
app/controllers/oidc_accounts_controller.rb (1)
skip_authentication(1-125)app/controllers/concerns/authentication.rb (2)
skip_authentication(11-14)create_session_for(40-44)app/models/current.rb (1)
user(8-10)app/models/oidc_identity.rb (1)
record_authentication!(9-11)
app/controllers/oidc_accounts_controller.rb (1)
app/controllers/concerns/authentication.rb (1)
create_session_for(40-44)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 67-67: [UnorderedKey] The OIDC_CLIENT_ID key should go before the OIDC_ISSUER key
(UnorderedKey)
[warning] 68-68: [UnorderedKey] The OIDC_CLIENT_SECRET key should go before the OIDC_ISSUER key
(UnorderedKey)
.env.local.example
[warning] 18-18: [UnorderedKey] The OIDC_CLIENT_ID key should go before the OIDC_ISSUER key
(UnorderedKey)
[warning] 19-19: [UnorderedKey] The OIDC_CLIENT_SECRET key should go before the OIDC_ISSUER key
(UnorderedKey)
🪛 GitHub Check: ci / lint
app/controllers/oidc_accounts_controller.rb
[failure] 2-2:
Layout/SpaceInsideArrayLiteralBrackets: Use space inside array brackets.
[failure] 2-2:
Layout/SpaceInsideArrayLiteralBrackets: Use space inside array brackets.
🔇 Additional comments (11)
app/assets/tailwind/application.css (1)
13-13: Import structure looks good.The import of the Google sign-in styles is properly placed and follows the existing pattern.
Note: Ensure the imported stylesheet addresses design system token usage (see comment on
google-sign-in.css).app/models/user.rb (1)
13-13: LGTM! Association properly configured.The
has_many :oidc_identitiesassociation withdependent: :destroyensures proper cleanup of linked OIDC identities when a user is deleted.config/locales/views/sessions/en.yml (1)
8-21: LGTM! Translation keys properly structured.The OpenID Connect and failure translations are correctly nested under the
sessionsnamespace and follow the existing locale file structure.app/models/oidc_identity.rb (1)
1-11: LGTM! Solid model foundation.The validations are appropriate, especially the uniqueness constraint on
uidscoped toprovider, which ensures each provider+uid pair is unique. Therecord_authentication!method provides a clean way to track authentication timestamps.app/controllers/sessions_controller.rb (3)
3-3: LGTM! Correct authentication bypass.Adding
openid_connectandfailureto the skip list is appropriate since these actions are part of the authentication flow and must be accessible to unauthenticated users.
30-68: Excellent security improvements! Previous vulnerabilities addressed.This implementation correctly addresses all the critical security concerns raised in previous reviews:
✅ Account takeover prevention (line 40): Now authenticates using
provider + uidlookup instead of email-only, preventing attackers from hijacking accounts by controlling matching email addresses on the OIDC provider.✅ MFA enforcement (lines 48-50): MFA verification is now required when
user.otp_required?is true, closing the MFA bypass vulnerability.✅ Nil safety (lines 34-37): Proper validation of auth data prevents NoMethodError exceptions.
The pending auth flow (lines 58-66) safely stores session data for the account linking process when no existing OIDC identity is found.
70-72: The translation key is correctly defined and the code requires no changes.The key
t(".failed")in thefailureaction properly resolves tosessions.failure.failed, which exists inconfig/locales/views/sessions/en.ymlwith the message "Could not authenticate." The translation is appropriately scoped to the failure action, separate from theopenid_connectaction's translation.app/controllers/oidc_accounts_controller.rb (4)
5-16: LGTM! Clean session validation.The action properly validates pending OIDC data and prepares instance variables for the view. The conditional check on line 15 prevents unnecessary database queries when email is missing.
18-60: Good security flow, but note code duplication.The password verification requirement (line 27) before linking is a critical security measure that prevents account takeover. The MFA flow integration (lines 47-49) is also correct.
However, lines 31-41 duplicate the OIDC identity creation logic. See the review comment on
app/models/oidc_identity.rblines 14-26 for a refactoring suggestion to useOidcIdentity.create_from_omniauthinstead.
62-77: LGTM! Clean form preparation.Pre-filling the user form with data from the OIDC provider (lines 72-75) provides a good user experience while still allowing the user to edit the information if needed.
79-124: Note code duplication and consider password recovery.Code duplication: Lines 103-113 duplicate the OIDC identity creation logic. See the review comment on
app/models/oidc_identity.rblines 14-26 for consolidation suggestions.Password lockout consideration: Line 88 generates a secure random password that the user never sees, meaning they can only authenticate via OIDC. If the OIDC provider becomes unavailable or the user loses access, they'll be locked out of their account. Consider whether you want to provide a password reset flow or document this as OIDC-only authentication.
Do you want to add a password recovery mechanism for OIDC-created accounts, or is OIDC-only authentication the intended behavior?
| def self.create_from_omniauth(auth, user) | ||
| create!( | ||
| user: user, | ||
| provider: auth.provider, | ||
| uid: auth.uid, | ||
| info: { | ||
| email: auth.info&.email, | ||
| name: auth.info&.name, | ||
| first_name: auth.info&.first_name, | ||
| last_name: auth.info&.last_name | ||
| } | ||
| ) | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Unused method creating code duplication.
This create_from_omniauth method is not invoked anywhere in the codebase. Instead, controllers directly call user.oidc_identities.create! with inline parameters (see app/controllers/oidc_accounts_controller.rb lines 31-41 and 103-113), duplicating the info extraction logic.
Additionally, this method doesn't set last_authenticated_at, but the controller code does.
Option 1 (recommended): Update this method to set last_authenticated_at and use it in controllers to eliminate duplication:
def self.create_from_omniauth(auth, user)
create!(
user: user,
provider: auth.provider,
uid: auth.uid,
info: {
email: auth.info&.email,
name: auth.info&.name,
first_name: auth.info&.first_name,
last_name: auth.info&.last_name
- }
+ },
+ last_authenticated_at: Time.current
)
endThen replace the direct create! calls in OidcAccountsController with:
# In create_link action (line 31)
oidc_identity = OidcIdentity.create_from_omniauth(
OpenStruct.new(
provider: @pending_auth["provider"],
uid: @pending_auth["uid"],
info: OpenStruct.new(@pending_auth.slice("email", "name", "first_name", "last_name"))
),
user
)
# Similar change in create_user action (line 103)Option 2: Remove this method entirely if you prefer the explicit approach.
🤖 Prompt for AI Agents
In app/models/oidc_identity.rb around lines 14 to 26, the create_from_omniauth
method is unused and duplicates controller logic; update this method to also set
last_authenticated_at (e.g. set to Time.current) and accept the auth object used
by controllers (supporting auth.info as a hash or OpenStruct) when building the
info hash, then replace the inline user.oidc_identities.create! calls in
app/controllers/oidc_accounts_controller.rb at lines 31-41 and 103-113 to call
OidcIdentity.create_from_omniauth(auth, user) so the extraction of
email/name/first_name/last_name and last_authenticated_at is centralized (or
remove the model method if you prefer the explicit controller approach).
| <div class="mb-6 p-4 bg-yellow-50 border border-yellow-200 rounded-md"> | ||
| <h3 class="text-sm font-medium text-yellow-800 mb-2">Verify Your Identity</h3> | ||
| <p class="text-sm text-yellow-700"> | ||
| To link your <%= @pending_auth["provider"] %> account<% if @pending_auth["email"].present? %> (<%= @pending_auth["email"] %>)<% end %>, | ||
| please verify your identity by entering your password. | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
Replace hardcoded color classes with design system tokens.
The alert boxes use hardcoded Tailwind color classes (bg-yellow-50, border-yellow-200, text-yellow-800, etc.) instead of functional design system tokens. This pattern repeats throughout the file (lines 36-42, 45-54).
As per coding guidelines, use functional tokens like bg-container, text-primary, border-primary instead of raw color classes. Consider creating alert component tokens in the design system (e.g., bg-alert-warning, text-alert-warning) or using existing semantic tokens.
🤖 Prompt for AI Agents
In app/views/oidc_accounts/link.html.erb around lines 6 to 12 (and similarly at
lines 36-42 and 45-54), replace hardcoded Tailwind color classes (bg-yellow-50,
border-yellow-200, text-yellow-800, text-yellow-700, etc.) with
semantic/functional design tokens such as bg-container or the project’s alert
tokens (e.g., bg-alert-warning, border-alert-warning, text-alert-warning or
text-primary) so the view uses the design system; update the class attributes to
use those tokens and, if the required alert tokens do not exist yet, add
corresponding tokens to the design system (or map them in the component helper)
before using them in the markup.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
test/controllers/oidc_accounts_controller_test.rb (4)
19-24: Remove unused helper method.The
set_pending_authhelper is never called in any test. All tests passpending_oidc_authdirectly via thesession:parameter instead. This is dead code.Apply this diff to remove the unused helper:
- def set_pending_auth(auth_data) - # Set session data by making a request that stores it - post sessions_path, env: { - "rack.session" => { pending_oidc_auth: auth_data } - }, params: {} - end -
51-66: Consider adding assertions for completeness.The test covers the critical path (OIDC identity creation), but could optionally verify:
last_authenticated_atis set on the created identity- A session is created (user is logged in)
pending_oidc_authis cleared from the sessionThese additions would make the test more comprehensive, though the current coverage is adequate for the core functionality.
Example additions:
# After line 65, add: identity = @user.oidc_identities.find_by( provider: pending_auth["provider"], uid: pending_auth["uid"] ) assert_not_nil identity.last_authenticated_at assert Session.exists?(user_id: @user.id)
82-94: Add assertions to verify complete MFA flow.The test verifies the redirect to MFA but doesn't confirm the expected session state. Based on the MFA flow in
test/controllers/sessions_controller_test.rb(lines 82-94), the controller should:
- Set
mfa_user_idin the session- NOT create a user session yet (authentication completes after MFA verification)
- Optionally create the OIDC identity (or defer until after MFA)
Add these assertions after line 93:
# Verify MFA flow state assert_equal @user.id, session[:mfa_user_id] assert_not Session.exists?(user_id: @user.id), "Session should not be created before MFA verification"
125-145: Comprehensive new user registration test - consider minor additions.The test thoroughly verifies user, OIDC identity, and family creation with correct data. Optionally, you could add assertions for:
last_authenticated_atbeing set on the OIDC identity- User being assigned to the newly created family
Example additions after line 144:
# Verify last_authenticated_at is set assert_not_nil oidc_identity.last_authenticated_at # Verify user is assigned to the new family assert_equal new_user.family, Family.order(:created_at).last
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/controllers/oidc_accounts_controller.rb(1 hunks)db/migrate/20251024083624_create_oidc_identities.rb(1 hunks)test/controllers/oidc_accounts_controller_test.rb(1 hunks)test/test_helper.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/controllers/oidc_accounts_controller.rb
🧰 Additional context used
📓 Path-based instructions (7)
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
Name Minitest files with *_test.rb and mirror the app/ structure under test/
test/**/*_test.rb: ALWAYS use Minitest + fixtures + Mocha for tests; NEVER RSpec or FactoryBot.
Use Mocha for mocking in tests when necessary.
Use VCR for external API tests.
test/**/*_test.rb: Always use Minitest for tests; do not use RSpec
Do not use factories (e.g., FactoryBot) in tests; rely on fixtures instead
For tests needing many records, use Rails helpers to construct data and inline the creation in the test
Only write tests for critical and important code paths
Avoid tests that merely verify framework/ActiveRecord behavior
Test boundaries correctly: for queries, assert returned values; for commands, assert collaborators are called with correct params
Never test the implementation details of one class in another class’s test suite
Use the mocha gem for stubs and mocks
Prefer OpenStruct for mock instances; use a mock class for complex cases
Only mock what’s necessary; don’t mock return values unless they are under test
Files:
test/controllers/oidc_accounts_controller_test.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
test/controllers/oidc_accounts_controller_test.rbtest/test_helper.rbdb/migrate/20251024083624_create_oidc_identities.rb
**/*.{rb,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{rb,js,jsx,ts,tsx}: Make changes atomic, testable, and explain their impact briefly in code suggestions.
Respect existing tests and add tests when changing critical logic.
Files:
test/controllers/oidc_accounts_controller_test.rbtest/test_helper.rbdb/migrate/20251024083624_create_oidc_identities.rb
**/test/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
**/test/**/*.rb: ALWAYS use Minitest and fixtures for Ruby tests; NEVER RSpec or factories.
Only test critical and important code paths; system tests should be used sparingly for critical user flows.
Files:
test/controllers/oidc_accounts_controller_test.rbtest/test_helper.rb
**/*.{rb,erb,haml,slim}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb,haml,slim}: UseCurrent.userfor the current user; do not usecurrent_user
UseCurrent.familyfor the current family; do not usecurrent_family
Ignore i18n methods; hardcode strings in English for now (do not useI18n.t,t, or similar)
Files:
test/controllers/oidc_accounts_controller_test.rbtest/test_helper.rbdb/migrate/20251024083624_create_oidc_identities.rb
db/migrate/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place database migrations under db/migrate/
Simple validations like null checks and unique constraints should be handled by database constraints in migration files.
ActiveRecord migrations must inherit from
ActiveRecord::Migration[7.2]; do not use version 8.0
Files:
db/migrate/20251024083624_create_oidc_identities.rb
db/migrate/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Enforce simple validations in the database (NULL constraints, unique indexes, etc.)
Files:
db/migrate/20251024083624_create_oidc_identities.rb
🧬 Code graph analysis (1)
test/controllers/oidc_accounts_controller_test.rb (4)
test/controllers/sessions_controller_test.rb (1)
setup(3-150)test/test_helper.rb (1)
user_password_test(77-79)app/models/user.rb (2)
setup_mfa!(126-132)enable_mfa!(134-139)app/controllers/oidc_accounts_controller.rb (1)
new_user(62-77)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
🔇 Additional comments (7)
db/migrate/20251024083624_create_oidc_identities.rb (1)
1-15: LGTM! Well-structured migration following Rails conventions.The migration correctly:
- Uses ActiveRecord::Migration[7.2] as required
- Enforces database-level constraints (NOT NULL, unique index)
- Uses UUID for primary and foreign keys
- Includes a unique composite index on
[provider, uid]to prevent duplicate OIDC identitiesThe
jsonbcolumn forinfoprovides flexibility for storing provider-specific data.test/controllers/oidc_accounts_controller_test.rb (6)
45-49: LGTM! Good negative test case.Properly verifies that the controller redirects when no pending authentication data exists in the session.
68-80: LGTM! Good negative test case.Properly verifies that linking fails with invalid credentials and no OIDC identity is created.
96-104: LGTM! Good negative test case.Properly validates that the controller requires pending authentication data.
118-123: LGTM! Good UI verification test.Properly checks that the new account creation option is displayed with the correct email.
147-153: LGTM! Good session verification test.This test nicely complements the user creation test by specifically verifying the session is established after OIDC registration.
155-160: LGTM! Good negative test case.Properly validates error handling when no pending authentication data exists.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/controllers/sessions_controller_test.rb (1)
111-133: LGTM! Account linking flow test is solid.The test correctly verifies that users without an existing OIDC identity are redirected to the account linking page.
Minor note: The text assertion on line 132 (
"To link your openid_connect account") couples the test to exact view wording, which could make the test brittle if the message changes. Consider using a more structural assertion (e.g., checking for a specific CSS class or data attribute) if this becomes an issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
db/schema.rb(4 hunks)test/controllers/oidc_accounts_controller_test.rb(1 hunks)test/controllers/sessions_controller_test.rb(2 hunks)test/test_helper.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/controllers/oidc_accounts_controller_test.rb
🧰 Additional context used
📓 Path-based instructions (6)
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
test/test_helper.rbdb/schema.rbtest/controllers/sessions_controller_test.rb
**/*.{rb,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{rb,js,jsx,ts,tsx}: Make changes atomic, testable, and explain their impact briefly in code suggestions.
Respect existing tests and add tests when changing critical logic.
Files:
test/test_helper.rbdb/schema.rbtest/controllers/sessions_controller_test.rb
**/test/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
**/test/**/*.rb: ALWAYS use Minitest and fixtures for Ruby tests; NEVER RSpec or factories.
Only test critical and important code paths; system tests should be used sparingly for critical user flows.
Files:
test/test_helper.rbtest/controllers/sessions_controller_test.rb
**/*.{rb,erb,haml,slim}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb,haml,slim}: UseCurrent.userfor the current user; do not usecurrent_user
UseCurrent.familyfor the current family; do not usecurrent_family
Ignore i18n methods; hardcode strings in English for now (do not useI18n.t,t, or similar)
Files:
test/test_helper.rbdb/schema.rbtest/controllers/sessions_controller_test.rb
db/schema.rb
📄 CodeRabbit inference engine (.cursor/rules/project-design.mdc)
Treat db/schema.rb as the authoritative source of truth for model relationships
Files:
db/schema.rb
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
Name Minitest files with *_test.rb and mirror the app/ structure under test/
test/**/*_test.rb: ALWAYS use Minitest + fixtures + Mocha for tests; NEVER RSpec or FactoryBot.
Use Mocha for mocking in tests when necessary.
Use VCR for external API tests.
test/**/*_test.rb: Always use Minitest for tests; do not use RSpec
Do not use factories (e.g., FactoryBot) in tests; rely on fixtures instead
For tests needing many records, use Rails helpers to construct data and inline the creation in the test
Only write tests for critical and important code paths
Avoid tests that merely verify framework/ActiveRecord behavior
Test boundaries correctly: for queries, assert returned values; for commands, assert collaborators are called with correct params
Never test the implementation details of one class in another class’s test suite
Use the mocha gem for stubs and mocks
Prefer OpenStruct for mock instances; use a mock class for complex cases
Only mock what’s necessary; don’t mock return values unless they are under test
Files:
test/controllers/sessions_controller_test.rb
🧬 Code graph analysis (1)
test/controllers/sessions_controller_test.rb (3)
app/controllers/mfa_controller.rb (1)
new(5-8)test/models/current_test.rb (1)
test(3-9)app/models/user.rb (2)
setup_mfa!(126-132)enable_mfa!(134-139)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
🔇 Additional comments (7)
test/test_helper.rb (1)
39-42: LGTM! Past review concern resolved.The OmniAuth test configuration is now correct. Setting
test_mode = trueenables mock authentication and prevents real OAuth provider requests during tests, which was the concern raised in the previous review. Allowing both GET and POST forallowed_request_methodsis standard practice for OIDC callback handling.db/schema.rb (2)
546-556: LGTM! Well-structured OIDC identities table.The
oidc_identitiestable schema is properly designed:
- Required fields (
user_id,provider,uid) are markednot null- Unique index on
(provider, uid)prevents duplicate identities from the same provider- JSONB
infofield with default empty object for flexible attribute storagelast_authenticated_attimestamp for tracking authentication events
948-948: LGTM! Proper foreign key constraint.The foreign key from
oidc_identitiestousersensures referential integrity and will cascade appropriately when users are deleted.test/controllers/sessions_controller_test.rb (4)
8-24: LGTM! Clean test setup and helper.The teardown properly clears OmniAuth mock state between tests, preventing test pollution. The
setup_omniauth_mockhelper is well-designed with sensible defaults and uses.compactto remove nil values from the info hash.
71-109: LGTM! Comprehensive authentication flow tests.These tests properly cover:
- Successful authentication with an existing OIDC identity (lines 71-88)
- MFA redirection when the user has MFA enabled (lines 90-109)
Both tests correctly verify session creation/non-creation and proper redirects. The MFA test appropriately checks that the session is NOT created immediately but instead stores the user ID for MFA verification.
135-146: LGTM! Proper error handling test.The test correctly simulates authentication failure by providing incomplete auth data (nil provider/uid) and verifies the system responds with an appropriate error message and redirect to login.
148-171: Excellent security test! Critical protection against account takeover.This test is particularly important as it verifies that authentication cannot succeed by email matching alone. The system correctly requires a matching OIDC identity (provider + uid pair) to authenticate, preventing attackers from taking over accounts by using the same email address with a different OIDC provider or account.
The assertion on line 166 explicitly confirms that no session is created for an unlinked identity, ensuring the user must go through the account linking flow first.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/controllers/sessions_controller.rb(2 hunks)config/initializers/omniauth.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/
Files:
app/controllers/sessions_controller.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
app/controllers/sessions_controller.rbconfig/initializers/omniauth.rb
**/*.{rb,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{rb,js,jsx,ts,tsx}: Make changes atomic, testable, and explain their impact briefly in code suggestions.
Respect existing tests and add tests when changing critical logic.
Files:
app/controllers/sessions_controller.rbconfig/initializers/omniauth.rb
**/app/**/*.{rb,erb,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Push Rails to its limits before adding new dependencies; a strong technical/business reason is required for new dependencies.
Files:
app/controllers/sessions_controller.rb
**/app/controllers/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Controllers should be skinny; business logic should go in models (
app/models/), not inapp/services/.
Files:
app/controllers/sessions_controller.rb
**/*.{rb,erb,haml,slim}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb,haml,slim}: UseCurrent.userfor the current user; do not usecurrent_user
UseCurrent.familyfor the current family; do not usecurrent_family
Ignore i18n methods; hardcode strings in English for now (do not useI18n.t,t, or similar)
Files:
app/controllers/sessions_controller.rbconfig/initializers/omniauth.rb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/controllers/sessions_controller.rb
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Store application and environment configuration under config/
Files:
config/initializers/omniauth.rb
🧬 Code graph analysis (2)
app/controllers/sessions_controller.rb (3)
app/controllers/oidc_accounts_controller.rb (1)
skip_authentication(1-125)app/controllers/concerns/authentication.rb (2)
skip_authentication(11-14)create_session_for(40-44)app/models/oidc_identity.rb (1)
record_authentication!(9-11)
config/initializers/omniauth.rb (1)
config/application.rb (1)
config(10-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
🔇 Additional comments (6)
app/controllers/sessions_controller.rb (3)
3-3: LGTM! Correctly exempts OIDC callback actions from authentication.The openid_connect and failure actions are callback entry points from the external OIDC provider and must skip authentication, just like the existing new and create actions.
30-68: LGTM! Security issues from previous reviews have been addressed.The implementation now correctly:
- Validates provider and uid presence before proceeding
- Looks up OIDC identities by
(provider, uid)tuple instead of email alone, preventing account takeover- Enforces MFA when
user.otp_required?is true, preventing MFA bypass- Redirects unknown identities to an explicit account-linking flow rather than auto-creating accounts
The session storage for pending auth is appropriately scoped and uses safe navigation for optional fields.
70-72: LGTM! Standard OmniAuth failure callback.The failure action appropriately redirects to the login page with an alert when OIDC authentication fails.
config/initializers/omniauth.rb (3)
7-9: LGTM! Environment variable validation with test-mode support.The initializer correctly validates required OIDC environment variables and permits test mode to proceed with fallback defaults.
18-22: LGTM! Client options correctly fall back to test defaults.The client_options configuration properly falls back to test values when environment variables are not set, since
ENV["KEY"]returnsnil(not an empty string) andnil || "default"works as intended.
24-27: LGTM! Proper fail-fast behavior for production.The initializer correctly implements fail-fast configuration validation: logging a warning in non-production environments while raising an error with specific details in production to prevent runtime OIDC failures.
- Set a config item `configuration.x.auth.oidc_enabled` - Hide button if disabled
sokie
left a comment
sokie
left a comment
There was a problem hiding this comment.
Things to address still, good for an issue:
- dead code path at
create_from_omniauth - Hardcoded color classes
- Button around create new user
@jjmata
I tested all paths and they work correctly:
- email and password login, still works
- register new user with email and pass works
- oauth new user works
- oauth existing user and link works
* Add OpenID Connect login support * Add docs for OIDC config with Google Auth * Use Google styles for log in - Add support for linking existing account - Force users to sign-in with passoword first, when linking existing accounts - Add support to create new user when using OIDC - Add identities to user to prevent account take-ver - Make tests mocking instead of being integration tests - Manage session handling correctly - use OmniAuth.config.mock_auth instead of passing auth data via request env * Conditionally render Oauth button - Set a config item `configuration.x.auth.oidc_enabled` - Hide button if disabled --------- Signed-off-by: Juan José Mata <[email protected]> Signed-off-by: soky srm <[email protected]> Co-authored-by: sokie <[email protected]>
commit 9fefe57 Author: João Felipe <[email protected]> Date: Wed Oct 29 02:15:14 2025 +0400 Feature/yahoo finance (we-promise#123) * Implement Yahoo Finance * Added tests * Updated hosting controller to check for managed app_mode instead of env_override * Suggestions from CodeRabbit and Fixes on tests * Remove Css changes * Fix yahoo finance impl and i18n * Updated view to use healthy method * remove usage * Updated env example * keep usage on class just to keep same format * Ci test * Remove some useless validations * Remove logs * Linter fixes * Broke this in my conflict merge * Wrong indentation level --------- Signed-off-by: Juan José Mata <[email protected]> Co-authored-by: Juan José Mata <[email protected]> commit 3910116 Author: Albert Solà <[email protected]> Date: Tue Oct 28 20:23:20 2025 +0000 Update pages/dashboard locales (we-promise#255) commit 4fb0a38 Author: soky srm <[email protected]> Date: Tue Oct 28 19:32:27 2025 +0100 Providers factory (we-promise#250) * Implement providers factory * Multiple providers sync support - Proper Multi-Provider Syncing: When you click sync on an account with multiple providers (e.g., both Plaid and SimpleFin), all provider items are synced - Better API: The existing account.providers method already returns all providers, and account.provider returns the first one for backward compatibility - Correct Holdings Deletion Logic: Holdings can only be deleted if ALL providers allow it, preventing accidental deletions that would be recreated on next sync TODO: validate this is the way we want to go? We would need to check holdings belong to which account, and then check provider allows deletion. More complex - Database Constraints: The existing validations ensure an account can have at most one provider of each type (one PlaidAccount, one SimplefinAccount, etc.) * Add generic provider_import_adapter * Finish unified import strategy * Update app/models/plaid_account.rb Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: soky srm <[email protected]> * Update app/models/provider/factory.rb Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: soky srm <[email protected]> * Fix account linked by plaid_id instead of external_id * Parse numerics to BigDecimal Parse numerics to BigDecimal before computing amount; guard nils. Avoid String * String and float drift; also normalize date. * Fix incorrect usage of assert_raises. * Fix linter * Fix processor test. * Update current_balance_manager.rb * Test fixes * Fix plaid linked account test * Add support for holding per account_provider * Fix proper account access Also fix account deletion for simpefin too * FIX match tests for consistency * Some more factory updates * Fix account schema for multipe providers Can do: - Account #1 → PlaidAccount + SimplefinAccount (multiple different providers) - Account #2 → PlaidAccount only - Account #3 → SimplefinAccount only Cannot do: - Account #1 → PlaidAccount + PlaidAccount (duplicate provider type) - PlaidAccount we-promise#123 → Account #1 + Account #2 (provider linked to multiple accounts) * Fix account setup - An account CAN have multiple providers (the schema shows account_providers with unique index on [account_id, provider_type]) - Each provider should maintain its own separate entries - We should NOT update one provider's entry when another provider syncs * Fix linter and guard migration * FIX linter issues. * Fixes - Remove duplicated index - Pass account_provider_id - Guard holdings call to avoid NoMethodError * Update schema and provider import fix * Plaid doesn't allow holdings deletion * Use ClimateControl for proper env setup * No need for this in .git --------- Signed-off-by: soky srm <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Juan José Mata <[email protected]> commit 72e7d77 Author: Juan José Mata <[email protected]> Date: Mon Oct 27 21:52:37 2025 +0100 Add onboarding state selector for self-hosted signup (we-promise#251) * Add onboarding modes to self-hosted signup * Style form consistently * Configure ONBOARDING_STATE via ENV commit dcb6748 Author: ampersandru <[email protected]> Date: Mon Oct 27 04:18:45 2025 -0700 Added a clickable drop down arrow that reveals list of instructions for Brand Fetch Client ID (we-promise#246) * Added a clickable drop down arrow that reveals list of instructions for Brand Fetch Client ID Signed-off-by: ampersandru <[email protected]> * Updated html code based on recommendations Signed-off-by: ampersandru <[email protected]> * Update _brand_fetch_settings.html.erb Signed-off-by: ampersandru <[email protected]> * Small edit for consistency with Twelve Data settings flow * HTML linting * Missed an extra closing tag. --------- Signed-off-by: ampersandru <[email protected]> Signed-off-by: Juan José Mata <[email protected]> Co-authored-by: Juan José Mata <[email protected]> commit cca14c9 Author: ampersandru <[email protected]> Date: Mon Oct 27 03:45:01 2025 -0700 Added a clickable drop down arrow that reveals list of instructions for Twelve Data Secret Key (we-promise#247) * Added a clickable drop down arrow that reveals list of instructions for Twelve Data Secret Key Signed-off-by: ampersandru <[email protected]> * Fixed html spacing Signed-off-by: ampersandru <[email protected]> * Make display even more compact * Remove <u> and use CSS classes instead --------- Signed-off-by: ampersandru <[email protected]> Co-authored-by: Juan José Mata <[email protected]> commit 0fc70e9 Author: Juan José Mata <[email protected]> Date: Sun Oct 26 23:16:25 2025 +0100 Add runServices for db and redis in devcontainer Signed-off-by: Juan José Mata <[email protected]> commit d3d91f4 Author: Juan José Mata <[email protected]> Date: Sun Oct 26 20:53:18 2025 +0100 Add PikaPods One-click installs Signed-off-by: Juan José Mata <[email protected]> commit 53eb90f Author: ampersandru <[email protected]> Date: Sun Oct 26 09:58:18 2025 -0700 Added instructions on where and how to obtain API key for Twelve Data (we-promise#235) * Added instructions on where and how to obtain API key for Twelve Data Signed-off-by: ampersandru <[email protected]> * added rel="noopener noreferrer" to links Signed-off-by: ampersandru <[email protected]> --------- Signed-off-by: ampersandru <[email protected]> commit a35a4ac Author: Juan José Mata <[email protected]> Date: Sun Oct 26 17:51:31 2025 +0100 Update version to 0.6.5-alpha.3 Prepare for alpha.3 Signed-off-by: Juan José Mata <[email protected]> commit f42e6e3 Author: Albert Solà <[email protected]> Date: Sun Oct 26 16:50:31 2025 +0000 Added translations for ca - Catalan (we-promise#238) * Add CA locales for models * Add CA locales for views * Use translations in activity feed * Additional CA locales * Fix typo --------- Co-authored-by: Juan José Mata <[email protected]> commit db19c95 Author: ampersandru <[email protected]> Date: Sun Oct 26 09:47:29 2025 -0700 Add instructions on how to obtain Brand Logo Client ID API (we-promise#234) * Add instructions on how to obtain Brand Logo Client ID API Signed-off-by: ampersandru <[email protected]> * updated html based on recommendations Signed-off-by: ampersandru <[email protected]> * added rel="noopener noreferrer" to links Signed-off-by: ampersandru <[email protected]> --------- Signed-off-by: ampersandru <[email protected]> commit 0b393a0 Author: OrangeDrangon <[email protected]> Date: Sun Oct 26 11:58:26 2025 -0400 add custom s3 support storage config option (we-promise#239) Options are documented here including an example on how to set a custom endpoint: https://guides.rubyonrails.org/active_storage_overview.html#s3-service-amazon-s3-and-s3-compatible-apis commit fea228d Author: luckyPipewrench <[email protected]> Date: Sun Oct 26 10:50:45 2025 -0400 Simplefin sync improvements (we-promise#240) * Fix syncing issues with new connections and accounts.. - Keep SimpleFin institution metadata strictly per account (`simplefin_accounts.org_data`). - Relax `simplefin_items` institution constraints to allow creating items before org data exists. - Remove code that copied the first account’s `org` onto `simplefin_items`. * Improve Simplefin Sync • SimpleFin: family “Sync” includes SimpleFin items; importer does unbounded discovery (with pending=1 fallback) before windowed fetch, for both regular and first syncs. • Stop populating item‑level institution fields; keep institution metadata per account. • Relax NOT NULL on item institution fields. • Post‑sync dashboard broadcasts are now guarded (UI cannot fail the job). • Show a friendly “daily refresh limit” banner on the SimpleFin card when the latest sync is rate‑limited. • Add bin/rails sure:simplefin:debug[ITEM_ID] to print latest sync, snapshot account count, simplefin_accounts count, and unlinked list. * Fixed double‑quoted strings, spacing around array brackets and commas * chore: ignore local .junie files * - Broadcast error logs now include full backtraces - SimpleFin discovery logic deduplicated fixed - app/models/simplefin_item/importer.rb --Added a concise docstring for perform_account_discovery describing purpose, steps, and side‑effects. --Added a docstring for fetch_accounts_data describing params and return value. commit f9f6274 Author: Juan José Mata <[email protected]> Date: Sat Oct 25 22:57:49 2025 +0200 Make logo work in dark mode Signed-off-by: Juan José Mata <[email protected]> commit 1ee20ab Author: Copilot <[email protected]> Date: Sat Oct 25 22:23:28 2025 +0200 Eliminate code duplication in OIDC identity creation (we-promise#230) * Eliminate duplication by using create_from_omniauth method - Updated OidcIdentity.create_from_omniauth to set last_authenticated_at - Refactored OidcAccountsController to use create_from_omniauth instead of direct create! calls - Updated test to verify last_authenticated_at is set by create_from_omniauth Co-authored-by: jjmata <[email protected]> * Extract auth hash building into private helper method - Added build_auth_hash helper method to eliminate OpenStruct creation duplication - Both create_link and create_user actions now use the same helper Co-authored-by: jjmata <[email protected]> * Linter fix * Fix button style on OIDC link step * Fix dark mode styles --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jjmata <[email protected]> Co-authored-by: Juan José Mata <[email protected]> commit b24b102 Author: luckyPipewrench <[email protected]> Date: Fri Oct 24 14:41:33 2025 -0400 Fix rounding issue (we-promise#226) Remove rounding from transactions. commit 03cac95 Author: soky srm <[email protected]> Date: Fri Oct 24 19:39:42 2025 +0200 Category improvements (we-promise#232) * Category improvements - Updated default categories to a more inclusive set - Updated default icon set for categories - Updated default categories colors to better separate then. * FIX tests * Better color pallettes for a few commit 962ddd1 Author: Juan José Mata <[email protected]> Date: Fri Oct 24 19:25:36 2025 +0200 Refresh README with new logo and LLM conversation in screenshot Signed-off-by: Juan José Mata <[email protected]> commit b001d1c Author: Marc <[email protected]> Date: Fri Oct 24 19:02:54 2025 +0200 I have created a one click deploy button for railway users 🧙♂️ (we-promise#228) * Add Railway deployment button to README Signed-off-by: Marc <[email protected]> commit 4ba8f32 Author: Juan José Mata <[email protected]> Date: Fri Oct 24 18:11:31 2025 +0200 Fix production OIDC regression Signed-off-by: Juan José Mata <[email protected]> commit 768e85c Author: Juan José Mata <[email protected]> Date: Fri Oct 24 16:07:45 2025 +0200 Add OpenID Connect login support (we-promise#77) * Add OpenID Connect login support * Add docs for OIDC config with Google Auth * Use Google styles for log in - Add support for linking existing account - Force users to sign-in with passoword first, when linking existing accounts - Add support to create new user when using OIDC - Add identities to user to prevent account take-ver - Make tests mocking instead of being integration tests - Manage session handling correctly - use OmniAuth.config.mock_auth instead of passing auth data via request env * Conditionally render Oauth button - Set a config item `configuration.x.auth.oidc_enabled` - Hide button if disabled --------- Signed-off-by: Juan José Mata <[email protected]> Signed-off-by: soky srm <[email protected]> Co-authored-by: sokie <[email protected]> commit d51ba51 Author: Copilot <[email protected]> Date: Fri Oct 24 13:33:06 2025 +0200 Fix Twelve Data API parsing errors causing "Current Market Price Unknown" (we-promise#224) * Add tests and fix for Twelve Data API parsing errors * Fix search_securities to handle nil data key --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jjmata <[email protected]> Co-authored-by: Juan José Mata <[email protected]> commit a8f318c Author: Copilot <[email protected]> Date: Fri Oct 24 12:04:19 2025 +0200 Fix "Messages is invalid" error for Ollama/custom LLM providers and add comprehensive AI documentation (we-promise#225) * Add comprehensive AI/LLM configuration documentation * Fix Chat.start! to use default model when model is nil or empty * Ensure all controllers use Chat.default_model for consistency * Move AI doc inside `hosting/` * Probably too much error handling --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jjmata <[email protected]> Co-authored-by: Juan José Mata <[email protected]> commit 4f44630 Author: Juan José Mata <[email protected]> Date: Fri Oct 24 00:16:29 2025 +0200 Bump version to 0.6.5-alpha.1 Signed-off-by: Juan José Mata <[email protected]> commit bb364fa Author: soky srm <[email protected]> Date: Fri Oct 24 00:08:59 2025 +0200 LLM cost estimation (we-promise#223) * Password reset back button also after confirmation Signed-off-by: Juan José Mata <[email protected]> * Implement a filter for category (we-promise#215) - Also implement an is empty/is null condition. * Implement an LLM cost estimation page Track costs across all the cost categories: auto categorization, auto merchant detection and chat. Show warning with estimated cost when running a rule that contains AI. * Update pricing * Add google pricing and fix inferred model everywhere. * Update app/models/llm_usage.rb Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: soky srm <[email protected]> * FIX address review * Linter * Address review - Lowered log level - extracted the duplicated record_usage method into a shared concern * Update app/controllers/settings/llm_usages_controller.rb Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: soky srm <[email protected]> * Moved attr_reader out of private --------- Signed-off-by: Juan José Mata <[email protected]> Signed-off-by: soky srm <[email protected]> Co-authored-by: Juan José Mata <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Add OpenID Connect login support * Add docs for OIDC config with Google Auth * Use Google styles for log in - Add support for linking existing account - Force users to sign-in with passoword first, when linking existing accounts - Add support to create new user when using OIDC - Add identities to user to prevent account take-ver - Make tests mocking instead of being integration tests - Manage session handling correctly - use OmniAuth.config.mock_auth instead of passing auth data via request env * Conditionally render Oauth button - Set a config item `configuration.x.auth.oidc_enabled` - Hide button if disabled --------- Signed-off-by: Juan José Mata <[email protected]> Signed-off-by: soky srm <[email protected]> Co-authored-by: sokie <[email protected]>
Summary
omniauth_openid_connectwith CSRF protection to support OpenID Connect loginsTesting
bundle exec rubocopbin/rails test(fails: could not connect to server: Connection refused)Summary by CodeRabbit
New Features
Bug Fixes / Error Handling
Localization
Configuration
Style
Database
Tests