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 support for Authentication #17

Closed
jmcristobal opened this issue Jul 16, 2021 · 9 comments
Closed

Add support for Authentication #17

jmcristobal opened this issue Jul 16, 2021 · 9 comments
Milestone

Comments

@jmcristobal
Copy link

jmcristobal commented Jul 16, 2021

Allow to configure authentication method for the Kafka Connect cluster. I've done a quick test (clumpsy, I know) and works adding the ClientHeaderParam anotation at interface level. The values should be obtained from the configuration, obviously.

I'm not an expert in this REST Client library, and probably there is a better way to implement this solution.

Also, this is only covering BasicAuth and other authentication methods could be included as well.

If this is out of scope, don't hesitate to close this one ;)

/*
 *  Copyright 2021 The original authors
 *
 *  Licensed under the Apache License, Version 2.0 (the "License");
 *  you may not use this file except in compliance with the License.
 *  You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 *  Unless required by applicable law or agreed to in writing, software
 *  distributed under the License is distributed on an "AS IS" BASIS,
 *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 *  See the License for the specific language governing permissions and
 *  limitations under the License.
 */
package dev.morling.kccli.service;

import java.util.Base64;
import java.util.List;
import java.util.Map;

import javax.ws.rs.DELETE;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.PUT;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;

import org.eclipse.microprofile.rest.client.annotation.ClientHeaderParam;
import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;

@Path("/")
@RegisterRestClient
@ClientHeaderParam(name = "Authorization", value = "{getBasicAuth}")
public interface KafkaConnectApi {

    @GET
    KafkaConnectInfo getWorkerInfo();

    @GET
    @Path("/connector-plugins")
    List<ConnectorPlugin> getConnectorPlugins();

    @GET
    @Path("/connectors/")
    List<String> getConnectors();

    @POST
    @Path("/connectors/")
    ConnectorStatusInfo createConnector(String config);

    @GET
    @Path("/connectors/{name}")
    ConnectorInfo getConnector(@PathParam("name") String name);

    @POST
    @Path("/connectors/{name}/restart")
    void restartConnector(@PathParam("name") String name);

    @DELETE
    @Path("/connectors/{name}")
    void deleteConnector(@PathParam("name") String name);

    @GET
    @Path("/connectors/{name}/status")
    ConnectorStatusInfo getConnectorStatus(@PathParam("name") String name);

    @GET
    @Path("/connectors/{name}/topics")
    Map<String, TopicsInfo> getConnectorTopics(@PathParam("name") String name);

    @GET
    @Path("/connectors/{name}/config")
    Map<String, String> getConnectorConfig(@PathParam("name") String name);

    @PUT
    @Path("/connectors/{name}/config")
    ConnectorStatusInfo updateConnector(@PathParam("name") String name, String config);

    @POST
    @Path("/connectors/{name}/tasks/{id}/restart")
    ConnectorInfo restartTask(@PathParam("name") String name, @PathParam("id") String id);

    default String getBasicAuth() {
        return "Basic " +
                Base64.getEncoder().encodeToString("username:password".getBytes());
    }
}
@tonyfosterdev
Copy link
Contributor

I would love to see this!

@gunnarmorling
Copy link
Collaborator

Same, same :) So far, no one has found the bandwidth for implementing it. It'd be a very valuable contribution.

@tonyfosterdev
Copy link
Contributor

tonyfosterdev commented Sep 4, 2021 via email

@gunnarmorling
Copy link
Collaborator

No worries, I'm sure we'll get it sorted out one way or another.

As a first step, I'm curious how a solution should look like. There's the concept of "contexts" in kcctl, which lets you switch between different named Kafka Connect environments, e.g. "local", "staging", "prod". Details of the contexts are stored in a file ~/.kcctl (KC URL, but also Kafka bootstrap servers for the upcoming offset display/reset feature).

Using this file for storing auth information per context would be an obvious choice, but of course there are security implications to consider. Ideally, the auth information should be stored encrypted, and decrypted upon usage. Which would be cumbersome to do for each usage, so doing something similar to SSH Agent may be interesting. It would be interesting to see how other CLI tools handle this sort of requirement.

@tonyfosterdev
Copy link
Contributor

tonyfosterdev commented Sep 4, 2021 via email

@tonyfosterdev
Copy link
Contributor

I dug into how kubectl used basic auth. This page lays it out well: https://unofficial-kubernetes.readthedocs.io/en/latest/concepts/cluster-administration/authenticate-across-clusters-kubeconfig/#example-kubeconfig-file

The credentials are stored plaintext. It may be nice, down the road, to use a cred store that is native to the OS, but that would require a few implementations.

@tonyfosterdev
Copy link
Contributor

PR for this is here for review: #68

gunnarmorling pushed a commit that referenced this issue Oct 5, 2021
gunnarmorling pushed a commit that referenced this issue Oct 5, 2021
gunnarmorling added a commit that referenced this issue Oct 5, 2021
* Showing more meaningful error message
* Avoiding Commons Lang dependency
* Updating versions
tonyfosterdev added a commit to tonyfosterdev/kcctl that referenced this issue Oct 7, 2021
tonyfosterdev added a commit to tonyfosterdev/kcctl that referenced this issue Oct 7, 2021
tonyfosterdev added a commit to tonyfosterdev/kcctl that referenced this issue Oct 7, 2021
tonyfosterdev added a commit to tonyfosterdev/kcctl that referenced this issue Oct 7, 2021
tonyfosterdev added a commit to tonyfosterdev/kcctl that referenced this issue Oct 7, 2021
tonyfosterdev added a commit to tonyfosterdev/kcctl that referenced this issue Oct 7, 2021
tonyfosterdev added a commit to tonyfosterdev/kcctl that referenced this issue Oct 7, 2021
gunnarmorling pushed a commit that referenced this issue Oct 8, 2021
gunnarmorling pushed a commit that referenced this issue Oct 8, 2021
gunnarmorling added a commit that referenced this issue Oct 8, 2021
* Showing more meaningful error message
* Avoiding Commons Lang dependency
* Updating versions
gunnarmorling pushed a commit that referenced this issue Oct 8, 2021
gunnarmorling pushed a commit that referenced this issue Oct 8, 2021
gunnarmorling pushed a commit that referenced this issue Oct 8, 2021
gunnarmorling added a commit that referenced this issue Oct 8, 2021
@tonyfosterdev
Copy link
Contributor

@jmcristobal I believe you should be all set now. This PR is merged.

@gunnarmorling
Copy link
Collaborator

Indeed, thanks again. Will close this one.

@gunnarmorling gunnarmorling added this to the 1.0.0.Beta1 milestone Oct 9, 2021
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

No branches or pull requests

3 participants