Added JWT auth + /pyewatercycle endpoint #29
Conversation
| lumped: | ||
| type: boolean | ||
| description: Whether model is lumped or non-lumped model | ||
| default: false |
There was a problem hiding this comment.
Perhaps instead of a boolean, we could use something like:
model_type:
type: string
description: Whether model is lumped or gridded (or ...)
default: gridded
There was a problem hiding this comment.
If you can think of a third type then I would switch to an enum. For now boolean is clear enough for me.
| forcing: | ||
| type: string | ||
| description: Directory name of forcing. Directory should contain `ewatercycle_forcing.yaml` file. | ||
| example: marrmot-m01_example_1989-1992_buffalo-river |
There was a problem hiding this comment.
This assumes that a (basic) forcing exists for each model instance in our explorer. An alternative would be to add a "generate forcing" cell. Not sure if we should support both, though. For now, compromise on having a short example forcing for each model?
There was a problem hiding this comment.
It would indeed be nice to generate forcings in notebook, for now I created #30 to add it later.
| if "forcing" in setup: | ||
| forcing = setup["forcing"] | ||
| if not forcing.startswith("/"): | ||
| forcing = forcing_root_dir + "/" + forcing |
There was a problem hiding this comment.
Yep will fix. Dont expect many Windows deployments, but better safe then sorry
| from ewatercycle_experiment_launcher.process import process_notebook | ||
|
|
||
|
|
||
| def notebook(setup: dict, forcing_root_dir: str) -> NotebookNode: |
There was a problem hiding this comment.
why is forcing_root_dir a separate parameter?
There was a problem hiding this comment.
Because it is set from command line, not from a request body. Also to make testing easier the notebook method is written without any flask globals.
Can be removed once eWaterCycle/ewatercycle#248 is implemented.
|
|
||
|
|
||
| def check_auth(username, password, required_scopes=None): | ||
| def check_basic_auth(username, password, required_scopes=None): |
There was a problem hiding this comment.
https://github.com/zalando/connexion/tree/main/examples/openapi3/basicauth used it, but you right also works without.
| timestamp = _current_timestamp() | ||
| payload = { | ||
| "iss": JWT_ISSUER, | ||
| "iat": int(timestamp), |
There was a problem hiding this comment.
Correct, copied it from https://github.com/zalando/connexion/blob/2dfd57dafbedff99c0a32616079f80c21b9de6d9/examples/openapi3/jwt/app.py#L22 which also has double cast
| def _current_timestamp() -> int: | ||
| return int(time.time()) |
There was a problem hiding this comment.
Only used once, perhaps no need for a function?
| @@ -84,16 +84,23 @@ To start launcher use | |||
| ```bash | |||
| # JUPYTERHUB_URL is URL where JupyterHub is running. If path like `/jupyter` then origin header is appended. | |||
| export JUPYTERHUB_URL=http://172.17.0.1:8000 | |||
| # JWT_SECRET is secret with which JWT tokens are encoded/decoded | |||
There was a problem hiding this comment.
I think the sudo on L24 is not needed. Might be better to avoid.
There was a problem hiding this comment.
On L54 I get failed authentication (on WSL2)
There was a problem hiding this comment.
sudo is needed for nodejs installed with apt. Switched to https://github.com/nvm-sh/nvm
| To generate a notebook you need to | ||
| 1. POST to /auth with Basic authentication to receive a JWT token | ||
| 2. POST to a notebook path, like /hello, with `Authorization: Bearer <jwt token>` header. |
There was a problem hiding this comment.
Getting 401 unauthorized for everything I try (WSL problem?)
There was a problem hiding this comment.
Hmm, JH uses the OS user accounts to authenticated against. Which works on my linux box. Not sure how accounts work in WSL. Mabye add user with sudo useradd someone;sudo passwd someone.
Co-authored-by: Peter Kalverla <[email protected]>
Co-authored-by: Peter Kalverla <[email protected]>
|
SonarCloud Quality Gate failed. |
Refs eWaterCycle/terriajs#8
Refs #27