Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

weaken the security warning. #917

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
5 participants
Member

karlitschek commented Dec 15, 2012

We have to discuss if this warning makes sense at all but implaying that the account might already be hacked is definitely too much.

See issue #854
#854

Please review @jancborchardt @LukasReschke Or someone else.

Frank Karlitschek weaken the security warning. We have to discuss if this warning makes…
… sense at all but implaying that the account might already be hacked is definitely too much.

See issue #854
#854
fc5362b
Member

LukasReschke commented Dec 16, 2012

Please check my remarks in #854

Frank Karlitschek remove the warning and write a log entry instead as discussed
fe96b89
Member

karlitschek commented Dec 20, 2012

@jancborchardt @LukasReschke @DeepDiver1975 Can I hve two reviewers?

Member

jancborchardt commented Dec 20, 2012

@karlitschek there’s a merge conflict with master, can you fix that before?

Member

karlitschek commented Dec 20, 2012

@jancborchardt I can fix that later. But is the patch OK?

Member

jancborchardt commented Dec 20, 2012

Yeah, tested and works good 👍 was just a bit worried because I was stuck in a merge conflict again. :)

Member

karlitschek commented Dec 20, 2012

Yes. It's a pain in the a... that you always have to rebase and update the pull request.

@LukasReschke LukasReschke commented on the diff Dec 20, 2012

lib/base.php
@@ -596,7 +596,7 @@ protected static function handleLogin() {
$error = array();
// remember was checked after last login
if (OC::tryRememberLogin()) {
- $error[] = 'invalidcookie';
+ OC_Log::write('core', 'Automatic logon rejected!', OC_Log::DEBUG);
@LukasReschke

LukasReschke Dec 20, 2012

Member

This is a duplicate because line 656 already creates a log entry.

(So please remove it)

Member

LukasReschke commented Dec 20, 2012

Count this as 👍 after you removed 599 :-)

@DeepDiver1975 DeepDiver1975 commented on the diff Dec 20, 2012

lib/base.php
@@ -596,7 +596,7 @@ protected static function handleLogin() {
$error = array();
// remember was checked after last login
if (OC::tryRememberLogin()) {
@DeepDiver1975

DeepDiver1975 Dec 20, 2012

Owner

BTW: Why the fxxx is a function returning true if it actually failed? CRAZY

Owner

DeepDiver1975 commented Dec 20, 2012

👍 for this PR - I'll soon submit one to cleanup this strange logic ....

Member

karlitschek commented Dec 22, 2012

@DeepDiver1975 So I leave this to you if you want to cleanup it anyways ;-)

Owner

DeepDiver1975 commented Dec 22, 2012

Hehe - thanks a lot! 😈

Member

karlitschek commented Dec 22, 2012

Sorry. You offered your help with the cleanup ;-)

@LukasReschke LukasReschke deleted the weaken_securitywarning branch Jan 8, 2013

root-11 commented Apr 8, 2013

Hi Guys,
Sorry to awaken an issue from the grave, but I just managed to recreate this issue in OC 5.0.3.

Automatic logon rejected!
If you did not change your password recently, your account may be compromised!
Please change your password to secure your account again.

Reading the thread above I can only think if some merge gone wrong during the last 4 months?

Perhaps it could be helpful to hint to users that, they should try resetting the cookies using (in chrome: right-click ==>> inspect element ==>> select tab named "resources" ==>> find the Cookies-leaf and select & delete all of them.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment