Merge branch...

Merge branch '52285-omniauth-jwt-provider-dont-support-rsa-ecdsa-and-other-ppk-algorithms' into 'master'

Support RSA and ECDSA algorithms in Omniauth JWT provider

Closes #52285

See merge request gitlab-org/gitlab-ce!23411

changelogs/unreleased/52285-omniauth-jwt-ppk-support.yml

+---
+title: Support RSA and ECDSA algorithms in Omniauth JWT provider
+merge_request: 23411
+author: Michael Tsyganov
+type: fixed

config/gitlab.yml.example

@@ -548,15 +548,15 @@ production: &base
       #     app_id: 'YOUR_APP_ID',
       #     app_secret: 'YOUR_APP_SECRET' }
       # - { name: 'jwt',
-      #     app_secret: 'YOUR_APP_SECRET',
       #     args: {
-      #             algorithm: 'HS256',
-      #             uid_claim: 'email',
-      #             required_claims: ["name", "email"],
-      #             info_map: { name: "name", email: "email" },
-      #             auth_url: 'https://example.com/',
-      #             valid_within: null,
-      #           }
+      #       secret: 'YOUR_APP_SECRET',
+      #       algorithm: 'HS256', # Supported algorithms: 'RS256', 'RS384', 'RS512', 'ES256', 'ES384', 'ES512', 'HS256', 'HS384', 'HS512'
+      #       uid_claim: 'email',
+      #       required_claims: ['name', 'email'],
+      #       info_map: { name: 'name', email: 'email' },
+      #       auth_url: 'https://example.com/',
+      #       valid_within: 3600 # 1 hour
+      #     }
       #   }
       # - { name: 'saml',
       #     label: 'Our SAML Provider',

doc/administration/auth/README.md

@@ -10,7 +10,7 @@ providers.
 - [LDAP](ldap.md) Includes Active Directory, Apple Open Directory, Open LDAP,
   and 389 Server
 - [OmniAuth](../../integration/omniauth.md) Sign in via Twitter, GitHub, GitLab.com, Google,
-  Bitbucket, Facebook, Shibboleth, Crowd, Azure and Authentiq ID
+  Bitbucket, Facebook, Shibboleth, Crowd, Azure, Authentiq ID, and JWT
 - [CAS](../../integration/cas.md) Configure GitLab to sign in using CAS
 - [SAML](../../integration/saml.md) Configure GitLab as a SAML 2.0 Service Provider
 - [Okta](okta.md) Configure GitLab to sign in using Okta

doc/administration/auth/jwt.md

@@ -26,15 +26,15 @@ JWT will provide you with a secret key for you to use.
     ```ruby
     gitlab_rails['omniauth_providers'] = [
       { name: 'jwt',
-        app_secret: 'YOUR_APP_SECRET',
         args: {
-                algorithm: 'HS256',
-                uid_claim: 'email',
-                required_claims: ["name", "email"],
-                info_maps: { name: "name", email: "email" },
-                auth_url: 'https://example.com/',
-                valid_within: nil,
-              }
+          secret: 'YOUR_APP_SECRET',
+          algorithm: 'HS256', # Supported algorithms: 'RS256', 'RS384', 'RS512', 'ES256', 'ES384', 'ES512', 'HS256', 'HS384', 'HS512'
+          uid_claim: 'email',
+          required_claims: ['name', 'email'],
+          info_maps: { name: 'name', email: 'email' },
+          auth_url: 'https://example.com/',
+          valid_within: 3600 # 1 hour
+        }
       }
     ]
     ```
@@ -43,15 +43,15 @@ JWT will provide you with a secret key for you to use.
 
     ```
     - { name: 'jwt',
-        app_secret: 'YOUR_APP_SECRET',
         args: {
-                algorithm: 'HS256',
-                uid_claim: 'email',
-                required_claims: ["name", "email"],
-                info_map: { name: "name", email: "email" },
-                auth_url: 'https://example.com/',
-                valid_within: null,
-              }
+          secret: 'YOUR_APP_SECRET',
+          algorithm: 'HS256', # Supported algorithms: 'RS256', 'RS384', 'RS512', 'ES256', 'ES384', 'ES512', 'HS256', 'HS384', 'HS512'
+          uid_claim: 'email',
+          required_claims: ['name', 'email'],
+          info_map: { name: 'name', email: 'email' },
+          auth_url: 'https://example.com/',
+          valid_within: 3600 # 1 hour
+        }
       }
     ```
 
@@ -60,7 +60,7 @@ JWT will provide you with a secret key for you to use.
 
 1.  Change `YOUR_APP_SECRET` to the client secret and set `auth_url` to your redirect URL.
 1.  Save the configuration file.
-1.  [Reconfigure GitLab][] or [restart GitLab][] for the changes to take effect if you
+1.  [Reconfigure][] or [restart GitLab][] for the changes to take effect if you
     installed GitLab via Omnibus or from source respectively.
 
 On the sign in page there should now be a JWT icon below the regular sign in form.
@@ -68,5 +68,5 @@ Click the icon to begin the authentication process. JWT will ask the user to
 sign in and authorize the GitLab application. If everything goes well, the user
 will be redirected to GitLab and will be signed in.
 
-[reconfigure GitLab]: ../restart_gitlab.md#omnibus-gitlab-reconfigure
+[reconfigure]: ../restart_gitlab.md#omnibus-gitlab-reconfigure
 [restart GitLab]: ../restart_gitlab.md#installations-from-source

lib/omni_auth/strategies/jwt.rb

 # frozen_string_literal: true
 
 require 'omniauth'
+require 'openssl'
 require 'jwt'
 
 module OmniAuth
@@ -37,7 +38,19 @@ module OmniAuth
       end
 
       def decoded
-        @decoded ||= ::JWT.decode(request.params['jwt'], options.secret, options.algorithm).first
+        secret =
+          case options.algorithm
+          when *%w[RS256 RS384 RS512]
+            OpenSSL::PKey::RSA.new(options.secret).public_key
+          when *%w[ES256 ES384 ES512]
+            OpenSSL::PKey::EC.new(options.secret).tap { |key| key.private_key = nil }
+          when *%w(HS256 HS384 HS512)
+            options.secret
+          else
+            raise NotImplementedError, "Unsupported algorithm: #{options.algorithm}"
+          end
+
+        @decoded ||= ::JWT.decode(request.params['jwt'], secret, true, { algorithm: options.algorithm }).first
 
         (options.required_claims || []).each do |field|
           raise ClaimInvalid, "Missing required '#{field}' claim" unless @decoded.key?(field.to_s)
@@ -45,7 +58,7 @@ module OmniAuth
 
         raise ClaimInvalid, "Missing required 'iat' claim" if options.valid_within && !@decoded["iat"]
 
-        if options.valid_within && (Time.now.to_i - @decoded["iat"]).abs > options.valid_within
+        if options.valid_within && (Time.now.to_i - @decoded["iat"]).abs > options.valid_within.to_i
           raise ClaimInvalid, "'iat' timestamp claim is too skewed from present"
         end
 

spec/lib/omni_auth/strategies/jwt_spec.rb

@@ -4,12 +4,10 @@ describe OmniAuth::Strategies::Jwt do
   include Rack::Test::Methods
   include DeviseHelpers
 
-  context '.decoded' do
-    let(:strategy) { described_class.new({}) }
+  context '#decoded' do
+    subject { described_class.new({}) }
     let(:timestamp) { Time.now.to_i }
     let(:jwt_config) { Devise.omniauth_configs[:jwt] }
-    let(:key) { JWT.encode(claims, jwt_config.strategy.secret) }
-
     let(:claims) do
       {
         id: 123,
@@ -18,19 +16,55 @@ describe OmniAuth::Strategies::Jwt do
         iat: timestamp
       }
     end
+    let(:algorithm) { 'HS256' }
+    let(:secret) { jwt_config.strategy.secret }
+    let(:private_key) { secret }
+    let(:payload) { JWT.encode(claims, private_key, algorithm) }
 
     before do
-      allow_any_instance_of(OmniAuth::Strategy).to receive(:options).and_return(jwt_config.strategy)
-      allow_any_instance_of(Rack::Request).to receive(:params).and_return({ 'jwt' => key })
+      subject.options[:secret] = secret
+      subject.options[:algorithm] = algorithm
+
+      expect_next_instance_of(Rack::Request) do |rack_request|
+        expect(rack_request).to receive(:params).and_return('jwt' => payload)
+      end
     end
 
-    it 'decodes the user information' do
-      result = strategy.decoded
+    ECDSA_NAMED_CURVES = {
+      'ES256' => 'prime256v1',
+      'ES384' => 'secp384r1',
+      'ES512' => 'secp521r1'
+    }.freeze
 
-      expect(result["id"]).to eq(123)
-      expect(result["name"]).to eq("user_example")
-      expect(result["email"]).to eq("user@example.com")
-      expect(result["iat"]).to eq(timestamp)
+    {
+      OpenSSL::PKey::RSA => %w[RS256 RS384 RS512],
+      OpenSSL::PKey::EC => %w[ES256 ES384 ES512],
+      String => %w[HS256 HS384 HS512]
+    }.each do |private_key_class, algorithms|
+      algorithms.each do |algorithm|
+        context "when the #{algorithm} algorithm is used" do
+          let(:algorithm) { algorithm }
+          let(:secret) do
+            if private_key_class == OpenSSL::PKey::RSA
+              private_key_class.generate(2048)
+                .to_pem
+            elsif private_key_class == OpenSSL::PKey::EC
+              private_key_class.new(ECDSA_NAMED_CURVES[algorithm])
+                .tap { |key| key.generate_key! }
+                .to_pem
+            else
+              private_key_class.new(jwt_config.strategy.secret)
+            end
+          end
+          let(:private_key) { private_key_class ? private_key_class.new(secret) : secret }
+
+          it 'decodes the user information' do
+            result = subject.decoded
+
+            expect(result).to eq(claims.stringify_keys)
+          end
+        end
+      end
     end
 
     context 'required claims is missing' do
@@ -43,7 +77,7 @@ describe OmniAuth::Strategies::Jwt do
       end
 
       it 'raises error' do
-        expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::Jwt::ClaimInvalid)
+        expect { subject.decoded }.to raise_error(OmniAuth::Strategies::Jwt::ClaimInvalid)
       end
     end
 
@@ -57,11 +91,12 @@ describe OmniAuth::Strategies::Jwt do
       end
 
       before do
-        jwt_config.strategy.valid_within = Time.now.to_i
+        # Omniauth config values are always strings!
+        subject.options[:valid_within] = 2.days.to_s
       end
 
       it 'raises error' do
-        expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::Jwt::ClaimInvalid)
+        expect { subject.decoded }.to raise_error(OmniAuth::Strategies::Jwt::ClaimInvalid)
       end
     end
 
@@ -76,11 +111,12 @@ describe OmniAuth::Strategies::Jwt do
       end
 
       before do
-        jwt_config.strategy.valid_within = 2.seconds
+        # Omniauth config values are always strings!
+        subject.options[:valid_within] = 2.seconds.to_s
       end
 
       it 'raises error' do
-        expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::Jwt::ClaimInvalid)
+        expect { subject.decoded }.to raise_error(OmniAuth::Strategies::Jwt::ClaimInvalid)
       end
     end
   end