Uploaded image for project: 'camunda BPM'
  1. camunda BPM
  2. CAM-12804

Make the default IdentityService more reusable for custom implementations

    • Icon: Task Task
    • Resolution: Fixed
    • Icon: L3 - Default L3 - Default
    • 7.15.0-alpha3, 7.15.0
    • 7.13.0
    • spring-boot
    • None

      Multiple methods of the IdentityServiceImpl call specific commands that make use of features of the UserEntity rather than just the User (e.g. SaveUserCmd that checks the password against the policy). Those usages of the UserEntity could also be done by the specific Identity Provider, like the DbIdentityServiceProvider.

      When customers write their own identity providers, they cannot easily reuse the IdentityServiceImpl as this now has a tight coupling to the UserEntity. By breaking this coupling and letting the provider take care of the type-specifc User class actions, we would allow to reuse the IdentityService in custom implementations.

      --------------------
      original description:
      Right now, when a user is saved the saveUser method in the IdentityServiceImpl class is called, which uses the SaveUserCmd class implementing the Command interface. This class casts the User object into UserEntity. Now, when implementing a custom identity provider, we would generally want to use our own user store and structure which would almost certainly be incompatible with UserEntity. This casting in SaveUserCmd is preventing the use of the default IdentityServiceImpl and as a workaround, I have to rewrite the save methods in a custom IdentityService.

      If the SaveUserCmd doesn't cast to UserEntity and all actions that needs to be performed on the UserEntity like calling the checkPasswordAgainstPolicy is delegated to the default identity provider instead, then there is no real need to override the default identity service itself which seems like a better design to me.

      The above issue is also present while saving groups and tenants.

        This is the controller panel for Smart Panels app

            [CAM-12804] Make the default IdentityService more reusable for custom implementations

            Hi sayakmukhopadhyay,

            thank you very much for reaching out to us with this request.

            I adjusted the type to Feature Request as I think this is rather about improving the current implementation to allow for a better reusability.
            A Bug Report would imply that the current implementation is not working as expected, which I think is not the case here.

            As for your request, I think this makes total sense in general and I like the idea.
            I will adjust the description a bit here and there to make this a bit clearer for a request. if you don't mind.

            I will also forward this to decision-making afterwards.
            In the meantime, it would be very helpful if you could provide a PR to our GitHub repository outlining the changes you have in mind in more detail.
            This would speed up the process of including this idea into the code tremendously.

            Thanks and best,
            Tobias

            Tobias Metzke-Bernstein added a comment - Hi sayakmukhopadhyay , thank you very much for reaching out to us with this request. I adjusted the type to Feature Request as I think this is rather about improving the current implementation to allow for a better reusability. A Bug Report would imply that the current implementation is not working as expected, which I think is not the case here. As for your request, I think this makes total sense in general and I like the idea. I will adjust the description a bit here and there to make this a bit clearer for a request. if you don't mind. I will also forward this to decision-making afterwards. In the meantime, it would be very helpful if you could provide a PR to our GitHub repository outlining the changes you have in mind in more detail. This would speed up the process of including this idea into the code tremendously. Thanks and best, Tobias

            Hi, thanks for clearing the feature request. Your description makes more sense and has got the idea behind this issue. I will try and get a PR done with the changes I had in mind. Thanks for looking into this.

            Sayak Mukhopadhyay added a comment - Hi, thanks for clearing the feature request. Your description makes more sense and has got the idea behind this issue. I will try and get a PR done with the changes I had in mind. Thanks for looking into this.

            Hi sayakmukhopadhyay,

            Your proposal makes a lot of sense and a pull request would be greatly appreciated. Please let us know if you have any questions, and feel free to raise an incomplete PR if you want to validate your approach first.

            Cheers,
            Thorben

            Thorben Lindhauer added a comment - Hi sayakmukhopadhyay , Your proposal makes a lot of sense and a pull request would be greatly appreciated. Please let us know if you have any questions, and feel free to raise an incomplete PR if you want to validate your approach first. Cheers, Thorben

            Hi
            I have created PR https://github.com/camunda/camunda-bpm-platform/pull/1157 but the PR is not ready. I am unable to figure out how to transmit the `skipPasswordPolicy` value from the default service to the default provider via a generic command class.

            Sayak Mukhopadhyay added a comment - Hi I have created PR https://github.com/camunda/camunda-bpm-platform/pull/1157 but the PR is not ready. I am unable to figure out how to transmit the `skipPasswordPolicy` value from the default service to the default provider via a generic command class.

            Hi sayakmukhopadhyay,

            Sorry for the late response on your PR. I'll answer your question directly on github. I'm afraid that I'm leaving for Christmas holidays next week, but I'd like to continue giving feedback once I'm back in January and help you make your PR a success.

            Cheers,
            Thorben

            Thorben Lindhauer added a comment - Hi sayakmukhopadhyay , Sorry for the late response on your PR. I'll answer your question directly on github. I'm afraid that I'm leaving for Christmas holidays next week, but I'd like to continue giving feedback once I'm back in January and help you make your PR a success. Cheers, Thorben

            Thorben Lindhauer added a comment - Merged via https://github.com/camunda/camunda-bpm-platform/commit/c303a1cca516292f95cf31243a77be91d4c4746b . Thank you raising and implementing this.

              thorben.lindhauer Thorben Lindhauer
              sayakmukhopadhyay Sayak Mukhopadhyay
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Created:
                Updated:
                Resolved: