OIDC Login (#201)

* Add first version of OIDC login Co-authored-by: Alexander Sohn <[email protected]> * Fix rubocop issues Co-authored-by: Alexander Sohn <[email protected]> * Revert DB Schema changes Co-authored-by: Alexander Sohn <[email protected]> * Revert DB Schema changes - like, eally Co-authored-by: Alexander Sohn <[email protected]> * Add tests Co-authored-by: Alexander Sohn <[email protected]> * Fix linter issues Co-authored-by: Alexander Sohn <[email protected]> * Remove schema changes and remove CSRF on certain route * Update schema to current version on main * Remove unlocalized text Co-authored-by: Alexander Sohn <[email protected]>
hpi-swt2 · Jan 6, 2023 · 7791efe · 7791efe
1 parent 7035919
commit 7791efe
Show file tree

Hide file tree

Showing 9 changed files with 176 additions and 8 deletions.
diff --git a/Gemfile b/Gemfile
@@ -58,6 +58,8 @@ gem "devise", github: "heartcombo/devise", ref: "f8d1ea90bc3" # https://steve-co
 gem "devise-i18n" # https://github.com/tigrish/devise-i18n
 gem "devise-bootstrap-views" # https://github.com/hisea/devise-bootstrap-views
 gem "devise-i18n-bootstrap" # https://github.com/maximalink/devise-i18n-bootstrap
+gem 'omniauth_openid_connect'
+gem "omniauth-rails_csrf_protection"
 
 # Use Active Storage variants [https://guides.rubyonrails.org/active_storage_overview.html#transforming-images]
 # gem "image_processing", "~> 1.2"

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -84,10 +84,13 @@ GEM
       tzinfo (~> 2.0)
     addressable (2.8.1)
       public_suffix (>= 2.0.2, < 6.0)
+    aes_key_wrap (1.1.0)
     ast (2.4.2)
+    attr_required (1.0.1)
     autoprefixer-rails (10.4.7.0)
       execjs (~> 2)
     bcrypt (3.1.18)
+    bindata (2.4.14)
     bindex (0.8.1)
     bootsnap (1.13.0)
       msgpack (~> 1.2)
@@ -125,9 +128,17 @@ GEM
     factory_bot_rails (6.2.0)
       factory_bot (~> 6.2.0)
       railties (>= 5.0.0)
+    faraday (2.7.2)
+      faraday-net_http (>= 2.0, < 3.1)
+      ruby2_keywords (>= 0.0.4)
+    faraday-follow_redirects (0.3.0)
+      faraday (>= 1, < 3)
+    faraday-net_http (3.0.2)
     ffi (1.15.5)
     globalid (1.0.0)
       activesupport (>= 5.0)
+    hashie (5.0.0)
+    httpclient (2.8.3)
     i18n (1.12.0)
       concurrent-ruby (~> 1.0)
     importmap-rails (1.1.5)
@@ -140,6 +151,12 @@ GEM
       actionview (>= 5.0.0)
       activesupport (>= 5.0.0)
     json (2.6.2)
+    json-jwt (1.16.1)
+      activesupport (>= 4.2)
+      aes_key_wrap
+      bindata
+      faraday (~> 2.0)
+      faraday-follow_redirects
     loofah (2.19.0)
       crass (~> 1.0.2)
       nokogiri (>= 1.5.9)
@@ -166,6 +183,27 @@ GEM
     nokogiri (1.13.9)
       mini_portile2 (~> 2.8.0)
       racc (~> 1.4)
+    omniauth (2.1.0)
+      hashie (>= 3.4.6)
+      rack (>= 2.2.3)
+      rack-protection
+    omniauth-rails_csrf_protection (1.0.1)
+      actionpack (>= 4.2)
+      omniauth (~> 2.0)
+    omniauth_openid_connect (0.5.0)
+      omniauth (>= 1.9, < 3)
+      openid_connect (~> 1.1)
+    openid_connect (1.4.2)
+      activemodel
+      attr_required (>= 1.0.0)
+      json-jwt (>= 1.15.0)
+      net-smtp
+      rack-oauth2 (~> 1.21)
+      swd (~> 1.3)
+      tzinfo
+      validate_email
+      validate_url
+      webfinger (~> 1.2)
     orm_adapter (0.5.0)
     parallel (1.22.1)
     parser (3.1.2.1)
@@ -181,6 +219,14 @@ GEM
       nio4r (~> 2.0)
     racc (1.6.0)
     rack (2.2.4)
+    rack-oauth2 (1.21.3)
+      activesupport
+      attr_required
+      httpclient
+      json-jwt (>= 1.11.0)
+      rack (>= 2.1.0)
+    rack-protection (3.0.5)
+      rack
     rack-test (2.0.2)
       rack (>= 1.3)
     rails (7.0.4)
@@ -297,6 +343,10 @@ GEM
       mini_portile2 (~> 2.8.0)
     stimulus-rails (1.1.1)
       railties (>= 6.0.0)
+    swd (1.3.0)
+      activesupport (>= 3)
+      attr_required (>= 0.0.5)
+      httpclient (>= 2.4)
     thor (1.2.1)
     tilt (2.0.11)
     timeout (0.3.0)
@@ -309,6 +359,12 @@ GEM
       concurrent-ruby (~> 1.0)
     unicode-display_width (2.3.0)
     uri (0.10.0)
+    validate_email (0.1.6)
+      activemodel (>= 3.0)
+      mail (>= 2.2.5)
+    validate_url (1.0.15)
+      activemodel (>= 3.0.0)
+      public_suffix
     warden (1.2.9)
       rack (>= 2.0.9)
     web-console (4.2.0)
@@ -320,6 +376,9 @@ GEM
       nokogiri (~> 1.6)
       rubyzip (>= 1.3.0)
       selenium-webdriver (~> 4.0)
+    webfinger (1.2.0)
+      activesupport
+      httpclient (>= 2.4)
     websocket (1.2.9)
     websocket-driver (0.7.5)
       websocket-extensions (>= 0.1.0)
@@ -345,6 +404,8 @@ DEPENDENCIES
   importmap-rails
   jbuilder
   net-http
+  omniauth-rails_csrf_protection
+  omniauth_openid_connect
   pg
   prawn
   puma (~> 5.0)

diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb
@@ -0,0 +1,19 @@
+class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController
+  # See https://github.com/omniauth/omniauth/wiki/FAQ#rails-session-is-clobbered-after-callback-on-developer-strategy
+  skip_before_action :verify_authenticity_token, only: :openid_connect
+  def openid_connect
+    @user = User.from_omniauth(request.env["omniauth.auth"])
+    if @user.persisted?
+      sign_in_and_redirect @user
+      set_flash_message(:notice, :success, kind: "OpenID Connect") if is_navigational_format?
+    else
+      set_flash_message(:alert, :failure, kind: OmniAuth::Utils.camelize(failed_strategy.name), reason: failure_message)
+      redirect_to root_path
+    end
+  end
+
+  def failure
+    set_flash_message(:alert, :failure, kind: OmniAuth::Utils.camelize(failed_strategy.name), reason: failure_message)
+    redirect_to root_path
+  end
+end
diff --git a/app/models/user.rb b/app/models/user.rb
@@ -3,7 +3,7 @@ class User < ApplicationRecord
   # Include default devise modules. Others available are:
   # :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
   devise :database_authenticatable, :registerable,
-         :recoverable, :rememberable, :validatable
+         :recoverable, :rememberable, :validatable, :omniauthable, omniauth_providers: [:openid_connect]
   has_many :notifications, dependent: :destroy
   has_and_belongs_to_many :items, join_table: "wishlist"
 
@@ -57,4 +57,12 @@ def to_member_of!(group)
     group.reload
     reload
   end
+
+  def self.from_omniauth(auth)
+    # Create user in database if it does not exist yet when logging in via OIDC
+    where(email: auth.info.email).first_or_create! do |user|
+      user.email = auth.info.email
+      user.password = Devise.friendly_token[0, 20]
+    end
+  end
 end
diff --git a/app/views/devise/shared/_links.html.erb b/app/views/devise/shared/_links.html.erb
@@ -1,3 +1,14 @@
+<%- if devise_mapping.omniauthable? %>
+  <%- resource_class.omniauth_providers.each do |provider| %>
+    <%= button_to(omniauth_authorize_path(resource_name, provider),
+                  class: 'btn btn-primary', id: "#{provider}-signin" ,
+                  method: :post,
+                  data: {disable_with: "<i class='fa fa-spinner fa-spin'></i>", turbo: "false"}) do %>
+      <%= t('.sign_in_with_provider', provider: OmniAuth::Utils.camelize(provider)) %>
+    <% end %>
+  <% end %>
+<% end %>
+
 <%- if controller_name != 'sessions' %>
   <%= link_to t(".sign_in"), new_session_path(resource_name) %><br />
 <% end %>
@@ -18,8 +29,3 @@
   <%= link_to t('.didn_t_receive_unlock_instructions'), new_unlock_path(resource_name) %><br />
 <% end %>
 
-<%- if devise_mapping.omniauthable? %>
-  <%- resource_class.omniauth_providers.each do |provider| %>
-    <%= link_to t('.sign_in_with_provider', provider: OmniAuth::Utils.camelize(provider)), omniauth_authorize_path(resource_name, provider), method: :post %><br />
-  <% end %>
-<% end %>
diff --git a/config/environments/test.rb b/config/environments/test.rb
@@ -57,4 +57,7 @@
 
   # Annotate rendered view with file names.
   # config.action_view.annotate_rendered_view_with_filenames = true
+
+  # Enable testing for OIDC
+  OmniAuth.config.test_mode = true
 end
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
@@ -272,7 +272,24 @@
   # ==> OmniAuth
   # Add a new OmniAuth provider. Check the wiki for more information on setting
   # up on your models and hooks.
-  # config.omniauth :github, 'APP_ID', 'APP_SECRET', scope: 'user,public_repo'
+  config.omniauth :openid_connect,
+                  name: :openid_connect,
+                  scope: %i[openid email profile],
+                  response_type: :code,
+                  client_options: {
+                    port: 443,
+                    scheme: "https",
+                    host: "oidc.hpi.de",
+                    # Instead of env vars, could also use Rails credentials store
+                    # env vars are set on deployed Heroku instance, default to HPI OpenID client setup for local dev
+                    # Requires server to be running on port 3000, as that is also set on the remote OIDC config (and is checked)
+                    identifier: ENV.fetch("OIDC_CLIENT_ID", "dev"),
+                    secret: ENV.fetch("OIDC_CLIENT_SECRET", "secret"),
+                    redirect_uri: "#{ENV['APP_BASE_URL'] || 'http://localhost:3000'}/users/auth/openid_connect/callback",
+                    authorization_endpoint: "/auth"
+                  },
+                  client_auth_method: :other,
+                  discovery: true
 
   # ==> Warden configuration
   # If you want to use other strategies, that are not supported by Devise, or

diff --git a/config/routes.rb b/config/routes.rb
@@ -11,7 +11,10 @@
   # Define your application routes per the DSL in https://guides.rubyonrails.org/routing.html
 
   # https://github.com/heartcombo/devise/blob/main/README.md
-  devise_for :users, controllers: { registrations: 'users' }
+  devise_for :users, controllers: { registrations: 'users', omniauth_callbacks: "users/omniauth_callbacks" }
+  devise_scope :user do
+    get 'profile', to: 'users#profile'
+  end
 
   resources :users
   post 'add_to_waitlist/:id', to: 'items#add_to_waitlist', as: 'add_to_waitlist'

diff --git a/spec/features/users/oidc_spec.rb b/spec/features/users/oidc_spec.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+describe "OpenId Connect Login", type: :feature do
+  context "with a valid OIDC session returned" do
+    before do
+      OmniAuth.config.mock_auth[:openid_connect] = OmniAuth::AuthHash.new(
+        provider: "openid_connect",
+        uid: "test.user",
+        info: {
+          email: "[email protected]"
+        }
+      )
+
+      visit new_user_session_path
+      find_by_id('openid_connect-signin').click
+    end
+
+    it "redirects to dashboard path" do
+      expect(page).to have_current_path(dashboard_path)
+    end
+
+    it "displays a success message" do
+      expect(page).to have_css(".alert-success")
+    end
+  end
+
+  context "with invalid oidc session returned" do
+    before do
+      @omniauth_logger = OmniAuth.config.logger
+      # Change OmniAuth logger (default output to STDOUT)
+      OmniAuth.config.logger = Rails.logger
+
+      OmniAuth.config.mock_auth[:openid_connect] = :invalid_credentials
+      visit new_user_session_path
+      find_by_id('openid_connect-signin').click
+    end
+
+    it "redirects to login path" do
+      expect(page).to have_current_path(new_user_session_path)
+    end
+
+    it "shows an error message" do
+      expect(page).to have_css(".alert-danger")
+    end
+
+  end
+end