Plots2: OAuth Login fail and Form Login fail for some users

[kevinzluo]

I already had an account created through Github with username kevinz_qu.

Chrome, Windows 10

[jonathanzhang53]

i Tried logging in with GitHub / google and it gave me a success message but it still showed login/signup at the top right

[geekychasser]

@kevinzluo i am facing same issue and i am also not able to login on my local server.

thank you

[SidharthBansal]

Google oauth working on Ubuntu + Chrome on pl.org

[SidharthBansal]

Github OAuth working on Ubuntu + Chrome on pl.org

[SidharthBansal]

@geekychaser getting similar errors for the localhost.

[SidharthBansal]

Login form working fine for the pl.org

[geekychasser]

@SidharthBansal but it is not working this side, i also tried on different browsers but the issue remains the same.
Thank you

[SidharthBansal]

Is it working on the pl.org?

On Sat, Jan 5, 2019 at 1:58 PM Rishabh Bothra [e-mail]
wrote:

@SidharthBansal https://github.com/SidharthBansal but it is not working
this side, i also tried on different browsers but the issue remains the
same.
Thank you

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4522#issuecomment-451637723,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUACQ1HQTNTN-MqRMNVuHNmRkY1jywrEks5vAGIQgaJpZM4Zt7OJ
.

[geekychasser]

no it is not working this side

[geekychasser]

so the issue has arised with this commit id 258bf087753c78b239c796c9fd5b02d007e4264c if you try to point head before this commit everything would work smoothly.
please correct me if i am wrong
thank you

[grvsachdeva]

@geekychaser the issue faced by you as described here - https://github.com/publiclab/plots2/issues/4522#issuecomment-451636135 is not an unexpected error. The alert shown in screenshot is rendered in case a moderator bans a user(in your case admin is banned). So, do you remember banning the admin or something like that? Thanks!

[SidharthBansal]

It is also appearing on my localhost. It is appearing for user, moderator
and admin all.

On Sat, Jan 5, 2019 at 2:17 PM Gaurav Sachdeva [e-mail]
wrote:

@geekychaser https://github.com/geekychaser the issue faced by you as
described here - #4522 (comment)
https://github.com/publiclab/plots2/issues/4522#issuecomment-451636135
is not an unexpected error. The alert shown in screenshot is rendered in
case a moderator bans a user(in your case admin is banned). So, do you
remember banning the admin or something like that? Thanks!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4522#issuecomment-451638784,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUACQ3xpiw3fuy6EN4XZZ4NGMmDQsROaks5vAGa2gaJpZM4Zt7OJ
.

[grvsachdeva]

It is also appearing on my localhost. It is appearing for user, moderator
and admin all.

You mean user banned alert?

[SidharthBansal]

Yeah

On Sat, Jan 5, 2019 at 2:19 PM Gaurav Sachdeva [e-mail]
wrote:

It is also appearing on my localhost. It is appearing for user, moderator
and admin all.

You mean user banned alert?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4522#issuecomment-451638910,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUACQ3rb-kFcgWAPbeJndzF-0rQvwS9qks5vAGc3gaJpZM4Zt7OJ
.

[grvsachdeva]

I am inspecting.

[geekychasser]

@geekychaser the issue faced by you as described here - #4522 (comment) is not an unexpected error. The alert shown in screenshot is rendered in case a moderator bans a user(in your case admin is banned). So, do you remember banning the admin or something like that? Thanks!

no i haven't banned admin or something like that, i faced this after updating my fork so this must be arrived due to last few commits and ion getting Head pointing few commits behind it again started working. thank you

[grvsachdeva]

Ok, run rake db:migrate and let me know the status. Thanks!

[SidharthBansal]

We tried rake db:reset and it doesn't work.

On Sat, Jan 5, 2019 at 2:24 PM Gaurav Sachdeva [e-mail]
wrote:

Ok, run rake db:migrate and let me know the status. Thanks!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4522#issuecomment-451639151,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUACQ4rAV_AFbYPUV4iSJA8ZNLoaUzu_ks5vAGhJgaJpZM4Zt7OJ
.

[SidharthBansal]

Gaurav is it also happening on your side?

