Store public keys on developer key [ci no-db-snapshot]

Closes PLAT-3684

Test Plan:
- Create a new developer key and call
  generate_rsa_keypair!
- Verify its public_jwk column was populated
- Verify its private_jwk attribute was populated
  (This attribute is volatile: It is only populated in memory, not
   persisted to the DB)

Change-Id: I8e1f0ebb597228f9e91f463d00bba1d360c27e6d
Reviewed-on: https://gerrit.instructure.com/161576
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
Reviewed-by: Marc Alan Phillips <mphillips@instructure.com>
QA-Review: Marc Alan Phillips <mphillips@instructure.com>
Product-Review: Weston Dransfield <wdransfield@instructure.com>

app/models/developer_key.rb

@@ -45,6 +45,9 @@ class DeveloperKey < ActiveRecord::Base
 
   validates_as_url :redirect_uri, allowed_schemes: nil
   validate :validate_redirect_uris
+  validate :validate_public_jwk
+
+  attr_reader :private_jwk
 
   scope :nondeleted, -> { where("workflow_state<>'deleted'") }
   scope :not_active, -> { where("workflow_state<>'active'") } # search for deleted & inactive keys
@@ -104,6 +107,13 @@ class DeveloperKey < ActiveRecord::Base
     self.api_key = CanvasSlug.generate(nil, 64) if overwrite || !self.api_key
   end
 
+  def generate_rsa_keypair!(overwrite: false)
+    return if public_jwk.present? && !overwrite
+    key_pair = Lti::RSAKeyPair.new
+    @private_jwk = key_pair.to_jwk
+    self.public_jwk = key_pair.public_jwk.to_h
+  end
+
   def set_auto_expire_tokens
     self.auto_expire_tokens = true
   end
@@ -227,6 +237,18 @@ class DeveloperKey < ActiveRecord::Base
 
   private
 
+  def validate_public_jwk
+    return true if public_jwk.blank?
+
+    if public_jwk['kty'] != Lti::RSAKeyPair::KTY
+      errors.add :public_jwk, "Must use #{Lti::RSAKeyPair::KTY} kty"
+    end
+
+    if public_jwk['alg'] != Lti::RSAKeyPair::ALG
+      errors.add :public_jwk, "Must use #{Lti::RSAKeyPair::ALG} alg"
+    end
+  end
+
   def invalidate_access_tokens_if_scopes_removed!
     return unless developer_key_management_and_scoping_on?
     return unless saved_change_to_scopes?

db/migrate/20180821193008_add_public_jwk_to_developer_keys.rb

@@ -0,0 +1,24 @@
+#
+# Copyright (C) 2018 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+
+class AddPublicJwkToDeveloperKeys < ActiveRecord::Migration[5.1]
+  tag :predeploy
+
+  def change
+    add_column :developer_keys, :public_jwk, :jsonb
+  end
+end

lib/lti/jwk_key_pair.rb

@@ -19,8 +19,17 @@ module Lti
   class JWKKeyPair
     attr_reader :public_key, :private_key, :alg, :use
     def to_jwk
-      time = Time.now.utc.iso8601
-      private_key.to_jwk(kid: time, alg:alg, use:use)
+      private_key.to_jwk(kid: kid, alg:alg, use:use)
+    end
+
+    def public_jwk
+      private_key.public_key.to_jwk(kid: kid, alg:alg, use:use)
+    end
+
+    private
+
+    def kid
+      @_kid ||= Time.now.utc.iso8601
     end
   end
 end

lib/lti/rsa_key_pair.rb

@@ -19,6 +19,7 @@ require 'openssl'
 
 module Lti
   class RSAKeyPair < JWKKeyPair
+    KTY = 'RSA'.freeze
     ALG = 'RS256'.freeze
     SIZE = 2048
     def initialize(use: 'sig')

spec/lib/lti/jwk_key_pair_spec.rb

@@ -27,4 +27,20 @@ describe Lti::JWKKeyPair do
       end
     end
   end
+
+  describe "public_jwk" do
+    it 'includes the public key in JWK format' do
+      Timecop.freeze(Time.zone.now) do
+        keys = Lti::RSAKeyPair.new
+        expect(keys.public_jwk).to include(keys.private_key.public_key.to_jwk(kid: Time.now.utc.iso8601))
+      end
+    end
+
+    it 'does not include the private key claims in JWK format' do
+      Timecop.freeze(Time.zone.now) do
+        keys = Lti::RSAKeyPair.new
+        expect(keys.public_jwk.keys).not_to include 'd', 'p', 'q', 'dp', 'dq', 'qi'
+      end
+    end
+  end
 end

spec/models/developer_key_spec.rb

@@ -70,6 +70,22 @@ describe DeveloperKey do
   end
 
   describe 'callbacks' do
+    describe 'public_jwk validations' do
+      subject { developer_key_saved }
+
+      before { subject.public_jwk = {invalid: 'test'} }
+
+      it 'verifies public_jwk kty is "RSA"' do
+        expect(subject.save).to eq false
+      end
+
+      it 'adds an error message whn public_jwk is invalid' do
+        subject.public_jwk = {invalid: 'test'}
+        subject.save
+        expect(subject.errors[:public_jwk]).to include 'Must use RSA kty'
+      end
+    end
+
     it 'does not validate scopes' do
       expect do
         DeveloperKey.create!(
@@ -362,6 +378,65 @@ describe DeveloperKey do
     expect(developer_key_saved.last_used_at).not_to be_nil
   end
 
+  describe '#generate_rsa_keypair!' do
+    context 'when "public_jwk" is already set' do
+      subject do
+        developer_key.generate_rsa_keypair!
+        developer_key
+      end
+
+      let(:developer_key) do
+        key = DeveloperKey.create!
+        key.generate_rsa_keypair!
+        key.save!
+        key
+      end
+      let(:public_jwk) { developer_key.public_jwk }
+
+      context 'when "override" is false' do
+        it 'does not change the "public_jwk"' do
+          expect(subject.public_jwk).to eq public_jwk
+        end
+
+        it 'does not change the "private_jwk" attribute' do
+          previous_private_key = developer_key.private_jwk
+          expect(subject.private_jwk).to eq previous_private_key
+        end
+      end
+
+      context 'when "override: is true' do
+        subject do
+          developer_key.generate_rsa_keypair!(overwrite: true)
+          developer_key
+        end
+
+        it 'does change the "public_jwk"' do
+          previous_public_key = developer_key.public_jwk
+          expect(subject.public_jwk).not_to eq previous_public_key
+        end
+
+        it 'does change the "private_jwk"' do
+          previous_private_key = developer_key.private_jwk
+          expect(subject.private_jwk).not_to eq previous_private_key
+        end
+      end
+    end
+
+    context 'when "public_jwk" is not set' do
+      subject { DeveloperKey.new }
+
+      before { subject.generate_rsa_keypair! }
+
+      it 'populates the "public_jwk" column with a public key' do
+        expect(subject.public_jwk['kty']).to eq Lti::RSAKeyPair::KTY
+      end
+
+      it 'populates the "private_jwk" attribute with a private key' do
+        expect(subject.private_jwk['kty']).to eq Lti::RSAKeyPair::KTY.to_sym
+      end
+    end
+  end
+
   describe "#redirect_domain_matches?" do
     it "should match domains exactly, and sub-domains" do
       developer_key_not_saved.redirect_uri = "http://example.com/a/b"