feat: use self-minted tokens for sessions#1249
feat: use self-minted tokens for sessions#1249leafty merged 47 commits intobuild/feat-self-mint-tokensfrom
Conversation
…tor' into leafty/feat-self-mint-tokens # Conflicts: # bases/renku_data_services/data_api/app.py # bases/renku_data_services/data_api/dependencies.py # components/renku_data_services/app_config/config.py # components/renku_data_services/authn/api/api.spec.yaml # components/renku_data_services/authn/api/apispec.py # components/renku_data_services/authn/api/blueprints.py # components/renku_data_services/authn/renku.py # test/components/renku_data_services/authn/test_renku.py
Co-authored-by: Samuel Gaist <samuel.gaist@idiap.ch>
Co-authored-by: Samuel Gaist <samuel.gaist@idiap.ch>
…tor' into leafty/feat-self-mint-tokens
Details: * Add the `RenkuSelfAuthenticator` class which authenticates internal tokens. (This is a new `authenticator`). * Add the `RenkuSelfTokenMint` class which can create internal tokens. Tokens are signed with the `HS512` algorithm. * Add the `POST /api/data/internal/authentication/token` endpoint to renew internal tokens. This endpoint has the same OpenAPI definition as `POST /api/data/oauth2/connections/:connection_id/token_endpoint`. OAuth 2.0 token endpoint to support applications running in sessions * Update the `pyjwt` dependency. Note: the next PRs will make use of the new internal tokens for `git-proxy`, `remote-session-controller` and `csi-rclone`.
…o leafty/feat-self-mint-tokens # Conflicts: # components/renku_data_services/app_config/config.py # components/renku_data_services/authn/api/apispec.py
Coverage Report for CI Build 24511689261Warning No base build found for commit Coverage: 86.505%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
d03cb1d to
d370f20
Compare
…o leafty/feat-self-mint-tokens
| return user | ||
|
|
||
| # NOTE: user is not None since there is at least one authenticator in the chain | ||
| assert user is not None |
There was a problem hiding this comment.
Is it a test leftover ?
Otherwise it should be turned into an exception.
There was a problem hiding this comment.
I don't think I follow: the assert will raise in tests but not in production mode and this assert makes a type assertion: user is None cannot happen, so the fact that nothing runs in production is OK.
olevski
left a comment
There was a problem hiding this comment.
One small change but we can live without that also. Up to you if you want to use it or not.
| class ChainedAuthenticator(Authenticator[AnyAPIUser]): | ||
| """Chain authenticators until a user is authenticated or all authenticators are tried.""" | ||
|
|
||
| token_field: str = "__not_used__" |
There was a problem hiding this comment.
This field should also never be changed here, right? And remember to import Final from typing and field from dataclasses
| token_field: str = "__not_used__" | |
| token_field: Final[str] = field(default="__not_used__", init=False) |
There was a problem hiding this comment.
...not sure if init=False makes it a bit too weird. Like it is a child of Authenticator but the API is now different. Maybe that is too much. And just Final is enough.
There was a problem hiding this comment.
The value of the field is never used, so it doesn't matter if it is changed. The value is there to indicate that it's not used directly but rather each authenticator in the chain will use its own value.
| [dict(id=provider.id, access_token_url=provider.access_token_url) for provider in git_providers] | ||
| ), | ||
| ), | ||
| client.V1EnvVar(name=f"{prefix}RENKU_AUTHENTICATION_VERSION", value="v2"), |
There was a problem hiding this comment.
That's neat. We can handle changes to the internal auth better if there are any in the future.
There was a problem hiding this comment.
Yes, though I will probably wipe the old code after a release. 😄
Use self-minted tokens for sessions.
Details:
POST /api/data/oauth2/connections/:connection_id/token_endpointAPI endpoint is updated to support only internal refresh tokensPR Stack: