Feat: FranceConnect 2.0 (0.27-stable)#922
Feat: FranceConnect 2.0 (0.27-stable)#922moustachu wants to merge 5 commits intorelease/0.27-stablefrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR backports FranceConnect v2 implementation from the main branch to the release/0.27-stable branch. The changes replace the deprecated omniauth-france_connect gem with a custom implementation based on OpenID Connect, updating authentication flows, configuration, UI elements, and tests.
Key Changes
- Removed dependency on the external
omniauth-france_connectgem and implemented a custom FranceConnect strategy using OpenID Connect - Refactored logout handling to use a new
Configuratorclass that manages omniauth provider settings dynamically from database and environment variables - Updated FranceConnect UI assets with new official branding images (Principal, Alternative, and Disabled button states in SVG and PNG formats)
Reviewed Changes
Copilot reviewed 27 out of 44 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
lib/omniauth/strategies/france_connect.rb |
New custom FranceConnect strategy extending OpenIDConnect with specific French authentication parameters |
lib/decidim_app/omniauth/configurator.rb |
New configurator class for dynamic omniauth provider configuration from multiple sources |
lib/decidim_app/omniauth/openid_connect_utils.rb |
Utility class for setting up OpenID Connect provider configurations |
lib/extends/controllers/decidim/devise/*_controller_extends.rb |
Refactored logout handling to use the new configurator pattern instead of session-based config |
config/initializers/omniauth_france_connect.rb |
Simplified initializer using the new configurator approach |
config/secrets.yml |
Updated FranceConnect configuration structure to match OpenID Connect requirements |
config/locales/*.yml |
Updated translation keys for FranceConnect configuration and removed deprecated entries |
app/views/decidim/devise/shared/_omniauth_buttons.html.erb |
Enhanced view to support custom button images with hover states |
spec/**/*_spec.rb |
Updated tests to match new authentication flows and expect correct redirect behavior |
app/packs/images/FranceConnect-Bouton/** |
Added official FranceConnect v2 button assets in multiple states and formats |
Gemfile & Gemfile.lock |
Removed omniauth-france_connect gem dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rescue Capybara::ElementNotFound | ||
| # do nothing | ||
| end |
There was a problem hiding this comment.
Empty rescue block swallows all Capybara::ElementNotFound exceptions without logging or handling. Consider logging the exception or documenting why it's safe to ignore.
| <% if provider[:icon_path].present? %> | ||
| <%= link_to decidim.send("user_#{name}_omniauth_authorize_path"), class: "button button--social--custom", method: :post do %> | ||
| <% if provider[:icon_hover_path].present? %> | ||
| <%= image_pack_tag provider[:icon_path], class: "button--has-hover", alt: t("devise.shared.links.sign_in_with_provider", provider: provider_name) %> | ||
| <%= image_pack_tag provider[:icon_hover_path], class: "button--is-hover", alt: t("devise.shared.links.sign_in_with_provider", provider: provider_name) %> | ||
| <% else %> | ||
| <%= image_pack_tag provider[:icon_path], alt: t("devise.shared.links.sign_in_with_provider", provider: provider_name) %> | ||
| <% end %> | ||
| <% end %> | ||
| <% else %> | ||
| <%= sso_provider_image(name, decidim.send("user_#{name}_omniauth_authorize_path"), link_class: "button button--social--custom") %> | ||
| <% end %> |
There was a problem hiding this comment.
The view references provider[:icon_path] and provider[:icon_hover_path] without checking if they exist in the provider hash. This could cause issues if these keys are not present. Consider using safe navigation or providing default values.
| logout_path = omniauth_config.options(:logout_path) | ||
| end | ||
|
|
||
| if provider.present? && logout_policy == "session.destroy" && logout_path.present? |
There was a problem hiding this comment.
Local variable logout_policy may be used before it is initialized.
| logout_path = omniauth_config.options(:logout_path) | ||
| end | ||
|
|
||
| if provider.present? && logout_policy == "session.destroy" && logout_path.present? |
There was a problem hiding this comment.
Local variable logout_path may be used before it is initialized.
2 participants
Backporting code for FranceConnect v2 on
release/0.27-stablebranch