Bug 48709 - Non documented ucr variables for radius
Non documented ucr variables for radius
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: Radius
UCS 4.4
Other Linux
: P5 normal (vote)
: UCS 4.4-3-errata
Assigned To: Julia Bremer
Sönke Schwardt-Krummrich
:
: 49621 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2019-02-19 17:18 CET by Jürn Brodersen
Modified: 2020-03-18 12:27 CET (History)
4 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 3: Simply Wrong: The implementation doesn't match the docu
Who will be affected by this bug?: 1: Will affect a very few installed domains
How will those affected feel about the bug?: 1: Nuisance – not a big deal but noticeable
User Pain: 0.017
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jürn Brodersen univentionstaff 2019-02-19 17:18:55 CET
Non documented ucr variables for radius

Not all ucr variables used in the freeradius templates are documented
Comment 1 Mathieu Simon 2020-01-10 15:29:26 CET
Hi

By grepping through the source code in the main git repository under services/univention-radius I've found the following undocumented UCR variables based on 4.4-3 Errata 413:

freeradius/auth/helper/ntlm/debug
freeradius/conf/auth-type/mschap/mppe
freeradius/conf/auth-type/mschap/ntdomainhack
freeradius/conf/log/auth
freeradius/conf/log/auth/badpass
freeradius/conf/log/auth/goodpass
freeradius/conf/auth-type/mschap/requireencryption
freeradius/conf/auth-type/mschap/strongencryption
freeradius/conf/log/strippednames
freeradius/conf/private/key/file
freeradius/conf/realm
freeradius/conf/servers/maxrequests
freeradius/conf/servers/spare/min
freeradius/conf/servers/spare/max
freeradius/conf/servers/start
freeradius/conf/starttls
freeradius/conf/users

Maybe this helps to come up with a PR.

Regards,
Mathieu
Comment 2 Max Pohle univentionstaff 2020-02-24 16:53:52 CET
Dear QA!

Please review my work.

Version: 6.0.2-19A~4.4.0.202002241643

Changes:
* doc/errata/staging/univention-radius.yaml
* services/univention-radius/debian/changelog
* services/univention-radius/debian/univention-radius.univention-config-registry- variables

Commits:
* 4a6070486945a7ca152d4e88eb2435ed4210672d
* [d87f807c3502a713a5a59d3167f1649623f0b825]
* 9e21bb7bf698b2bc31e4d0ccb88a74160dde6171
Comment 3 Mathieu Simon 2020-03-01 15:15:37 CET
Hi

I've made some commends on the main commit, mostly hyphen issues in the german translation which happens to me as well (and have been schooled by someone else in the past...). ;-)

For the record:
https://github.com/univention/univention-corporate-server/commit/9e21bb7bf698b2bc31e4d0ccb88a74160dde6171#diff-1cab77bf6841b9e4dbf29722555b2802R56

Oh and please consider marking as duplicate or resolving #49621 as well. 

Thanks for adding these descriptions, I'm looking forward for the actual release.

Regards
Mathieu
Comment 4 Max Pohle univentionstaff 2020-03-02 11:07:26 CET
*** Bug 49621 has been marked as a duplicate of this bug. ***
Comment 5 Mathieu Simon 2020-03-03 06:22:55 CET
Hi

One of your Univention-collegues has also jumped on the commenting-bandwagon on Github. ;-)

Anyhow, ntdomainhack could be completely dropped as I only discovered now by accident that the only remaining reference to this variable is found in univention-radius.postinst

How do you handle the removal of an unused UCR variable on already-installed systems so they don't linger around until someon discovers them? I.e. programmatic removal during a package update or make a release notes entry in an upcoming point release?

And would you appreciate a PR fixing the mentioned comments by both your collegue and me? 

Regards
Mathieu
Comment 6 Sönke Schwardt-Krummrich univentionstaff 2020-03-03 09:13:04 CET
There are several comments for your commit that have been made via github:
https://github.com/univention/univention-corporate-server/commit/9e21bb7bf698b2bc31e4d0ccb88a74160dde6171
Comment 7 Sönke Schwardt-Krummrich univentionstaff 2020-03-03 09:20:41 CET
(In reply to Mathieu Simon from comment #5)
> Anyhow, ntdomainhack could be completely dropped as I only discovered now by
> accident that the only remaining reference to this variable is found in
> univention-radius.postinst

Thanks for pointing this out.
 
> How do you handle the removal of an unused UCR variable on already-installed
> systems so they don't linger around until someon discovers them? I.e.
> programmatic removal during a package update or make a release notes entry
> in an upcoming point release?

Usually, if there is a replacement variable, we "move" the value of the old variable to the new variable. And as you described, this is nothing, we will do in an errata update but a patchlevel/minor/major update would be a good plache.

> And would you appreciate a PR fixing the mentioned comments by both your
> collegue and me? 

Afaik, Max is already working on it.

But apart from that we would appreciate pull requests. Most of the time it makes sense to ask in advance if the PR has a chance to be taken on short notice, so that the work is not unnecessary or lies around too long.
Comment 8 Julia Bremer univentionstaff 2020-03-16 15:13:11 CET
Package: univention-radius
Version: 6.0.2-20A~4.4.0.202003161507
Branch: ucs_4.4-0
Scope: errata4.4-3

ce9a116c2d Bug #48709: yaml
06f4297d45 Bug #48709: Fix typos in UCR-Variable description

I fixed the typos which were pointed out by Mathieu Simon, thank you very much :-)
The obsolete UCR variable ntdomainhack stays for now, since we shouldn't remove it in an errata update.
Comment 9 Sönke Schwardt-Krummrich univentionstaff 2020-03-16 21:26:16 CET
OK: code change
OK: installation
OK: update
OK: changelog entry
OK: advisory
OK: functional change
OK: package built and installable
Comment 10 Erik Damrose univentionstaff 2020-03-18 12:27:39 CET
<http://errata.software-univention.de/ucs/4.4/489.html>