oarevalo / BugLogHQ

BugLogHQ is a tool to centralize the handling of automated bug reports from multiple applications. BugLogHQ provides a unified view of error messages sent from any number of applications, allowing the developer to search, graph, forward, and explore the bug reports submitted by the applications.
http://www.bugloghq.com
155 stars 67 forks source link

CORS #128

Closed coryasilva closed 8 years ago

coryasilva commented 8 years ago

What do you think of allowing CORS on the rest endpoint. Currently we are using our AngularJS logging Module with these slight modifications the the REST listener.

Now as BugLogHQ is concerned I would imagine there should be some switches for this logic in the UI. When you have time let me know what you think.

(Sorry I accidentally included my changes from pull request #127, but this pull request was only meant to start dialogue.)

oarevalo commented 8 years ago

Hey Cory, thanks for the suggestion. I'm not too familiar with CORS, but can you explain why would there be a need for any UI toggling of this? it seems to me that the implementation is very transparent and only activated when explicitly calling the OPTIONS method. If anything a top-level config setting on the main config file to allow the CORS support to be toggled off completely (I would be inclined for the 'On' default)

I'd suggest though to use a regular CFINCLUDE and/or include this functionality directly in the bugLogListenerREST.cfm file. If this is something that is required on every request to the listener, creating and invoking a CFC sounds like unnecessary overhead.

coryasilva commented 8 years ago

If a CORS header exists more than once in a response it will throw an error on a client web browser. This could happen if a server admin set the CORS headers on the web server itself. For example in IIS you can make these settings in the "HTTP Responses". Though, I prefer to have them in the code... Having CORS on by default sounds great!

Also it would be nice if we could alter the Access-Control-Allow-Origin string value in the UI in order to restrict cross origin access to a select few domains.

Perhaps the UI pieces can be added at a later date or be included in the configuration xml; what do you think?

Regarding your last comment; I suck at Coldfusion and will just add that into the bugLogListenerREST.cfm as you suggested.

oarevalo commented 8 years ago

Makes sense, in that case I think we could add it only at the config file level and then maybe on a future release do it on the UI based on user feedback.

My suggestion would be to add the following settings to the buglog-config.xml.cfm file:

<setting name="cors.enabled">true</setting>
<setting name="cors.allowOrigin">*</setting>

This way anyone deploying BugLogHQ can tailor it to their environment, while still providing a "relaxed" default config.

coryasilva commented 8 years ago

Got it; how do I best get setting from the buglog-config.xml.cfm?

oarevalo commented 8 years ago

Typically you would do:

var serviceLoader = createObject("component", "bugLog.components.service").init( instance );
var setting_value = serviceLoader.getConfig().getSetting( setting_name );

Where you can get the value of instance from bugLogListenerREST.cfm (after the cfparams).

However... the ServiceLoader is already being instantiated inside listener.cfc on the same listeners directory, inside logEntry(). So what you could do to avoid the double instantiation of the same CFC is move the instantiation of the service loader in listener.cfc to the init() method, and add a new method to the listener to either handle the CORS stuff or to retrieve all the needed data for it (enabled/disabled, allow-origin value) so you can handle it on the CFC