added app://. for ALLOWED_CORS_ORIGINS

[bwship]

This allows the Commandeer App to talk to LocalStack.

[bwship]

@thrau - here is the updated PR. This one is the correct one. It is just two additional lines, and two lines of comments.

[dfangl]

Hi! First of all, the PR is fine, thanks for fixing it!

Regarding the concrete implications: Would it be possible, as an alternative to this PR, to add a onBeforeSendHeaders listener (https://www.electronjs.org/docs/latest/api/web-request#webrequestonbeforesendheadersfilter-listener) to Commandeer, which drops the origin header (for requests to localstack)? This would seem like a cleaner approach to me, in comparison to allowing the "app://" origin. It was necessary for dynamodb nosql, as we could not really impact this there.

LocalStack will allow any request without an Origin header set (as this is the behavior for awscli as well, for example), so dropping it when connecting to LocalStack seems like a valid solution, I am thinking of something like this:

const { session } = require('electron')

const filter = {
  urls: ['*://localhost:*', '*://localhost.localstack.cloud*']
}

session.defaultSession.webRequest.onBeforeSendHeaders(filter, (details, callback) => {
      details.requestHeaders['Origin'] = null;
      callback({cancel: false, requestHeaders: details.requestHeaders});
    });

Without knowing too much about commandeer, this could work. If this is too much work or not feasible, we are happy to merge this PR.

[bwship]

Daniel,

What you laid our was a great solution. I had to modify it slightly to work, but got a new release out with that in. You can close out this PR, as it is working now!

Cheers, Bob Wall

(703) 919-3750

On Tue, Aug 9, 2022 at 6:19 AM Daniel Fangl @.***> wrote:

Hi! First of all, the PR is fine, thanks for fixing it!

Regarding the concrete implications: Would it be possible, as an alternative to this PR, to add a onBeforeSendHeaders listener ( https://www.electronjs.org/docs/latest/api/web-request#webrequestonbeforesendheadersfilter-listener) to Commandeer, which drops the origin header (for requests to localstack)? This would seem like a cleaner approach to me, in comparison to allowing the "app://" origin. It was necessary for dynamodb nosql, as we could not really impact this there.

LocalStack will allow any request without an Origin header set (as this is the behavior for awscli as well, for example), so dropping it when connecting to LocalStack seems like a valid solution, I am thinking of something like this:

const { session } = require('electron') const filter = { urls: ['://localhost:', '://localhost.localstack.cloud']} session.defaultSession.webRequest.onBeforeSendHeaders(filter, (details, callback) => { details.requestHeaders['Origin'] = null; callback({cancel: false, requestHeaders: details.requestHeaders}); });

Without knowing too much about commandeer, this could work. If this is too much work or not feasible, we are happy to merge this PR.

— Reply to this email directly, view it on GitHub https://github.com/localstack/localstack/pull/6611#issuecomment-1209192708, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKDYISCBBZJKSJSBJ5BW3LVYIWCHANCNFSM555BI6KQ . You are receiving this because you authored the thread.Message ID: @.***>

[dfangl]

Great to hear that! Will close now.