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

fix(HMS-3152): remove pattern matching from location name #23

Merged
merged 1 commit into from
Sep 28, 2024

Conversation

pvoborni
Copy link
Contributor

@pvoborni pvoborni commented Sep 23, 2024

Location name should be a DNS label and could be matched by a regex.

Correct one could be "^[a-zA-Z0-9]([a-zA-Z0-9\\-]*[a-zA-Z0-9])*$".

PR #21 has a discussion about it. But users can defined more values in IPA location in the FreeIPA server than what the RFCs allow.

From this PoV, it is more important for idmsvc to pass registration even with slightly invalid DNS label than being valid but failing. Idmsvc doesn't use the value for DNS operations.

@pvoborni
Copy link
Contributor Author

pvoborni commented Sep 23, 2024

Btw a follow-up task might be to loosen the check at https://github.com/podengo-project/ipa-hcc/blob/main/src/ipahcc/client/auto_enrollment.py#L90

@frasertweedale
Copy link
Contributor

Btw a follow-up task might be to loosen the check at https://github.com/podengo-project/ipa-hcc/blob/main/src/ipahcc/client/auto_enrollment.py#L90

That uses ipalib.util.validate_dns_label, so I think there's nothing to do (unless ipalib.util.validate_dns_label is also too strict).

@frasertweedale
Copy link
Contributor

I think this is the better change to merge. Thank you for the detailed justification in the commit message - but perhaps could you also include a comment in the yaml itself, so that no one will be tempted to put a restriction back onto it? :)

Copy link
Contributor

@frasertweedale frasertweedale left a comment

Choose a reason for hiding this comment

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

One request - provisional ACK, feel free to merge it after adding the comment.

Location name should be a DNS label and could be matched by a regex.

Correct one could be "^[a-zA-Z0-9]([a-zA-Z0-9\\-]*[a-zA-Z0-9])*$".

PR podengo-project#21 has a
discussion about it. But users can defined more values in IPA
location in the FreeIPA server than what the RFCs allow.

From this PoV, it is more important for idmsvc to pass registration
even with slightly invalid DNS label than being valid but failing.
Idmsvc doesn't use the value for DNS operations.

Signed-off-by: Petr Vobornik <[email protected]>
@pvoborni pvoborni changed the title fix: remove pattern matching from location name fix(HMS-3152): remove pattern matching from location name Sep 27, 2024
@frasertweedale frasertweedale merged commit 8e2bc0b into podengo-project:main Sep 28, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants