Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config.php - Disable Remember Login #1358

Closed
MeCias opened this issue Sep 11, 2016 · 39 comments
Closed

config.php - Disable Remember Login #1358

MeCias opened this issue Sep 11, 2016 · 39 comments
Labels
1. to develop Accepted and waiting to be taken care of enhancement feature: authentication

Comments

@MeCias
Copy link

MeCias commented Sep 11, 2016

Hi guys,

You are offering for a lot of events off and on switches in the config.php even for session_keepalive. For the Remember Login there is just the possibility to change the remember_login_cookie_lifetime.
It would be great to switch off and hide the Remember Login via config.php as well, since the App that exists for this seems to be discontinued and ever since experimental.

Thanks in advance for considering it.

@chaos-prevails
Copy link

are there any plans to be able to hide the remember login via config.php or other means (except the discontinued plugin?)

AFAIK #1347 does not fix this.

@ghost
Copy link

ghost commented Jan 16, 2017

The setting 'remember_login_cookie_lifetime' => 0, does not work?

@chaos-prevails
Copy link

no, unfortunately not. Checkbox stays.
to remove the checkbox, see #1347 (comment)

it would be easier if the checkbox is removed/hidden as soon as 'remember_login_cookie_lifetime' is set to 0

@MariusBluem MariusBluem changed the title Feature request: config.php - Disable Remember Login config.php - Disable Remember Login Feb 12, 2017
@Wikinaut
Copy link
Contributor

The Disable Remember Login App appears to be not working with NC 12.0.0. I would also prefer what has been proposed here: add a configuration option to disable the check box on the login screen.

@alve89
Copy link

alve89 commented Jun 7, 2017

This might be a security sensitive issue if users are authenticated against an external identity provider. So this feature is not only a nice-to-have but rather a must-have.

@GitHubUser4234
Copy link
Contributor

Yeah, since this commit removed the ability to disable the "Stay logged in" checkbox via the rememberlogin flag in the info.xml of an app, it looks like CSS / PHP hacking are the only option to disable it, that's really sad 😢

@MorrisJobke
Copy link
Member

I don't really see the background of this one here. What use case should be solved by this? If the user clicks actively on a "remember me" checkbox maybe the intention of this user is to keep logged in. All that is solved by this is that an admin makes the live of a user more hard and pulls rights from the users. If an admin thinks that it's users are not smart enough to figure out that this may be a problem, then it's maybe better to not give the user an account at all.

Adding a feature flag here also doesn't help a lot for the maintenance of Nextcloud itself. If you really really really want to drop this option: then implement an app, that overwrites the CSS to hide this feature and overwrites the session variable that sets this flag in the PHP, but it is very unlikely that this will be implemented in the server itself, because we see this as a valuable feature and something that makes the life of our users a lot easier.

I will close this ticket here. Sorry for the inconvenience.

@Wikinaut
Copy link
Contributor

Wikinaut commented Aug 31, 2017

@MorrisJobke There was (is) still an App for this, but it does not work any more (since about version 9 or 10 owncloud/nextcloud).

@Wikinaut Wikinaut reopened this Aug 31, 2017
@chaos-prevails
Copy link

@MorrisJobke Many services also have minimum password requirements. These rules are there because many users would otherwise choose weak passwords. I think disabling the remember me checkbox would fall into the same category. It prevents users from saving login credentials on computers where they should normally be not saved

@MariusBluem
Copy link
Member

Maybe somebody wants to add a checkbox for this into the password_policy-app?!

@MorrisJobke
Copy link
Member

It prevents users from saving login credentials on computers where they should normally be not saved

That is the reason that this is disabled by default. If you fear this, then maybe set the session to a super short time span. I don't see, why completely disable that feature helps somebody.

@MorrisJobke There was (is) still an App for this, but it does not work any more (since about version 9 or 10 owncloud/nextcloud).

Yes, but this will not be implemented in the server in itself. It will always be in an app, because our goal at Nextcloud is to make live easier and not harder. And additionally it is quite unlikely that the server team itself will maintain this app. Somebody can implement this app and maintain it. That is the reason why I closed this ticket in the server repo, because this is the bug and feature tracker of the server component itself and not the feature tracker for all the app wishes out there.

We also need to somehow organise ourselves and dumping random feature wishes in it, that are better to be implemented in a separate app does not help us.

If the previous working app is broken, then report it to the maintainer of this app and not in the server.

Please keep this ticket closed. Thanks

@Wikinaut
Copy link
Contributor

Then please remove the box fully: "Stay logged in". it is unsafe as such.

@MorrisJobke
Copy link
Member

Then please remove the box fully: "Stay logged in". it is unsafe as such.

Then we should not run servers in the internet 😉 they are unsafe as such

@Wikinaut
Copy link
Contributor

@LukasReschke Please tell Morris, that the box on the login page should be removed. It is unsafe to have the box, because when a user logs in in an Internet Café or so and clicks the box, the credentials are saved.

@Wikinaut
Copy link
Contributor

Wikinaut commented Aug 31, 2017

@MorrisJobke

Then we should not run servers in the internet 😉 they are unsafe as such

Unsachliches Argument.

@MorrisJobke
Copy link
Member

MorrisJobke commented Aug 31, 2017

it is unsafe as such.

If this is the case, then most of companies in the internet business do it completely wrong. Sometimes you should not look at how other projects do it, but often it's quite good, to also think a bit more about it and not just randomly kill stuff.

@LukasReschke Please tell Morris, that the box on the login page should be removed. It is unsafe to have the box, because when a user logs in in an Internet Café or so and clicks the box, the credentials are saved.

There is an easy solution for this rare case: Just don't tick the box, which is the default scenario.

@GitHubUser4234
Copy link
Contributor

@MorrisJobke: There are use cases for this: In some projects (including ours), users are not allowed to use this feature for their own safety! Please give us the option back. Implementation-wise it doesn't look like much effort either?

@MariusBluem
Copy link
Member

MariusBluem commented Aug 31, 2017

Please give us the option back.

A not longer maintained 3rdparty-app is nothing we have removed. It was the decision of the developer to not continue the development.

Implemetation-wise it doesn’t look like much effort either?

If you think so, I do not understand, why you don’t take the time to create an app (maybe based on https://apps.owncloud.com/content/show.php/Disable+Remember+Login?content=162551) and submit it in our App Store ... this is how open source works ;)

If you don’t know how this can be done, I cannot understand how you can say, that this would not cost much effort :)

@ghost
Copy link

ghost commented Aug 31, 2017

@GitHubUser4234 @Wikinaut Use the app Custom CSS and hide the login checkbox (and other elements like the contacts menu). Problem solved.

https://apps.nextcloud.com/apps/theming_customcss
https://github.com/juliushaertl/theming_customcss

@nextcloud nextcloud deleted a comment from alve89 Sep 1, 2017
@GitHubUser4234
Copy link
Contributor

@MariusBluem Wrong. Have a look again at my first comment above. Besides, we never used that third-party app before, but put the flag into a config of an own custom app which has a totally different purpose - not a nice solution, but still better than source code hacking. Being able to set the value in config.php would certainly be a much cleaner solution. So please try to be more constructive here.

@xraMsamohT Yep, that's what we did, but having to maintain and potentially update custom source code with every Nextcloud release is far from ideal. Agreeing with @alve89 in that regard.

@GitHubUser4234
Copy link
Contributor

@alve89 Aaaaargh, I accidentally deleted your comment, (combination of mobile phone and fat fingers), I'm really sorry, could you repost it? Thanks ~

@alve89
Copy link

alve89 commented Sep 5, 2017

If you don't know how this can be done, I cannot understand how you can say, that this would not cost much effort

I can't see the point to make this available with an app - why not only with an option within the config? Because THIS wouldn't cost any effort to write one if-clause.

@MorrisJobke
Of course you want to make the life of users more easy - but if admins please for this feature it shouldn't be ignored! A discussion helps the other side to see and understand the reasons - just ignoring it by closing the thread isn't that userfriendly.
Every admin is free to make the remember function usable or not. If one wants to hide it for several reeasons he should be able to do this!

@nextcloud nextcloud deleted a comment from alve89 Sep 5, 2017
@GitHubUser4234
Copy link
Contributor

For some projects, data protection is really essential. It could be as critical as data you have in Online-Banking. It would be unimaginable to find a "Remember me?" checkbox for Online-Banking access, wouldn't it?

@LukasReschke Would be glad to hear your comment also :)

@ChristophWurst
Copy link
Member

GitHubUser4234 deleted a comment from alve89 41 minutes ago

Again? 😮

@alve89
Copy link

alve89 commented Sep 5, 2017

@ChristophWurst

No, it's still there. Fortunately. 😊

@MorrisJobke MorrisJobke reopened this Sep 6, 2017
@MorrisJobke MorrisJobke added 1. to develop Accepted and waiting to be taken care of feature: authentication labels Sep 6, 2017
@MorrisJobke
Copy link
Member

Just to say: This is only an enhancement ticket and nobody can guarantee, that this will implemented at all. Pull requests are obviously welcome. ;)

@GitHubUser4234
Copy link
Contributor

Just to say: This is only an enhancement ticket and nobody can guarantee, that this will implemented at all. Pull requests are obviously welcome. ;)

Thanks @MorrisJobke 👍 At least this opens a door to potential contributors, knowing that such enhancement would actually be accepted.

Hey guys, anyone having some spare resources to add the feature? For a start, one could probably have a look at this commit and rollback the changes, but instead of looking for the rememberlogin in an app's info.xml, one would simply lookup the flag in config.php.

@rmsmgaspar
Copy link

Hi,
You can comment the lines with this info in the login page.
Tested and worked.

@wehkah
Copy link

wehkah commented Feb 25, 2018

I am puzzled because no one has mentioned the obvious reason why this option ("remember me") should be removable from the login page: if users set this option and loose their devices, then any one who finds the devices will be able to access the clouded data as well. This might be just slightly embarrassing when it concerns personal data of an unprivileged user, but it becomes a security breach if it happens to a privileged user or even an (sub-)admin.

@Wikinaut
Copy link
Contributor

@wehkah (my comment August 2017:) @LukasReschke Please tell Morris, that the box on the login page should be removed. It is unsafe to have the box, because when a user logs in in an Internet Café or so and clicks the box, the credentials are saved.

@chaos-prevails
Copy link

I agree with @Wikinaut and @wehkah : this is a security issue.

Nextcloud is advocating security and privacy. At the same time this checkbox can cause grave security implications.

First, my experience is, that unfortunately, many users choose the most convenient setup over time, even if it compromises security. There are password policies (length, complexity) for the same reasons: most users need compulsion.

Second, there are alternatives for a fast login, like keepass and other password managers. They auto-complete username+password in a heartbeat.

@ChristophWurst
Copy link
Member

Second, there are alternatives for a fast login, like keepass and other password managers. They auto-complete username+password in a heartbeat.

Interesting that you're mentioning this in this context, where one claims that Nextcloud is insecure because of the remember-login feature.

@chaos-prevails would you be interested in working this? I could give you some pointers to get started - just let me know!

@chaos-prevails
Copy link

Hi @ChristophWurst ,

yes please give me some pointers. I assume the goal is to have the checkbox configurable via the config.inc.php file?

