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

Add option for custom auth #1280

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

patrykkotlowski-dsstream
Copy link

@patrykkotlowski-dsstream patrykkotlowski-dsstream commented Aug 30, 2024

Custom authentication

This PR provides an option for the users to use custom authentication mechanism.

What's changed:

  • New options were added to the configuration
  • There is an option to create custom oauth provider
  • There is an option for custom JWT token check.

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Sep 26, 2024
@dokterbob dokterbob added enhancement New feature or request and removed bug Something isn't working labels Sep 26, 2024
@dokterbob
Copy link
Collaborator

Thanks for the contrib @patrykkotlowski-dsstream!

Do you see any way of using this work to arrive at a more generic pluggable auth? We're really looking forward to move support of auth frameworks more towards the community, so we'd rather remove than add new methods.

But if yours could be refactored towards having more pluggable auth methods (e.g. all auth methods are classes) that can be easily swapped, which can be returned from a single call back hook, that might be really nice.

In addition, before we merge this in, we'd really like to see both E2E and unit tests. Otherwise, it's really hard for us to ensure that it won't break time and again while building on other stuff.

What do you think?

@patrykkotlowski-dsstream
Copy link
Author

Hi, thank you for the feedback!

I would be more than happy to add both E2E and unit tests to ensure the stability of the implementation. Also, I’m definitely interested in refactoring the auth methods in the future to make them more pluggable, as suggested. However, if it’s okay with you, I’d prefer to handle that refactor in a separate PR, since I’m currently focused on adding the CustomAuth provider as quickly as possible.

Let me know your thoughts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants