Skip to content

Commit

Permalink
Handle unauthorized requests accordingly
Browse files Browse the repository at this point in the history
The XHR token was not set in the HTTP header, which could lead to an unauthorized uploaded (and after that a rejection, because the CSRF - token is also in the submitted form). Now the server is rejecting before the upload, if the CSRF - token is not correct.
  • Loading branch information
tvdeyen authored and sascha-karnatz committed Dec 13, 2023
1 parent a53c560 commit 5aba11c
Show file tree
Hide file tree
Showing 12 changed files with 34 additions and 26 deletions.
4 changes: 3 additions & 1 deletion app/controllers/alchemy/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ def permission_denied(exception = nil)
#{current_alchemy_user.inspect}
WARN
end
if current_alchemy_user
if request.format.json?
render json: {message: Alchemy.t("You are not authorized")}, status: :unauthorized
elsif current_alchemy_user
handle_redirect_for_user
else
handle_redirect_for_guest
Expand Down
11 changes: 1 addition & 10 deletions app/controllers/concerns/alchemy/admin/uploader_responses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ module Admin
module UploaderResponses
extend ActiveSupport::Concern

included do
rescue_from ActiveRecord::ActiveRecordError, with: :error_uploaded_response
end

def successful_uploader_response(file:, status: :created)
message = Alchemy.t(:upload_success,
scope: [:uploader, file.class.model_name.i18n_key],
Expand All @@ -34,15 +30,10 @@ def failed_uploader_response(file:)

private

def error_uploaded_response(exception)
message = Alchemy.t(:error, scope: :uploader, error: exception.message)
render json: {growl_message: message}, status: 500
end

def uploader_response(file:, message:)
{
files: [file.to_jq_upload],
growl_message: message
message: message
}
end
end
Expand Down
1 change: 0 additions & 1 deletion app/javascript/alchemy_admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ setDefaultAnimation("tooltip.hide", {
}
})


// Global Alchemy object
if (typeof window.Alchemy === "undefined") {
window.Alchemy = {}
Expand Down
16 changes: 12 additions & 4 deletions app/javascript/alchemy_admin/components/uploader.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { AlchemyHTMLElement } from "./alchemy_html_element"
import { Progress } from "./uploader/progress"
import { FileUpload } from "./uploader/file_upload"
import { translate } from "alchemy_admin/i18n"
import { getToken } from "alchemy_admin/utils/ajax"

export class Uploader extends AlchemyHTMLElement {
static properties = {
Expand Down Expand Up @@ -81,6 +82,9 @@ export class Uploader extends AlchemyHTMLElement {
fileUploadCount++

request.open("POST", form.action)
request.setRequestHeader("X-CSRF-Token", getToken())
request.setRequestHeader("X-Requested-With", "XMLHttpRequest")
request.setRequestHeader("Accept", "application/json")
request.send(formData)
}

Expand All @@ -93,10 +97,14 @@ export class Uploader extends AlchemyHTMLElement {

// create progress bar
this.uploadProgress = new Progress(fileUploads)
this.uploadProgress.onComplete = () => {
// wait three seconds to see the result of the progressbar
setTimeout(() => (this.uploadProgress.visible = false), 1500)
setTimeout(() => this.dispatchCustomEvent("Upload.Complete"), 2000)
this.uploadProgress.onComplete = (status) => {
if (status === "successful") {
// wait three seconds to see the result of the progressbar
setTimeout(() => (this.uploadProgress.visible = false), 1500)
setTimeout(() => this.dispatchCustomEvent("Upload.Complete"), 2000)
} else {
this.dispatchCustomEvent("Upload.Failure")
}
}

document.body.append(this.uploadProgress)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ export class FileUpload extends AlchemyHTMLElement {
get responseMessage() {
try {
const response = JSON.parse(this.request.responseText)
return response["growl_message"]
return response["message"]
} catch (error) {
return translate("Could not parse JSON result")
}
Expand Down
4 changes: 2 additions & 2 deletions app/javascript/alchemy_admin/components/uploader/progress.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class Progress extends AlchemyHTMLElement {
this.querySelector(`.overall-upload-value`).textContent = overallUploadSize

if (this.finished) {
this.onComplete()
this.onComplete(status)
}

this.className = status
Expand All @@ -90,7 +90,7 @@ export class Progress extends AlchemyHTMLElement {
* it would be possible to trigger the event here, but the dispatching would happen
* in the scope of that component and can't be cached o uploader - component level
*/
onComplete() {}
onComplete(_status) {}

/**
* @returns {boolean}
Expand Down
3 changes: 2 additions & 1 deletion app/javascript/alchemy_admin/utils/ajax.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function buildPromise(xhr) {
})
}

function getToken() {
export function getToken() {
const metaTag = document.querySelector('meta[name="csrf-token"]')
return metaTag.attributes.content.textContent
}
Expand Down Expand Up @@ -53,6 +53,7 @@ export default function ajax(method, path, data, accept = "application/json") {
xhr.open(method, url.toString())
xhr.setRequestHeader("Content-type", "application/json; charset=utf-8")
xhr.setRequestHeader("Accept", accept)
xhr.setRequestHeader("X-Requested-With", "XMLHttpRequest")
xhr.setRequestHeader("X-CSRF-Token", getToken())

if (data && method.toLowerCase() !== "get") {
Expand Down
2 changes: 1 addition & 1 deletion lib/alchemy/test_support/shared_uploader_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
expect(response.media_type).to eq("application/json")
expect(response.status).to eq(422)
json = JSON.parse(response.body)
expect(json).to have_key("growl_message")
expect(json).to have_key("message")
expect(json).to have_key("files")
end
end
4 changes: 2 additions & 2 deletions spec/controllers/alchemy/admin/attachments_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ module Alchemy
expect(response.media_type).to eq("application/json")
expect(response.status).to eq(201)
json = JSON.parse(response.body)
expect(json).to have_key("growl_message")
expect(json).to have_key("message")
expect(json).to have_key("files")
end
end
Expand Down Expand Up @@ -134,7 +134,7 @@ module Alchemy
expect(response.media_type).to eq("application/json")
expect(response.status).to eq(202)
json = JSON.parse(response.body)
expect(json).to have_key("growl_message")
expect(json).to have_key("message")
expect(json).to have_key("files")
end

Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/alchemy/admin/pictures_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ module Alchemy
expect(response.media_type).to eq("application/json")
expect(response.status).to eq(201)
json = JSON.parse(response.body)
expect(json).to have_key("growl_message")
expect(json).to have_key("message")
expect(json).to have_key("files")
end
end
Expand Down
7 changes: 7 additions & 0 deletions spec/javascript/alchemy_admin/components/uploader.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import { Uploader } from "alchemy_admin/components/uploader"

jest.mock("alchemy_admin/utils/ajax", () => {
return {
__esModule: true,
getToken: () => "123"
}
})

describe("alchemy-uploader", () => {
/**
* @type {Uploader}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ describe("alchemy-file-upload", () => {
describe("successful server response", () => {
beforeEach(() => {
const xhrMock = mockXMLHttpRequest(200, {
growl_message: "Foo Bar"
message: "Foo Bar"
})
renderComponent(testFile, xhrMock)
component.request.onload()
Expand All @@ -150,7 +150,7 @@ describe("alchemy-file-upload", () => {
describe("failed server response", () => {
beforeEach(() => {
const xhrMock = mockXMLHttpRequest(400, {
growl_message: "Error: Foo Bar"
message: "Error: Foo Bar"
})
renderComponent(testFile, xhrMock)
component.request.onload()
Expand Down

0 comments on commit 5aba11c

Please sign in to comment.