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

Stops display of the token if not JWT and if Openshift cluster fetches the username from the master API #275

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

phantomjinx
Copy link
Member

@phantomjinx phantomjinx commented Dec 20, 2023

Asks the Openshift cluster for the username associated with the given token.

If using form-login and connecting to Openshift, it is possible to obtain the username associated with the login token using the OS REST API.

Adds a small function to the form-service.ts that fetches the username upon successful login.

Screenshot_20231220_190358

@phantomjinx
Copy link
Member Author

Was experimenting with the cluster REST API and tried this out. Seems to work correctly but will probably need tidying up if we want to include it. Anyway, post what you think of it.

Copy link
Member

@tadayosi tadayosi left a comment

Choose a reason for hiding this comment

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

The username resolution logic looks good, but there are other comments that need to be addressed.

@tadayosi
Copy link
Member

And while it's fine to support such case, it's important that it's just an edge case and should never impede the normal case with K8s and serviceaccount token by adding extra load.

@phantomjinx phantomjinx force-pushed the username-openshift-api-exp branch 2 times, most recently from 24dd89d to edf3ce2 Compare February 27, 2024 13:40
@phantomjinx
Copy link
Member Author

And while it's fine to support such case, it's important that it's just an edge case and should never impede the normal case with K8s and serviceaccount token by adding extra load.

Despite attempts to avoid an enquiry of the API Server to determine if it belongs to an OpenShift cluster, I have found no other way to get that result. The form config makes no mention of the type of cluster (we could include it but then it is dependent on the user correctly supplying it).

So the latest version:

  • no longer displays the token as the subject;
  • Evaluates the response of the base /api to determine if it contains the core OpenShift groups;
  • Only in the event that the cluster is OpenShift will it then enquire as to the username.

@phantomjinx phantomjinx force-pushed the username-openshift-api-exp branch from edf3ce2 to 936bf87 Compare February 27, 2024 13:47
@phantomjinx phantomjinx marked this pull request as ready for review February 27, 2024 13:47
@phantomjinx phantomjinx changed the title experiment: fetches the username for the token from Openshift cluster Stops display of the token if not JWT and if Openshift cluster fetches the username from the master API Feb 27, 2024
@tadayosi
Copy link
Member

@phantomjinx

Despite attempts to avoid an enquiry of the API Server to determine if it belongs to an OpenShift cluster, I have found no other way to get that result. The form config makes no mention of the type of cluster (we could include it but then it is dependent on the user correctly supplying it).

Just an idea to explore: What about using local storage and cache the result on whether it's OCP or K8s there? The reasoning is that local storage is tied to a hostname and it's highly unlikely that the same hostname changes the type of cluster. If the result is cached, we only need to inquire once. If a similar inquiry has been done in other places in hawtio-online, we might also be able to use the cached result to remove extra burden on API server further.

@tadayosi
Copy link
Member

Yet another option might be that the nginx server holds the result of the inquiry [1] somewhere on the server in some way and provides an extra endpoint like /cluster-type which returns whether it's on OCP or K8s. That way we only need to inquire to the nginx server but not to the API server.

[1]

check_openshift_api() {
APISERVER="https://${CLUSTER_MASTER:-kubernetes.default.svc}"
SERVICEACCOUNT=/var/run/secrets/kubernetes.io/serviceaccount
TOKEN=$(cat ${SERVICEACCOUNT}/token)
CACERT=${SERVICEACCOUNT}/ca.crt
STATUS_CODE=$(curl --cacert ${CACERT} --header "Authorization: Bearer ${TOKEN}" -X GET ${APISERVER}/apis/apps.openshift.io/v1 --write-out '%{http_code}' --silent --output /dev/null)
if [ "${STATUS_CODE}" != "200" ]; then
OPENSHIFT=false
fi
echo OpenShift API: ${OPENSHIFT} - ${STATUS_CODE} ${APISERVER}/apis/apps.openshift.io/v1
}
check_openshift_api

…o#271)

* In the corner-case that form-login is used to login to an OpenShift
  cluster, JWT decode is not able to decrypt it to provide the username.
  However, it is possible to fetch the user using the Openshift API.

* To discover the subject:
  1. Try retrieving the username using JWT decode;
  2. Determine if the cluster is OpenShift by asking the master URI
  3. In the event that the cluster is OpenShift then ask the master URI
     for the username
@phantomjinx phantomjinx force-pushed the username-openshift-api-exp branch from 936bf87 to e41053d Compare February 28, 2024 19:29
@phantomjinx
Copy link
Member Author

Went with the second approach, ie. injecting the flag from the server by adding it to osconsole/config.json.

* Since nginx host server already enquires as whether the cluster is
  OpenShift, make this property available to hawtio by adding it to the
  osconsole/config.json

* The effect is that the form-service no longer has to interrogate the
  master API to determine the same result.
@phantomjinx phantomjinx force-pushed the username-openshift-api-exp branch from e41053d to d925249 Compare February 28, 2024 19:31
@phantomjinx phantomjinx merged commit 6cd0728 into hawtio:main Feb 29, 2024
3 checks passed
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