Merge branch 'master' into dependabot/bundler/flipper-and-flipper-act…

…ive_record-and-flipper-ui-1.2.2

Gemfile

@@ -188,6 +188,7 @@ group :development do
   gem 'rubocop-rails'
   gem 'spring'
   gem 'spring-commands-rspec'
+  gem 'spring-commands-rubocop'
   gem 'web-console'
 
   gem 'rack-mini-profiler', '< 3.0.0'

Gemfile.lock

@@ -98,11 +98,12 @@ GEM
     activejob (7.0.8)
       activesupport (= 7.0.8)
       globalid (>= 0.3.6)
-    activemerchant (1.123.0)
+    activemerchant (1.133.0)
       activesupport (>= 4.2)
       builder (>= 2.1.2, < 4.0.0)
       i18n (>= 0.6.9)
       nokogiri (~> 1.4)
+      rexml (~> 3.2.5)
     activemodel (7.0.8)
       activesupport (= 7.0.8)
     activerecord (7.0.8)
@@ -221,7 +222,7 @@ GEM
       ruby-rc4 (>= 0.1.5)
     concurrent-ruby (1.2.3)
     connection_pool (2.4.1)
-    crack (0.4.6)
+    crack (1.0.0)
       bigdecimal
       rexml
     crass (1.0.6)
@@ -379,7 +380,8 @@ GEM
       rspec (>= 2.0, < 4.0)
     jsonapi-serializer (2.2.0)
       activesupport (>= 4.2)
-    jwt (2.7.1)
+    jwt (2.8.0)
+      base64
     knapsack_pro (6.0.4)
       rake
     language_server-protocol (3.17.0.3)
@@ -579,7 +581,7 @@ GEM
     redcarpet (3.6.0)
     redis (5.1.0)
       redis-client (>= 0.17.0)
-    redis-client (0.19.1)
+    redis-client (0.20.0)
       connection_pool
     regexp_parser (2.9.0)
     reline (0.4.1)
@@ -681,7 +683,7 @@ GEM
     semantic_range (3.0.0)
     shoulda-matchers (6.1.0)
       activesupport (>= 5.2.0)
-    sidekiq (7.2.1)
+    sidekiq (7.2.2)
       concurrent-ruby (< 2)
       connection_pool (>= 2.3.0)
       rack (>= 2.2.4)
@@ -702,6 +704,8 @@ GEM
     spring (4.1.3)
     spring-commands-rspec (1.0.4)
       spring (>= 0.9.1)
+    spring-commands-rubocop (0.4.0)
+      spring (>= 1.0)
     sprockets (3.7.2)
       concurrent-ruby (~> 1.0)
       rack (> 1, < 3)
@@ -778,7 +782,7 @@ GEM
       activesupport
       faraday (~> 2.0)
       faraday-follow_redirects
-    webmock (3.20.0)
+    webmock (3.21.2)
       addressable (>= 2.8.0)
       crack (>= 0.3.2)
       hashdiff (>= 0.4.0, < 2.0.0)
@@ -923,6 +927,7 @@ DEPENDENCIES
   spreadsheet_architect
   spring
   spring-commands-rspec
+  spring-commands-rubocop
   state_machines-activerecord
   stimulus_reflex (= 3.5.0.rc3)
   stimulus_reflex_testing

app/controllers/admin/oidc_settings_controller.rb

@@ -2,6 +2,13 @@
 
 module Admin
   class OidcSettingsController < Spree::Admin::BaseController
-    def index; end
+    def index
+      @account = spree_current_user.oidc_account
+    end
+
+    def destroy
+      spree_current_user.oidc_account&.destroy
+      redirect_to admin_oidc_settings_path
+    end
   end
 end

app/controllers/omniauth_callbacks_controller.rb

@@ -2,7 +2,7 @@
 
 class OmniauthCallbacksController < Devise::OmniauthCallbacksController
   def openid_connect
-    spree_current_user.link_from_omniauth(request.env["omniauth.auth"])
+    OidcAccount.link(spree_current_user, request.env["omniauth.auth"])
 
     redirect_to admin_oidc_settings_path
   end

app/helpers/shop_helper.rb

@@ -22,7 +22,7 @@ def shop_tabs
       { name: 'home', title: t(:shopping_tabs_home), show: show_home_tab?,
         default: show_home_tab? },
       { name: 'shop', title: t(:shopping_tabs_shop), show: !require_customer?,
-        default: !show_home_tab? },
+        default: !show_home_tab?, shop: true },
       { name: 'about', title: t(:shopping_tabs_about), show: true },
       { name: 'producers', title: t(:shopping_tabs_producers), show: true },
       { name: 'contact', title: t(:shopping_tabs_contact), show: true },

app/models/oidc_account.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+class OidcAccount < ApplicationRecord
+  belongs_to :user, class_name: "Spree::User"
+
+  # When a user authenticates via token, the `uid` should be mapped to only one
+  # OFN user and therefore it needs to be unique.
+  validates :uid, presence: true, uniqueness: true
+
+  def self.link(user, auth)
+    attributes = {
+      user_id: user.id,
+      provider: auth.provider,
+      uid: auth.uid,
+      token: auth.dig(:credentials, :token),
+      refresh_token: auth.dig(:credentials, :refresh_token),
+    }
+    # This skips validations but we have database constraints in place.
+    # We may replace this at some point.
+    upsert_all([attributes], unique_by: [:user_id]) # rubocop:disable Rails/SkipsModelValidations
+  end
+end

app/models/spree/ability.rb

@@ -177,7 +177,7 @@ def add_enterprise_management_abilities(user)
 
       can [:admin, :create], :manager_invitation
 
-      can [:admin, :index], :oidc_setting
+      can [:admin, :index, :destroy], :oidc_setting
 
       can [:admin, :create], Voucher
     end

app/models/spree/gateway/stripe_sca.rb

@@ -4,7 +4,6 @@
 require 'stripe/credit_card_cloner'
 require 'stripe/authorize_response_patcher'
 require 'stripe/payment_intent_validator'
-require 'active_merchant/billing/gateways/stripe_payment_intents_decorator'
 require 'active_merchant/billing/gateways/stripe'
 
 module Spree
@@ -44,7 +43,7 @@ def stripe_account_id
         StripeAccount.find_by(enterprise_id: preferred_enterprise_id)&.stripe_user_id
       end
 
-      # NOTE: the name of this method is determined by Spree::Payment::Processing
+      # NOTE: this method is required by Spree::Payment::Processing
       def purchase(money, creditcard, gateway_options)
         begin
           payment_intent_id = fetch_payment_intent(creditcard, gateway_options)
@@ -64,7 +63,7 @@ def capture(money, payment_intent_id, gateway_options)
         provider.capture(money, payment_intent_id, options)
       end
 
-      # NOTE: the name of this method is determined by Spree::Payment::Processing
+      # NOTE: this method is required by Spree::Payment::Processing
       def charge_offline(money, creditcard, gateway_options)
         customer, payment_method =
           Stripe::CreditCardCloner.new(creditcard, stripe_account_id).find_or_clone
@@ -75,7 +74,7 @@ def charge_offline(money, creditcard, gateway_options)
         failed_activemerchant_billing_response(e.message)
       end
 
-      # NOTE: the name of this method is determined by Spree::Payment::Processing
+      # NOTE: this method is required by Spree::Payment::Processing
       def authorize(money, creditcard, gateway_options)
         authorize_response =
           provider.authorize(*options_for_authorize(money, creditcard, gateway_options))
@@ -84,26 +83,27 @@ def authorize(money, creditcard, gateway_options)
         failed_activemerchant_billing_response(e.message)
       end
 
-      # NOTE: the name of this method is determined by Spree::Payment::Processing
-      def void(response_code, _creditcard, gateway_options)
-        payment_intent_id = response_code
-        payment_intent_response = Stripe::PaymentIntent.retrieve(payment_intent_id,
-                                                                 stripe_account: stripe_account_id)
+      # NOTE: this method is required by Spree::Payment::Processing
+      def void(payment_intent_id, _creditcard, gateway_options)
+        payment_intent_response = Stripe::PaymentIntent.retrieve(
+          payment_intent_id, stripe_account: stripe_account_id
+        )
         gateway_options[:stripe_account] = stripe_account_id
 
         # If a payment has been confirmed it can't be voided by Stripe, and must be refunded instead
         if voidable?(payment_intent_response)
-          provider.void(response_code, gateway_options)
+          provider.void(payment_intent_id, gateway_options)
         else
-          provider.refund(refundable_amount(payment_intent_response), response_code,
-                          gateway_options)
+          provider.refund(
+            payment_intent_response.amount_received, payment_intent_id, gateway_options
+          )
         end
       end
 
-      # NOTE: the name of this method is determined by Spree::Payment::Processing
-      def credit(money, _creditcard, response_code, gateway_options)
+      # NOTE: this method is required by Spree::Payment::Processing
+      def credit(money, _creditcard, payment_intent_id, gateway_options)
         gateway_options[:stripe_account] = stripe_account_id
-        provider.refund(money, response_code, gateway_options)
+        provider.refund(money, payment_intent_id, gateway_options)
       end
 
       def create_profile(payment)
@@ -119,11 +119,6 @@ def voidable?(payment_intent_response)
         VOIDABLE_STATES.include? payment_intent_response.status
       end
 
-      def refundable_amount(payment_intent_response)
-        payment_intent_response.amount_received -
-          payment_intent_response.charges.data.map(&:amount_refunded).sum
-      end
-
       # In this gateway, what we call 'secret_key' is the 'login'
       def options
         options = super

app/models/spree/user.rb

@@ -42,6 +42,7 @@ class User < ApplicationRecord
     has_many :credit_cards, dependent: :destroy
     has_many :report_rendering_options, class_name: "::ReportRenderingOptions", dependent: :destroy
     has_many :webhook_endpoints, dependent: :destroy
+    has_one :oidc_account, dependent: :destroy
 
     accepts_nested_attributes_for :enterprise_roles, allow_destroy: true
     accepts_nested_attributes_for :webhook_endpoints
@@ -51,22 +52,13 @@ class User < ApplicationRecord
 
     validates :email, 'valid_email_2/email': { mx: true }, if: :email_changed?
     validate :limit_owned_enterprises
-    validates :uid, uniqueness: true, if: lambda { uid.present? }
-
-    # Same validation as in the openid_connect gem.
-    # This validator is totally outdated but we indirectly depend on it.
-    validates :uid, email: true, if: lambda { uid.present? }
 
     class DestroyWithOrdersError < StandardError; end
 
     def self.admin_created?
       User.admin.count > 0
     end
 
-    def link_from_omniauth(auth)
-      update!(provider: auth.provider, uid: auth.uid)
-    end
-
     # Whether a user has a role or not.
     def has_spree_role?(role_in_question)
       spree_roles.where(name: role_in_question.to_s).any?

app/views/admin/oidc_settings/index.html.haml

@@ -7,14 +7,26 @@
   %h2= t(".connect")
   %br
 
-  - if spree_current_user.provider == 'openid_connect' && spree_current_user.uid.present?
-    = t(".already_connected")
-    = spree_current_user.uid
+  - if @account
+    = t(".connected", uid: @account.uid)
     %br
     %br
 
     = t(".view_account")
     = link_to t(".les_communs_link"), "#{ Devise.omniauth_configs[:openid_connect].options[:issuer] }/account"
+    %br
+    %br
+    = button_to t(".disconnect"), admin_oidc_setting_path(@account), method: :delete
+
+    - if @account.refresh_token.blank?
+      %br
+      %br
+      %p= t(".note_expiry")
+      %br
+      %br
+      = button_to t(".refresh"),
+        Spree::Core::Engine.routes.url_helpers.spree_user_openid_connect_omniauth_authorize_path(auth_type: "login"),
+        data: { method: :post, "ujs-navigate": "false" }
 
   - else
     = t(".link_your_account")

app/views/shopping_shared/_tabs.html.haml

@@ -1,6 +1,6 @@
 - if (@order&.distributor || current_distributor) == current_distributor
 
-  #shop-tabs{"data-controller": "tabs-and-panels", "data-action": "orderCycleSelected@window->tabs-and-panels#activateDefaultPanel", "data-tabs-and-panels-class-name-value": "selected"}
+  #shop-tabs{"data-controller": "tabs-and-panels", "data-action": "orderCycleSelected@window->tabs-and-panels#activateShopPanel", "data-tabs-and-panels-class-name-value": "selected"}
     .tab-buttons
       .flex.row
         .columns.small-12.large-8
