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

Update LNbits to 0.12.10 #1358 #1370

Merged
merged 6 commits into from
Sep 3, 2024
Merged

Update LNbits to 0.12.10 #1358 #1370

merged 6 commits into from
Sep 3, 2024

Conversation

Ghooosstt
Copy link
Contributor

Update LNbits to the latest version

@Ghooosstt
Copy link
Contributor Author

This is the latest version of LNbits and it has a lot of changes/updates, but I don't know how to test it on my umbrel which works on my raspberry pi 5. Anyone is able to check if the change of the Docker image is sufficient for this to work or if any further modifications are needed?

@nmfretz
Copy link
Contributor

nmfretz commented Aug 28, 2024

Thanks very much for this update @Ghooosstt. And sorry for the delay on my end while I've been away.

I have tested and this is working well for me. However, before updating this I just want to bring in the LNbits team to check something:

@motorina0, I understand that LNbits allows super user registration as of 0.12.0. Prior to 0.12.0 you guys added a way to create a superuser by using the environment variable SUPER_USER in the docker-compose.yml and then having the user navigate to something like umbrel.local:3007/uuidv4/<your-lnbits-password>. This workaround allowed umbrel users to have an admin account that they could manage extensions from.

Context: lnbits/lnbits#1688

What I'm wondering is if we are now okay to remove the SUPER_USER environment variable from the docker-compose.yml and get rid of the current superuser instructions:

image

In my testing, removing the above items works fine because:

  1. Fresh installs (of 0.12.0 and up) are prompted to create a superuser account through the UI:
image
  1. Legacy installs on umbrelOS that were installed pre 0.12.0 can still access their super user accounts by either:
    a) logging in through the UI with the appropriate user ID from their legacy admin account:
image

b) using either the old format from bookmarking back in the day:
http://umbrel.local:3007/wallet?usr=<user-id>&wal=<wallet-id>

or even using the uuidv4 method:
umbrel.local:3007/uuidv4/<password from the app store>

Again, this works fine in my testing, but I'd appreciate confirmation or letting me know if I've missed something.

@nmfretz nmfretz mentioned this pull request Aug 28, 2024
@Ghooosstt
Copy link
Contributor Author

Thank you for your help on this pull request @nmfretz! 🧡
I hope @motorina0 will approve your analysis!

@nmfretz
Copy link
Contributor

nmfretz commented Aug 30, 2024

You're welcome @Ghooosstt. I've contacted Vlad offline and he can take a look at this early next week.

@motorina0
Copy link

@nmfretz you are correct. The new fresh install page will allow the user to directly create the admin account.
Therefore the docker environment is not required anymore.

LGTM, thanks for updating LNbits!

Copy link

github-actions bot commented Sep 3, 2024

🎉   Linting finished with no errors or warnings   🎉

Thank you for your submission! This is an automated linter that checks for common issues in pull requests to the Umbrel App Store.

Please review the linting results below and make any necessary changes to your submission.

Linting Results

Severity File Description
ℹ️ lnbits/docker-compose.yml Potentially using unsafe user in service "web":
The default container user "root" can lead to security vulnerabilities. If you are using the root user, please try to specify a different user (e.g. "1000:1000") in the compose file or try to set the UID/PUID and GID/PGID environment variables to 1000.

Legend

Symbol Description
Error: This must be resolved before this PR can be merged.
⚠️ Warning: This is highly encouraged to be resolved, but is not strictly mandatory.
ℹ️ Info: This is just for your information.

@nmfretz
Copy link
Contributor

nmfretz commented Sep 3, 2024

Excellent, thanks very much for the review and confirmation @motorina0. I have cleaned up the legacy superuser items from the compose file and app manifest, and added a reminder in the release notes for legacy LNbits installs on how to access their admin accounts.

Tested again. Going live.

@nmfretz nmfretz merged commit 6022091 into getumbrel:master Sep 3, 2024
1 check passed
@Ghooosstt
Copy link
Contributor Author

Thank you so much @nmfretz and @motorina0 !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants