Skip to content

Commit

Permalink
instfs login pixel, send cookies on instfs upload
Browse files Browse the repository at this point in the history
refs RECNVS-106, fixes RECNVS-257

test-plan:
[upload sends inst-fs credentials]
 - set `REQUIRE_AUTH: 'true'` in your inst-fs docker-compose.local.yml
   and ensure your jwt secret and oauth flow are synced between canvas
   and inst-fs
 - turn on inst-fs in canvas
 - delete any cookies on the api.instfs.docker domain
 - login to canvas and upload a file on the user's files page, should
   work
[regression]
 - turn inst-fs back off and configure canvas for S3 storage
 - upload a file on the user's files page, should work
 - configure canvas for local storage, repeat

Change-Id: If646a6630554da5778852d8eb1abc976cd77886d
Reviewed-on: https://gerrit.instructure.com/138539
Reviewed-by: Andrew Huff <[email protected]>
Tested-by: Jenkins
QA-Review: Collin Parrish <[email protected]>
Product-Review: Jacob Fugal <[email protected]>
  • Loading branch information
lukfugl committed Jan 27, 2018
1 parent 0a15ef7 commit bb4cea2
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 8 deletions.
4 changes: 4 additions & 0 deletions app/controllers/login/shared.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ def successful_login(user, pseudonym, otp_passed = false)
end
end

# ensure the next page rendered includes an instfs pixel to log them in
# there
session.delete(:shown_instfs_pixel)

if pseudonym.account_id != (@real_domain_root_account || @domain_root_account).id
flash[:notice] = t("You are logged in at %{institution1} using your credentials from %{institution2}",
institution1: (@real_domain_root_account || @domain_root_account).name,
Expand Down
6 changes: 4 additions & 2 deletions app/jsx/shared/upload_file.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,17 @@ export function completeUpload(preflightResponse, file, options={}) {
upload_params = upload_params || {};
success_url = success_url || upload_params.success_url;
const ajaxLib = options.ajaxLib || axios;
const isToS3 = !!success_url;

// post upload
// xsslint xssable.receiver.whitelist formData
const formData = new FormData();
Object.entries(upload_params).forEach(([key, value]) => formData.append(key, value));
formData.append(file_param, file, options.filename);
const upload = ajaxLib.post(upload_url, formData, {
responseType: (success_url ? 'document' : 'json'),
onUploadProgress: options.onProgress
responseType: (isToS3 ? 'document' : 'json'),
onUploadProgress: options.onProgress,
withCredentials: !isToS3
});

// finalize upload
Expand Down
2 changes: 2 additions & 0 deletions app/views/layouts/_foot.html.erb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
<%= InstFS.login_pixel(@current_user, session) %>

<script>
INST = <%= raw(inst_env.to_json) %>;
ENV = <%= raw StringifyIds.recursively_stringify_ids(js_env.clone).to_json %>;
Expand Down
37 changes: 31 additions & 6 deletions lib/inst_fs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ def enabled?
Canvas::Plugin.find('inst_fs').enabled?
end

def login_pixel(user, session)
if !session[:shown_instfs_pixel] && user && app_host
session[:shown_instfs_pixel] = true
pixel_url = login_pixel_url(token: session_jwt(user))
%Q(<img src="#{pixel_url}" alt="" role="presentation" />).html_safe
end
end

def authenticated_url(attachment, options={})
query_params = { token: access_jwt(attachment, options) }
query_params[:download] = 1 if options[:download]
Expand Down Expand Up @@ -69,19 +77,27 @@ def setting(key)
nil
end

def service_url(path, query_params=nil)
url = "#{app_host}#{path}"
url += "?#{query_params.to_query}" if query_params
url
end

def login_pixel_url(query_params)
service_url("/session/ensure", query_params)
end

def access_url(attachment, query_params)
"#{app_host}/files/#{attachment.instfs_uuid}/#{attachment.filename}?#{query_params.to_query}"
service_url("/files/#{attachment.instfs_uuid}/#{attachment.filename}", query_params)
end

def thumbnail_url(attachment, query_params)
"#{app_host}/thumbnails/#{attachment.instfs_uuid}?#{query_params.to_query}"
service_url("/thumbnails/#{attachment.instfs_uuid}", query_params)
end

def upload_url(token=nil)
query_string = { token: token }.to_query if token
url = "#{app_host}/files"
url += "?#{query_string}" if query_string
url
query_string = { token: token } if token
service_url("/files", query_string)
end

def access_jwt(attachment, options={})
Expand All @@ -104,5 +120,14 @@ def upload_jwt(user, capture_url, capture_params)
capture_params: capture_params
}, expires_in.from_now, self.jwt_secret)
end

def session_jwt(user)
expires_in = Setting.get('instfs.session_jwt.expiration_minutes', '5').to_i.minutes
Canvas::Security.create_jwt({
iat: Time.now.utc.to_i,
user_id: user.global_id&.to_s,
resource: '/session/ensure'
}, expires_in.from_now, self.jwt_secret)
end
end
end
54 changes: 54 additions & 0 deletions spec/javascripts/jsx/shared/uploadFileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -273,3 +273,57 @@ test('completeUpload appends avatar include in GET after inst-fs upload if optio
done();
});
})

test('completeUpload to S3 posts withCredentials false', assert => {
const done = assert.async()
const successUrl = 'http://successUrl';

const postStub = sinon.stub();
const getStub = sinon.stub();
postStub.returns(Promise.resolve({ data: {} }));
getStub.returns(Promise.resolve({ data: {} }));

const fakeAjaxLib = {
post: postStub,
get: getStub
};

const preflightResponse = {
upload_url: 'http://uploadUrl',
success_url: successUrl
};
const file = sinon.stub();
const options = { ajaxLib: fakeAjaxLib };

completeUpload(preflightResponse, file, options).then(() => {
ok(postStub.calledWith(sinon.match.any, sinon.match.any, sinon.match({
withCredentials: false
})), 'withCredentials is false');
done();
});
})

test('completeUpload to non-S3 posts withCredentials true', assert => {
const done = assert.async()

const postStub = sinon.stub();
const getStub = sinon.stub();
postStub.returns(Promise.resolve({ data: {} }));
getStub.returns(Promise.resolve({ data: {} }));

const fakeAjaxLib = {
post: postStub,
get: getStub
};

const preflightResponse = { upload_url: 'http://uploadUrl' };
const file = sinon.stub();
const options = { ajaxLib: fakeAjaxLib };

completeUpload(preflightResponse, file, options).then(() => {
ok(postStub.calledWith(sinon.match.any, sinon.match.any, sinon.match({
withCredentials: true
})), 'withCredentials is true');
done();
});
})

0 comments on commit bb4cea2

Please sign in to comment.