Add session expire functionality based on inactivity
Describe your changes
I have implemented a session timeout feature based on inactivity, forcing re-authentication with every netbird up. This was necessary because, while evaluating if Netbird was suitable for my use case, I noticed that authentication is only required after the login expiration period has elapsed. For security reasons, I wanted access to be required with every connection: this way, the authentication process with the identity provider is triggered and, by using a session timeout setting in Keycloak, credentials are requested each time.
Building on the existing approach for login expiration, I implemented inactivity expiration by checking the status of a peer: after a configurable period of time following netbird down, the peer shows login required.
I also added a setting, inactivityExpirationEnabled, that can enable or disable the inactivity expiration feature through the APIs.
I needed this functionality due to a business requirement and thought it might be useful to you as well.
Checklist
- [ ] Is it a bug fix
- [ ] Is a typo/documentation fix
- [x] Is a feature enhancement
- [ ] It is a refactor
- [ ] Created tests that fail without the change (if possible)
- [ ] Extended the README / documentation, if necessary
Hello @ctrl-zzz can you check the failing tests and Sonar checks?
We will review the feature and give you a feedback ASAP
Hello, I believe they are about the Cognitive Complexity of some methods. Since it's a new feature I added some new properties that had to be checked as well for some functionalities. I guess there needs to be less conditions to be checked, but not checking them wouldn't make the methods work as intended
Hello, I believe they are about the Cognitive Complexity of some methods. Since it's a new feature I added some new properties that had to be checked as well for some functionalities. I guess there needs to be less conditions to be checked, but not checking them wouldn't make the methods work as intended
thanks, there are some optimizations that can done, see some suggestions below:
For func (am *DefaultAccountManager) UpdateAccountSettings, you can move some of the new checks into a separate method or function, and that should reduce complexity.
For func (am *DefaultAccountManager) MarkPeerConnected you can rewrite it like this:
if peer.AddedWithSSOLogin() {
if peer.LoginExpirationEnabled && account.Settings.PeerLoginExpirationEnabled {
am.checkAndSchedulePeerLoginExpiration(ctx, account)
}
if peer.InactivityExpirationEnabled && account.Settings.PeerInactivityExpirationEnabled {
am.checkAndSchedulePeerInactivityExpiration(ctx, account)
}
}
Quality Gate passed
Issues
0 New issues
2 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code