Workaround for user_saml-application to return lower-case realm when configured through environment-variable. #273
Conversation
…onfigured through environment-variable. Signed-off-by: marioqxx <marioqxx@users.noreply.github.com>
…onfigured through environment-variable. Signed-off-by: marioqxx <marioqxx@users.noreply.github.com>
…onfigured through environment-variable. Signed-off-by: marioqxx <marioqxx@users.noreply.github.com>
|
Not sure what happened, but almost all of the errors from "ansible-lint" refer to naming-convention issues, which seem to have its origin in a change of the ansible-lint checks. Almost all errors come from something other than the patch. |
|
Hello there @marioqxx However, 🤔 I feels that this part of code is more specific to your deployment than a generic installation process. regards. |
|
@marioqxx you can now rebase to get rid of annoying errors on ansible-lint. |
|
@staticdev: Done. Thank you. @aalaesar: You raise a valid question, I expected this question and appreciate you taking your time on this topic. Moreover, I questioned myself whether or not to add this here. I did conclude that it might be helpful and thus went forward. Certainly, you may come to a different conclusion and I'm OK with that too. So what is the use-scenario: You want single-sign-on and administer your users centrally via LDAP, such as through OpenLDAP or Active directory. In my use-case I run a samba domain controller and provide single-sign-on for nextcloud as well as central user-administration by the samba domain-controller. Single-sign-on is then accomplished through user_saml-module. I cannot judge how often such a setup exists, because NC has put this behind a paywall. I do respect NC's decision and I do have an interest that NC is a prosperous company as well! The issue that this patch works around is there since at least 2017 (see the date of the case I refer in the README) and it pops up on several further places in the github.com issue-tracker for the user_saml project. The most recent entry I think is from 2022/2021 and basically it concludes that this won't be changed. Therefore, the workaround more or less becomes a permanent solution and it cannot be expected that it is not needed anymore in the near future. In order to make this work-around work properly, I have decided not to use ansible-patch module, but instead have chosen to use blockinfile. The way it is implemented makes it resilient to changes and the script would work for any user_saml-Version back till 2017 (and maybe earlier). I think it is safe to assume that the same holds true for future versions, including changes in user_saml-app that also change SAMLController.php. The idea then to include it in this script, but not outside in another playbook was also driven by the thought, that it might be helpful for others and furthermore, the patch requires some pre-requisites, which can be easily checked in this ansible-script but would require duplicate but consistent configuration settings when placed outside. E.g. "nextcloud-root-directory", check presence of user_saml application and the like. In summary, from implementation point of view it nicely fits in here. I also thought, that it is an quite compact and local change to this ansible script and which does not need extra servicing. It then also adds only a single configuration parameter and furthermore, the README adds value in the way that it explains an issue and helps others, avoiding them lengthy trouble-shooting and searching or even giving up at all. |
Signed-off-by: marioqxx <marioqxx@users.noreply.github.com>
|
@staticdev: I've updated the README according your suggestion. I have added you to my private repository, which is a ansible-role for installing and setting up a samba domain-controller. This isn't yet at a state to become public and I'll do this after the new Debian release is out and tested against it. |
|
Hello there ! Thank you for your effort to explain it well in the readme 👍 |
No description provided.