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

App Submission: WebCheck #1742

Merged
merged 28 commits into from
Nov 12, 2024
Merged

App Submission: WebCheck #1742

merged 28 commits into from
Nov 12, 2024

Conversation

dennysubke
Copy link
Contributor

App Submission

WebCheck

...

Icon

GhoYf27 - Imgur

...

Gallery images

1QWFJhW - Imgur

g9q7xuV - Imgur

3KVJib8 - Imgur

cs0lZzo - Imgur

FGIpxV4 - Imgur

...

I have tested my app on:

  • umbrelOS on a Raspberry Pi
  • umbrelOS on an Umbrel Home
  • umbrelOS on Linux VM

@dennysubke
Copy link
Contributor Author

dennysubke commented Nov 5, 2024

The WebCheck app offers powerful website checks with a modern, user-friendly UI. Loading times, server responses and errors are displayed quickly and clearly. In my opinion this is perfect for integration in Umbrel. 😎

I tested everything so far in my Community Store:
https://github.com/dennysubke/dennys-umbrel-app-store

Copy link
Contributor

@nmfretz nmfretz left a comment

Choose a reason for hiding this comment

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

Excellent submission @dennysubke! We'll do a once-over on the gallery assets before going live.

In the meantime, can you please take a look at these small changes:

@@ -0,0 +1,13 @@
version: '3.9'
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's change this to version 3.7 to ensure compatibility with past OS versions. It should be fine as-is, but I'd like to err on the side of caution.

FYI, this will change in our upcoming apps refactor, but for now let's always use 3.7/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood! :) I will take this into account in the future.

environment:
APP_HOST: webcheck_server_1
APP_PORT: 3000
PROXY_AUTH_ADD: "false"
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we remove PROXY_AUTH_ADD: "false" entirely so that this app gets umbrel's transparent auth proxy?

That way the WebCheck UI inherits the security properties of our umbrel auth:

  • By default, we only let requests through the proxy if the user already has a valid auth cookie from the Umbrel homescreen. So since the WebCheck dashboard has no auth of its own, it will still be protected by ours. This gives a good UX because there's zero friction for Umbrel users... they can just access the dashboard without re-entering any credentials if they are already logged in to their Umbrel.

  • It also has the benefit of inheriting other security properties of Umbrel auth, such as 2FA if they have it enabled on their Umbrel. They would then get 2FA security for all apps behind the auth proxy too.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, removing PROXY_AUTH_ADD: "false" is a good idea if we want the WebCheck UI to inherit Umbrel’s transparent authentication proxy.

Comment on lines +38 to +41
gallery:
- 1.jpg
- 2.jpg
- 3.jpg
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you have 4 lovely gallery images, you can add a 4.jpg here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tip! :)

@dennysubke
Copy link
Contributor Author

Excellent submission @dennysubke! We'll do a once-over on the gallery assets before going live.

In the meantime, can you please take a look at these small changes:

@nmfretz Short message: All changes have been made. Thank you for your valuable help, and let me know if there's anything else you'd like to comment on. 👍

@nmfretz
Copy link
Contributor

nmfretz commented Nov 12, 2024

Perfect, thanks for those final changes @dennysubke! Tested and going live.

image

Copy link

⚠️   Linting finished with 1 warning   ⚠️

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
ℹ️ webcheck/docker-compose.yml Potentially using unsafe user in service "server":
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.
⚠️ webcheck/umbrel-app.yml "icon" and "gallery" needs to be empty for new app submissions:
The "icon" and "gallery" fields must be empty for new app submissions as it is being created by the Umbrel team.

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 nmfretz merged commit 5f7672e into getumbrel:master Nov 12, 2024
1 check passed
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.

2 participants