From 9503ff20cefcfc39cb451562d7ec4fd440198564 Mon Sep 17 00:00:00 2001
From: Thorsten Eckel <te@znuny.com>
Date: Mon, 8 Jul 2019 17:18:14 +0200
Subject: [PATCH] Maintenance: Applied required changes to resolve 'omniauth'
 CVE-2015-9284

While Zammad was not affected it was using GET requests. These are now prohibited because they enable the attack vector (in other scenarios).
Since it's not a general 'rails' or 'omnniauth' issue but an issue affecting scenarios where both are combined. It is required to use the 'omniauth-rails_csrf_protection' gem instead of 'omniauth' for those scenarios.

More information can be found here:
https://github.com/omniauth/omniauth/pull/809
https://github.com/omniauth/omniauth/pull/809#issuecomment-497376674
https://nvd.nist.gov/vuln/detail/CVE-2015-9284
https://github.com/cookpad/omniauth-rails_csrf_protection
https://github.com/rubysec/ruby-advisory-db/pull/390#issuecomment-509108186
---
 .gitlab-ci.yml                                |  2 +-
 Gemfile                                       |  4 +++-
 Gemfile.lock                                  |  5 ++++-
 .../javascripts/app/views/login.jst.eco       | 19 +++++++++++--------
 app/assets/stylesheets/zammad.scss            |  8 +++++++-
 spec/requests/o_auth_spec.rb                  |  2 +-
 6 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 586eea5d5..b3ccef350 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -102,7 +102,7 @@ pre:bundle-audit:
   script:
     - gem install bundler-audit
     - bundle-audit update
-    - bundle-audit
+    - bundle-audit --ignore CVE-2015-9284
 
 pre:github:
   <<: *pre_stage
diff --git a/Gemfile b/Gemfile
index 48132f759..4aeb6bd33 100644
--- a/Gemfile
+++ b/Gemfile
@@ -64,7 +64,9 @@ gem 'doorkeeper'
 gem 'oauth2'
 
 # authentication - third party
-gem 'omniauth'
+gem 'omniauth-rails_csrf_protection'
+
+# authentication - third party providers
 gem 'omniauth-facebook'
 gem 'omniauth-github'
 gem 'omniauth-gitlab'
diff --git a/Gemfile.lock b/Gemfile.lock
index eb7dc751f..fbfbc5d14 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -340,6 +340,9 @@ GEM
     omniauth-oauth2 (1.6.0)
       oauth2 (~> 1.1)
       omniauth (~> 1.9)
+    omniauth-rails_csrf_protection (0.1.2)
+      actionpack (>= 4.2)
+      omniauth (>= 1.3.1)
     omniauth-twitter (1.4.0)
       omniauth-oauth (~> 1.1)
       rack
@@ -596,7 +599,6 @@ DEPENDENCIES
   mysql2 (= 0.4.10)
   net-ldap
   oauth2
-  omniauth
   omniauth-facebook
   omniauth-github
   omniauth-gitlab
@@ -604,6 +606,7 @@ DEPENDENCIES
   omniauth-linkedin-oauth2
   omniauth-microsoft-office365
   omniauth-oauth2
+  omniauth-rails_csrf_protection
   omniauth-twitter
   omniauth-weibo-oauth2
   pg (= 0.21.0)
diff --git a/app/assets/javascripts/app/views/login.jst.eco b/app/assets/javascripts/app/views/login.jst.eco
index 135c1c859..26b9dbaab 100644
--- a/app/assets/javascripts/app/views/login.jst.eco
+++ b/app/assets/javascripts/app/views/login.jst.eco
@@ -48,21 +48,24 @@
             <% end %>
           </div>
 
-          <% if !_.isEmpty(@auth_providers): %>
+        </form>
+        <% if !_.isEmpty(@auth_providers): %>
           <div class="separator">
             <span class="separator-text"><%- @T('or sign in using') %></span>
           </div>
 
           <div class="auth-providers">
             <% for auth_provider in @auth_providers: %>
-              <a class="auth-provider auth-provider--<%= auth_provider.class %>" href="<%= auth_provider.url %>">
-                <%- @Icon("#{auth_provider.class}-button", 'provider-icon') %>
-                <span class="provider-name"><%- @T(auth_provider.name) %></span>
-              </a>
+              <form method="post" action="<%= auth_provider.url %>">
+                <input type="hidden" name="authenticity_token" value="<%= Spine.Ajax.defaults.headers['X-CSRF-Token'] %>">
+                <button type="submit" class="auth-provider auth-provider--<%= auth_provider.class %>">
+                  <%- @Icon("#{auth_provider.class}-button", 'provider-icon') %>
+                  <span class="provider-name"><%- @T(auth_provider.name) %></span>
+                </button>
+              </form>
             <% end %>
           </div>
-          <% end %>
-        </form>
+        <% end %>
       </div>
 
       <p>
@@ -85,4 +88,4 @@
     <%- @T('Powered by') %>
     <a href="https://zammad.org" target="_blank"><%- @Icon('logotype', 'logotype') %></a>
   </div>
-</div>
\ No newline at end of file
+</div>
diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss
index a9be9d691..0f99a7a5d 100644
--- a/app/assets/stylesheets/zammad.scss
+++ b/app/assets/stylesheets/zammad.scss
@@ -2969,9 +2969,14 @@ ol.tabs li {
   flex-wrap: wrap;
   justify-content: space-between;
   margin-bottom: -10px;
+  
+  > form {
+    width: calc(33.33% - 6px);
+  }
 }
 
 .auth-provider {
+  width: 100%;
   height: 40px;
   padding: 0 10px 0 7px;
   margin-bottom: 10px;
@@ -2981,7 +2986,8 @@ ol.tabs li {
   display: flex;
   align-items: center;
   text-decoration: none;
-  width: calc(33.33% - 6px);
+  border: none;
+  text-align: initial;
 
   &.auth-provider--wide {
     padding-right: 25px;
diff --git a/spec/requests/o_auth_spec.rb b/spec/requests/o_auth_spec.rb
index 1d86492aa..bab1c6a3b 100644
--- a/spec/requests/o_auth_spec.rb
+++ b/spec/requests/o_auth_spec.rb
@@ -5,7 +5,7 @@ RSpec.describe 'OAuth', type: :request do
   describe 'request handling' do
 
     it 'does o365 - start' do
-      get '/auth/microsoft_office365'
+      post '/auth/microsoft_office365'
       expect(response).to have_http_status(:found)
       expect(response.body).to include('https://login.microsoftonline.com/common/oauth2/v2.0/authorize')
       expect(response.body).to include('redirect_uri=http%3A%2F%2Fzammad.example.com%2Fauth%2Fmicrosoft_office365%2Fcallback')