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

Basic Auth implementation does not use login credentials provided in options.auth #204

Closed
orolle opened this issue Jan 7, 2018 · 7 comments
Assignees

Comments

@orolle
Copy link

orolle commented Jan 7, 2018

In release v1.0.0 basic auth is implemented. This implenetation uses only credentials in provided in the database url in the form of https://user:pwd@domain.com/database. Credentials provided in options.auth are ignored.

Expected Behavior and Current Behavior

var db1 = new Pouchdb("https://domain.com/database", {"auth": {"username":"user", "password": "pwd"}})
var db2 = new Pouchdb(" https://user:pwd@domain.com/database");
/* db1 and db2 should behave exactly the same but do not*/

Possible Solution

In commit function getBasicAuthHeaders(db) {...} uses just the db.name to create HTTP Basic Auth header, but should also use the other options provided into Pouchdb() constructor. It seems all other HTTP header options in options are also ignored like for custom headers.

Steps to Reproduce (for bugs)

var db1 = new Pouchdb("https://domain.com/database", {"auth": {"username":"user", "password": "pwd"}});
db.getUser("user").then(...);

The above code fails.

Context

Firstly, my usernames are email addresses. therefore i cannot use the Basic Auth in url.
Secondly, Apple Safari (iOS, desktop) has a very restrictive cookie policy which disallows web sites to see cookies from another site if the user himself did not visit the other site with visible content too. Pure CORS with session cookies in not possible with safari. workarounds exists but complicated (iframes, redirects). Therefore basic auth over HTTPS is a good solution for that issue.

Your Environment

  • Version of PouchDB Authentication: 1.1.0
  • Version of PouchDB: 6.3.4
  • Platform name and version: Chrome, Firefox, Safari
  • Operating System and version: Linux, iOS
  • Server: CouchDB 2.0.0

Remarks

When the fix is released, can you ping me so that I can update the clojurescript wrapper on https://clojars.org/

@ptitjes
Copy link
Collaborator

ptitjes commented Jan 7, 2018

Thanks @orolle for the very good bug report! I will look into this soon.
I did not know about the auth option in PouchDB constructor, so I have to dig the code on what remains in db.__opts about this in order to fix getBasicAuthHeaders().

@ptitjes ptitjes self-assigned this Jan 7, 2018
ptitjes added a commit to ptitjes/pouchdb-authentication that referenced this issue Jan 13, 2018
Previously only Basic Authentication information from url was used.
Now, also information from db.__opts.auth is used.

Fixes pouchdb-community#204.
@ptitjes
Copy link
Collaborator

ptitjes commented Jan 13, 2018

@orolle #208 does seem to do the job. Would you mind testing it before I merge ?

@orolle
Copy link
Author

orolle commented Jan 14, 2018

@ptitjes I tried to get the test to run, but I have a wired npm issue (nothing to do with your code) and I have no idea how to fix this. I have too little experience with npm and node.js.
I reviewed your code and tests. Your code looks good. The test covers the change. I am very happy with the result :-)

@orolle orolle closed this as completed Jan 14, 2018
@ptitjes
Copy link
Collaborator

ptitjes commented Jan 14, 2018

@orolle OK I'll merge it now then. I'll do a release in the next days and I'll ping you then !

@ptitjes
Copy link
Collaborator

ptitjes commented Jan 20, 2018

@orolle
Copy link
Author

orolle commented Jan 22, 2018

@ptitjes thanks! The release to cljsjs is just a matter of a few days :-)
cljsjs/packages#1464

@orolle
Copy link
Author

orolle commented Jan 24, 2018

Thanks a lot! the fix is now in production and our safari users are happy :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants