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: pluggable auth plugins #945

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

barakalon
Copy link

What do these changes do?

First off - thanks for all the work on this package :)

This PR introduces a framework for auth plugins. This will make it easier to add auth plugins.

Why?

I need to implement the kerberos plugin. We also use this client with services that only imitate the mysql server (i.e. starrocks and proprietary services based on mysql-mimic), and I might need proprietary auth plugins.

Are there changes in behavior for the user?

Nope. All the existing auth_plugins should work as-is.

Related issue number

n/a

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • [] Documentation reflects the changes
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> (e.g. 588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the PR
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: Fix issue with non-ascii contents in doctest text files.

This has tests in the sense that existing tests don't fail. But I don't think the existing tests for auth are that extensive, so it might warrant some more testing.

As for documentation - I'm not sure this should be documented just yet.

@barakalon
Copy link
Author

@barakalon
Copy link
Author

@Nothing4You any thoughts on this one?

@Nothing4You
Copy link
Collaborator

Hi @barakalon,

thank you for the PR, unfortunately it'll probably be a few more weeks until I'll have time to review this.

@barakalon
Copy link
Author

Sounds good 👍

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