On Sat, Jan 5, 2019 at 2:26 PM Sidharth Bansal <
[email protected]> wrote:

We tried rake db:reset and it doesn't work.

On Sat, Jan 5, 2019 at 2:24 PM Gaurav Sachdeva [e-mail]
wrote:

Ok, run rake db:migrate and let me know the status. Thanks!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4522#issuecomment-451639151,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUACQ4rAV_AFbYPUV4iSJA8ZNLoaUzu_ks5vAGhJgaJpZM4Zt7OJ
.

[grvsachdeva]

No, i am not facing any issue. I remember running rake db:migrate when the pull from master was large.

[geekychasser]

@gauravano facing same after rake db:migrate

[jywarren]

Is it possible we did not synchronize user status before the DrupalUsers
deprecation? I thought we were already running status from rusers.status
but maybe we weren't?

If so we could still manually copy them all over as we haven't dropped the
table.

On Sat, Jan 5, 2019, 4:03 AM Rishabh Bothra <[email protected] wrote:

@gauravano https://github.com/gauravano facing same after rake
db:migrate

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4522#issuecomment-451639621,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJ9H3joD774JPUjFjNOoTGLBK8F7Tks5vAGo9gaJpZM4Zt7OJ
.

[jywarren]

Oh, so it seems to me that we are only seeing the banned error on localhost installs, not on pl.org. I'm changing the title to match. I confirmed also i can log in with GitHub OAuth on pl.org without trouble.

Checking user.rb we set status = 1 in the create method, so it should be OK.

Previously status was stored in DrupalUsers table but now in User - so could the User records have been status != 1 for some reason?

In addition i wonder if a lot of previously banned users have now been unbanned if we didn't copy over the drupal_user.status. Hmm.

[jywarren]

No, for a long time we've been synchronizing status:

https://github.com/publiclab/plots2/blob/d55049cf4953998b85a9a78722379f07fd42bef8/app/models/drupal_user.rb#L84-L88

[jywarren]

And everything was synchronized here: https://github.com/publiclab/plots2/pull/2157/files

[oorjitchowdhary]

@kevinzluo i am facing same issue and i am also not able to login on my local server.

thank you

I too am getting this error.. Unable to login

[jywarren]

Should we push a migration where on development it resets user status for
seed users? Lots are being affected by this :-(

On Mon, Jan 7, 2019 at 10:32 AM Oorjit Chowdhary [e-mail]
wrote:

@kevinzluo https://github.com/kevinzluo i am facing same issue and i am
also not able to login on my local server.
[image: image]
https://user-images.githubusercontent.com/24489162/50721936-6b991980-10ed-11e9-90e9-acd50f1cad96.png

thank you

I too am getting this error.. Unable to login

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4522#issuecomment-451973301,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJ0qwsXh3SPLT2gabSKbMWKPZ36_Lks5vA2iGgaJpZM4Zt7OJ
.

[geekychasser]

@jywarren yes sir it will be great if you will push these changes. We are having trouble at our laptops.
Thanks you

[jywarren]

Hi @gauravano i want to reserve this issue for this also-important problem in development:

But I don't think it's exactly the same (although it may be related to it) as #4569.

[grvsachdeva]

how about trying to logging in as banned user via OAuth, if it shows the "signed in" message without actually being logged in then it means it is same banned user problem and we are showing wrong alert in OAuth case.

[grvsachdeva]

Actually, i have an extra gmail account, trying now.

[jywarren]

Hi, @gauravano that sounds promising, but how do you know this? Can you walk us through the code to demonstrate this?

[grvsachdeva]

hey, @jywarren I have just reproduced this issue - https://github.com/publiclab/plots2/issues/4569#issuecomment-454212914 and also saw that you were successful too in reproducing.

Do you think drupal_user deprecation is related to it? Can't say how only some of the users are banned. We should work to identify the wrong banned users. And, yes the wrong alert also needs to be corrected for OAuth.

[jywarren]

I just created a new user via OAuth and it got status = 0. Maybe, creation of user record with oauth doesn't set a default status = 1?

[jywarren]

But what about people who already had an account?

[jywarren]

Confirmed, here is the user creation code, it doesn't create users with status = 1. Proposing a change right now. But we should think of existing users and how it happened to them too. Or were all people speaking up with brand new accounts?

[jywarren]

https://github.com/publiclab/plots2/blame/master/app/models/user.rb#L401-L417

[jywarren]

We need to fix the notice delay too, how is notice passed on this line? see https://github.com/publiclab/plots2/blob/master/app/controllers/user_sessions_controller.rb#L58

[jywarren]

Checklist:

• [x] test status on oauth creation
• [x] confirm fix on oauth creation status
• [x] figure out if and how already-existing users got banned?
• [x] figure out how to unban lots of people? most since the drupal_users merge who do not have otherwise spammed content like comments or posts.

[jywarren]

Aha - the synchronization code did not apply on initial creation, esp via create_with_omniauth! https://github.com/publiclab/plots2/pull/2157/files

OK, so perhaps: people had accounts but they'd created them via OAuth, and actually had status = 0 (banned) from the start. But we only checked their matching DrupalUsers.status, which was 1 (not banned). But once we deprecated DrupalUsers code, their original User.status started being used, rejecting them. Possible?

[jywarren]

i bet login check was wired in https://github.com/publiclab/plots2/blame/master/app/controllers/application_controller.rb#L77
AHA! yes! https://github.com/publiclab/plots2/blame/2e33bcb229e77cf4994487a8d9439ce784dc09f8/app/controllers/application_controller.rb#L77
ok great

[jywarren]

so we can filter by the user.password_checkerfield to see who is OAuth-created, because of https://github.com/publiclab/plots2/blob/master/app/models/user.rb#L407

[jywarren]

OK, we've run:

User.where.not(password_checker: 0).where(status: 0).each do |user|
  if user.notes.count == 0 || user.notes.collect(&:status).include?(1) || user.comments.collect(&:status).include?(1)
    user.status = 1
    user.save
  end
end

[jywarren]

Please check if you're able to log in now!

[jywarren]

Reopening for a few follow-up steps:

• [x] 4151819 should not have passed if it's really testing what we were hoping for, so I'd like to re-examine
• [x] I forgot that the solution above doesn't exempt people with no comments... probably doesn't make a difference but i want to think through it carefully to be sure
• [ ] address delayed notice with flash.now[...] but see above link to how notice is passed...?

Thanks @gauravano ! Pushing to stable too.

[jywarren]

Confirmed this works on stable -- though i had to manually redirect myself back to stable.publiclab.org!

[jywarren]

OK and reattempting the test failure worked!

[jywarren]

Only 6 ppl got through my filter due to the comments mistake; ["rinkithakur19923", "abusaud23", "rbhatia46", "vladlenariston", "serafimrostiw", "rahamath_prizm"] - one is for sure a real person. All are status = 1 now. Removing the others as I checked their comments and they are spam.

[jywarren]

Now we just need to be sure we address: address delayed notice with flash.now[...] but see above link to how notice is passed...?

[jywarren]

@gauravano would you mind taking a look at this last piece?

[jywarren]

And let's properly report through errors like "you've been banned" when using the OAuth login flow!

[grvsachdeva]

@jywarren I will take a look at the alert part. Thanks!

[jywarren]

Maybe we could write tests here to look for appropriate alert messages for different outcomes like banned status:

https://github.com/publiclab/plots2/blob/f1e8e497c4bc01cf0cb9151578fbfa1bce875c37/test/functional/user_sessions_controller_test.rb#L60-L74

[jywarren]

Hi @gauravano perhaps we should open a more specific follow up issue for this section and highlight where the alerts are being generated both in the oauth flow and in the basic login where the messages are correct (as a model for how to refactor the oauth versions). What do you think?

[grvsachdeva]

Hey @jywarren I opened a issue at #4691. Thanks!

[jywarren]

Awesome. Thanks a lot!

On Jan 22, 2019 10:19 AM, "Gaurav Sachdeva" [e-mail]
wrote:

Hey @jywarren https://github.com/jywarren I opened a issue at #4691
https://github.com/publiclab/plots2/issues/4691. Thanks!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4522#issuecomment-456396103,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJ5LbuTqi3OmbLbTCWWGkbs2GN8nAks5vFw_CgaJpZM4Zt7OJ
.