Skip to content

Conversation

@tvdijen
Copy link
Member

@tvdijen tvdijen commented Jan 14, 2026

Ensure that the NAS identifier is always set to meet standards compliance when no NAS-IP-Address is set

@codecov
Copy link

codecov bot commented Jan 14, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (aa03381) to head (be4e9ac).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##             master     #28   +/-   ##
========================================
  Coverage      0.00%   0.00%           
+ Complexity       26      25    -1     
========================================
  Files             1       1           
  Lines            91      89    -2     
========================================
+ Misses           91      89    -2     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tvdijen tvdijen requested a review from restena-sw January 14, 2026 09:12
@restena-sw
Copy link
Collaborator

You should entirely comment out line 160 (nasIpAddress). With the updated upstream code, it is very complex to determine whether the RADIUS request will be sent as IPv4 or IPv6; if it happens to be IPv6, setting NAS-IP-Address will fail.
As per standards, setting Nas-Identifier is sufficient.

Copy link
Collaborator

@restena-sw restena-sw left a comment

Choose a reason for hiding this comment

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

looks good

@tvdijen tvdijen merged commit 9087a96 into master Jan 14, 2026
36 checks passed
@tvdijen tvdijen deleted the feature/fix-ipv6 branch January 14, 2026 10:07
tvdijen added a commit that referenced this pull request Jan 14, 2026
)

* Always set the NAS identifier; if not set fall back to the hostname

* Refrain from setting NAS-IP-Address
@tvdijen
Copy link
Member Author

tvdijen commented Jan 14, 2026

Tagged v2.1.1

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