IdStory IdM modules » Registration (reg)

Implemetation is in psourek/1094-activation can you please do the review?

I made review (czechidm version 8.0.0 and reg 1.2.0-SNAPSHOT), there is result from my tests:

• documentation for new behavior and application properties missing,
• I have set this property: idm.pub.reg.note.required=true I'm not able made new reqistration see picture. Check this attribute during creting default fields or create some changelog,
  
• not localized message:
  
• default password field is generated as simple text, also I'm not able create password with confirm filed (2x text filed with retype password)
  ,
• i'm not able create new registration, when I has defined password policy for registration module (idm.sec.reg.passwordPolicy), password does not match password policy, but my password matches with policy (is longer than 6 characters)
• some unused vars and console logs on frontend:
  

      7:16  warning  "Advanced" is defined but never used        no-unused-vars
     46:11  warning  "showPassword" is defined but never used    no-unused-vars
     64:5   warning  Unexpected console statement                no-console
    101:5   warning  Unexpected console statement                no-console
    106:12  warning  "showSaveButton" is defined but never used  no-unused-vars
    110:7   warning  "showLogin" is defined but never used       no-unused-vars
    111:7   warning  "showPassword" is defined but never used    no-unused-vars
    118:11  warning  "isRequiredNote" is defined but never used  no-unused-vars
  

• in Registration.js code in comments lines 54,90 the code is unless?,
• props showNote, noteRequired, showPassword and showLogin is probably unless, or?,
• when I show registration form for first time is show this message, message isn't even localized, but better than this message will be show loading
  
• null pointer while registration:
  

  java.lang.NullPointerException: null
      at eu.bcvsolutions.idm.reg.service.impl.DefaultRegIdentityService.prepareIdentity(DefaultRegIdentityService.java:197)
  

  
  
• user after success registration stay on registration form, there was redirect to login page,
• isn't possible confirm registration, after I click on URL I obtain this message:
  
• for configuration RegConfiguration missing javadoc for some methods,
• new result codes are not localized.

Please fix this issues, for now I'm not able test self registration, after your fix I will be continue with review.

This ticket sleeps too long. Please finish it so we are finally able to use it on our procejt

Fixed previous issues. Documentation is in wiki. Added changelog.

Can you please make a review?

Thanks for fixing the previous mistakes, I made second review

• project has some errors in IDE,
• tests not pass,
• when I post registration (first init) is validate only on backend, you just return "Some mandatory fields were empty" which? Why is not validate form at frontend? You can use validate by EAV form,
  
• first initialized reg form hasn't localization and password field (some customers are very sensitive for missing localization)
  
• show loading is still not implemented, and message "no attributes" is hardly written in code:
  
• After successful registration is user redirect to login page without any message, try to consult with someone behavior after registration,
• attribute selfRegistered isn't checked after registration, this critical backward compatible issues, on some project is this used for some check,
• missing localization for password field:
  
• missing localization for terms field:
  
• bad EN localization for agree box:
  
• without own module I'm not possible add localization for password attribute renderer:
  
• some strange behavior I fill all fields in registration and I still receive "some mandatory fields were empty":
  
  

  2018-11-23 07:55:46.448  WARN 23645 --- [nio-8080-exec-1] e.b.i.c.e.ExceptionControllerAdvice      : [440f8886-6c02-4958-b397-f4e8fc448931] 
  
  eu.bcvsolutions.idm.core.api.exception.ResultCodeException: Missing registration data test
      at eu.bcvsolutions.idm.reg.service.impl.DefaultRegRegistrationValidator.lambda$3(DefaultRegRegistrationValidator.java:87)
      at java.util.ArrayList.forEach(ArrayList.java:1257)
      at eu.bcvsolutions.idm.reg.service.impl.DefaultRegRegistrationValidator.validateBasicFields(DefaultRegRegistrationValidator.java:80)
      at eu.bcvsolutions.idm.reg.service.impl.DefaultRegRegistrationValidator.validate(DefaultRegRegistrationValidator.java:60)
      at eu.bcvsolutions.idm.reg.service.impl.DefaultRegRegistrationService.processRegistrationRequest(DefaultRegRegistrationService.java:46)
      at eu.bcvsolutions.idm.reg.service.impl.DefaultRegRegistrationService$$FastClassBySpringCGLIB$$6bd062d5.invoke(<generated>)
  

• EAV from doesn't have validation for email this is result, when I fill invalid email:
  
• registration with password policy doens't work. I set all these configuration properties: idm.pub.reg.attributes.password.attr, idm.pub.reg.passwordInputEnabled, idm.sec.reg.passwordPolicy and after I put password that pass with the password policy I still receive error: "Password does not match password policy". Problem is probably with the stream. After I enabled password policy for reg module registration will not work.
  

  final GuardedString passAttr = Optional.ofNullable(userRegistration.getRegistrationAtrributes()).orElse(emptyList()).stream()
              .filter(a -> a.getFormAttribute().equals(passwordFormAttribute.getId()))
              .map(a -> new GuardedString(a.getShortTextValue()))
              .findFirst().orElse(null);
  

• notification template need probably some reimplement because now some prerequisites missing eq:
  
• missing localization for message:
  
• activation mode use same notification as registration mode, is this OK?
• after successful activation, is user just redirect into login page without message (same as registration),
• after successful activation isn't possible to login into CzechIdM because filled password by registration doesn't work. Password is still the old, but user will not know it,
• notification with denied registration doesn't contain message:
  
• when I fill email address into registration (activation mode), the email address isn't used. Existing user in idm hasn't email address. Now I'm not able complete the activation mode, because email is not send. This is probably wanted behavior idk, try to consult with someone,
• configuration property for terms and note is not used now, but still they still exist in documentation,
• it was removed some methods from interfaces now isn't module fully compatible, version must be patched to new major version.

Documentation with diagram is awesome, now I fully understand hybrid and activation mode. Also refactored whole documentation is very good idea. Thank you for that.

I fixed above issues. Here is some additional info:

• project has some errors in IDE - forgot to commit, FIXED
• tests not pass - this was due to the different order of test execution. I tuned the tests against Jenkins so i believe it is OK now
• when I post registration (first init) is validate only on backend, you just return "Some mandatory fields were empty" which? Why is not validate form at frontend? You can use validate by EAV form - this validation is there only as a secondary backend validation, which tells user, that they forgot to fill out attributes, which are used for identity search. I fixed initialization, so that it now sets up registration form properly (identity search attribute is marked as required in form definition).
• first initialized reg form hasn't localization and password field (some customers are very sensitive for missing localization) - after consultation with Ondra, we decided that localization would not be supported, because there is currently not a way to define localization in a differrent module than core.
• show loading is still not implemented, and message "no attributes" is hardly written in code - I added localization for the mentioned message, but i could not fix the showLoading even with Ondra helping me. Problem is IMO in UiUtils method isShowLoading, which returns false if given entity in store is null.
• After successful registration is user redirect to login page without any message, try to consult with someone behavior after registration - added message
• attribute selfRegistered isn't checked after registration, this critical backward compatible issues, on some project is this used for some check - added selfRegistered boolean attribute to reg form definition. It is not shown in registration form for user to click on it.
• missing localization for terms field - localized
• bad EN localization for agree box: - fixed
• without own module I'm not possible add localization for password attribute renderer - i consulted this woth Ondra and we decided that the logic will stay the same, but i changed localization key not to look into core for localization keys
• some strange behavior I fill all fields in registration and I still receive "some mandatory fields were empty" - fixed, but it is still not working with CHAR eav type due to #1397
• registration with password policy doens't work. I set all these configuration properties: idm.pub.reg.attributes.password.attr, idm.pub.reg.passwordInputEnabled, idm.sec.reg.passwordPolicy and after I put password that pass with the password policy I still receive error: "Password does not match password policy". Problem is probably with the stream. After I enabled password policy for reg module registration will not work. - I cannot reproduce this, can you please check it again? See attached GIF
• notification template need probably some reimplement because now some prerequisites missing eq: - i just marked firstName and lastName as required attributes when default reg form is initialized
• missing localization for message: - fixed
• activation mode use same notification as registration mode, is this OK? - I believe it is. These modes are used exclusively, so there is no need for multiple notification templates IMO. We can always add another template later, but in my projects there is no need for that at this point.
• after successful activation, is user just redirect into login page without message (same as registration), - added message
• after successful activation isn't possible to login into CzechIdM because filled password by registration doesn't work. Password is still the old, but user will not know it, - After consultation we decided not to support password change with activation mode due to security risks
• notification with denied registration doesn't contain message: - added body text to deny notification
• when I fill email address into registration (activation mode), the email address isn't used. Existing user in idm hasn't email address. Now I'm not able complete the activation mode, because email is not send. This is probably wanted behavior idk, try to consult with someone, - I consulted it with Alena and we agreed that it is OK. Maybe we can talk about some feature which would handle this situation, but for now i think this is sufficient enough
• configuration property for terms and note is not used now, but still they still exist in documentation, - fixed
• it was removed some methods from interfaces now isn't module fully compatible, version must be patched to new major version - This is todo for new version release

Could you please look at the changes ive made and review it again? Thanks.

I made review:

• en localization for module contains more keys than cz localization (Petr Hanak's tests did not pass :(),
• Recaptcha missing, now isn't possible use this feature, without this feature isn't possible use it on some project that use public self registration,
• thanks for add missing attribute localization, but this error message is still in ugly state, without panel or something, I know is minor things but some customers are sensitive with items without padding or panels,
  
• feature generating login doesn't work, this is critically bug for projects that use this feature, because Server error was thrown (Generator and idm.pub.reg.loginGenerator was set). I simulate same behavior as in project:
  
  

Your attached GIF missing.

For now I'm done with review, these thinks are showstoper for some project. Tomorow we will consult about solution with Marcel, because some project wainting for the registration module.

Ive fixed all mentioned issues. I also added mussing gif. I sincerely hope that it is ok now.

OK if is all fixed, you can release it.