AuthUser does not properly set user as authenticated

Issue #323 resolved
Matt Zuba
created an issue

I'm not sure if this is intentional or an oversight, so I wanted to file the bug anyway.

When attempting to authenticate a user with a userid or apikey, the AuthUser function doesn't set a user as authenticated if it successfully loaded. (And as an aside, when trying to auth by a username that does not exist, it is automatically created (not sure if this is intended or not either)).

Patch attached if this is indeed a bug...

Comments (7)

  1. Marcin Kuzminski repo owner

    access to web part of RhodeCode by API KEY is intentionally not logging in users. I consider auto login with this key as a security threat. The only part of RhodeCode that's using this (besides the API) is the RSS/ATOM feed, for example to use it to read the log with RSS reader without need to login.

    But maybe it's worth to put this option in the configuration file for admins to decide how this should behave.

  2. Matt Zuba reporter

    Maybe I don't understand the purpose of the AuthUser class then? What's the difference, from a developer standpoint, if I pass an API key or a username to the AuthUser class? Regardless of what is being passed, passwords aren't checked, yet passing a username set's a user to authenticated, whereas an apikey or userid does not. All I can tell the function is doing is verifying that a user with one of those pieces of information passed (api key, user id or username) exists and then loads up all of their information. Why does passing a username qualify a user as logged in, but a user id or api key doesn't?

    I'm writing a python SSH handler script utilizing RhodeCode's internals and Mercurial's internals, and I need to pass something to the script to let it know who the user is who SSH'd in. I wanted to use username, but if you pass a username to the AuthUser class, and that user doesn't exist, that function (indirectly) ends up creating a new user, definitely not my desired outcome.

    Is there another function/class I should be looking at to properly verify and load a user's information and permissions based on a apikey, username or user id?

  3. Matt Zuba reporter

    Ok, I looked through the code and saw how the AuthUser class is being used and understand now the design decision. That leaves me with a question of using the username in the AuthUser function then... Should AuthUser be creating a user that doesn't exist when a username is passed in?

  4. Marcin Kuzminski repo owner

    AuthUser, never creates an user, auth() function does that but the reason it does is if ldap login is enabled, and IF user authenticated correctly with ldap server, than we need a corresponding user in rhodecode database for permissions management, so it creates an user then on the fly.

  5. Matt Zuba reporter

    It is creating users though if they don't exist. If I pass in a username to AuthUser, login_auth_container is called with the username, and login_auth_container then checks if the user exists. If not, it populates some dummy data then calls UserModel.create_for_container_auth, which then creates a new user and then AuthUser sets that user as authenticated.

    from paste.deploy import appconfig
    from rhodecode.config.environment import load_environment
    from rhodecode.lib.auth import AuthUser
    from rhodecode.model import init_model
    from sqlalchemy import engine_from_config
    if __name__ == "__main__":
        # Load the environment for RhodeCode so we can utilize it's database
        conf = appconfig('config:development.ini', relative_to = './')
        config = load_environment(conf.global_conf, conf.local_conf)
        engine = engine_from_config(conf, 'sqlalchemy.db1.')
        user = AuthUser(username='asdfasdfasdfa')
        print user

    Check the database after running that and you should see a new junk user created.

  6. Marcin Kuzminski repo owner

    O right, i totally forgot about container Auth which was added from a fork, and which does create an user. But calling AuthUser with username is only for container auth like apache sso etc.

    Not sure if it's a good decision to do it at that level.

  7. Log in to comment