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

Avoid an error if ipadefaultemaildomain if not defined #1289

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

proxyconcept
Copy link

The IPA configuration ipadefaultemaildomain is optional (in this case, the key don't exist).

It's usefull to unset the default email domain, because it disables the auto-generation of an email address when creating a new user. But when we have this type of configuration, the module ipauser always failed (with or without email) :

msg: '''ipadefaultemaildomain'''

While it's permitted by the FreeIPA server to have no default email domain, the module assumes that there must be one configured.

This PR simply check if this parameter exists. If not, AND if ever the module is called with a "short" email address that needs to be extended, so no miracle : it will failed with a message like this

msg: 'user_mod: testuser: invalid ''email'': invalid e-mail format: testuser@None'

But in this case I think it is a legitimate failure (the request is not compatible with the server configuration) and the message remains relatively understandable.

The IPA configuration "ipadefaultemaildomain" is optional (in this case the key don't exist)
@t-woerner
Copy link
Member

@proxyconcept Thanks for the PR.
Yes, the current behaviour of the module is wrong. Additionally to your change there needs to be an extra check to ensure that the task fails with an invalid email address and not defaultemaildomain set.

@rjeffman
Copy link
Member

In my opinion, the default e-mail domain configuration is broken on FreeIPA side (https://pagure.io/freeipa/issue/9680).

I suggest we wait for a decision/fix there before we change anything.

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

Successfully merging this pull request may close these issues.

3 participants