backdrop-contrib / simplesamlphp_auth

Support SAML for authentication of users. The module will auto-provision user accounts and dynamically assign roles in Backdrop CMS if you want it to.
https://backdropcms.org/project/simplesamlphp_auth
GNU General Public License v2.0
2 stars 3 forks source link

Extend configuration to allow editing text for login link #11

Closed irinaz closed 1 year ago

irinaz commented 1 year ago

Fixes #9 Extend configuration to allow editing text for login link, backport from Drupal 7

irinaz commented 1 year ago

I made changes, but on my test instance I am not getting correct text in login url :(

irinaz commented 1 year ago

@laryn , I am mark my PR ready for review, though I know that it has at least one problem. Fixes #9

irinaz commented 1 year ago

Ready for testing and review, fixes #9

laryn commented 1 year ago

@irinaz So I think you have it working? The config and update hooks remain in order to get it to a fully ready PR to submit to the maintainer for consideration. Still have some energy?

irinaz commented 1 year ago

Yes, totally! I very much appreciate you mentioning me on hooks implementation!

On 2023-01-17 14:15, Laryn wrote:

@irinaz [1] So I think you have it working? The config and update hooks remain in order to get it to a fully ready PR to submit to the maintainer for consideration. Still have some energy?

-- Reply to this email directly, view it on GitHub [2], or unsubscribe [3]. You are receiving this because you were mentioned.Message ID: @.***>

Links:

[1] https://github.com/irinaz [2] https://github.com/backdrop-contrib/simplesamlphp_auth/pull/11#issuecomment-1386154459 [3] https://github.com/notifications/unsubscribe-auth/AATBT3ITC625KXVENKIVUVLWS4KW3ANCNFSM6AAAAAATZZ3TQQ

laryn commented 1 year ago

So I would say adding a default value in the config file is a good next step, so new installations have a default value.

And then an update hook in the install file, which will set the default value for anybody who has already installed the module and doesn't have a value for this setting yet.

For the update hook, we'll use hook_update_N (docs). Since this module is currently in 2.x you could create hook_update_1200() for this update.

keiserjb commented 1 year ago

I agree with adding a default value.

irinaz commented 1 year ago

@joelsteidl @keiserjb @laryn , please let me know what needs to happen to move forward. I believe that default is added in s https://github.com/irinaz/simplesamlphp_auth/blob/ef7945485cd41c8c0add8374f404589efc4b8db9/simplesamlphp_auth.install#L73 - please correct me if I am wrong.
I am not sure if need update hook.

laryn commented 1 year ago

@irinaz I have done a bunch of merging today and merged a version of this PR. It would be great if you can test the latest (still unreleased) version of this module to make sure all is working well for you.