Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

configurable CORS on login resources #762

Closed
jchris opened this issue Mar 30, 2015 · 6 comments · Fixed by #770
Closed

configurable CORS on login resources #762

jchris opened this issue Mar 30, 2015 · 6 comments · Fixed by #770
Assignees
Labels
Milestone

Comments

@jchris
Copy link
Contributor

jchris commented Mar 30, 2015

The first implementation of CORS support was conservative about enabling login via CORS.

However most apps will want to enable login from CORS hosts, simply because that is where the UI is located. So we need to make an option to enable CORS for logins.

I think this could be another field in the CORS config object, named sessionOrigin which would have an array of string origins as it's value.

For sugar, we could also have the option of true in cases where there will be many origins and all should have login access.

This config would then be consulted in places like this:

if len(h.rq.Header["Origin"]) > 0 {

I'm labeling this a bug because CORS is not useful for anything besides toy apps without this. Tag @jamiltz b/c he's about to run into this.

N.B. It would be easy to convince me that we should just remove the protection from the session endpoints, and if CORS is turned on at all, it's turned on for session stuff too.

@jchris jchris added the bug label Mar 30, 2015
@jchris
Copy link
Contributor Author

jchris commented Mar 30, 2015

Link to example CORS config so you don't have to dig it up yourself https://github.com/couchbase/sync_gateway/blob/master/examples/cors_admin_party.json

@jamesnocentini
Copy link
Contributor

While working on a Todo app with facebook authentication and CORS enabled on SG I was getting back a No CORS response on /db/_facebook which is expected since CORS is currently always disabled on login endpoints. To log users in, @jchris suggested to perform the login on the same origin as SG by hosting an html page as attachment (e.g. /db/loginpage/index.html).

On the app, the user flow would be: webappdomain.com -> sgdomain.com/db/loginpage/index.html -> webappdomain.com.

This requires the attachment to be publicly accessible. SG could provide an api in the config file to allow a set of docs to be public for this use case.

@jchris jchris self-assigned this Apr 6, 2015
@jchris
Copy link
Contributor Author

jchris commented Apr 6, 2015

@jamiltz am I right that you'd prefer to just be able to turn on CORS access for login, instead of some kinda HTML hack?

@jamesnocentini
Copy link
Contributor

@jchris Having CORS enabled with login would be even easier yes but I'm not sure what the security implications would be.

Then I'm guessing web apps would have to set the .withCredentials flag to true to send the SG cookie in authed requests? (https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS#Requests_with_credentials)

@jchris
Copy link
Contributor Author

jchris commented Apr 6, 2015

I think in the case of a FB token, the security situation is clear b/c they make login apps use an app id that matches an origin. But for user/pass sessions it's harder b/c you don't want just any site to put up a UI that accepts user passwords.

@jchris
Copy link
Contributor Author

jchris commented Apr 6, 2015

I'm in the code implementing now, and that last point makes me think we might want different policy for different endpoints. This is one of those things that will probably get controlled by a proxy in a deployed app, but SG should ship with something workable for getting started purposes.

jchris pushed a commit that referenced this issue Apr 6, 2015
jchris added a commit that referenced this issue Apr 7, 2015
@jchris jchris removed the in progress label Apr 7, 2015
@zgramana zgramana modified the milestone: 1.1.0 Jun 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants