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

Cannot save LDAP settings if Server URI ends with custom domain(i.e. not ending with com, net, org, etc) #3646

Closed
SylvainMartel opened this issue Apr 8, 2019 · 4 comments

Comments

@SylvainMartel
Copy link

SylvainMartel commented Apr 8, 2019

ISSUE TYPE
  • Bug Report
COMPONENT NAME
  • UI
SUMMARY
ENVIRONMENT
  • AWX version: 4.0.0
  • AWX install method: docker on linux
  • Ansible version: 2.7.9
  • Operating System: Centos 7
  • Web Browser: chrome
STEPS TO REPRODUCE
  1. Go to Settings -> AUTHENTICATION.
  2. In the Sub Category drop-down, select LDAP.
  3. In the LDAP Server URI field, attempt to add a URI that ends with a custom internal domain like: ldap://servername.cotoso.coto.so3:389
  4. Hit Save
EXPECTED RESULTS

It should save the config

ACTUAL RESULTS

Error message stating: Failed to save settings. Returned status: 400

The field then display : Enter a valid URL
But the url is valid, for an internal domain.

ADDITIONAL INFORMATION
@ryanpetrello
Copy link
Contributor

I'm able to reproduce this:

image

@fosterseth
Copy link
Member

the error occurs if a number is present anywhere in the top-level domain portion of the url, e.g. .so3, s3o, 3so, etc.

the full url is validated using the built-in django URLValidator, which checks against a pre-defined reg expression to make sure the string looks like a valid url
(https://docs.djangoproject.com/en/2.2/ref/validators/#urlvalidator)

one workaround would be to replace any given url top level domain portion with something we know works, e.g. replace
ldap://servername.cotoso.coto.so3:389
with
ldap://servername.cotoso.coto.com:389

and sending that into the run_validations() method. That way it checks the validity of the rest of the url and url structure without getting hung up on the top level domain

another option is to write a custom regex and send that into the validator

or we can simply disallow non-conventional urls and give the user a more detailed error message instead of the generic 400

or, we can remove this particular URLValidator altogether so the check never occurs.

@fosterseth
Copy link
Member

fosterseth pushed a commit to fosterseth/awx that referenced this issue Sep 19, 2019
…ustom regex that will be used to validate a URI provided by users. The URI is eventually passed through a django class, URLValidator. This class has a default regex that does not accept numbers in the top level domain. The custom regex, which is passed into URLValidator via **kwargs, simply offers number capabilites for the top level domain portion of the full url. This is a bug fix related to issue ansible#3646.
fosterseth pushed a commit to fosterseth/awx that referenced this issue Sep 20, 2019
- API error if LDAPServerFieldURI contains a number in the top level domain
- Add custom regex in LDAPServerFieldURI class that is passed to django
  URLValidator
- The custom regex allows for numbers to be present in the top level domain
- Unit tests check that valid URIs pass through URLValidator, and that
  invalid URIs raise the correct exception
- Related to issue ansible#3646
fosterseth pushed a commit to fosterseth/awx that referenced this issue Sep 20, 2019
- API error if LDAPServerURIField contains a number in the top level domain
- Add custom regex in LDAPServerURIField class that is passed to django
  URLValidator
- The custom regex allows for numbers to be present in the top level domain
- Unit tests check that valid URIs pass through URLValidator, and that
  invalid URIs raise the correct exception
- Related to issue ansible#3646
fosterseth added a commit to fosterseth/awx that referenced this issue Sep 20, 2019
- Bug: API error if LDAPServerURIField contains a number in the top level domain
- Add custom regex in LDAPServerURIField class that is passed to django
  URLValidator
- The custom regex allows for numbers to be present in the top level domain
- Unit tests check that valid URIs pass through URLValidator, and that
  invalid URIs raise the correct exception
- Related to issue ansible#3646
@kdelee
Copy link
Member

kdelee commented Sep 20, 2019

Tested out with a variety of different URIs, I'm happy with the unit test coverage that was added. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants