[Bug 36369] New: Make APIClient be more useful
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 Bug ID: 36369 Summary: Make APIClient be more useful Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org I file this report for discussion. In my opinion, there's too much duplicated code on the API client libraries. And the APIClient "class" is just a shortcut to avoid instantiating the module-specific API clients: ```javascript import ERMAPIClient from "./erm-api-client"; ... export const APIClient = { erm: new ERMAPIClient(), ``` Then used everywhere like this: ```javascript import { APIClient } from "../../fetch/api-client.js" ... const client = APIClient.erm await client.agreements.count().then( ``` IMHO, we need a real APIClient class that would implement generic methods like: ```javascript create: params => this.post({ endpoint: params.endpoint ? params.endpoint : "/", body: params.resource, headers: params.headers }), ... getAll: params => this.get({ endpoint: params.endpoint ? params.endpoint : "/", params.query, headers: params.headers, }), ... count: params => this.count({ endpoint: params.endpoint ? params.endpoint : "/", "?" + new URLSearchParams({ _page: 1, _per_page: 1, ...(params.query && { q: JSON.stringify(params.query) }), }), }), ``` and the API Client classes (like ERMAPIClient) should inherit from it, somehow. For doing so, we will face another design roadblock: While I uderstand that ERMAPIClient is a single place to have all the ERM module-needed API actions, it still feels weird from an OOP perspective how things are organized. I feel like we should have individual classes, like: ```javascript export RecordSourcesAPIClient extends APIClient ``` and then have a new type of class along this lines: ```javascript import CitiesAPIClient from "./cities-api-client"; import RecordSourcesAPIClient from "./record-sources-api-client"; ... export const AdminAPIClient = { cities: new CitiesAPIClient(), record_sources: new RecordSourcesAPIClient(), ... }; ``` this way, we will avoid repeating ourselves (less bugs!) and the code is (I expect) more reusable. I also think the `embed` handling should just be a list for the different methods, like: embed: [ 'patron', 'patron.category' ] and generically handled by APIClient. No need to build the headers manually everywhere. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com, | |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com, | |matt.blenkinsop@ptfs-europe | |.com, | |paul.derscheid@lmscloud.de, | |pedro.amorim@ptfs-europe.co | |m, tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 --- Comment #1 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- I think this makes sense. Tomas are you able to provide a PoC we can use as a starting point to look at / discuss? -- 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=36369 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Is there any reason not to build the javascript version of our OpenAPI spec and use that internally? -- 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=36369 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Pedro Amorim from comment #1)
I think this makes sense. Tomas are you able to provide a PoC we can use as a starting point to look at / discuss?
Yes, it is my plan for this morning. -- 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=36369 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Kyle M Hall from comment #2)
Is there any reason not to build the javascript version of our OpenAPI spec and use that internally?
You mean for validating outgoing requests? -- 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=36369 --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Tomás Cohen Arazi from comment #4)
(In reply to Kyle M Hall from comment #2)
Is there any reason not to build the javascript version of our OpenAPI spec and use that internally?
You mean for validating outgoing requests?
I just did some tests and at least the clients generated by https://editor.swagger.io/ do not validate data. Their are other SDK generators out there but this one at least doesn't do it. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 --- Comment #6 from Paul Derscheid <paul.derscheid@lmscloud.de> --- If you want to validate input before passing it to api clients, please use zod or something similar. That makes it very declarative and easy to maintain. Here's a list of options compared by performance: https://moltar.github.io/typescript-runtime-type-benchmarks/ -- 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=36369 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=36369 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 164201 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164201&action=edit Bug 36369: Introduce an APIClientBase class -- 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=36369 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Sorry for the delay y'all. I didn't manage to make this work. But I think it highlights the idea of what I tried to propose. api-client-base.js could be implemented as an http-client.js tweak, but I wanted to make it self-contained to ease explaining it. And also, I prefer we use the already implemented method names instead of plain HTTP verbs. Happy to Meet/Jitsi/Zoom to talk about this if needed. Please take this as a constructive review of the already great toolset y've all built. -- 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=36369 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164201|0 |1 is obsolete| | --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 164205 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164205&action=edit Bug 36369: Introduce an APIClientBase class -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 0xfun <me@paulderscheid.xyz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |me@paulderscheid.xyz --- Comment #10 from 0xfun <me@paulderscheid.xyz> --- I like it. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 --- Comment #11 from Jonathan Druart <jonathan.druart@gmail.com> --- It could work to reduce the number of lines in the -api-client.js. Two points however: 1. The endpoint need to be configurable. 'this._baseURL + "/" + id' is too simple and does not work for everything (eg. /patrons/{patron_id}/holds, /trains/{train_id}/items/{item_id}) 2. So far we have decided to provide what is implemented/used. With this we will offer in the view usage of routes that may not be implemented yet. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 --- Comment #12 from Paul Derscheid <paul.derscheid@lmscloud.de> --- I have implemented something similar before. In this case we can't have simplicity and flexibility. The more flexible the api-client-base is, the more complex it will become. I think we should agree on the interface (and its quirks) first and then structure the base class around it. Maybe a meeting like Tomas suggested would be helpful. -- 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=36369 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #11)
It could work to reduce the number of lines in the -api-client.js.
Two points however: 1. The endpoint need to be configurable. 'this._baseURL + "/" + id' is too simple and does not work for everything (eg. /patrons/{patron_id}/holds, /trains/{train_id}/items/{item_id})
In my opinion, such cases should be handled, maybe with some more parameters to api-client-base. I totally get it and saw it in the code.
2. So far we have decided to provide what is implemented/used. With this we will offer in the view usage of routes that may not be implemented yet.
Am I right to interpret what you say like: 'if the Vue page doesn't allow deleting the resource, we shouldn't implement a delete() method'? It feels to me that this shouldn't be an issue... and would allow people to reuse this more? -- 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=36369 --- Comment #14 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #13)
(In reply to Jonathan Druart from comment #11)
2. So far we have decided to provide what is implemented/used. With this we will offer in the view usage of routes that may not be implemented yet.
Am I right to interpret what you say like: 'if the Vue page doesn't allow deleting the resource, we shouldn't implement a delete() method'?
It feels to me that this shouldn't be an issue... and would allow people to reuse this more?
Yes it is what I meant for "used". And I agree it is not really a problem. However it can be a problem for "implemented": If the endpoint does not exist in the controller should we provide it in the api-client? It does not make sense to me. -- 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=36369 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #14)
(In reply to Tomás Cohen Arazi from comment #13)
(In reply to Jonathan Druart from comment #11)
2. So far we have decided to provide what is implemented/used. With this we will offer in the view usage of routes that may not be implemented yet.
Am I right to interpret what you say like: 'if the Vue page doesn't allow deleting the resource, we shouldn't implement a delete() method'?
It feels to me that this shouldn't be an issue... and would allow people to reuse this more?
Yes it is what I meant for "used". And I agree it is not really a problem. However it can be a problem for "implemented": If the endpoint does not exist in the controller should we provide it in the api-client? It does not make sense to me.
I think it is fair that a dev can try, but then get a 404. I would like to hear from others. -- 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=36369 --- Comment #16 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #14)
(In reply to Tomás Cohen Arazi from comment #13)
(In reply to Jonathan Druart from comment #11)
2. So far we have decided to provide what is implemented/used. With this we will offer in the view usage of routes that may not be implemented yet.
Am I right to interpret what you say like: 'if the Vue page doesn't allow deleting the resource, we shouldn't implement a delete() method'?
It feels to me that this shouldn't be an issue... and would allow people to reuse this more?
Yes it is what I meant for "used". And I agree it is not really a problem. However it can be a problem for "implemented": If the endpoint does not exist in the controller should we provide it in the api-client? It does not make sense to me.
Maybe we can just code an override for not implemented verbs/endpoints that throws an exception. So devs don't get confused? I'm willing to be back to this work/discussion. -- 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=36369 Michał <schodkowy.omegi-0r@icloud.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |schodkowy.omegi-0r@icloud.c | |om --- Comment #17 from Michał <schodkowy.omegi-0r@icloud.com> --- Imho if you want to re-do an API client, it would make a lot of sense to do it in the most modern/universal and proper way possible. So something that could be used standalone also outside of the Koha UIs, maybe even on a separate repo and published on NPM (both to allow external JS programs to interact with Koha, but to also ensure it's written in an objective manner, with no hacks or short-circuits that'd unnecessarily bind it inherently to the Koha UI?). Then it also begs to finally use TypeScript, as type-safe API client is a very good thing, and you have OpenAPI specs that define everything about the APIs. So not doing that would be a big waste. The only problem is that a project like https://openapi-ts.dev/ for automatically translating OpenAPI specs to TypeScript types requires OpenAPI 3.x in current versions, but Koha still didn't migrate from 2.x (would be a good opportunity to do it). Of course the JS parts could still use it after building and not be too concerned about it. But in a proper setup, even then, a proper IDE setup could still give developers the type hints in their IntelliSense, which'd still be an easy win. So we should use something like that. Another win of using automated OpenAPI->TypeScript type generation would be that new endpoints defined in the schema could already be available for usage without manually needing to define them in yet another place. And without needing to manually type out all the code to handle the new endpoints (which would be pretty nice if we want to accelerate the API development and phase out the old one and classic Perl scripting) There's two projects: 1. https://openapi-ts.dev/ 2. https://heyapi.dev/ It seems that #1 focuses on more "bare" but usable schema generation, which can be used with their openapi-fetch client to access APIs in a type-safe but close to the original specs manner. For example (mind you that TS detects the exact path used by strings and type-checks the parameters!!): const { data, error } = await client.GET("/blogposts/{post_id}", { params: { path: { post_id: "my-post" }, query: { version: 2 }, }, }); It seems that #2 generates more of "prettier" SDKs already and focuses on direct integrations with plugins and middlewares etc. Their example is more like: const { data, error } = await getPetById({ path: { // random id 1-10 petId: BigInt(Math.floor(Math.random() * (10 - 1 + 1) + 1)), }, }); Not sure which approach seems nicer. Also technically it could be that simply generated types lived on a separate repo generated for all versions (including point-releases or major version tags), with clients being more responsible with how they'd directly pull/integrate them? I think everything really depends whether people would prefer using something like in #1 and maybe writing some more functional simpler wrappers around some of them (such as `blogposts.getPostById(id: number)`, or directly use something as in #2 which is something in between. Also the #2 having integrations with something like Zod or TanStack Query could prove quite useful in the future developments with Vue components (validating and auto-fetching of stuff etc.). I mean just look at the example here: https://tanstack.com/query/latest/docs/framework/vue/quick-start It would remain to be checked which one would work better with paginated endpoints though. It seems the #2's TanStack Query's integration has some support for infinite queries there in that regard. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 --- Comment #18 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- *** Bug 42753 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |ASSIGNED --- Comment #19 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Hi all, While working on CSRF enforcement for the REST API (Bug 34451), I needed a centralized place to handle API-level concerns - CSRF token injection, embed header handling - without duplicating logic across 26 client files. That led me to Bug 42753 (cleaning up hardcoded x-koha-embed headers), and then I realized I was essentially circling back to what I proposed here over two years ago. So I finally did it. The patch introduces a KohaApiClient base class that all API clients now extend. It centralizes: - HttpClient instantiation (no more passing the class as a constructor arg) - The embed parameter (declarative arrays -> x-koha-embed header, automatically) - CSRF token handling (inherited from HttpClient, applied consistently) All 26 clients are converted. All hardcoded x-koha-embed headers are replaced with the declarative embed: [...] idiom. This also resolves Bug 42753. Patches incoming. PS: I'm happy with the idea of auto-generating the client, but that belongs to a separate bug IMHO, and can be made on top of this. -- 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=36369 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164205|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=36369 --- Comment #20 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 199843 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199843&action=edit Bug 36369: Introduce KohaApiClient base class Introduce KohaApiClient as the base class for all API clients. This class centralizes: - HttpClient instantiation with a base URL - The embed parameter: callers declare embeds as an array, and _resolveEmbed() translates it to the x-koha-embed HTTP header All 26 API client classes now extend KohaApiClient instead of receiving HttpClient as a constructor argument. All hardcoded x-koha-embed headers in client methods are replaced with the declarative embed: [...] parameter. The api-client.js orchestrators (vanilla JS and Vue) no longer import or pass HttpClient - they simply instantiate clients with no arguments. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ qa -c 1 => SUCCESS: QA passes 3. Verify ERM, preservation, SIP2, ILL, acquisitions, checkout modules still function (embeds correctly sent) 4. 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=36369 --- Comment #21 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 199844 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199844&action=edit Bug 36369: (follow-up) Move CSRF token handling from HttpClient to KohaApiClient HttpClient is a generic HTTP utility with no knowledge of Koha. CSRF tokens are a Koha API concern: generated server-side by Koha::Token->generate_csrf() and embedded in the page's meta tag. This patch moves CSRF responsibility to the correct layer: - KohaApiClient._readCsrfToken() reads the token from the DOM - KohaApiClient._withCsrf() injects the CSRF-TOKEN header into state-changing requests (post, put, patch, delete) - HttpClient no longer has any CSRF awareness This makes HttpClient reusable as a generic fetch wrapper, and keeps Koha-specific API semantics (embed, CSRF) centralized in KohaApiClient. Signed-off-by: Tomas 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=36369 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #22 from Jonathan Druart <jonathan.druart@gmail.com> --- Error: Apply failed: Git command (git am -3 /tmp/C3pWJ48E7t/0001-199843.patch) failed: error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/js/fetch/acquisition-api-client.js). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |34451 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199843|0 |1 is obsolete| | Attachment #199844|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=36369 --- Comment #23 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200182 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200182&action=edit Bug 36369: Introduce KohaApiClient base class Introduce KohaApiClient as the base class for all API clients. This class centralizes: - HttpClient instantiation with a base URL - The embed parameter: callers declare embeds as an array, and _resolveEmbed() translates it to the x-koha-embed HTTP header All 26 API client classes now extend KohaApiClient instead of receiving HttpClient as a constructor argument. All hardcoded x-koha-embed headers in client methods are replaced with the declarative embed: [...] parameter. The api-client.js orchestrators (vanilla JS and Vue) no longer import or pass HttpClient - they simply instantiate clients with no arguments. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ qa -c 1 => SUCCESS: QA passes 3. Verify ERM, preservation, SIP2, ILL, acquisitions, checkout modules still function (embeds correctly sent) 4. 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=36369 --- Comment #24 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200183 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200183&action=edit Bug 36369: (follow-up) Move CSRF token handling from HttpClient to KohaApiClient HttpClient is a generic HTTP utility with no knowledge of Koha. CSRF tokens are a Koha API concern: generated server-side by Koha::Token->generate_csrf() and embedded in the page's meta tag. This patch moves CSRF responsibility to the correct layer: - KohaApiClient._readCsrfToken() reads the token from the DOM - KohaApiClient._withCsrf() injects the CSRF-TOKEN header into state-changing requests (post, put, patch, delete) - HttpClient no longer has any CSRF awareness This makes HttpClient reusable as a generic fetch wrapper, and keeps Koha-specific API semantics (embed, CSRF) centralized in KohaApiClient. Signed-off-by: Tomas 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=36369 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42811 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42811 [Bug 42811] Migrate x-koha-embed headers to embed keyword in API clients -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200182|0 |1 is obsolete| | Attachment #200183|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=36369 --- Comment #25 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 200638 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200638&action=edit Bug 36369: Introduce KohaApiClient base class Introduce KohaApiClient as the base class for all API clients. This class centralizes: - HttpClient instantiation with a base URL - The embed parameter: callers declare embeds as an array, and _resolveEmbed() translates it to the x-koha-embed HTTP header All 26 API client classes now extend KohaApiClient instead of receiving HttpClient as a constructor argument. All hardcoded x-koha-embed headers in client methods are replaced with the declarative embed: [...] parameter. The api-client.js orchestrators (vanilla JS and Vue) no longer import or pass HttpClient - they simply instantiate clients with no arguments. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ qa -c 1 => SUCCESS: QA passes 3. Verify ERM, preservation, SIP2, ILL, acquisitions, checkout modules still function (embeds correctly sent) 4. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 --- Comment #26 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 200639 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200639&action=edit Bug 36369: (follow-up) Move CSRF token handling from HttpClient to KohaApiClient HttpClient is a generic HTTP utility with no knowledge of Koha. CSRF tokens are a Koha API concern: generated server-side by Koha::Token->generate_csrf() and embedded in the page's meta tag. This patch moves CSRF responsibility to the correct layer: - KohaApiClient._readCsrfToken() reads the token from the DOM - KohaApiClient._withCsrf() injects the CSRF-TOKEN header into state-changing requests (post, put, patch, delete) - HttpClient no longer has any CSRF awareness This makes HttpClient reusable as a generic fetch wrapper, and keeps Koha-specific API semantics (embed, CSRF) centralized in KohaApiClient. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36369 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42876 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42876 [Bug 42876] Update CSRF token to not reach it does not expire -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org