Add out of the box CORS support

[e0ipso]

Things to discuss:

• [ ] Enable CORS support for '*' out of the box as a default.
• [ ] Allow changing the '*' for a list of allowed domains using the UI installer.
• [ ] Provide a contenta-cors-enable drush command. Other?

[dawehner]

Added a 3rd point.

[dawehner]

Note: I tried to implement the drush command using https://github.com/klausi/yaml_comments but sadly this libraryt has some bugs with counting the file numbers.

[dawehner]

dawehner @justafish Didn't you used you maintain the CORS module, you could have an opinion on #19

[2:27 PM] 
@e0ipso ... do you plan to change the drupal internal names as well?

[2:27 PM] 
e0ipso @dawehner no, just the API output

[2:28 PM] 
(and input)

[2:28 PM] 
dawehner sure, no worries :slightly_smiling_face:

[2:30 PM] 
e0ipso I’d say _I’ll let you know the details_, but after @dpolant’s patch I won’t need to. Docson will let you know :slightly_smiling_face:

[2:30 PM] 
@dawehner going for lunch. ttyl

[2:30 PM] 
dawehner haha

[2:30 PM] 
@dpolant++

[2:30 PM] 
@e0ipso have a good one.

[2:30 PM] 
@e0ipso wait

[2:33 PM] 
@e0ipso its not even late breakfast time yet

[4:18 PM] 
justafish
@dawehner I’m not sure a drush command is necessary. I think a note in the Wiki/README is fine

[4:18 PM] 
but other than, that :+1: for enabling *

[4:29 PM] 
dawehner @justafish so it shouldn't be enabled by default?

[4:43 PM] 
justafish
@dawehner enable it by default, explain in the wiki/readme how to restrict by domain

[4:44 PM] 
I don’t think it’s worth abstracting into a drush command, and I think it’s better for people to learn exactly what that setting does and how it works

[4:48 PM] 
dawehner fair point. Educating people is a good thing indeed!

[5:05 PM] 
e0ipso I’m cautious about  `'*'` by default

[5:06 PM] 
Browsers implement CORS as a directive

[5:06 PM] 
for security reasons

[5:07 PM] 
I think that removing that for Contenta sites

[5:07 PM] 
would make less technical people more vulnerable

[5:07 PM] 
to session ridding attacks

[5:08 PM] 
Bear in mind that less technical people are also the ones that are less likely to know about those attacks```

Given thgat 

[dawehner]

We had the idea to just expose it for localhost, aka.

    enabled: true
    allowedHeaders:
      - '*'
    allowedMethods:
      - '*'
    allowedOrigins:
      - localhost
    exposedHeaders: false
    maxAge: false
    supportsCredentials: false

, but well, this doesn't work, as it does check the port, aka. 8000 for me, so this didn't worked as expected.

[dawehner]

We have documentation about CORS now, see https://github.com/contentacms/contenta_jsonapi#cors

[e0ipso]

I think a sensible default can be to use the port that drush runserver uses by default, since the installer will use that one.

[dawehner]

Well it's sensible but also pointless isn't it? The app will not run on this particular port, right?

On Mon, 29 May 2017, 21:45 Mateu Aguiló Bosch, notifications@github.com wrote:

I think a sensible default can be to use the port that drush runserver uses by default, since the installer will use that one.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/contentacms/contenta_jsonapi/issues/19#issuecomment-304728152, or mute the thread https://github.com/notifications/unsubscribe-auth/AABz7i3Hwr-TBONwreJSuLRBk4XhyTvgks5r-y5OgaJpZM4NoohQ .

[e0ipso]

:man_facepalming: you are right.

[e0ipso]

3000 and 8000 seem to be the most popular. Whatever value works for me.

[dawehner]

Oh yeah I've seen 3000 4000 and 8000. Works for me

On Mon, 29 May 2017, 22:23 Mateu Aguiló Bosch, notifications@github.com wrote:

3000 and 8000 seem to be the most popular. Whatever value works for me.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/contentacms/contenta_jsonapi/issues/19#issuecomment-304732908, or mute the thread https://github.com/notifications/unsubscribe-auth/AABz7hge2cr4ckvSl5_TkOXrUEuk63voks5r-zdVgaJpZM4NoohQ .