@ChristophWurst
Copy link
Member

  • Login template that shows the checkbox:
    <?php if (!$_['hideRemeberLoginState']) { ?>
    <div class="remember-login-container">
    <?php if ($_['rememberLoginState'] === 0) { ?>
    <input type="checkbox" name="remember_login" value="1" id="remember_login" class="checkbox checkbox--white">
    <?php } else { ?>
    <input type="checkbox" name="remember_login" value="1" id="remember_login" class="checkbox checkbox--white" checked="checked">
    <?php } ?>
    <label for="remember_login"><?php p($l->t('Stay logged in')); ?></label>
    </div>
    <?php } ?>
  • Controller route handler that shows the login form:
    /**
    * @PublicPage
    * @NoCSRFRequired
    * @UseSession
    *
    * @param string $user
    * @param string $redirect_url
    * @param string $remember_login
    *
    * @return TemplateResponse|RedirectResponse
    */
    public function showLoginForm($user, $redirect_url, $remember_login) {
    if ($this->userSession->isLoggedIn()) {
    return new RedirectResponse(OC_Util::getDefaultPageUrl());
    }
    $parameters = array();
    $loginMessages = $this->session->get('loginMessages');
    $errors = [];
    $messages = [];
    if (is_array($loginMessages)) {
    list($errors, $messages) = $loginMessages;
    }
    $this->session->remove('loginMessages');
    foreach ($errors as $value) {
    $parameters[$value] = true;
    }
    $parameters['messages'] = $messages;
    if ($user !== null && $user !== '') {
    $parameters['loginName'] = $user;
    $parameters['user_autofocus'] = false;
    } else {
    $parameters['loginName'] = '';
    $parameters['user_autofocus'] = true;
    }
    if (!empty($redirect_url)) {
    $parameters['redirect_url'] = $redirect_url;
    }
    $parameters['canResetPassword'] = true;
    $parameters['resetPasswordLink'] = $this->config->getSystemValue('lost_password_link', '');
    if (!$parameters['resetPasswordLink']) {
    if ($user !== null && $user !== '') {
    $userObj = $this->userManager->get($user);
    if ($userObj instanceof IUser) {
    $parameters['canResetPassword'] = $userObj->canChangePassword();
    }
    }
    } elseif ($parameters['resetPasswordLink'] === 'disabled') {
    $parameters['canResetPassword'] = false;
    }
    $parameters['alt_login'] = OC_App::getAlternativeLogIns();
    $parameters['rememberLoginState'] = !empty($remember_login) ? $remember_login : 0;
    $parameters['hideRemeberLoginState'] = !empty($redirect_url) && $this->session->exists('client.flow.state.token');
    if ($user !== null && $user !== '') {
    $parameters['loginName'] = $user;
    $parameters['user_autofocus'] = false;
    } else {
    $parameters['loginName'] = '';
    $parameters['user_autofocus'] = true;
    }
    $parameters['throttle_delay'] = $this->throttler->getDelay($this->request->getRemoteAddress());
    // OpenGraph Support: http://ogp.me/
    Util::addHeader('meta', ['property' => 'og:title', 'content' => Util::sanitizeHTML($this->defaults->getName())]);
    Util::addHeader('meta', ['property' => 'og:description', 'content' => Util::sanitizeHTML($this->defaults->getSlogan())]);
    Util::addHeader('meta', ['property' => 'og:site_name', 'content' => Util::sanitizeHTML($this->defaults->getName())]);
    Util::addHeader('meta', ['property' => 'og:url', 'content' => $this->urlGenerator->getAbsoluteURL('/')]);
    Util::addHeader('meta', ['property' => 'og:type', 'content' => 'website']);
    Util::addHeader('meta', ['property' => 'og:image', 'content' => $this->urlGenerator->getAbsoluteURL($this->urlGenerator->imagePath('core','favicon-touch.png'))]);
    return new TemplateResponse(
    $this->appName, 'login', $parameters, 'guest'
    );
    }
  • Controller route handler that handles login form submission:
    /**
    * @PublicPage
    * @UseSession
    * @NoCSRFRequired
    * @BruteForceProtection(action=login)
    *
    * @param string $user
    * @param string $password
    * @param string $redirect_url
    * @param boolean $remember_login
    * @param string $timezone
    * @param string $timezone_offset
    * @return RedirectResponse
    */
    public function tryLogin($user, $password, $redirect_url, $remember_login = false, $timezone = '', $timezone_offset = '') {
    if(!is_string($user)) {
    throw new \InvalidArgumentException('Username must be string');
    }
    // If the user is already logged in and the CSRF check does not pass then
    // simply redirect the user to the correct page as required. This is the
    // case when an user has already logged-in, in another tab.
    if(!$this->request->passesCSRFCheck()) {
    return $this->generateRedirect($redirect_url);
    }
    if ($this->userManager instanceof PublicEmitter) {
    $this->userManager->emit('\OC\User', 'preLogin', array($user, $password));
    }
    $originalUser = $user;
    // TODO: Add all the insane error handling
    /* @var $loginResult IUser */
    $loginResult = $this->userManager->checkPasswordNoLogging($user, $password);
    if ($loginResult === false) {
    $users = $this->userManager->getByEmail($user);
    // we only allow login by email if unique
    if (count($users) === 1) {
    $previousUser = $user;
    $user = $users[0]->getUID();
    if($user !== $previousUser) {
    $loginResult = $this->userManager->checkPassword($user, $password);
    }
    }
    }
    if ($loginResult === false) {
    $this->logger->warning('Login failed: \''. $user .'\' (Remote IP: \''. $this->request->getRemoteAddress(). '\')', ['app' => 'core']);
    // Read current user and append if possible - we need to return the unmodified user otherwise we will leak the login name
    $args = !is_null($user) ? ['user' => $originalUser] : [];
    if (!is_null($redirect_url)) {
    $args['redirect_url'] = $redirect_url;
    }
    $response = new RedirectResponse($this->urlGenerator->linkToRoute('core.login.showLoginForm', $args));
    $response->throttle(['user' => $user]);
    $this->session->set('loginMessages', [
    ['invalidpassword'], []
    ]);
    return $response;
    }
    // TODO: remove password checks from above and let the user session handle failures
    // requires https://github.com/owncloud/core/pull/24616
    $this->userSession->completeLogin($loginResult, ['loginName' => $user, 'password' => $password]);
    $this->userSession->createSessionToken($this->request, $loginResult->getUID(), $user, $password, (int)$remember_login);
    // User has successfully logged in, now remove the password reset link, when it is available
    $this->config->deleteUserValue($loginResult->getUID(), 'core', 'lostpassword');
    $this->session->set('last-password-confirm', $loginResult->getLastLogin());
    if ($timezone_offset !== '') {
    $this->config->setUserValue($loginResult->getUID(), 'core', 'timezone', $timezone);
    $this->session->set('timezone', $timezone_offset);
    }
    if ($this->twoFactorManager->isTwoFactorAuthenticated($loginResult)) {
    $this->twoFactorManager->prepareTwoFactorLogin($loginResult, $remember_login);
    $providers = $this->twoFactorManager->getProviders($loginResult);
    if (count($providers) === 1) {
    // Single provider, hence we can redirect to that provider's challenge page directly
    /* @var $provider IProvider */
    $provider = array_pop($providers);
    $url = 'core.TwoFactorChallenge.showChallenge';
    $urlParams = [
    'challengeProviderId' => $provider->getId(),
    ];
    } else {
    $url = 'core.TwoFactorChallenge.selectChallenge';
    $urlParams = [];
    }
    if (!is_null($redirect_url)) {
    $urlParams['redirect_url'] = $redirect_url;
    }
    return new RedirectResponse($this->urlGenerator->linkToRoute($url, $urlParams));
    }
    if ($remember_login) {
    $this->userSession->createRememberMeToken($loginResult);
    }
    return $this->generateRedirect($redirect_url);
    }
  • Method that handles the post-login magic: https://github.com/nextcloud/server/blob/master/lib/private/User/Session.php#L337-L375
  • Method that does the cookie magic: https://github.com/nextcloud/server/blob/master/lib/private/User/Session.php#L851-L872

It should be easy to locate the corresponding test cases that have to be adapted and extended for this feature.

@BloodyIron
Copy link

There used to be an app for ownCloud that literally enabled you to disable the checkbox. What happened to that? I can't find the app any more now that I've moved to nextCloud D:

@Wikinaut
Copy link
Contributor

@BloodyIron the app is not working since a long time - you cannot "disable" the remember setting with the app, I guess, this is why the app has been removed (for both owncloud and nextcloud).

@BloodyIron
Copy link

So how about we have this as a built-in feature then already?

@GitHubUser4234
Copy link
Contributor

If I'm not mistaken, "Remember Me" is now always enabled (see #9109), but according to #13747 it can be disabled by setting remember_login_cookie_lifetime to 0. Please correct me if I'm wrong, otherwise this can probably be closed.

@CarlSchwan
Copy link
Member

The login view was ported to vue and there is no such checkbox anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of enhancement feature: authentication
Projects
None yet
Development

No branches or pull requests