Closed fdurand closed 3 years ago
It seems like the host is passed in as an array.
@jrouzierinverse Might be normal here if we want to provide multiple hostnames in the host field for redundancy when talking to the AD source over LDAP.
I'm reopening because integration tests failed this night when creating AD sources. I will investigate.
Error seen in logs:
Mar 23 04:21:28 pfcen7dev packetfence: pfperl-api(12851) ERROR: Attribute (host) does not pass the type constraint because: Validation failed for 'ArrayRef[Str]' with value 172.17.17.100 at constructor pf::Authentication::Source::ADSource::new (defined at /usr/local/pf/lib/pf/Authentication/Source/ADSource.pm line 96) line 174.
Mar 23 04:21:28 pfcen7dev packetfence: pfperl-api(12851) DEBUG: 500 Internal Server Error (1.448034s, 0.691/s) (Mojolicious::Controller::rendered)
when creating a source with following payload:
{
"administration_rules": null,
"authentication_rules": [
{
"id": "catchall",
"description": null,
"match": "all",
"actions": [
{
"type": "set_role",
"value": "machine_employee"
},
{
"type": "set_access_duration",
"value": "5m"
}
],
"conditions": []
}
],
"basedn": "dc=example,dc=lan",
"binddn": "vagrant-domain@example.lan",
"cache_match": "0",
"connection_timeout": 1,
"description": "172.17.17.100 machine authentication",
"email_attribute": "mail",
"encryption": "starttls",
"host": "172.17.17.100",
"id": "dot1x_eap_peap_machine_auth",
"monitor": "1",
"password": "VagrantPass1",
"port": "389",
"read_timeout": 10,
"realms": "",
"scope": "sub",
"searchattributes": "",
"set_access_durations_action": null,
"shuffle": "0",
"type": "AD",
"usernameattribute": "servicePrincipalName",
"write_timeout": 5
}
Change "host": "172.17.17.100",
to "host": ["172.17.17.100"],
Hello @jrouzierinverse,
It will certainly fix integration tests issue but it's a regression that will impact many PacketFence users. We need to have a backward compatibility.
Reopening because creating an AD source with host=172.17.17.100
works but testing source failed.
In fact, when sending payload mentioned in previous comment, AD source is created with a host
parameter set to an empty value:
[dot1x_eap_peap_machine_auth]
type=AD
description=172.17.17.100 machine authentication
password=VagrantPass1
realms=
searchattributes=
monitor=1
read_timeout=10
host=
scope=sub
usernameattribute=servicePrincipalName
connection_timeout=1
binddn=vagrant-domain@example.lan
encryption=starttls
port=389
write_timeout=5
email_attribute=mail
basedn=dc=example,dc=lan
shuffle=0
set_access_durations_action=
cache_match=0
EDIT: there is no issue when creating source from API.
@jrouzierinverse, do you have a fix ? This issue prevents integration tests to run correctly.
On top of that, you can create an AD source from GUI but after you did it, your source is saved and when you want to edit it, you got: Unhandled source type
Integration tests are still failing on that part. I will look into this.
Now authentication source is created correctly: host
parameter has correct value in authentication.conf
but test of source failed when sending following payload:
POST https://172.17.17.10:1443/api/v1/config/sources/test
{
"basedn": "dc=example,dc=lan",
"binddn": "vagrant-domain@example.lan",
"connection_timeout": 1,
"description": "172.17.17.100 machine authentication",
"encryption": "starttls",
"host": "172.17.17.100",
"id": "dot1x_eap_peap_machine_auth",
"password": "VagrantPass1",
"port": "389",
"read_timeout": 10,
"scope": "sub",
"type": "AD",
"usernameattribute": "servicePrincipalName",
"write_timeout": 5
}
Result: Can't connect to server or bind with 'vagrant-domain@example.lan' on :
/api/v1/config/sources/test
doesn't accept host
as string
.
I updated JSON payload in integration tests (in 9b60774f396486ac299b339db75bafd6d759f362) to send host
as an array for creation and test (like frontend did).
However, I keep this issue open to track two things:
host
as a string for creation but not for test (which doesn't make sense IMHO)@jrouzierinverse, how do you want to handle that ?
Since the host can take both a string and array an upgrade notes is not needed at the moment. I agree with the test should take both string and array that was fixed in b312a99.
Describe the bug![image](https://user-images.githubusercontent.com/1553962/111509726-6602d980-8723-11eb-997b-1739d7eaf989.png)