Branding issue - Saidit cookie: reddit_session.

submitted by RavAshi from self.SaidIt

sorted by:
top
newinsightfulfun
you are viewing a single comment's thread.

view the rest of the comments →

[–]magnora7 2 insightful - 1 fun2 insightful - 0 fun3 insightful - 1 fun -  (8 children)

Thanks for the report. Is it the name of the cookie, or something within the cookie? This one could be a pain to rename... /u/d3rr what do you think about this one

edit: It looks like it's a development.ini setting, maybe we can just change it in the development.update file and make ini and that'll solve it?

edit2: There are also mentions of "reddit_session" in post_tests.py and tracker.py. We're probably not even using the one in tracker, but we can change those both too to saidit_session and that should get everything?

[–][deleted] 2 insightful - 1 fun2 insightful - 0 fun3 insightful - 1 fun -  (7 children)

Good sleuthing on this one. I'd say that the names of these cookies are not user facing, so this is less of a branding issue and more of a philosophical issue.

We're clearly a reddit clone and give reddit credit, so what's the point of removing all traces of the word "reddit"? Do we also need to rename our database which is named reddit? Another argument against is that changing these cookie names could have unanticipated consequences, like RES and/or the reddit mobile app (which we have no current plans to use) might be looking for these cookies by name.

So I could go either way on this one, but there are never ending mentions of reddit everywhere and I wonder about the practical value of changing them.

If we change this one, we should also change 

admin_cookie = reddit_admin

[–]magnora7 2 insightful - 1 fun2 insightful - 0 fun3 insightful - 1 fun -  (6 children)

I'd say that the names of these cookies are not user facing, so this is less of a branding issue and more of a philosophical issue.

Yeah I was thinking the same at first, but honestly this is different from the database. Some people do go through their cookies, but no one will ever see internal variables. I think if people other than us can see it, we should change it. And this guy found this cookie instance, so I think we should change it. If it's really as easy as changing those 3 instances, I think we should do it. If it's some 2 hour process, then we could skip it, I don't think it's worth dedicating a lot of time to. But let's try the easy way and see if it works to fix it. If it doesn't, we'll re-evaluate.

Changing all these instances has a couple good consequences:

  1. Branding, to show saidit is polished and finished for the user as much as possible.

  2. If someone wants to deploy saidit on their own server, they can simply find and replace saidit with something else. We weren't able to do that with reddit because the name is embedded in all these random variables, but someone could do that with saidit. So we're creating a package that can be easily re-branded for others to deploy if they want. Unlike the reddit code.

  3. The cookie stored on the user's computer should have the correct site name on it, just for security and trust reasons. It's the only file we really store on their computer, so let's at least name it correctly

Those are my reasons why it's worth it, even though it seems like a small issue. Still worth 15 mins of trying, but not worth hours and hours. I have no desire to rename anything that isn't customer-facing, but this cookie is, imo. I would consider html and css code labels as not customer-facing. And database stuff is definitely not customer-facing.

If it's going to break RES or the mobile stuff, then it should contain the text "reddit_session" somewhere. If it doesn't, we should be good to go.

So we can just change all 3 of those instances (adding the line to development.update), then do 'make ini' right? Then that's it? I guess I'll try it on the dev server first, might as well.

[–][deleted] 2 insightful - 1 fun2 insightful - 0 fun3 insightful - 1 fun -  (5 children)

Alright, I'm convinced. You're right, people do go through their cookies.

So we can just change all 3 of those instances (adding the line to development.update), then do 'make ini' right?

Yes, and then a reddit-restart. Also make sure your entry to development.update gets put in the [DEFAULT] section. There's only like 3 or 4 sections but it has to be right.

Also, I think this is going to log everyone out. Writing a cookie migration script is possible, but that kills the 15m approach.

[–]magnora7 2 insightful - 1 fun2 insightful - 0 fun3 insightful - 1 fun -  (4 children)

Cool. Alright I changed the development.update to include this in the general section:

login_cookie = saidit_session

name of the admin cookie

admin_cookie = saidit_admin

name of the otp cookie

otp_cookie = saidit_otp

then did make ini, flush, and restart. Seems to still work. I don't think it's storing cookies because of the certificate not existing?

I'm going to try it out on saidit now, and then I'll change those two python references.

[–][deleted] 2 insightful - 1 fun2 insightful - 0 fun3 insightful - 1 fun -  (3 children)

we did a forced logout, but the cookie rename looks correct to me, nice.

[–]magnora7 2 insightful - 1 fun2 insightful - 0 fun3 insightful - 1 fun -  (2 children)

Sweet. Just finished updating everything in post_tests.py and tracker.py (which I'm pretty certain is not being used at all). That's it! Should be good to go.

[–][deleted] 2 insightful - 1 fun2 insightful - 0 fun3 insightful - 1 fun -  (1 child)

I have always tried to get the internal tracking going, to make the sub traffic stats page work. I don't think it's working, but none of it is disabled intentionally as of now.

[–]magnora7 2 insightful - 1 fun2 insightful - 0 fun3 insightful - 1 fun -  (0 children)

Ah ok, good to know, thanks. I just figured from its folder location that it's unused.