MycroftAI / personal-backend

WORK IN PROGRESS: A Flask personal backend alternative for running your own version of https://home.mycroft.ai
Apache License 2.0
114 stars 32 forks source link

New account creation requires email #14

Closed el-tocino closed 5 years ago

el-tocino commented 5 years ago

Thinking this would be handled better via a local config or utility.
Might be useful to create a 1st run script that does a bit of this (cert setup, admin account creation) perhaps.

el-tocino commented 5 years ago

To add on to this, if you're trying to send from a gmail account, you have to enable 'less secure app access' (google it). Also double check the imap/smtp settings and verify the domain/port. You at least get a copy of the sent email in your sent items. Messages, when received, went to spam, as well.

JarbasAl commented 5 years ago

a startup script makes sense, creating admin accounts and ask about using ssl / creating certs, but it should not be mandatory, i'd like to keep this pip installable and not requiring user interaction to install, install should be separate from configuration

i still think user accounts should be by email, but a script to add new users directly is also maybe a good idea

should we create a /bin folder with a couple bash scripts?

or maybe change setup.py to add some command line options #7 , i like that better

about google, should we add that to the readme?

el-tocino commented 5 years ago

A startup script should be optional. Not opposed to the email setup, but a local setup tool would be useful as well.

cmdrogogov commented 5 years ago

Would it be possible to consider tying user authentication into IPA, OpenLDAP or other directory services?

el-tocino commented 5 years ago

Possible, but would take some effort by a motivated person. :)

flatsiedatsie commented 5 years ago

I was also surprised I had to give an email address to create a local user account, since I'm using this on the same Pi as MyCroft.

flatsiedatsie commented 5 years ago

I have circumvented this by changing some code. It would be nice to make this requirement configurable.

First, I changed this function in frontend/utils.py

def add_user(username, password, email, mail_sender):
    token = generate_confirmation_token(email)
    with session_scope() as s:
        try:
            u = User(name=username, password=password, mail=email, token=token)
            #confirm_url = url_for('confirm', token=token, _external=True)
            #html = render_template('activate.html', confirm_url=confirm_url)
            #subject = "Please confirm your email"
            #send_confirmation_mail(email, subject, html, mail_sender)
            s.add(u)
            s.commit()
            return True

Then, I added one line to the signup routine in frontend/main.py to immediately confirm the user.

    @app.route('/signup', methods=['GET', 'POST'])
    @noindex
    @donation
    def signup():
        if not session.get('logged_in'):
            form = LoginForm(request.form)
            if request.method == 'POST':
                username = request.form['username'].lower()
                password = utils.hash_password(request.form['password'])
                email = request.form['email']
                if form.validate():
                    print("form valid")
                    if utils.username_taken(username):
                        flash("Username taken")
                        return json.dumps({'status': 'Username taken'})
                    if utils.mail_taken(email):
                        flash("Email taken")
                        return json.dumps({'status': 'Email taken'})
                    if utils.add_user(username, password, email, mail_sender):
                        session['logged_in'] = True
                        session['username'] = username
                        utils.change_user(confirmed=True, confirmed_on=time.time()) # ADDED THIS LINE
                        flash("Signup successful")
                        return json.dumps({'status': 'Signup successful'})
                    flash("Signup failed")
                    return json.dumps({'status': 'Signup failed'})
                flash('All fields required')
                return json.dumps({'status': 'All fields required'})
        return redirect(url_for('unconfirmed'))
JarbasAl commented 5 years ago

originally i assumed the backend would be on an open network (say a digital ocean account) and decided to require email, even if not on an open network i did not assume you would run 1 instance in each device, the idea being that the backend manages all your devices (mainly handling STT at current state)

to add an account without email you can interact with the database directly, should a cli/example script be made to handle this? maybe also a cli tool to pair a device

for this use case (backend on device) other option would be to make a service that would connect to the mycroft messagebus, this could listen for pairing messages and auto-pair

flatsiedatsie commented 5 years ago

should a cli/example script be made to handle this? maybe also a cli tool to pair a device

That would be great. I had a look in the current database example and that helped a lot to understand what was going on. I also used a sqlite database management tool to look into the device database. Using the database script added a strange user, and for this user the password wasn't hashed. When I got the process working and could sign up normally, the data looked different.

hooddanielc commented 5 years ago

I think adding functionality to disable user signup/signin would increase complexity and might introduce accidental back doors. The steps required by the installer is minimal if the installer has a gmail account.

  1. First start the server to generate a new config file
  2. Add and app password to your gmail account by following this guide
  3. Change the config file mail_user to the email you want to use in ~/.mycroft/personal_backend/personal_backend.conf
  4. Change the config file mail_password to the password generated (step 2) in ~/.mycroft/personal_backend/personal_backend.conf
  5. Restart the server

I've tested this using current revision and it is working.

flatsiedatsie commented 5 years ago

That is fine, but I was referred to this project because I wanted to have a version of MyCroft that runs completely cloudless.

Having to make a connection to a mail server invalidates this proposition.

It seems that this project is more of a "team backend" or "company backend" than a "personal backend"? It's not really geared towards use by individuals?

JarbasAl commented 5 years ago

@flatsiedatsie the project is in a very early stage, the first concern was to mimic the official backend for compatibility

you make a valid point, i will make this configurable and optional, but enabled by default

flatsiedatsie commented 5 years ago

Thank you, that sounds great!

JarbasAl commented 5 years ago

solved by #43 and #44