@@ -10,7 +10,7 @@
         .columns.large-4.show-for-large-up
           = render partial: "shopping_shared/order_cycles"
     - shop_tabs.each do |tab|
-      %div{id: "#{tab[:name]}_panel", "data-tabs-and-panels-target": "panel #{'default' if tab[:default]}" } 
+      %div{id: "#{tab[:name]}_panel", "data-tabs-and-panels-target": "panel #{'default' if tab[:default]} #{'shop' if tab[:shop]}" } 
         .page-view
           - if tab[:custom]
             = render "shopping_shared/tabs/custom"

app/webpacker/controllers/tabs_and_panels_controller.js

@@ -1,7 +1,7 @@
 import { Controller } from "stimulus";
 
 export default class extends Controller {
-  static targets = ["tab", "panel", "default"];
+  static targets = ["tab", "panel", "default", "shop"];
   static values = { className: String };
 
   connect() {
@@ -31,6 +31,10 @@ export default class extends Controller {
     this._activateByHash(`#${this.defaultTarget.id}`);
   }
 
+  activateShopPanel() {
+    this._activateByHash(`#${this.shopTarget.id}`);
+  }
+
   _activateByHash(hash) {
     this.tabTargets.forEach((tab) => {
       if (tab.hash == hash) {

bin/rubocop

@@ -0,0 +1,4 @@
+#!/usr/bin/env ruby
+load File.expand_path("spring", __dir__)
+require 'bundler/setup'
+load Gem.bin_path('rubocop', 'rubocop')

bin/spring

@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-# This file loads Spring without using loading other gems in the Gemfile, in order to be fast.
+# This file loads Spring without loading other gems in the Gemfile in order to be fast.
 # It gets overwritten when you run the `spring binstub` command.
 
 if !defined?(Spring) && [nil, "development", "test"].include?(ENV["RAILS_ENV"])

config/initializers/devise.rb

@@ -145,7 +145,13 @@
 
 if ENV["OPENID_APP_ID"].present? && ENV["OPENID_APP_SECRET"].present?
   Devise.setup do |config|
-    protocol = Rails.env.development? ? "http://" : "https://"
+    site = if Rails.env.development?
+             # The lescommuns server accepts localhost:3000 as valid.
+             # So you can test in development.
+             "http://localhost:3000"
+           else
+             "https://#{ENV["SITE_URL"]}"
+           end
     config.omniauth :openid_connect, {
       name: :openid_connect,
       issuer: "https://login.lescommuns.org/auth/realms/data-food-consortium",
@@ -158,7 +164,7 @@
       client_options: {
         identifier: ENV["OPENID_APP_ID"],
         secret: ENV["OPENID_APP_SECRET"],
-        redirect_uri: "#{protocol}#{ENV["SITE_URL"]}/user/spree_user/auth/openid_connect/callback",
+        redirect_uri: "#{site}/user/spree_user/auth/openid_connect/callback",
         jwks_uri: 'https://login.lescommuns.org/auth/realms/data-food-consortium/protocol/openid-connect/certs'
       }
     }

config/locales/en.yml

@@ -1712,10 +1712,15 @@ en:
       index:
         title: "OIDC Settings"
         connect: "Connect Your Account"
-        already_connected: "Your account is already linked to this DFC authorization account:"
+        disconnect: "Disconnect"
+        connected: "Your account is linked to %{uid}."
         les_communs_link: "Les Communs Open ID server"
         link_your_account: "You need first to link your account with the authorization provider used by DFC (Les Communs Open ID Connect)."
         link_account_button: "Link your Les Communs OIDC Account"
+        note_expiry: |
+          Tokens to access connected apps have expired. Please refresh your
+          account connection to keep all integrations working.
+        refresh: "Refresh authorisation"
         view_account: "To view your account, see:"
     subscriptions:
       index:

config/routes/admin.rb

@@ -113,7 +113,7 @@
       put :unpause, on: :member
     end
 
-    resources :oidc_settings, only: :index
+    resources :oidc_settings, only: [:index, :destroy]
 
     resources :subscription_line_items, only: [], format: :json do
       post :build, on: :collection