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

AGENT-950: Implement Separate JWT Tokens for Different User Personas #9039

2 changes: 1 addition & 1 deletion cmd/node-joiner/testdata/add-nodes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ unqualified-search-registries = []
CLUSTER_ID=c37c9544-4320-4380-9d8b-0753a4d9ea57
CLUSTER_NAME=ostest
CLUSTER_API_VIP_DNS_NAME=api.ostest.test.metalkube.org
AGENT_AUTH_TOKEN_EXPIRY=.*
AUTH_TOKEN_EXPIRY=.*
-- expected/grub.cfg --
.*fips=1.*
-- expected/import-cluster-config.json --
Expand Down
6 changes: 3 additions & 3 deletions data/data/agent/files/usr/local/bin/add-node.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ cluster_id=""
while [[ "${cluster_id}" = "" ]]
do
# Get cluster id
cluster_id=$(curl_assisted_service "/clusters" | jq -r .[].id)
cluster_id=$(curl_assisted_service "/clusters" GET | jq -r .[].id)
if [[ "${cluster_id}" = "" ]]; then
sleep 2
fi
Expand All @@ -24,7 +24,7 @@ status_issue="90_add-node"
host_ready=false
while [[ $host_ready == false ]]
do
host_status=$(curl_assisted_service "/infra-envs/${INFRA_ENV_ID}/hosts" | jq -r ".[].status")
host_status=$(curl_assisted_service "/infra-envs/${INFRA_ENV_ID}/hosts" GET | jq -r ".[].status")
if [[ "${host_status}" != "known" ]]; then
printf '\\e{yellow}Waiting for the host to be ready' | set_issue "${status_issue}"
sleep 10
Expand All @@ -33,7 +33,7 @@ do
fi
done

HOST_ID=$(curl_assisted_service "/infra-envs/${INFRA_ENV_ID}/hosts" | jq -r '.[].id')
HOST_ID=$(curl_assisted_service "/infra-envs/${INFRA_ENV_ID}/hosts" GET | jq -r '.[].id')
printf '\nHost %s is ready for installation\n' "${HOST_ID}" 1>&2
clear_issue "${status_issue}"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ status_issue="65_token"

export TZ=UTC
check_token_expiry() {
expiry_epoch=$(date -d "${AGENT_AUTH_TOKEN_EXPIRY}" +%s)
expiry_epoch=$(date -d "${AUTH_TOKEN_EXPIRY}" +%s)
current_epoch=$(date +%s)

if [ "$current_epoch" -gt "$expiry_epoch" ]; then
Expand Down
31 changes: 11 additions & 20 deletions data/data/agent/files/usr/local/bin/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,14 @@ curl_assisted_service() {
local additional_options=("${@:3}") # Capture all arguments starting from the third one
local baseURL="${SERVICE_BASE_URL}api/assisted-install/v2"

case "${method}" in
"POST")
curl -s -S -X POST "${additional_options[@]}" "${baseURL}${endpoint}" \
-H "Authorization: ${AGENT_AUTH_TOKEN}" \
-H "accept: application/json" \
-H "Content-Type: application/json" \
;;
"PATCH")
curl -s -S -X PATCH "${additional_options[@]}" "${baseURL}${endpoint}" \
-H "Authorization: ${AGENT_AUTH_TOKEN}" \
-H "accept: application/json" \
-H "Content-Type: application/json" \
;;
"GET")
curl -s -S -X GET "${additional_options[@]}" "${baseURL}${endpoint}" \
-H "Authorization: ${AGENT_AUTH_TOKEN}" \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PATCH doesn't need the ${authz} token?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes for PATCH got added here via rebase and originally came from a commit on Sep 30 i.e. faaddc1 however I think its a wrong commit because PATCH is not used in any curl requests in installer repo. The correct use is in appliance here and here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, for now the appliance isn't directly using the code (the func was copied to the codebase instead).
Will probably be handled later on to reuse this func, so I think it'd be safer to update the PATCH flow as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I will update the PATCH flow here(even if its unused now). Please note , later in the appliance, you will need to pass the USER_AUTH_TOKEN

-H "Accept: application/json"
;;
esac
}
headers=(
-s -S
-H "Authorization: ${USER_AUTH_TOKEN}"
-H "accept: application/json"
)

[[ "$method" == "POST" || "$method" == "PATCH" ]] && headers+=(-H "Content-Type: application/json")

