compdemocracy / polis

:milky_way: Open Source AI for large scale open ended feedback
https://pol.is
GNU Affero General Public License v3.0
744 stars 173 forks source link

Embeds from dockerised server don't allow voting with data-xid set #287

Open joshsmith2 opened 4 years ago

joshsmith2 commented 4 years ago

Hello,

I've encountered an issue with the following setup:

Here's what the HTML snippet used for the embed looks like:

<div class='polis'
      data-conversation_id='1idfromaconversation' 
      data-xid='6345789'></div>
<script async src='http://mydocker-composeserverip/embed.js'></script>

I found I needed to change the line var polisUrl = "http://pol.is"; to var polisUrl = "http://mydocker-composeserverip" in order to get the convo embed to work. Having done this, everything works fine, but only if I take out data-xid. Leaving it in, the conversation loads, and the first statement presents itself, but voting gives me this error:

Apologies, your vote failed to send. Please check your connection and try again.

Here's the error in the console:

responseText: "Error: there is no unique or exclusion constraint matching the ON CONFLICT specification
   at module.exports.Client._emitResult (/app/node_modules/pg-native/index.js:173:26)
   at module.exports.Client._read (/app/node_modules/pg-native/index.js:215:31)
   at PQ.emit (events.js:198:13)
   at PQ.EventEmitter.emit (domain.js:448:20)
From previous event:
   at pgQueryP_impl (/app/server.js:151:10)
   at pgQueryP (/app/server.js:166:10)
   at createXidRecord (/app/server.js:949:10)
   at createDummyUser.then (/app/server.js:1010:18)
From previous event:
   at shouldCreateXidEntryPromise.then (/app/server.js:1008:34)
From previous event:
   at /app/server.js:1004:42
From previous event:
   at getXidRecordByXidOwnerId (/app/server.js:993:90)
   at getConversationInfoByConversationId.then (/app/server.js:2223:14)
   at runCallback (timers.js:705:18)
   at tryOnImmediate (timers.js:676:5)
   at processImmediate (timers.js:658:5)
   at process.topLevelDomainCallback (domain.js:126:23)
From previous event:
   at doXidConversationIdAuth (/app/server.js:2222:65)
   at /app/server.js:2297:11
From previous event:
   at doAuth (/app/server.js:2263:14)
   at /app/server.js:2349:7
   at callbacks (/app/node_modules/express/lib/router/index.js:164:37)
   at param (/app/node_modules/express/lib/router/index.js:138:11)
   at pass (/app/node_modules/express/lib/router/index.js:145:5)
   at nextRoute (/app/node_modules/express/lib/router/index.js:100:7)
   at callbacks (/app/node_modules/express/lib/router/index.js:167:11)
   at middleware_check_if_options (/app/server.js:14714:14)
   at callbacks (/app/node_modules/express/lib/router/index.js:164:37)
   at param (/app/node_modules/express/lib/router/index.js:138:11)
   at pass (/app/node_modules/express/lib/router/index.js:145:5)
   at nextRoute (/app/node_modules/express/lib/router/index.js:100:7)
   at callbacks (/app/node_modules/express/lib/router/index.js:167:11)
   at addCorsHeader (/app/server.js:2572:12)
   at callbacks (/app/node_modules/express/lib/router/index.js:164:37)
   at param (/app/node_modules/express/lib/router/index.js:138:11)
   at pass (/app/node_modules/express/lib/router/index.js:145:5)
   at Router._dispatch (/app/node_modules/express/lib/router/index.js:173:5)
   at Object.router [as handle] (/app/node_modules/express/lib/router/index.js:33:10)↵"

As far as I can tell, this relates to createXIDRecord, at 1010:18 of server.js, which does have a mention of ON_CONFLICT

function createXidRecord(ownerUid, uid, xid, x_profile_image_url, x_name, x_email) {
  return pgQueryP("insert into xids (owner, uid, xid, x_profile_image_url, x_name, x_email) values ($1, $2, $3, $4, $5, $6) " +
    "on conflict (owner, xid) do nothing;", [
      ownerUid,
      uid,
      xid,
      x_profile_image_url || null,
      x_name || null,
      x_email || null,
    ]);
}

