OPNsense Forum

English Forums => Development and Code Review => Topic started by: wheldom01 on April 02, 2020, 10:37:50 pm

Title: Access Servers - Radius: Adding default radius realm to configuration page
Post by: wheldom01 on April 02, 2020, 10:37:50 pm

Hi Forum,

I've just created the ability to have OPNsense add a default realm to a user logging in via radius. Mainly for use with the captive portal. However I've got a couple of questions.

1. Firstly would anyone else find this functionality useful?

2. My changes are across the following files:
   src/opnsense/mvc/app/library/OPNsense/Auth/Radius.php
   src/www/system_authservers.php

Which from my reading of  the documentation span the old and new code bases. Is that correct?
As such I'm struggling to find where/how field validation is carried out. A pointer or two in the right
direction would be much appreciated.

3. What is the pull request process?
Is there a documented process? Apologies if I've missed it in the development documentation.
Is the best practice to post here for a code review first?

Many thanks in advance.

Martin

Title: Re: Access Servers - Radius: Adding default radius realm to configuration page
Post by: mimugmail on April 03, 2020, 05:59:03 am

Field validation in new code are in mvc/app/model.../bla.xml
On old code you can search for "preg" in source files.

Create an account in github, fork opnsense/core, do you changes local in your fork, create a pull request ( button are all there :) )

Title: Re: Access Servers - Radius: Adding default radius realm to configuration page
Post by: wheldom01 on April 04, 2020, 09:14:03 am

The reason I couldn't find any form validation is because there isn't any for the Servers configuration section.
So from my perspective there are 3 options:

1. Update the form with my new fields and don't validate them.
2. Validate my new fields by adding preg matches to the existing code.
3. Write the Servers section under the new module mechanism.

Options 1 and 2 in my opinion range from horrible to undesirable.

I'm guessing that the preferred solution here is a re write?
And that his section of the code hasn't been done already as no one will have had a reason to touch it?

So I guess my long winded question is. Is anyone working on this at the moment?
If so do they need some help?

If not them I'm going to start turning this into a module.

Title: Re: Access Servers - Radius: Adding default radius realm to configuration page
Post by: mimugmail on April 04, 2020, 10:08:42 am

you can add a detailed feature request in github, maybe also ad jumps in to help you out.
i don't touch legacy code and auth section is out of scope for me.