Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,11 @@
// Make sure SELinux does not disable with access to host filesystems like tmp
"--security-opt=label=disable"
],
"forwardPorts": [
5173
],
"portsAttributes": {
"5173": {
"onAutoForward": "ignore"
}
},
"mounts": [
// Mount in the user terminal config folder so it can be edited
{
Expand Down
12 changes: 12 additions & 0 deletions notes
Comment thread
Matt-Carre marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# refresh_token: dict[str, str] = (

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

are these notes intended to be committed?

# # pyright: ignore[reportUnknownVariableType]
# keycloak_openid.refresh_token( # pyright: ignore[reportUnknownMemberType]
# token["refresh_token"]
# )
# )
# print(refresh_token)

# save token and refresh token to file
# if they're present, check if access token is valid, if no but refresh token is
# , just refresh token
# if invalid, run whole thing
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ dependencies = [
"hera",
"requests",
"PyYAML",
"python-keycloak",
] # Add project dependencies here, e.g. ["click", "numpy"]
dynamic = ["version"]
license.file = "LICENSE"
Expand Down
181 changes: 0 additions & 181 deletions src/python_interface_to_workflows/auth/get_token.py

This file was deleted.

47 changes: 47 additions & 0 deletions src/python_interface_to_workflows/auth/keycloak_checker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import os

from keycloak import KeycloakOpenID
from keycloak.pkce_utils import generate_code_challenge, generate_code_verifier

from python_interface_to_workflows.auth.open_auth_url import open_auth_url


def return_key(dev: bool) -> str:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think that we should configure tooling to grant ordinary users access to the staging cluster.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

style suggestion: I'm not sure that I would correctly guess what keycloak_check.return_key does from this name without reading the code. consider a more descriptive name.

match dev:
case True:
keycloak_openid = KeycloakOpenID(
server_url="https://identity-test.diamond.ac.uk/",
client_id="workflows-ui-dev",
realm_name="dls",
client_secret_key="",
pool_maxsize=1,
)
case False:
keycloak_openid = KeycloakOpenID(
client_id="workflows-dashboard",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

for now it would be better to use workflows-cli.

we maybe should consider a new client for the python interface.

server_url="https://identity-test.diamond.ac.uk/",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this client is intended for use on indentity, not identity-test

realm_name="dls",
client_secret_key="",
pool_maxsize=1,
)
code_verifier = generate_code_verifier()
code_challenge, code_challenge_method = generate_code_challenge(code_verifier)
auth_url = keycloak_openid.auth_url(
redirect_uri="http://localhost:5173/",
scope="openid posix-uid profile email fedid",
state="",
code_challenge=code_challenge,
code_challenge_method=code_challenge_method,
)
open_auth_url(auth_url)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what happens when a user is submitting from a headless terminal?

token: dict[str, str] = ( # pyright: ignore[reportUnknownVariableType]
keycloak_openid.token( # pyright: ignore[reportUnknownMemberType]
grant_type="authorization_code",
code=os.environ["AUTH"],
redirect_uri="http://localhost:5173/",
code_verifier=code_verifier,
)
)
os.environ["REFRESH"] = token["refresh_token"]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why are you storing this in the python process environment variables?

os.environ["TOKEN"] = token["access_token"]
return token["access_token"] # pyright: ignore[reportUnknownArgumentType]
41 changes: 41 additions & 0 deletions src/python_interface_to_workflows/auth/open_auth_url.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import os
import socket
import urllib.parse
import webbrowser
from http.server import BaseHTTPRequestHandler, HTTPServer
from typing import cast


class _ReusingHTTPServer(HTTPServer):
allow_reuse_address = True
auth_code: str


class CallbackHandler(BaseHTTPRequestHandler):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

does this need to be public?

def do_GET(self):
query = urllib.parse.urlparse(self.path).query
params = urllib.parse.parse_qs(query)
if "code" in params:
cast(_ReusingHTTPServer, self.server).auth_code = params["code"][0]
self.send_response(200)
self.end_headers()
self.wfile.write(b"Authorization successful. You can close this window.")
else:
self.send_response(400)
self.end_headers()
self.wfile.write(b"Missing authorization code.")


def open_auth_url(auth_url: str):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

missing return type hint

httpd = _ReusingHTTPServer(("localhost", 5173), CallbackHandler)
webbrowser.open(auth_url)
try:
httpd.handle_request()
os.environ["AUTH"] = httpd.auth_code
except OSError:
os.environ["AUTH"] = ""
print("ERROR: Port in use. Please restart your terminal.")
exit(1)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

style suggestion: consider the principle of least surprise: would a caller of this function expect it to exit the process on error?

finally:
httpd.socket.shutdown(socket.SHUT_RDWR)
httpd.server_close()
5 changes: 4 additions & 1 deletion src/python_interface_to_workflows/submit_to_graphql.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
from gql import Client, gql
from gql.transport.aiohttp import AIOHTTPTransport

from python_interface_to_workflows.auth.keycloak_checker import return_key


def submit_to_graphql():
token = return_key(dev=True)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

comment: the current implementation requires the user to open a browser and log in every time that submit_to_graphql is called. if you are not already planning to do so, you may consider revising this approach in future PRs.

transport = AIOHTTPTransport(
url="https://staging.workflows.diamond.ac.uk/graphql",
headers={"Authorization": "Bearer "}, # after Bearer = access token
headers={"Authorization": f"Bearer {token}"},
)
client = Client(
transport=transport,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import os

from hera.shared import global_config
from hera.workflows import (
DAG,
Expand All @@ -10,13 +12,13 @@
from hera.workflows import models as m
from hera.workflows.archive import NoneArchiveStrategy

global_config.host = "https://argo-workflows.staging.workflows.diamond.ac.uk/"
global_config.token = ""
global_config.image = "python:3.10"
global_config.host = os.environ.get("HOST")
global_config.image = str(os.environ.get("IMAGE"))
global_config.token = os.environ.get("TOKEN")


@script(
image="python:3.10",
image=str(os.environ.get("IMAGE")),
command=["python"],
volume_mounts=[m.VolumeMount(name="tmpdir", mount_path="/tmp")],
)
Expand All @@ -33,7 +35,7 @@ def install_dependencies():

@script(
command=["python"],
image="python:3.10",
image=str(os.environ.get("IMAGE")),
outputs=Parameter(
name="out-parameters", value_from=m.ValueFrom(path="/tmp/parameters.json")
),
Expand Down Expand Up @@ -74,7 +76,7 @@ def generate_parameters(

@script(
command=["/tmp/venv/bin/python"],
image="python:3.10",
image=str(os.environ.get("IMAGE")),
volume_mounts=[m.VolumeMount(name="tmpdir", mount_path="/tmp")],
outputs=[
Parameter(
Expand Down Expand Up @@ -125,7 +127,7 @@ def create_pattern(

@script(
command=["/tmp/venv/bin/python"],
image="python:3.10",
image=str(os.environ.get("IMAGE")),
volume_mounts=[m.VolumeMount(name="tmpdir", mount_path="/tmp")],
outputs=Artifact(
name="hdf5output",
Expand Down Expand Up @@ -155,7 +157,7 @@ def to_hdf5(paths: str):
with Workflow(
generate_name="hera-example-", # when running on graphql this should be name
entrypoint="workflowentry",
namespace="ks10000-3",
namespace=str(os.environ("NAMESPACE")),
api_version="argoproj.io/v1alpha1",
kind="Workflow", # ClusterWorkflowTemplate", when on graphql
labels={"workflows.diamond.ac.uk/science-group-examples": "true"},
Expand Down
Loading
Loading