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

feat(plugins) set generic X-Credential-Identifier #5516

Merged
merged 7 commits into from
Feb 4, 2020

Conversation

bungle
Copy link
Member

@bungle bungle commented Jan 31, 2020

Summary

The PR #4993 implemented X-Credential-Identifier for JWT Plugin and it was decided
at time that we should add support for this less opinionated field name on other auth
plugins too. This commit adds it to other bundled auth plugins too:

  • feat(basic-auth) set generic X-Credential-Identifier
  • feat(key-auth) set generic X-Credential-Identifier
  • feat(ldap-auth) set generic X-Credential-Identifier
  • feat(oauth2) set generic X-Credential-Identifier

It also adds two more commits:

  • chore(jwt) make set_consumer consistent with other auth plugins
  • chore(constants) add note about CREDENTIAL_USERNAME deprecations

This PR makes set_consumer consistent on all the bundled auth plugins.

Issues Resolved

This PR closes the PR #5201 by @lucasmoreno (only difference being more tests added and header name changed to Kong generic X-Credential-Identifier. Thanks @lucasmoreno, grab your contributor T-shirt at here: https://github.com/Kong/kong/blob/master/CONTRIBUTING.md#contributor-t-shirt (you can link to this PR).

…redential-Username)

### Summary

The PR #4993 implemented `X-Credential-Identifier` for `JWT Plugin` and it was decided
at time that we should add support for this less opinionated field name on other auth
plugins too. This commit adds it to `Basic Auth Plugin`.
@bungle bungle changed the title Feat/credential identifier feat(plugins) set generic X-Credential-Identifier Jan 31, 2020
kong/plugins/jwt/handler.lua Outdated Show resolved Hide resolved
kong/plugins/key-auth/handler.lua Outdated Show resolved Hide resolved
…dential-Username)

### Summary

The PR #4993 implemented `X-Credential-Identifier` for `JWT Plugin` and it was decided
at time that we should add support for this less opinionated field name on other auth
plugins too. This commit adds it to `Key Auth Plugin`.
…edential-Username)

### Summary

The PR #4993 implemented `X-Credential-Identifier` for `JWT Plugin` and it was decided
at time that we should add support for this less opinionated field name on other auth
plugins too. This commit adds it to `HMAC Auth Plugin`.
…edential-Username)

### Summary

The PR #4993 implemented `X-Credential-Identifier` for `JWT Plugin` and it was decided
at time that we should add support for this less opinionated field name on other auth
plugins too. This commit adds it to `LDAP Auth Plugin`.
…ntial-Username)

### Summary

The PR #4993 implemented `X-Credential-Identifier` for `JWT Plugin` and it was decided
at time that we should add support for this less opinionated field name on other auth
plugins too. This commit adds it to `OAuth2 Plugin`.

This feature was originally implemented by @lucasmoreno on PR #5201. Thank you, Lucas,
grab your T-shirt:
https://github.com/Kong/kong/blob/master/CONTRIBUTING.md#contributor-t-shirt!

I just refactored @lucasmoreno's code to use our generic `X-Credential-Identifier`
header instead of the proposed `X-Credential-Client-Id`.

### Issues Resolved

Closes #5201
### Summary

Updates `JWT Plugin's` `set_consumer` function to be consistent with
other auth plugins.
### Summary

Right now some plugins that used to set `X-Credential-Username`
are now also setting `X-Credential-Identifier` with the same
value. We want to get rid of `X-Credential-Username` at some
point in favor for `X-Credential-Identifier`. This commit just
adds a TODO comment there.
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.

None yet

4 participants