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

Cleanup Users and ZAuth redundancies #3913

Closed
Guite opened this Issue Jul 6, 2018 · 4 comments

Comments

Projects
None yet
2 participants

@Guite Guite added this to the 2.1.0 milestone Jul 6, 2018

@craigh

This comment has been minimized.

Member

craigh commented Jul 6, 2018

the validators are intentionally duplicated

consider - ZAuth may not be the module which authenticates the user (e.g. OAuth, etc). The Authentication module must validate its own values. In addition, the core must validate users as they are added.

The modvar being used should be obtained from the ZikulaUsersModule not ZikulaZAuthModule that appears to be a mistake.

UsersConstant::MODVAR_REGISTRATION_ILLEGAL_DOMAINS => '',

@Guite

This comment has been minimized.

Member

Guite commented Jul 6, 2018

I understand that each authentication module may use it's own validation logic. But why can't ZAuth reuse the email validator from the Users module if it utilises the same validation approach?

@craigh

This comment has been minimized.

Member

craigh commented Jul 6, 2018

I wrestled with all these decisions some time ago. At the time, I determined this was the best approach. I cannot recall all my thoughts, but suffice to say that all options were carefully considered. Self-containment was certainly part of the thought process. Obviously, I am typically not in favor of code duplication... but at the time, it made sense for me to do so.

@Guite

This comment has been minimized.

Member

Guite commented Jul 6, 2018

Okay. I will then just fix the wrong modvar reference.

@Guite Guite modified the milestones: 2.1.0, 1.5.8 Jul 6, 2018

@Guite Guite closed this in b144d91 Jul 6, 2018

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