allow multiple inst-fs secrets for key rotation

refs SAS-1215

test-plan:
- specs confirm existing behavior is still fine
- inst-fs secret can be set to a space separated list of multiple keys
  and:
  - signing still works as before, using the foremost key
  - validation of jwt to /api/v1/files/capture can use any of the keys

Change-Id: I0edc478ec07b44b197b5d9b066b3e0ef552966a8
Reviewed-on: https://gerrit.instructure.com/206932
Tested-by: Jenkins
Reviewed-by: Nathan Mills <[email protected]>
QA-Review: Jacob Fugal <[email protected]>
Product-Review: Jacob Fugal <[email protected]>

app/controllers/files_controller.rb

@@ -857,9 +857,7 @@ def api_capture
     end
 
     # check service authorization
-    begin
-      Canvas::Security.decode_jwt(params[:token], [ InstFS.jwt_secret ])
-    rescue
+    unless InstFS.validate_capture_jwt(params[:token])
       head :forbidden
       return
     end

lib/inst_fs.rb

@@ -66,11 +66,23 @@ def app_host
       setting("app-host")
     end
 
-    def jwt_secret
+    def jwt_secrets
       secret = setting("secret")
-      if secret
-        Base64.decode64(secret)
-      end
+      return [] unless secret
+      secret.split(/\s+/).map{ |key| Base64.decode64(key) }
+    end
+
+    def jwt_secret
+      # if there are multiple keys (to allow for validating during key
+      # rotation), the foremost is used for signing
+      jwt_secrets.first
+    end
+
+    def validate_capture_jwt(token)
+      Canvas::Security.decode_jwt(token, jwt_secrets)
+      true
+    rescue
+      false
     end
 
     def upload_preflight_json(context:, root_account:, user:, acting_as:, access_token:, folder:, filename:,

spec/apis/v1/files_controller_api_spec.rb

@@ -79,7 +79,7 @@ def call_course_create_file
     it 'includes include capture param in inst_fs token' do
       secret = 'secret'
       allow(InstFS).to receive(:enabled?).and_return true
-      allow(InstFS).to receive(:jwt_secret).and_return(secret)
+      allow(InstFS).to receive(:jwt_secrets).and_return([secret])
       json = call_course_create_file
       query = Rack::Utils.parse_nested_query(URI(json['upload_url']).query)
       payload = Canvas::Security.decode_jwt(query['token'], [secret])
@@ -332,7 +332,7 @@ def call_create_success(params = {})
 
     before do
       allow(InstFS).to receive(:enabled?).and_return true
-      allow(InstFS).to receive(:jwt_secret).and_return(secret)
+      allow(InstFS).to receive(:jwt_secrets).and_return([secret])
     end
 
     it "is not available without the InstFS feature" do

spec/controllers/files_controller_spec.rb

@@ -1189,7 +1189,7 @@ def submit_file
   describe "POST api_capture" do
     before :each do
       allow(InstFS).to receive(:enabled?).and_return(true)
-      allow(InstFS).to receive(:jwt_secret).and_return("jwt signing key")
+      allow(InstFS).to receive(:jwt_secrets).and_return(["jwt signing key"])
       @token = Canvas::Security.create_jwt({}, nil, InstFS.jwt_secret)
     end
 

spec/lib/inst_fs_spec.rb

@@ -23,20 +23,44 @@
     before do
       @app_host = 'http://test.host'
       @secret = "supersecretyup"
+      @rotating_secret = "anothersecret"
+      @secrets = [@secret, @rotating_secret]
+      encoded_secrets = @secrets.map{ |secret| Base64.encode64(secret) }.join(" ")
       allow(InstFS).to receive(:enabled?).and_return(true)
       allow(Canvas::DynamicSettings).to receive(:find).with(any_args).and_call_original
       allow(Canvas::DynamicSettings).to receive(:find).
         with(service: "inst-fs", default_ttl: 5.minutes).
         and_return({
           'app-host' => @app_host,
-          'secret' => Base64.encode64(@secret)
+          'secret' => encoded_secrets
         })
     end
 
-    it "returns decoded base 64 secret" do
+    it "returns primary decoded base 64 secret" do
       expect(InstFS.jwt_secret).to eq(@secret)
     end
 
+    it "returns all decoded base 64 secrets" do
+      expect(InstFS.jwt_secrets).to eq(@secrets)
+    end
+
+    context "validate_capture_jwt" do
+      it "returns true for jwt signed with primary key" do
+        token = Canvas::Security.create_jwt({}, nil, @secret, :HS512)
+        expect(InstFS.validate_capture_jwt(token)).to eq(true)
+      end
+
+      it "returns true for jwt signed with rotating key" do
+        token = Canvas::Security.create_jwt({}, nil, @rotating_secret, :HS512)
+        expect(InstFS.validate_capture_jwt(token)).to eq(true)
+      end
+
+      it "returns false for jwt signed with bogus key" do
+        token = Canvas::Security.create_jwt({}, nil, "boguskey", :HS512)
+        expect(InstFS.validate_capture_jwt(token)).to eq(false)
+      end
+    end
+
     context "authenticated_url" do
       before :each do
         @attachment = attachment_with_context(user_model)
@@ -529,8 +553,13 @@ def claims_for(options)
     it "instfs is not enabled" do
       expect(InstFS.enabled?).to be false
     end
+
     it "doesn't error on jwt_secret" do
       expect(InstFS.jwt_secret).to be_nil
     end
+
+    it "returns empty list of secrets" do
+      expect(InstFS.jwt_secrets).to eq([])
+    end
   end
 end