Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OAuth authentication: bugfix and enhancements #495

Merged
merged 6 commits into from
Oct 24, 2018

Conversation

veggiematts
Copy link
Contributor

@veggiematts veggiematts commented Oct 3, 2018

This patch provides a bugfix and several enhancements for OAuth authentication in the Koha connector:

  • Fix incorrect use of "Authorization" header.
  • Add better error handling
  • Authentication token is now stored in session and re-used until expired.
  • Replace incorrect call to Koha API.

Test plan:
Koha version >= 18.05 needed.

  • Coral: Install Unirest and league/oauth2-client in resources/api_client (composer might be the easiest way to do it)
  • Koha: Install Net::OAuth2::AuthorizationServer and enable RESTOAuth2ClientCredentials syspref.
  • Koha: Create a user for the REST API, edit this user, click on more -> manage API Keys -> Generate new client id/secret pair
  • Coral: edit organizations/admin/configuration.ini as such:
[ils]
ilsConnector=koha
ilsVendorRole="Vendor"
ilsApiUrl="http://yourkoha.tld/api/v1"
ilsAdminUrl="http://admin.yourkoha.tld/"
oauthid="<paste Client ID generated in Koha>"
oauthsecret="<paste Secret generated in Koha>"

@veggiematts veggiematts added bug This is a bug (not an enhancement) enhancement This is an enhancement (not a bug) labels Oct 3, 2018
@jeffnm
Copy link
Contributor

jeffnm commented Oct 23, 2018

@veggiematts We aren't Koha users, so I can't really test this well. The code looks good to me, but is there anyone else you could get to test this who uses Koha and Coral?

Since nothing in these changes should cause any issues for non-koha users (no one else is using the ils api, right?), I am happy to accept Biblibre testing, but it would be nice to have another user if possible.

Also, do you think we should add installer support for setting those [ils] configurations?

@veggiematts
Copy link
Contributor Author

is there anyone else you could get to test this who uses Koha and Coral?

I'm trying to :)

no one else is using the ils api, right?

That is correct.

Also, do you think we should add installer support for setting those [ils] configurations?

I think that would be a good thing, but not top-priority (as using the command line is mandatory anyway for installing dependencies with composer)

(3.* is needed for Unirest\Request\Body)
@LibClaire
Copy link

I have tested this with OAuth Authentication.

I was able to create vendors (Organizations) in Coral which appeared in my Koha Vendors and was able to pull Koha vendor information into new organization in Coral. Worked as expected.

@jeffnm
Copy link
Contributor

jeffnm commented Oct 24, 2018

Thanks @LibClaire and @veggiematts, that's good enough for me. I'll merge.

@jeffnm jeffnm merged commit 7c8fa69 into coral-erm:development Oct 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a bug (not an enhancement) enhancement This is an enhancement (not a bug)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants