-
Notifications
You must be signed in to change notification settings - Fork 3
Database refactor #123
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
Open
gsanchietti
wants to merge
39
commits into
main
Choose a base branch
from
user_refactor
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Database refactor #123
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9a4e9a6
to
1770889
Compare
16122a9
to
66ad62a
Compare
f50a3c3
to
e81764f
Compare
Avoid misalignment between VPN configuration and database: now the database is the source of truth for the VPN config
Resolve some known CVEs
51974f9
to
22fcce5
Compare
The update password part was removed, but it's still used by the UI
a4bba1b
to
b0fa344
Compare
The SECRETES_DIR env var is not required anymore
f21ea82
to
03e37fb
Compare
edospadoni
approved these changes
Jul 7, 2025
ba9d36d
to
d7d07c0
Compare
Make sure that concurrent access to active tokens is safe
Ease troubleshooting if easyresa fails
This info can be used inside the UI to show if a unit belongs to a group
d7d07c0
to
402c2d0
Compare
Make sure that only the user with id 1 is set as admin
6bb61fd
to
567f33d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: NethServer/nethsecurity#1300
This pull request introduces significant changes to the API and VPN container.
The focus is on improving overall security.
Main changes
report
database: the database is has not been renamed to avoid issues on migration. The vpn container now read and writes directly to the database: this also replaces VPN configuration via CCD files.ALLOWED_IPS
,PUBLIC_ENDPOINTS
andLISTEN_ADDRESS
is now possible to restrict the controller access for security reasons: only the/register
public endpoint can be public, units can then be modified to push all traffic through the VPNAPI changes
GET
,POST
,PUT
, andDELETE
operations to create, update, list, and delete unit groups./platform
for retrieving NS8 information than can be exposed inside the UI/auth
endpoint to check API server status, this can be used by podman to restart the controller if in faulty state/auth/<unit_id>
endpoint with basic authentication, it can be used to forward authentication from other middleware like traefik, it returns 200 if the user can access the unitvpn_address
andapi_port
inside the/register
response payload: these new fields can be used to configure the unit to push data inside the VPNConfiguration updates
LISTEN_ADDRESS
with a comma-separated list to support multiple listen addresses: with a minor change inside the/register
ENCRYPTION_KEY
andPLATFORM_INFO
environment variables for sensitive data encryption and platform metadata storage.ALLOWED_IPS
option, a comma-separated list of allowed IPs, if empty, all IPs are allowed, default is emptyPUBLIC_ENDPOINTS
option, a comma-separated list of public endpoints, that can be accessed even ifALLOWED_IPS
is set, default is emptyIf ALLOWED_IPS is set, the public endpoints should allow registration and ingestions from units, a good value should be:
/api/ingest,/api/units/register
Breaking changes⚠️
PUT /account
API now requires unit groups and can't be used to change the account passoword: the controller UI must ber refactored to follow new rulesDeprecations
OpenVPNStatusDir
,CredentialsDir
,SecretsDir
) as deprecated, signaling their removal in future versions.tokens
,credentials
).Dependency Changes
dgryski/dgoogauth
withpquerna/otp
for OTP functionality and addedgoogle/uuid
for UUID generation.Grafana authentication integration (this WILL NOT be implemented)
If Grafana authentication is integrated with the controller, all users must be created inside the controller itself, including guest users. To support guest users the following changes are needed:
/auth
controller API for non-admin usersChanges to other parts (not implemented yet)
/register
response