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

Use @chained_auth_function with core auth functions #4491

Merged
merged 5 commits into from
Jun 3, 2019

Conversation

alycejenni
Copy link
Contributor

This is essentially an updated PR based on #4248 opened by @ashleysommer several months ago, with the formatting fixes and a new test. I take zero credit for the original fix!

Proposed fixes:

From the original PR:

Some authentication plugins implement chained auth functions on actions such as user_create and user_update. If no other plugins provide these functions, then CKAN raises the exception "The auth 'user_create' is not found for chained auth". The CKAN built-in auth system however does provide these functions. So it makes sense to fallback to having the chained auth function chain to the corresponding built-in function.

This is an example of one such plugin which attempts to chain off 'user_create' and 'user_update' and fails:
https://github.com/NaturalHistoryMuseum/ckanext-ldap/blob/6f9aec4edadaf792e347b4d20adad2c2322c167f/ckanext/ldap/logic/auth/create.py#L12

I also fixed one other small bug in authz.py: the error message displays the auth_function name, but chained functions are partial functions and don't have names, so it was failing. It now uses the name of the .func if it's a partial.

Features:

  • includes tests covering changes
  • includes updated documentation
  • includes user-visible changes
  • includes API changes
  • includes bugfix for possible backport

Please [X] all the boxes above that apply

ashleysommer and others added 3 commits October 4, 2018 20:42
also fixed a related bug: error message for unauthenticated users would break for chained auth functions because partials don't have `__name__` attributes
NotAuthorized exception was not being thrown in CircleCI tests - perhaps because it was using the previous user?
@wardi wardi self-assigned this Oct 9, 2018
@alycejenni alycejenni mentioned this pull request Oct 17, 2018
5 tasks
@kmbn kmbn added the Awaiting tech team feedback This PR or issue needs feedback from the tech team. label Jan 24, 2019
@amercader
Copy link
Member

amercader commented Jun 3, 2019

@wardi I took the liberty of reviewing, fixing and merging this as I was working on it.

@amercader amercader removed the Awaiting tech team feedback This PR or issue needs feedback from the tech team. label Jun 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants