pouchdb-community / pouchdb-authentication

User authentication plugin for PouchDB and CouchDB.
Apache License 2.0
775 stars 118 forks source link

Compatibility with PouchDB 7 #254

Closed leonid-shevtsov closed 1 year ago

leonid-shevtsov commented 5 years ago

I've seen #238, but it looked abandoned, and there was refactoring seemingly unrelated to PouchDB 7. I figured it's easier to start over. Sorry about that.

Needless to say, this changeset is not compatible with PouchDB 6 anymore.

leonid-shevtsov commented 5 years ago

There are some issues in the tests:

jlami commented 5 years ago

Can we use the api.fetch that pouchdb-adapter-http exposes? https://github.com/pouchdb/pouchdb/blob/858660e9b37959d96eebf667ea9b8b1273f0d62c/packages/node_modules/pouchdb-adapter-http/src/index.js#L437 That one also supports 'overloading' so a user can give their own fetch function to modifiy the request.

We recently found out that we needed to overrule a signup request when logged in as a non-admin user to be able to prevent credentials from being included. I don't really see a way of doing this with your setup.

If we switched to api.fetch it might also prevent the need for getBasicAuthHeaders, since those seem to be handled there: https://github.com/pouchdb/pouchdb/blob/858660e9b37959d96eebf667ea9b8b1273f0d62c/packages/node_modules/pouchdb-adapter-http/src/index.js#L169-L176

leonid-shevtsov commented 5 years ago

@jlami good point.

I was already using the fetch from pouchdb-adapter-http, which is available through PouchDB.fetch and in db.fetch, so it did support overloading

However, I did some digging and turns out, if you use db.fetch, you can pass it paths instead of URLs, and it will build the URL for you. And as you said, it also sets auth headers and enables cookies. This removes a lot of code from pouchdb-authentication.

Unfortunately this PR won't work until there's a new version of PouchDB released; this fix in particular is absolutely necessary.

leonid-shevtsov commented 5 years ago

I made a PR to pouchdb to expose the fetchJSON API so I can remove the implementation I made in this one.

https://github.com/pouchdb/pouchdb/issues/7768

mqtik commented 4 years ago

Any updates on this?

ptitjes commented 1 year ago

Thank you @leonid-shevtsov. Sorry for the very late merge...

leonid-shevtsov commented 1 year ago

No worries! Thank you so much for merging.