Skip to content
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

Added owner fields in radius filter #6324

Merged
merged 1 commit into from
Jul 1, 2021
Merged

Conversation

fdurand
Copy link
Member

@fdurand fdurand commented Apr 23, 2021

Description

Add owner in the radius filter.

Impacts

Radius filters

Delete branch after merge

YES

Checklist

  • Document the feature
  • Add unit tests
  • Add acceptance tests (TestLink)

NEWS file entries

Enhancements

  • Added the owner info in the radius filters

@fdurand
Copy link
Member Author

fdurand commented Jun 29, 2021

BUMP

Copy link
Contributor

@nqb nqb left a comment

Choose a reason for hiding this comment

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

When there is no pid defined for a node (MAC authentication), would it not generate errors in log ?

I didn't test.

@julsemaan
Copy link
Collaborator

When there is no pid defined for a node (MAC authentication), would it not generate errors in log ?

I didn't test.

There should always be a PID AFAIK since default is in the person table

Copy link
Contributor

@nqb nqb left a comment

Choose a reason for hiding this comment

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

Work as expected. No error messages for MAC Auth.

I was able to test RADIUS filters for authorize and accounting. Accounting requires filter_in_packetfence_accounting=enabled in pf.conf

@nqb nqb merged commit 186054e into devel Jul 1, 2021
@nqb nqb deleted the feature/owner_in_radius_filter branch July 1, 2021 08:16
nqb added a commit that referenced this pull request Jul 1, 2021
satkunas pushed a commit that referenced this pull request Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants