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

[FEAT] Auth, Login, OAuth, create account with username and password #1653 #2092

Merged
merged 140 commits into from
Dec 12, 2023

Conversation

motorina0
Copy link
Collaborator

@motorina0 motorina0 commented Nov 10, 2023

Summary

Add login functionality. Supported methods:

  • user-id-only
    • the current (legacy) method
    • the usr (user id) from the url params is automatically removed
  • username-password
    • allow user to register * login with a username and password
    • an email is not required, but the user can configure one via SSO
  • google-auth - use google SSO
  • github-auth - use github SSO

The auth fields are documented in .env.example.

Based on the AUTH_ALLOWED_METHODS different Login/Register options will be available to the end user.

Login Page (all options)
image

Login Page (no usr allowed)
image

Google SSO:
https://github.com/lnbits/lnbits/assets/2951406/5f7ce5a7-7a9a-479b-b948-71c5cfbb39d4

On the Account Settings page the user can verify its email address via SSO (google and github at the moment).
image

Test Scenarios

Scenario 1 (backwards compatibility)

  • access with usr in URL query params
  • test Login with user ID
  • test Create New Wallet
    • image

Scenario 2

  • Register (username & password)
  • Login with the above username and password

Scenario 3

  • Logout

Scenario 4

  • Login with Google
  • Login with Github

Scenario 5 (Account Settings page)

  • set username (username cannot be updated)
  • update other values (first name, last name, picture, etc)
    • image

Scenario 6 (Change password)

  • on Account Settings click Change Password
  • if the user was registered via SSO the the old password is not required (only the new one)
  • otherwise the old password is required

Scenario 7 (add email)

  • an account that was created via the usr or Register flow has no email
  • it is possible for the user to attach a validated email using SSO:
    • go to Account Settings
    • if no email is present then the SSO options are available
    • select one of them and authenticate
    • at the end of the flow, the user should have the validated email configured
    • emails cannot be changed or manually edited
    • image

Scenario 8 (token expire)

  • set a low value (1 minute) for AUTH_TOKEN_EXPIRE_MINUTES
  • login and refresh after 1 min
  • the user should be redirected to the login page and a info message displayed

Dependencies

  • itsdangerous is required by from starlette.middleware.sessions import SessionMiddleware

Notes

Stuff that is still open but the PR can be reviewed without:

  • i18n
  • warning message on login/logout (if email not set)
  • find place for long description text on Login page
  • CI failures: nix and a regtest call

prusnak

This comment was marked as resolved.

@motorina0

This comment was marked as resolved.

)
from ..models import CreateUser, LoginUser

user_router = APIRouter()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would be nice to add more openapi documentation to APIRouter and the corresponding user_router routes

Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Attention: 267 lines in your changes are missing coverage. Please review.

Comparison is base (e76ba62) 60.45% compared to head (a2402f7) 59.39%.

Files Patch % Lines
lnbits/core/views/auth_api.py 23.14% 166 Missing ⚠️
lnbits/core/crud.py 17.91% 55 Missing ⚠️
lnbits/decorators.py 57.50% 17 Missing ⚠️
lnbits/helpers.py 46.66% 8 Missing ⚠️
lnbits/app.py 33.33% 6 Missing ⚠️
lnbits/core/views/generic.py 45.45% 6 Missing ⚠️
lnbits/core/helpers.py 40.00% 3 Missing ⚠️
lnbits/core/migrations.py 66.66% 2 Missing ⚠️
lnbits/core/views/api.py 33.33% 2 Missing ⚠️
lnbits/settings.py 92.85% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #2092      +/-   ##
==========================================
- Coverage   60.45%   59.39%   -1.06%     
==========================================
  Files          58       59       +1     
  Lines        8464     8832     +368     
==========================================
+ Hits         5117     5246     +129     
- Misses       3347     3586     +239     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@motorina0 motorina0 marked this pull request as ready for review November 21, 2023 20:11
pyproject.toml Outdated Show resolved Hide resolved
@dni dni changed the title Login improve [FEAT] Auth, Login, OAuth, create account with username and password #1653 Nov 22, 2023
@dni dni added this to the 0.12.0 milestone Nov 22, 2023
@dni dni added enhancement Make something better core Core of LNbits labels Nov 22, 2023
@jackstar12

This comment was marked as resolved.

@prusnak
Copy link
Collaborator

prusnak commented Dec 11, 2023

Question: @motorina0 When LNBITS_ALLOW_NEW_ACCOUNTS is set to False:

  1. does it prevent creation of new users with username and password (username-password method)?
  2. does it prevent creation of new users via Google SSO and Github SSO (google-auth and github-auth methods)?

@motorina0
Copy link
Collaborator Author

Question: @motorina0 When LNBITS_ALLOW_NEW_ACCOUNTS is set to False:

  1. does it prevent creation of new users with username and password (username-password method)?
  2. does it prevent creation of new users via Google SSO and Github SSO (google-auth and github-auth methods)?
  • 1: yes
  • 2: yes

lnbits/helpers.py Outdated Show resolved Hide resolved


def is_valid_username(username: str) -> bool:
username_regex = r"\b(?=[a-zA-Z0-9._]{2,20}$)(?!.*[_.]{2})[^_.].*[^_.]\b"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am clueless. Where does this come from and why is it so complicated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adapted from here: https://stackoverflow.com/questions/12018245/regular-expression-to-validate-username

- allowed characters: a-zA-Z0-9._
- username is 2-20 characters long
- no __ or _. or ._ or .. inside
- no _ or . at the beginning
- no _ or . at the end

dni and others added 2 commits December 12, 2023 12:27
wip wip wip

update deps

revert tests

formatting

bundle

refactor into new init

fix latest fastapi issue

fixup, working again

bundle

no more superuser url!

delete cookie on logout

add usr login feature

remove unused tests

fix node management

bundle
@dni dni merged commit c909371 into dev Dec 12, 2023
23 checks passed
@dni dni deleted the login_improve branch December 12, 2023 10:38
@prusnak prusnak mentioned this pull request Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core of LNbits enhancement Make something better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants