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

Changed to passing a Domain object, hardened the lints and update the readme.md #6

Merged
merged 17 commits into from
Feb 3, 2024

Conversation

bsutton
Copy link
Collaborator

@bsutton bsutton commented Dec 8, 2023

This is what I've done.
See what you think.

Copy link

codecov bot commented Dec 8, 2023

Codecov Report

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

Comparison is base (39d0dc2) 35.19% compared to head (60b43ed) 35.00%.

Files Patch % Lines
lib/src/letsencrypt.dart 22.50% 93 Missing ⚠️
lib/src/certificates_handler_io.dart 63.55% 43 Missing ⚠️
lib/src/certs_handler.dart 19.35% 25 Missing ⚠️
lib/src/logging.dart 57.14% 9 Missing ⚠️
lib/src/domain_certificate_file_path.dart 0.00% 8 Missing ⚠️
lib/src/security_context_builder.dart 0.00% 7 Missing ⚠️
lib/src/domain.dart 33.33% 6 Missing ⚠️
lib/src/check_certificate_status.dart 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master       #6      +/-   ##
==========================================
- Coverage   35.19%   35.00%   -0.20%     
==========================================
  Files           2        9       +7     
  Lines         429      440      +11     
==========================================
+ Hits          151      154       +3     
- Misses        278      286       +8     
Flag Coverage Δ
unittests 35.00% <39.50%> (-0.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@gmpassos
Copy link
Owner

gmpassos commented Dec 8, 2023

Let's upgrade to:

lints: ^3.0.0

@bsutton
Copy link
Collaborator Author

bsutton commented Dec 8, 2023

As requested I've move to lints 3.x

… that you can configure the server on an alternate port. I now pass the httpPort down into the challenge so it can set the url correctly.
@gmpassos
Copy link
Owner

gmpassos commented Dec 8, 2023

Let's bump the version to 1.3.0

@bsutton
Copy link
Collaborator Author

bsutton commented Dec 9, 2023

I now have a secure server up and running using the new code :)

…crypt ctor as we had methods that needed access to the port. The bindingAddress could have been left on the startServer method but it seemed more logic for these three values to be together.

Change the version to 2.0.0 to reflect the breaking change.
… the alternate port passed in the ctor causing the call to fail.
…t runs a renewal service that restarts the server as required.
@bsutton
Copy link
Collaborator Author

bsutton commented Dec 9, 2023

I think this is now ready to be merged unless you have any specific issues.

The key point is the breaking change to startServer

@gmpassos
Copy link
Owner

Please, check if any dependency need to be updated:

acme_client: ^1.3.0

@gmpassos
Copy link
Owner

Hi, I will check this PR this week. Do you have any update to it?

@bsutton
Copy link
Collaborator Author

bsutton commented Jan 22, 2024 via email

## Permissions
On Linux you need to be root (sudo) to open a port below 1024. If you try
to start your server with the default ports (80, 443) you will fail.

Copy link
Owner

Choose a reason for hiding this comment

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

Also add a documentation for setcap to avoid execution as root:

Enabling the dart VM:

sudo setcap 'cap_net_bind_service=+ep' /usr/lib/dart/bin/dart

Enabling a self-executable (from dart compile exe your_tool.dart):

sudo setcap 'cap_net_bind_service=+ep' your_tool.exe

@bsutton
Copy link
Collaborator Author

bsutton commented Jan 23, 2024 via email

@gmpassos
Copy link
Owner

Nice package!

I usually avoid running server processes as root due to security risks. I recommend using setcap on the binary to enable the server to listen on low ports. I don't think we should advise executing server processes as root.

@bsutton
Copy link
Collaborator Author

bsutton commented Jan 24, 2024 via email

@gmpassos gmpassos merged commit 819c0d0 into gmpassos:master Feb 3, 2024
3 of 4 checks passed
@gmpassos
Copy link
Owner

gmpassos commented Feb 3, 2024

FYI:

Preparing release and testing it in real projects:
#12

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