curl "${headers[@]}" -X "${method}" "${additional_options[@]}" "${baseURL}${endpoint}"

}
2 changes: 1 addition & 1 deletion data/data/agent/files/usr/local/bin/start-agent.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ INFRA_ENV_ID=""
until [[ $INFRA_ENV_ID != "" && $INFRA_ENV_ID != "null" ]]; do
sleep 5
>&2 echo "Querying assisted-service for infra-env-id..."
INFRA_ENV_ID=$(curl_assisted_service "/infra-envs" GET | jq -r .[0].id)
INFRA_ENV_ID=$(curl_assisted_service "/infra-envs" GET | jq -r '.[0].id')
done
echo "Fetched infra-env-id and found: $INFRA_ENV_ID"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
AUTH_TYPE=none
AUTH_TYPE={{.AuthType}}
DB_HOST=127.0.0.1
DB_NAME=installer
DB_PASS=admin
Expand All @@ -20,4 +20,5 @@ OPENSHIFT_INSTALL_RELEASE_IMAGE_MIRROR={{.ReleaseImageMirror}}
STORAGE=filesystem
INFRA_ENV_ID={{.InfraEnvID}}
EC_PUBLIC_KEY_PEM={{.PublicKeyPEM}}
AGENT_AUTH_TOKEN={{.Token}}
USER_AUTH_TOKEN={{.UserAuthToken}}
WATCHER_AUTH_TOKEN={{.WatcherAuthToken}}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[Unit]
Description=service that displays a message if agent auth token is expired.
Description=service that displays a message if auth tokens are expired.
Wants=network-online.target
After=network-online.target agent-interactive-console.service
ConditionPathExists=/etc/assisted/add-nodes.env
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ EnvironmentFile=/usr/local/share/assisted-service/assisted-service.env
EnvironmentFile=/etc/assisted/add-nodes.env
ExecStartPre=/bin/rm -f %t/%n.ctr-id
ExecStartPre=/usr/local/bin/wait-for-assisted-service.sh
ExecStart=podman run --net host --cidfile=%t/%n.ctr-id --cgroups=no-conmon --log-driver=journald --rm --pod-id-file=%t/assisted-service-pod.pod-id --replace --name=agent-import-cluster -v /etc/assisted/clusterconfig:/clusterconfig -v /etc/assisted/manifests:/manifests -v /etc/assisted/extra-manifests:/extra-manifests {{ if .HaveMirrorConfig }}-v /etc/containers:/etc/containers{{ end }} {{.CaBundleMount}} --env SERVICE_BASE_URL --env OPENSHIFT_INSTALL_RELEASE_IMAGE_MIRROR --env CLUSTER_ID --env CLUSTER_NAME --env CLUSTER_API_VIP_DNS_NAME --env AGENT_AUTH_TOKEN $SERVICE_IMAGE /usr/local/bin/agent-installer-client importCluster
ExecStart=podman run --net host --cidfile=%t/%n.ctr-id --cgroups=no-conmon --log-driver=journald --rm --pod-id-file=%t/assisted-service-pod.pod-id --replace --name=agent-import-cluster -v /etc/assisted/clusterconfig:/clusterconfig -v /etc/assisted/manifests:/manifests -v /etc/assisted/extra-manifests:/extra-manifests {{ if .HaveMirrorConfig }}-v /etc/containers:/etc/containers{{ end }} {{.CaBundleMount}} --env SERVICE_BASE_URL --env OPENSHIFT_INSTALL_RELEASE_IMAGE_MIRROR --env CLUSTER_ID --env CLUSTER_NAME --env CLUSTER_API_VIP_DNS_NAME --env USER_AUTH_TOKEN $SERVICE_IMAGE /usr/local/bin/agent-installer-client importCluster
ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id
ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ EnvironmentFile=/usr/local/share/assisted-service/agent-images.env
EnvironmentFile=/usr/local/share/assisted-service/assisted-service.env
ExecStartPre=/bin/rm -f %t/%n.ctr-id
ExecStartPre=/usr/local/bin/wait-for-assisted-service.sh
ExecStart=podman run --net host --cidfile=%t/%n.ctr-id --cgroups=no-conmon --log-driver=journald --rm --pod-id-file=%t/assisted-service-pod.pod-id --replace --name=agent-register-cluster -v /etc/assisted/manifests:/manifests -v /etc/assisted/extra-manifests:/extra-manifests {{ if .HaveMirrorConfig }}-v /etc/containers:/etc/containers{{ end }} {{.CaBundleMount}} --env SERVICE_BASE_URL --env OPENSHIFT_INSTALL_RELEASE_IMAGE_MIRROR --env AGENT_AUTH_TOKEN $SERVICE_IMAGE /usr/local/bin/agent-installer-client registerCluster
ExecStart=podman run --net host --cidfile=%t/%n.ctr-id --cgroups=no-conmon --log-driver=journald --rm --pod-id-file=%t/assisted-service-pod.pod-id --replace --name=agent-register-cluster -v /etc/assisted/manifests:/manifests -v /etc/assisted/extra-manifests:/extra-manifests {{ if .HaveMirrorConfig }}-v /etc/containers:/etc/containers{{ end }} {{.CaBundleMount}} --env SERVICE_BASE_URL --env OPENSHIFT_INSTALL_RELEASE_IMAGE_MIRROR --env USER_AUTH_TOKEN $SERVICE_IMAGE /usr/local/bin/agent-installer-client registerCluster
ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id
ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ EnvironmentFile=/etc/assisted/rendezvous-host.env
EnvironmentFile=/usr/local/share/assisted-service/agent-images.env
EnvironmentFile=/usr/local/share/assisted-service/assisted-service.env
ExecStartPre=/bin/rm -f %t/%n.ctr-id
ExecStart=podman run --net host --cidfile=%t/%n.ctr-id --cgroups=no-conmon --log-driver=journald --rm --pod-id-file=%t/assisted-service-pod.pod-id --replace --name=agent-register-infraenv -v /etc/assisted/manifests:/manifests --env SERVICE_BASE_URL --env IMAGE_TYPE_ISO --env AGENT_AUTH_TOKEN $SERVICE_IMAGE /usr/local/bin/agent-installer-client registerInfraEnv
ExecStart=podman run --net host --cidfile=%t/%n.ctr-id --cgroups=no-conmon --log-driver=journald --rm --pod-id-file=%t/assisted-service-pod.pod-id --replace --name=agent-register-infraenv -v /etc/assisted/manifests:/manifests --env SERVICE_BASE_URL --env IMAGE_TYPE_ISO --env USER_AUTH_TOKEN $SERVICE_IMAGE /usr/local/bin/agent-installer-client registerInfraEnv
ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id
ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id

Expand Down
2 changes: 1 addition & 1 deletion data/data/agent/systemd/units/apply-host-config.service
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ EnvironmentFile=/usr/local/share/assisted-service/assisted-service.env
ExecStartPre=/bin/rm -f %t/%n.ctr-id
ExecStartPre=/bin/mkdir -p %t/agent-installer /etc/assisted/hostconfig
ExecStartPre=/usr/local/bin/wait-for-assisted-service.sh
ExecStart=podman run --net host --cidfile=%t/%n.ctr-id --cgroups=no-conmon --log-driver=journald --restart=on-failure:10 --pod-id-file=%t/assisted-service-pod.pod-id --replace --name=apply-host-config -v /etc/assisted/hostconfig:/etc/assisted/hostconfig -v %t/agent-installer:/var/run/agent-installer:z --env SERVICE_BASE_URL --env INFRA_ENV_ID --env WORKFLOW_TYPE --env AGENT_AUTH_TOKEN $SERVICE_IMAGE /usr/local/bin/agent-installer-client configure
ExecStart=podman run --net host --cidfile=%t/%n.ctr-id --cgroups=no-conmon --log-driver=journald --restart=on-failure:10 --pod-id-file=%t/assisted-service-pod.pod-id --replace --name=apply-host-config -v /etc/assisted/hostconfig:/etc/assisted/hostconfig -v %t/agent-installer:/var/run/agent-installer:z --env SERVICE_BASE_URL --env INFRA_ENV_ID --env WORKFLOW_TYPE --env USER_AUTH_TOKEN $SERVICE_IMAGE /usr/local/bin/agent-installer-client configure
ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id
ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id

Expand Down
8 changes: 4 additions & 4 deletions pkg/agent/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,25 +69,25 @@ func NewCluster(ctx context.Context, assetDir, rendezvousIP, kubeconfigPath, ssh
czero := &Cluster{}
capi := &clientSet{}

var authToken string
var watcherAuthToken string
var err error

switch workflowType {
case workflow.AgentWorkflowTypeInstall:
authToken, err = FindAuthTokenFromAssetStore(assetDir)
watcherAuthToken, err = FindAuthTokenFromAssetStore(assetDir)
if err != nil {
return nil, err
}
case workflow.AgentWorkflowTypeAddNodes:
authToken, err = gencrypto.GetAuthTokenFromCluster(ctx, kubeconfigPath)
watcherAuthToken, err = gencrypto.GetAuthTokenFromCluster(ctx, kubeconfigPath)
if err != nil {
return nil, err
}
default:
return nil, fmt.Errorf("AgentWorkflowType value not supported: %s", workflowType)
}

restclient := NewNodeZeroRestClient(ctx, rendezvousIP, sshKey, authToken)
restclient := NewNodeZeroRestClient(ctx, rendezvousIP, sshKey, watcherAuthToken)

kubeclient, err := NewClusterKubeAPIClient(ctx, kubeconfigPath)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/agent/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type NodeZeroRestClient struct {
}

// NewNodeZeroRestClient Initialize a new rest client to interact with the Agent Rest API on node zero.
func NewNodeZeroRestClient(ctx context.Context, rendezvousIP, sshKey, token string) *NodeZeroRestClient {
func NewNodeZeroRestClient(ctx context.Context, rendezvousIP, sshKey, watcherAuthToken string) *NodeZeroRestClient {
restClient := &NodeZeroRestClient{}

// Get SSH Keys which can be used to determine if Rest API failures are due to network connectivity issues
Expand All @@ -48,7 +48,7 @@ func NewNodeZeroRestClient(ctx context.Context, rendezvousIP, sshKey, token stri
Path: client.DefaultBasePath,
}

config.AuthInfo = gencrypto.UserAuthHeaderWriter(token)
config.AuthInfo = gencrypto.WatcherAuthHeaderWriter(watcherAuthToken)

client := client.New(config)

Expand Down Expand Up @@ -137,7 +137,7 @@ func FindAuthTokenFromAssetStore(assetDir string) (string, error) {

var authToken string
if authConfig != nil {
authToken = authConfig.(*gencrypto.AuthConfig).AgentAuthToken
authToken = authConfig.(*gencrypto.AuthConfig).WatcherAuthToken
}

return authToken, nil
Expand Down
25 changes: 18 additions & 7 deletions pkg/asset/agent/gencrypto/auth_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,33 @@ import (
"github.com/pkg/errors"
)

// UserAuthHeaderWriter sets the JWT authorization token.
func UserAuthHeaderWriter(token string) runtime.ClientAuthInfoWriter {
// WatcherAuthHeaderWriter sets the JWT authorization token.
func WatcherAuthHeaderWriter(token string) runtime.ClientAuthInfoWriter {
return runtime.ClientAuthInfoWriterFunc(func(r runtime.ClientRequest, _ strfmt.Registry) error {
return r.SetHeaderParam("Authorization", token)
return r.SetHeaderParam("Watcher-Authorization", token)
})
}

// ParseExpirationFromToken checks if the token is expired or not.
func ParseExpirationFromToken(tokenString string) (time.Time, error) {
// ParseToken checks if the token string is valid or not and returns JWT token claim.
func ParseToken(tokenString string) (jwt.MapClaims, error) {
token, _, err := new(jwt.Parser).ParseUnverified(tokenString, jwt.MapClaims{})
if err != nil {
return time.Time{}, err
return nil, err
}
claims, ok := token.Claims.(jwt.MapClaims)
if !ok {
return time.Time{}, errors.Errorf("malformed token claims in url")
return nil, errors.Errorf("malformed token claims in url")
}
return claims, nil
}

// ParseExpirationFromToken checks if the token is expired or not.
// Returns zero time on error for consistent return type; caller should ignore time on error.
// Otherwise returns the token expiry time.
func ParseExpirationFromToken(tokenString string) (time.Time, error) {
claims, err := ParseToken(tokenString)
if err != nil {
return time.Time{}, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something but I wonder why its necessary to return time on a failure since it seems to be unused on failure in gencrypto/authconfig.go. If its needed please add a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the comments

}
exp, ok := claims["exp"].(float64)
if !ok {
Expand Down
4 changes: 2 additions & 2 deletions pkg/asset/agent/gencrypto/auth_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ func TestParseExpirationFromToken(t *testing.T) {
assert.NotEmpty(t, privateKey)
assert.NoError(t, err)

tokenNoExp, err := generateToken(privateKey, nil)
tokenNoExp, err := generateToken("userAuth", privateKey, nil)
assert.NotEmpty(t, tokenNoExp)
assert.NoError(t, err)

expiry := time.Now().UTC().Add(30 * time.Second)
tokenWithExp, err := generateToken(privateKey, &expiry)
tokenWithExp, err := generateToken("userAuth", privateKey, &expiry)
assert.NotEmpty(t, tokenWithExp)
assert.NoError(t, err)

Expand Down
Loading