As far as I can see, the above code looks fine; on opening the postgres container up and running it with dummy values it seemed to work. Oddly, various console.log( commands which I could see in server.js, and might have helped with debugging, weren't printing (though this may be a config thing)

As a note, I've tested setting data-xid on conversations hosted on https://pol.is, and that works fine.

patcon commented 4 years ago

Thanks for filing this :)

Likely related: https://github.com/pol-is/polisServer/issues/92

It's hard to debug this since removing pol.is as the hardcoded url (https://github.com/pol-is/polisServer/issues/288) is something that prob needs to be dealt with first, before figuring out what's going on with xid stuff.

I'd recommend testing without changing the pol.is domain first, by modifying /etc/hosts to point pol.is to your server IP. That way, you can set if that fixes your error, and know whether the root cause is (1) making SERVICE_URL configurable, or (2) a true bug with data-xid.

Would you be able to test that first?

joshsmith2 commented 4 years ago

Thanks pat - yes, happy to. Looks like I'll need to sort out https on the hosted server first, but that's a good prompt as it needs doing anyway. Will update here when done + tested.

joshsmith2 commented 4 years ago

A thought - might this have something to do with .xip.io?

Take 123.123.123.123 as the ip of my Pol.is server. Without embedding, voting on 123.123.123.123.xip.io/convoid works fine, but voting on 123.123.123.123/convoid causes a familiar looking error:

"Cannot POST /_domainWhitelistError_api/v3/votes"

I'm going to set up SSL to test this in any case - but I'll see if I can get rid of that error too. I guess it would help to have a sense of what .xip.io was doing in the pol.is context so I can move away from it

patcon commented 4 years ago

Looks like I'll need to sort out https on the hosted server first

Re: HTTPS. Feel free to steal the approach from #253, as I needed to get insecure https working in order to get facebook login working in development

Notably, that is just insecure self-signed, so not helpful for production, only testing.

Having said that, mailcow-dockerized tackles SSL using letsencrypt, and uses docker-compose for standing up a production instance. We could eventually learn from that for our prod setup -- was hoping to discuss using that approach when we're further along. But self-signed is still worth getting in, imho

~~What config looks like for administrators: https://mailcow.github.io/mailcow-dockerized-docs/firststeps-ssl/ Code that allows setup: https://github.com/mailcow/mailcow-dockerized/tree/master/data/Dockerfiles/acme~~

EDIT: Spun docker+letsencrypt convo into new issue: https://github.com/pol-is/polisServer/issues/289

joshsmith2 commented 4 years ago

Re: HTTPS. Feel free to steal the approach from #253, as I needed to get insecure https working in order to get facebook login working in development

Amazing, thanks - testing now

patcon commented 4 years ago

Without embedding, voting on 123.123.123.123.xip.io/convoid works fine, but voting on 123.123.123.123/convoid causes a familiar looking error:

I'm assuming you've still got DEV_MODE=true? That's really permissive (*.xip.io is hardcoded into whitelists), and skirts some work that still needs to be done to make polis prod-ready. Despite the breaks, you'll probably want to disable DEV_MODE to know that things are being properly addressed by your fixes.

We're kinda getting into more general topics, so how about this: Let's continue discussion async in gitter, since this is a bit more general, then we can come back here when we've teased apart smaller tasks? :) https://gitter.im/pol-is/polisDeployment?at=5ed7da394c9b0f060d40eb1a

joshsmith2 commented 4 years ago

Sounds perfect, see you on Gitter

patcon commented 4 years ago

@joshsmith2 Hiya! Did you ever get this one sorted? If so, do you have a fork you could link, or any context you could share on how you resolved? (sorry if you did this in chat and I neglected to link back here...!)

joshsmith2 commented 3 years ago

^^ The above pull request resolves this issue - there was what is (maybe) a type in the UNIQUE constraint for the xids table