[Bug 37711] New: IdP auto-register should work on the staff interface
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Bug ID: 37711 Summary: IdP auto-register should work on the staff interface Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Authentication Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org CC: dpavlin@rot13.org Right now it only works if the login happens in the OPAC. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | CC| |nick@bywatersolutions.com, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 170643 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170643&action=edit Bug 37711: DB update -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 170644 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170644&action=edit Bug 37711: DBIC schema -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 170645 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170645&action=edit Bug 37711: API changes This patch makes the API representation of the identity providers domains match the new metadata. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 170646 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170646&action=edit Bug 37711: Domains CRUD update This patch adapts the `Identity provider domains` CRUD page to handle the new configuration options. To test: 1. Have an IdP with some domains set. Make sure there's at least one domain with auto-register enabled and one with it disabled. 2. Apply this patches 3. Run: $ ktd --shell k$ updatedatabase => SUCCESS: Updates successfully 4. Run: k$ yarn build 5. Refresh the IdP domains page => SUCCESS: The domains table shows both OPAC and staff auto-register options. => SUCCESS: The staff auto-register is disabled for both domains => SUCCESS: The OPAC auto-register matches what was set for each domain regarding auto-register (remember that before this patchset, auto-register only works for the OPAC). 6. Play with editing/adding/removing domain configurations => SUCCESS: Things work as expected, values are persisted on the DB correctly. 7. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 170647 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170647&action=edit Bug 37711: Add support for staff interface auto-register on IdP Before this patches, only OPAC auto-registration at login using OAuth2/OIDC IdPs was allowed. This patch makes the staff interface to be able to auto-register new users. On doing it, I had to track down the possible values for the `interface` parameter in the case of the staff interface. Koha::Template::Plugin::AuthClient uses 'staff' internally so I sticked with it. The whole area needs more param validation and exceptions to avoid mistakes for not knowing the valid values. I added some safeguards on the Koha::REST::Plugin::Auth::IdP helpers. They get tested on the added tests. To test: 1. Apply this patch 2. Run: $ prove t/db_dependent/Koha/REST/Plugin/Auth/IdP.t => SUCCESS: Tests pass! 3. Have a valid OAuth... 4. Enable Staff auto-register 5. Login with an external IdP on the staff interface => SUCCESS: It gets registered! Note: it will probably lack permissions to access the staff interface, but you can check on the DB it got registered. 6. Sign off :-D Sponsored-by: ByWater Solutions -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170643|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 174090 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174090&action=edit Bug 37711: DB update Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170644|0 |1 is obsolete| | --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 174091 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174091&action=edit Bug 37711: DBIC schema Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170645|0 |1 is obsolete| | --- Comment #8 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 174092 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174092&action=edit Bug 37711: API changes This patch makes the API representation of the identity providers domains match the new metadata. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170646|0 |1 is obsolete| | --- Comment #9 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 174093 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174093&action=edit Bug 37711: Domains CRUD update This patch adapts the `Identity provider domains` CRUD page to handle the new configuration options. To test: 1. Have an IdP with some domains set. Make sure there's at least one domain with auto-register enabled and one with it disabled. 2. Apply this patches 3. Run: $ ktd --shell k$ updatedatabase => SUCCESS: Updates successfully 4. Run: k$ yarn build 5. Refresh the IdP domains page => SUCCESS: The domains table shows both OPAC and staff auto-register options. => SUCCESS: The staff auto-register is disabled for both domains => SUCCESS: The OPAC auto-register matches what was set for each domain regarding auto-register (remember that before this patchset, auto-register only works for the OPAC). 6. Play with editing/adding/removing domain configurations => SUCCESS: Things work as expected, values are persisted on the DB correctly. 7. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170647|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 174094 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174094&action=edit Bug 37711: Add support for staff interface auto-register on IdP Before this patches, only OPAC auto-registration at login using OAuth2/OIDC IdPs was allowed. This patch makes the staff interface to be able to auto-register new users. On doing it, I had to track down the possible values for the `interface` parameter in the case of the staff interface. Koha::Template::Plugin::AuthClient uses 'staff' internally so I sticked with it. The whole area needs more param validation and exceptions to avoid mistakes for not knowing the valid values. I added some safeguards on the Koha::REST::Plugin::Auth::IdP helpers. They get tested on the added tests. To test: 1. Apply this patch 2. Run: $ prove t/db_dependent/Koha/REST/Plugin/Auth/IdP.t => SUCCESS: Tests pass! 3. Have a valid OAuth... 4. Enable Staff auto-register 5. Login with an external IdP on the staff interface => SUCCESS: It gets registered! Note: it will probably lack permissions to access the staff interface, but you can check on the DB it got registered. 6. Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Needs Signoff |Signed Off --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- All working well in testing.. permissions mapping could be fun but is for a followup bug I reckon. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #12 from David Cook <dcook@prosentient.com.au> --- (In reply to Martin Renvoize (ashimema) from comment #11)
All working well in testing.. permissions mapping could be fun but is for a followup bug I reckon.
Agreed. That one is scarier... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #174090|0 |1 is obsolete| | Attachment #174091|0 |1 is obsolete| | Attachment #174092|0 |1 is obsolete| | Attachment #174093|0 |1 is obsolete| | Attachment #174094|0 |1 is obsolete| | --- Comment #13 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 185244 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185244&action=edit Bug 37711: DB update Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #14 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 185245 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185245&action=edit Bug 37711: DBIC schema Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #15 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 185246 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185246&action=edit Bug 37711: API changes This patch makes the API representation of the identity providers domains match the new metadata. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #16 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 185247 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185247&action=edit Bug 37711: Domains CRUD update This patch adapts the `Identity provider domains` CRUD page to handle the new configuration options. To test: 1. Have an IdP with some domains set. Make sure there's at least one domain with auto-register enabled and one with it disabled. 2. Apply this patches 3. Run: $ ktd --shell k$ updatedatabase => SUCCESS: Updates successfully 4. Run: k$ yarn build 5. Refresh the IdP domains page => SUCCESS: The domains table shows both OPAC and staff auto-register options. => SUCCESS: The staff auto-register is disabled for both domains => SUCCESS: The OPAC auto-register matches what was set for each domain regarding auto-register (remember that before this patchset, auto-register only works for the OPAC). 6. Play with editing/adding/removing domain configurations => SUCCESS: Things work as expected, values are persisted on the DB correctly. 7. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #17 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 185248 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185248&action=edit Bug 37711: Add support for staff interface auto-register on IdP Before this patches, only OPAC auto-registration at login using OAuth2/OIDC IdPs was allowed. This patch makes the staff interface to be able to auto-register new users. On doing it, I had to track down the possible values for the `interface` parameter in the case of the staff interface. Koha::Template::Plugin::AuthClient uses 'staff' internally so I sticked with it. The whole area needs more param validation and exceptions to avoid mistakes for not knowing the valid values. I added some safeguards on the Koha::REST::Plugin::Auth::IdP helpers. They get tested on the added tests. To test: 1. Apply this patch 2. Run: $ prove t/db_dependent/Koha/REST/Plugin/Auth/IdP.t => SUCCESS: Tests pass! 3. Have a valid OAuth... 4. Enable Staff auto-register 5. Login with an external IdP on the staff interface => SUCCESS: It gets registered! Note: it will probably lack permissions to access the staff interface, but you can check on the DB it got registered. 6. Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #18 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Rebased. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=40596 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #19 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Hmm.. It's been a while.. I'm going to upgrade my SO to a straight QA as there's seems to be hardly anyone with the knowledge to dig in and QA here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #20 from David Cook <dcook@prosentient.com.au> --- (In reply to Martin Renvoize (ashimema) from comment #19)
Hmm.. It's been a while.. I'm going to upgrade my SO to a straight QA as there's seems to be hardly anyone with the knowledge to dig in and QA here.
It's been tough to make this one a priority since my libraries just need self-registration via the OPAC, but I do have the knowledge... I'll take a little look too just to double-check... My only real concern was the permissions thing, but I think that's left out here, fortunately. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #21 from David Cook <dcook@prosentient.com.au> --- Failing QA, as there's an inconsistency between the atomic update and kohastructure.sql -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #22 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 185244 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185244 Bug 37711: DB update Review of attachment 185244: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=37711&attachment=185244) ----------------------------------------------------------------- ::: installer/data/mysql/atomicupdate/bug_37711.pl @@ +22,5 @@
+ if ( !column_exists( 'identity_provider_domains', 'auto_register_staff' ) ) { + $dbh->do( + q{ + ALTER TABLE identity_provider_domains + ADD COLUMN `auto_register_staff` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'Allow user auto register (OPAC)'
The comment here should be: 'Allow user auto register (Staff interface)' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #23 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 185247 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185247 Bug 37711: Domains CRUD update Review of attachment 185247: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=37711&attachment=185247) ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_provider_domains.tt @@ +322,5 @@
</div> [% END %]
+ <div id="toolbar" class="btn-toolbar"> + <a class="btn btn-default" id="new_identity_provider_domain" href="/cgi-bin/koha/admin/identity_providers.pl?domain_ops=1&identity_provider_id=[%- identity_provider_id | html -%]&op=add_form"
I thought that we were using & instead of & these days, but not really an issue... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #24 from David Cook <dcook@prosentient.com.au> --- Sorry I was going to keep testing but I've got caught up in other things... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #25 from David Cook <dcook@prosentient.com.au> --- Failed QA for a few more things: - If you select "Auto register" "Yes" on http://localhost:8081/cgi-bin/koha/admin/identity_providers.pl?op=add_form, and then you go to review the domain for that IdP, it'll say "Auto register (OPAC):" "Don't allow" and "Auto register (Staff)" "Don't allow". - When viewing the "Identity provider domains for '<idp>'", there are 2 tables. One appears to be the old table format which has only 1 column "Auto register". This is the table that is populated. Beneath that table there is another one with the correct 2 "Auto register" columns, but no data is provided. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #26 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #25)
- When viewing the "Identity provider domains for '<idp>'", there are 2 tables. One appears to be the old table format which has only 1 column "Auto register". This is the table that is populated. Beneath that table there is another one with the correct 2 "Auto register" columns, but no data is provided.
Note that this 2nd table appears correctly when you go into edit a particular domain for an IdP, so this is probably an easy fix on the listing view page -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #27 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Thanks for the valuable feedback, dcook! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #28 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi (tcohen) from comment #27)
Thanks for the valuable feedback, dcook!
No worries. I tried 😅 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |ASSIGNED --- Comment #29 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to David Cook from comment #25)
Failed QA for a few more things:
- If you select "Auto register" "Yes" on http://localhost:8081/cgi-bin/koha/admin/identity_providers.pl?op=add_form, and then you go to review the domain for that IdP, it'll say "Auto register (OPAC):" "Don't allow" and "Auto register (Staff)" "Don't allow".
- When viewing the "Identity provider domains for '<idp>'", there are 2 tables. One appears to be the old table format which has only 1 column "Auto register". This is the table that is populated. Beneath that table there is another one with the correct 2 "Auto register" columns, but no data is provided.
Ooof. That's a terrible rebase :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #185244|0 |1 is obsolete| | Attachment #185245|0 |1 is obsolete| | Attachment #185246|0 |1 is obsolete| | Attachment #185247|0 |1 is obsolete| | Attachment #185248|0 |1 is obsolete| | --- Comment #30 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 186978 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186978&action=edit Bug 37711: DB update Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #31 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 186979 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186979&action=edit Bug 37711: DBIC schema Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #32 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 186980 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186980&action=edit Bug 37711: API changes This patch makes the API representation of the identity providers domains match the new metadata. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #33 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 186981 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186981&action=edit Bug 37711: Domains CRUD update This patch adapts the `Identity provider domains` CRUD page to handle the new configuration options. To test: 1. Have an IdP with some domains set. Make sure there's at least one domain with auto-register enabled and one with it disabled. 2. Apply this patches 3. Run: $ ktd --shell k$ updatedatabase => SUCCESS: Updates successfully 4. Run: k$ yarn build 5. Refresh the IdP domains page => SUCCESS: The domains table shows both OPAC and staff auto-register options. => SUCCESS: The staff auto-register is disabled for both domains => SUCCESS: The OPAC auto-register matches what was set for each domain regarding auto-register (remember that before this patchset, auto-register only works for the OPAC). 6. Play with editing/adding/removing domain configurations => SUCCESS: Things work as expected, values are persisted on the DB correctly. 7. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #34 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 186982 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186982&action=edit Bug 37711: Add support for staff interface auto-register on IdP Before this patches, only OPAC auto-registration at login using OAuth2/OIDC IdPs was allowed. This patch makes the staff interface to be able to auto-register new users. On doing it, I had to track down the possible values for the `interface` parameter in the case of the staff interface. Koha::Template::Plugin::AuthClient uses 'staff' internally so I sticked with it. The whole area needs more param validation and exceptions to avoid mistakes for not knowing the valid values. I added some safeguards on the Koha::REST::Plugin::Auth::IdP helpers. They get tested on the added tests. To test: 1. Apply this patch 2. Run: $ prove t/db_dependent/Koha/REST/Plugin/Auth/IdP.t => SUCCESS: Tests pass! 3. Have a valid OAuth... 4. Enable Staff auto-register 5. Login with an external IdP on the staff interface => SUCCESS: It gets registered! Note: it will probably lack permissions to access the staff interface, but you can check on the DB it got registered. 6. Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #35 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to David Cook from comment #22)
+ ALTER TABLE identity_provider_domains + ADD COLUMN `auto_register_staff` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'Allow user auto register (OPAC)'
The comment here should be: 'Allow user auto register (Staff interface)'
Fixed inline. Good catch!
+ <a class="btn btn-default" id="new_identity_provider_domain" href="/cgi-bin/koha/admin/identity_providers.pl?domain_ops=1&identity_provider_id=[%- identity_provider_id | html -%]&op=add_form"
I thought that we were using & instead of & these days, but not really an issue...
The file is full of those. Maybe a separate bug to review how we build URLs globally and add a check somewhere?
- If you select "Auto register" "Yes" on http://localhost:8081/cgi-bin/koha/admin/identity_providers.pl?op=add_form, and then you go to review the domain for that IdP, it'll say "Auto register (OPAC):" "Don't allow" and "Auto register (Staff)" "Don't allow".
- When viewing the "Identity provider domains for '<idp>'", there are 2 tables. One appears to be the old table format which has only 1 column "Auto register". This is the table that is populated. Beneath that table there is another one with the correct 2 "Auto register" columns, but no data is provided.
Bad rebase. Solved. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #186981|0 |1 is obsolete| | Attachment #186982|0 |1 is obsolete| | --- Comment #36 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187000 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187000&action=edit Bug 37711: Domains CRUD update This patch adapts the `Identity provider domains` CRUD page to handle the new configuration options. To test: 1. Have an IdP with some domains set. Make sure there's at least one domain with auto-register enabled and one with it disabled. 2. Apply this patches 3. Run: $ ktd --shell k$ updatedatabase => SUCCESS: Updates successfully 4. Run: k$ yarn build 5. Refresh the IdP domains page => SUCCESS: The domains table shows both OPAC and staff auto-register options. => SUCCESS: The staff auto-register is disabled for both domains => SUCCESS: The OPAC auto-register matches what was set for each domain regarding auto-register (remember that before this patchset, auto-register only works for the OPAC). 6. Play with editing/adding/removing domain configurations => SUCCESS: Things work as expected, values are persisted on the DB correctly. 7. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #37 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187001 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187001&action=edit Bug 37711: Add support for staff interface auto-register on IdP Before this patches, only OPAC auto-registration at login using OAuth2/OIDC IdPs was allowed. This patch makes the staff interface to be able to auto-register new users. On doing it, I had to track down the possible values for the `interface` parameter in the case of the staff interface. Koha::Template::Plugin::AuthClient uses 'staff' internally so I sticked with it. The whole area needs more param validation and exceptions to avoid mistakes for not knowing the valid values. I added some safeguards on the Koha::REST::Plugin::Auth::IdP helpers. They get tested on the added tests. To test: 1. Apply this patch 2. Run: $ prove t/db_dependent/Koha/REST/Plugin/Auth/IdP.t => SUCCESS: Tests pass! 3. Have a valid OAuth... 4. Enable Staff auto-register 5. Login with an external IdP on the staff interface => SUCCESS: It gets registered! Note: it will probably lack permissions to access the staff interface, but you can check on the DB it got registered. 6. Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #38 from David Cook <dcook@prosentient.com.au> --- Marking "Failed QA". When adding an identity provider for the first time via http://localhost:8081/cgi-bin/koha/admin/identity_providers.pl?op=add_form there is only one "Auto register" option. If you select "Yes", when you go to review the domains for that IdP, you'll see that it says "No" for both "Auto register (OPAC)" and "Auto register (Staff)". -- Other than that... I think it's all good. Once that last bit is fixed, I think it'll be good to go. The functionality all works as expected otherwise. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187000|0 |1 is obsolete| | --- Comment #39 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187034 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187034&action=edit Bug 37711: Domains CRUD update This patch adapts the `Identity provider domains` CRUD page to handle the new configuration options. To test: 1. Have an IdP with some domains set. Make sure there's at least one domain with auto-register enabled and one with it disabled. 2. Apply this patches 3. Run: $ ktd --shell k$ updatedatabase => SUCCESS: Updates successfully 4. Run: k$ yarn build 5. Refresh the IdP domains page => SUCCESS: The domains table shows both OPAC and staff auto-register options. => SUCCESS: The staff auto-register is disabled for both domains => SUCCESS: The OPAC auto-register matches what was set for each domain regarding auto-register (remember that before this patchset, auto-register only works for the OPAC). 6. Play with editing/adding/removing domain configurations => SUCCESS: Things work as expected, values are persisted on the DB correctly. 7. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187034|0 |1 is obsolete| | --- Comment #40 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187037 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187037&action=edit Bug 37711: Domains CRUD update This patch adapts the `Identity provider domains` CRUD page to handle the new configuration options. To test: 1. Have an IdP with some domains set. Make sure there's at least one domain with auto-register enabled and one with it disabled. 2. Apply this patches 3. Run: $ ktd --shell k$ updatedatabase => SUCCESS: Updates successfully 4. Run: k$ yarn build 5. Refresh the IdP domains page => SUCCESS: The domains table shows both OPAC and staff auto-register options. => SUCCESS: The staff auto-register is disabled for both domains => SUCCESS: The OPAC auto-register matches what was set for each domain regarding auto-register (remember that before this patchset, auto-register only works for the OPAC). 6. Play with editing/adding/removing domain configurations => SUCCESS: Things work as expected, values are persisted on the DB correctly. 7. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #41 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187038 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187038&action=edit Test patch description -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #42 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187039 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187039&action=edit Test patch description 2 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #43 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187040 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187040&action=edit Test patch description 1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #44 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187041 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187041&action=edit Test patch description 2 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #45 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187042 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187042&action=edit Test patch description 3 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #46 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187043 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187043&action=edit Debug test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187038|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187039|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187040|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187041|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187042|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187043|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #47 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- I fixed the issue, @dcook. Sorry, for the noise, I decided to fix the 'Network error' git-bz was returning and used this bug to debug it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |The 'auto-register' feature release notes| |can now be enabled in the | |staff interface. | | | |Previously, this | |functionality was only | |available in the OPAC and | |could not be used from the | |staff side. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |dcook@prosentient.com.au |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #186978|0 |1 is obsolete| | --- Comment #48 from David Cook <dcook@prosentient.com.au> --- Created attachment 187068 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187068&action=edit Bug 37711: DB update Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #186979|0 |1 is obsolete| | --- Comment #49 from David Cook <dcook@prosentient.com.au> --- Created attachment 187069 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187069&action=edit Bug 37711: DBIC schema Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #186980|0 |1 is obsolete| | --- Comment #50 from David Cook <dcook@prosentient.com.au> --- Created attachment 187070 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187070&action=edit Bug 37711: API changes This patch makes the API representation of the identity providers domains match the new metadata. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187001|0 |1 is obsolete| | --- Comment #51 from David Cook <dcook@prosentient.com.au> --- Created attachment 187071 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187071&action=edit Bug 37711: Add support for staff interface auto-register on IdP Before this patches, only OPAC auto-registration at login using OAuth2/OIDC IdPs was allowed. This patch makes the staff interface to be able to auto-register new users. On doing it, I had to track down the possible values for the `interface` parameter in the case of the staff interface. Koha::Template::Plugin::AuthClient uses 'staff' internally so I sticked with it. The whole area needs more param validation and exceptions to avoid mistakes for not knowing the valid values. I added some safeguards on the Koha::REST::Plugin::Auth::IdP helpers. They get tested on the added tests. To test: 1. Apply this patch 2. Run: $ prove t/db_dependent/Koha/REST/Plugin/Auth/IdP.t => SUCCESS: Tests pass! 3. Have a valid OAuth... 4. Enable Staff auto-register 5. Login with an external IdP on the staff interface => SUCCESS: It gets registered! Note: it will probably lack permissions to access the staff interface, but you can check on the DB it got registered. 6. Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187037|0 |1 is obsolete| | --- Comment #52 from David Cook <dcook@prosentient.com.au> --- Created attachment 187072 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187072&action=edit Bug 37711: Domains CRUD update This patch adapts the `Identity provider domains` CRUD page to handle the new configuration options. To test: 1. Have an IdP with some domains set. Make sure there's at least one domain with auto-register enabled and one with it disabled. 2. Apply this patches 3. Run: $ ktd --shell k$ updatedatabase => SUCCESS: Updates successfully 4. Run: k$ yarn build 5. Refresh the IdP domains page => SUCCESS: The domains table shows both OPAC and staff auto-register options. => SUCCESS: The staff auto-register is disabled for both domains => SUCCESS: The OPAC auto-register matches what was set for each domain regarding auto-register (remember that before this patchset, auto-register only works for the OPAC). 6. Play with editing/adding/removing domain configurations => SUCCESS: Things work as expected, values are persisted on the DB correctly. 7. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #53 from David Cook <dcook@prosentient.com.au> --- Looking good. Thanks, Tomas! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Passed QA |Failed QA --- Comment #54 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- There is a problem here when I try to updatedatabase: {UNKNOWN}: DBIx::Class::ResultSource::column_info(): No such column auto_register at /kohadevbox/koha/Koha/Schema/Result/IdentityProviderDomain.pm line 222 Compilation failed in require at /usr/share/perl5/Class/C3/Componentised.pm line 146. at /usr/share/perl5/Class/C3/Componentised.pm line 151 Compilation failed in require at /kohadevbox/koha/C4/Installer.pm line 30. BEGIN failed--compilation aborted at /kohadevbox/koha/C4/Installer.pm line 30. Compilation failed in require at /kohadevbox/koha/installer/data/mysql/updatedatabase.pl line 41. BEGIN failed--compilation aborted at /kohadevbox/koha/installer/data/mysql/updatedatabase.pl line 41. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #55 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Lucas Gass (lukeg) from comment #54)
There is a problem here when I try to updatedatabase:
{UNKNOWN}: DBIx::Class::ResultSource::column_info(): No such column auto_register at /kohadevbox/koha/Koha/Schema/Result/IdentityProviderDomain.pm line 222 Compilation failed in require at /usr/share/perl5/Class/C3/Componentised.pm line 146. at /usr/share/perl5/Class/C3/Componentised.pm line 151 Compilation failed in require at /kohadevbox/koha/C4/Installer.pm line 30. BEGIN failed--compilation aborted at /kohadevbox/koha/C4/Installer.pm line 30. Compilation failed in require at /kohadevbox/koha/installer/data/mysql/updatedatabase.pl line 41. BEGIN failed--compilation aborted at /kohadevbox/koha/installer/data/mysql/updatedatabase.pl line 41.
That's because you skipped the DBIx::Class schema update from the second patch. I know it is a chicken-egg problem because of the boolean attributes... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #56 from David Cook <dcook@prosentient.com.au> --- Now I'm trying to remember how I updated the database... I might've done a reset_all which would use the kohastructure.sql I think, although I thought that I tried an upgradedatabase.pl as well... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #57 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #56)
Now I'm trying to remember how I updated the database... I might've done a reset_all which would use the kohastructure.sql I think, although I thought that I tried an upgradedatabase.pl as well...
I just re-tested with a fresh main and the following works: 1. git bz apply 37711 2. sudo koha-upgrade-schema kohadev Upgrading database schema for kohadev DEV atomic update /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_37711.pl [22:20:32]: Bug 37711 - Allow setting auto-register per interface (identity providers) Added column 'identity_provider_domains.auto_register_opac' Added column 'identity_provider_domains.auto_register_staff' Removed column 'identity_provider_domains.auto_register' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #58 from David Cook <dcook@prosentient.com.au> --- Oh... but then that got me thinking... If you apply the patches, restart Koha, but then rely on the web installer... this won't work. Going to http://localhost:8081/ will yield a 500 error with the following in the plack-error.log: Template process failed: undef error - DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'domains.auto_register_opac' in 'field list' at /kohadevbox/koha/Koha/Objects.pm line 319 Template process failed: undef error - DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'domains.auto_register_opac' in 'field list' at /kohadevbox/koha/Koha/Objects.pm line 319 -- While the Debian packages are the preferred way to go, I think that we are still supporting the web installer, and these patches would break the web installer it seems. The only solution I can think of would be to change "use Koha::Auth::Identity::Providers;" to "require Koha::Auth::Identity::Providers;" and move it into the try/catch block with "Koha::Auth::Identity::Providers->search". When using the web installer, it would fail, but it should fail gracefully. Gracefully in the sense that they're able to complete the web installer and maybe have a message saying they need to restart for the upgraded libraries to come into effect. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #59 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #58)
The only solution I can think of would be...
Hmm maybe overly optimistic because I change that and it still fails with a 500 error. There's no stack trace so it's not clear where this code is being called. Doing a bit of grepping, and it looks like probably a few other places, since it looks like it's the "->next" call which is triggering the error. Maybe Koha/Template/Plugin/AuthClient.pm, Koha/Auth/Client.pm, and/or somewhere else. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #60 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187848 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187848&action=edit Bug 37711: (follow-up) Fix AuthClient plugin database upgrade handling The AuthClient template plugin was causing 500 errors during database upgrades when the identity_providers schema was out of sync. The plugin tried to query the new 'auto_register_opac' column before the database schema was updated, creating a chicken-egg problem. This patch wraps the database query in Try::Tiny to handle schema mismatches gracefully during upgrades, allowing the upgrade process to continue without crashing the authentication interface. Test plan: 1. On main, apply all bug 37711 patches BUT THIS ONE 2. Simulate an upgrade by: * Edit Koha.pm so it points to the next version * Move the atomicupdate file to the right number in db_revs 3. Restart everything: $ ktd --shell k$ restart_all 4. Access http://kohadev-intra.localhost => FAIL: You get a 500! With a stack trace! 5. Apply this patch 6. Repeat 3 and 4 => SUCCESS: No 500 error, redirects to installer 7. Proceed with the upgrade: => SUCCESS: It works as it should 8. Run tests: k$ prove -v t/db_dependent/Template/Plugin/AuthClient.t => SUCCESS: All tests pass 9. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #61 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to David Cook from comment #59)
(In reply to David Cook from comment #58)
The only solution I can think of would be...
Hmm maybe overly optimistic because I change that and it still fails with a 500 error. There's no stack trace so it's not clear where this code is being called.
I submitted a patch that addresses the chicken-egg problem. Which you identified correctly to be related to the Koha::Template::Plugin::AuthClient class (instantiating the Koha::Identity::Providers). This highlighted Koha was breaking badly if there was an error fetching the IdPs. I decided it should just warn about the situation, and move forward so wrapped it in a try/catch block. And added missing tests for the template plugin while in the area. I set it back to SO so you can give it a last try for a final QA. Really glad you caught this. Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #62 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi (tcohen) from comment #61)
(In reply to David Cook from comment #59)
(In reply to David Cook from comment #58)
The only solution I can think of would be...
Hmm maybe overly optimistic because I change that and it still fails with a 500 error. There's no stack trace so it's not clear where this code is being called.
I submitted a patch that addresses the chicken-egg problem. Which you identified correctly to be related to the Koha::Template::Plugin::AuthClient class (instantiating the Koha::Identity::Providers).
This highlighted Koha was breaking badly if there was an error fetching the IdPs. I decided it should just warn about the situation, and move forward so wrapped it in a try/catch block. And added missing tests for the template plugin while in the area.
I set it back to SO so you can give it a last try for a final QA.
Really glad you caught this. Thanks!
Apologies for the delay in responding. Life's been a bit nuts. The additional patch looks good at a glance. I applied the patch and manually triggered the web installer and that worked well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187848|0 |1 is obsolete| | --- Comment #63 from David Cook <dcook@prosentient.com.au> --- Created attachment 188619 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188619&action=edit Bug 37711: (follow-up) Fix AuthClient plugin database upgrade handling The AuthClient template plugin was causing 500 errors during database upgrades when the identity_providers schema was out of sync. The plugin tried to query the new 'auto_register_opac' column before the database schema was updated, creating a chicken-egg problem. This patch wraps the database query in Try::Tiny to handle schema mismatches gracefully during upgrades, allowing the upgrade process to continue without crashing the authentication interface. Test plan: 1. On main, apply all bug 37711 patches BUT THIS ONE 2. Simulate an upgrade by: * Edit Koha.pm so it points to the next version * Move the atomicupdate file to the right number in db_revs 3. Restart everything: $ ktd --shell k$ restart_all 4. Access http://kohadev-intra.localhost => FAIL: You get a 500! With a stack trace! 5. Apply this patch 6. Repeat 3 and 4 => SUCCESS: No 500 error, redirects to installer 7. Proceed with the upgrade: => SUCCESS: It works as it should 8. Run tests: k$ prove -v t/db_dependent/Template/Plugin/AuthClient.t => SUCCESS: All tests pass 9. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #64 from David Cook <dcook@prosentient.com.au> --- Thanks for your patience, Tomas, and thanks for the patches! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 --- Comment #65 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 25.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Laura.escamilla@bywatersolu | |tions.com Status|Pushed to main |Needs documenting Version(s)|25.11.00 |25.11.00, 25.05.08 released in| | --- Comment #66 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Backported to 25.05 thanks all! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37711 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |baptiste.wojtkowski@biblibr | |e.com --- Comment #67 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Enhancement, not backported -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org