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

Support for brod_gssapi and other custom brod sasl authenticators #82

Closed
ramonpin opened this issue Mar 28, 2022 · 7 comments · Fixed by #85
Closed

Support for brod_gssapi and other custom brod sasl authenticators #82

ramonpin opened this issue Mar 28, 2022 · 7 comments · Fixed by #85

Comments

@ramonpin
Copy link

As stated in the documentation the BroadwayKafka.Producer module is able to pass any configuration on the client_config option down to the :brod.start_client/3.

Right now the code contains a validation that restricts the sasl configuration options to the values:

  • {:sasl, {:plain, user, password}}
  • {:sasl, {:scram_sha_256, user, password}}
  • {:sasl, {:scram_sha_512, user, password}}

But :brod also supports using authentication plugins as:

  • {:sasl, {:callback :module, opts}}

For example you can use GSSAPI auth with kerberos by using the brod_gssapi plugin and the following client configuration:

  • {:sasl, {:callback, :brod_gssapi, {:gssapi, keytab, principal}}}

Are you considering to allow this functionality?

@ramonpin
Copy link
Author

ramonpin commented Mar 28, 2022

From source code 'brod_client.ex':

  defp validate_option(:sasl, :undefined),
    do: {:ok, :undefined}

  defp validate_option(:sasl, value) do
    with {mechanism, username, password}
         when mechanism in [:plain, :scram_sha_256, :scram_sha_512] and
                is_binary(username) and
                is_binary(password) <- value do
      {:ok, value}
    else
      _value -> validation_error(:sasl, "a tuple of SASL mechanism, username and password", value)
    end
  end

I can try to make a PR adding the validation needed for the callback related tuple to that function but don't know if you expect any other problems down the stream.

@josevalim
Copy link
Member

A PR is welcome. We can either stop validating it or validate this additional value. Both approaches are fine!

@aravindanck
Copy link
Contributor

We're also looking to have this option as part of Broadway Kafka. Just curious @ramonpin - which SASL mechanism are you trying to support?

@ramonpin
Copy link
Author

ramonpin commented Mar 29, 2022

@aravindanck our intention is to have support for the SASL_PLAINTEXT with GSSAPI at least. We probably will need to make several consecutive PRs to other erlang/kafka's ecosystem proyects used by this module.

@aravindanck
Copy link
Contributor

@ramonpin Is there a PR for this one already? If not, are you planning to raise one soon?

@ramonpin
Copy link
Author

Sorry guys some problems in other areas of the application leave me no time for this.

@aravindanck great work from your side. Where you able to make it work with brod_gssapi library?

Hoping to contribute soon.

@aravindanck
Copy link
Contributor

@ramonpin It should work for brod_gssapi library, as the validation check enables you to work with any SASL callback module. I tried this with a callback module built for a different SASL mechanism

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 a pull request may close this issue.

3 participants