Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

update: headers for CORS #133

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions app/controllers/ldp/ReadWriteWebControllerGeneric.scala
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ trait ReadWriteWebControllerGeneric extends ReadWriteWebControllerTrait {
private def allowHeaders(authResult: AuthResult[NamedResource[Rdf]]): List[(String, String)] = {
val modesAllowed = authResult.authInfo.modesAllowed
val isLDPC = authResult.result.isInstanceOf[LocalLDPC[_]]
val allow = "Allow" -> {
val allow = "Access-Control-Allow-Methods" -> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LDP spec requires the Allow. See ¶ 4.2.8.2 of the LDP spec. So it can't be replaced the way it is done here.
The Access-Control-Allow-Methods is part of the CORS standard and has a different meaning. The reason for its existence has to be looked at in a lot greater detail to understand what the dangers associated with it are.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I hadn't found that. I'm wondering why they didn't use the same header. Can there be cases where the two headers have different values?
Otherwise I'm just going to duplicate it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is that the Allow header is saying: you can use these methods as you are authenticated now. The Access-Control-Allow-Methods field is saying what you can do if a JS script is holding a gun to your head and telling you exactly what to do. There are certain JS scripts that are allowed to take over your brain perhaps with the Origin header, but not all.
So in the case that the RDF is completely public then the two should be the same. What is really needed is in the WebID profile of the user who is authenticated, for him to specify the JS origins he accepts ( As long as an arbitrary JS can't just edit that file. )

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! Not sure to get the gun metaphore, but I'll add both.
I agree with you for the list in the webId. I guess the server could have its own black list as well. I'm wondering how we can ask the user if the origin should be added to the trusted ones. I guess we could do a redirection to the server holding the WebId. Not sure if it can be done on an OPTIONS request though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I have written up a fuller explanation of what I think is actually needed on the Web Access Control wiki page.
https://www.w3.org/wiki/WebAccessControl#Cors_User_Agents

I'll see if I get some feedback on this from the larger community. But let me see what you think.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a pretty good solution. One remark, the CORS problem is not limited to Write operations. Even giving access to a script to private data may be a real issue, as that script can then send that data somewhere else.

Another detail, I'm not sure to understand why the term class is used to specify the origin in the example ACL.

val headerStr = modesAllowed.collect {
case Method.Append =>
authResult.result match {
Expand Down Expand Up @@ -204,7 +204,9 @@ trait ReadWriteWebControllerGeneric extends ReadWriteWebControllerTrait {
writerFor[Rdf#Graph](request).map { wr =>
val headers =
"Access-Control-Allow-Origin" -> "*" ::
"Accept-Patch" -> Syntax.SparqlUpdate.mimeTypes.head.mime :: //todo: something that is more flexible
"Access-Control-Allow-Headers" -> "content-type, If-Match, Link, Slug" ::
"Access-Control-Expose-Headers" -> "Etag" ::
"Accept-Patch" -> Syntax.SparqlUpdate.mimeTypes.head.mime :: //todo: something that is more flexible
linkHeaders(ldpr) ::
commonHeaders
result(200, wr, Map(headers: _*))(ldpr.relativeGraph)
Expand Down