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

Fix/anonymous to accept a consumer #2035

Merged
merged 6 commits into from
Feb 21, 2017
Merged

Fix/anonymous to accept a consumer #2035

merged 6 commits into from
Feb 21, 2017

Conversation

Tieske
Copy link
Member

@Tieske Tieske commented Jan 31, 2017

Summary

current implementation for anonymous access does not allow to run plugins on anonymous consumers. This change changes the boolean setting to accept anonymous access into a uuid setting which can hold the uuid of the consumer to associate with the anonymous access.

When this user is set, the credentials will not be set, and the header X-Anonymous-Consumer: true will still be set.

Full changelog

Updated plugins:

  • basic-auth
  • key-auth
  • oauth2
  • hmac-auth
  • jwt
  • ldap-auth

@Tieske Tieske added the pr/changes requested Changes were requested to this PR by a maintainer. Please address them and ping back once done. label Jan 31, 2017
@Tieske Tieske self-assigned this Jan 31, 2017
@Tieske Tieske added this to the 0.10 RC milestone Jan 31, 2017
@thibaultcha
Copy link
Member

Going down this road is still a terrible idea on the long term imo, due to the concerning amount of code being duplicated. As suggested before, another way to implement this would be that such authentication plugins could comply to an interface, and the core could detect a plugin is an authentication one, and execute them. Depending on the return value of a plugin, we can determine:

  • errors (500)
  • non authenticated calls (403)
  • mis-authenticated calls (401)

And the core can take appropriate actions from there: setting authentication headers, allowing a call to be proxied as "anonymous", responding with HTTP 401 or 403, etc...

@Tieske
Copy link
Member Author

Tieske commented Feb 2, 2017

Agreed, should be revised, but as part of plugin api refactoring.

@Tieske Tieske added pr/status/needs review and removed pr/changes requested Changes were requested to this PR by a maintainer. Please address them and ping back once done. labels Feb 2, 2017
@Tieske Tieske requested a review from thibaultcha February 2, 2017 22:09
@Tieske Tieske merged commit 91a5738 into release/0.10.0 Feb 21, 2017
@Tieske Tieske deleted the fix/anonymous branch February 21, 2017 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants