Skip to content

feat: add support for ssh pubkeys storing / validating / retrieving - #1441

Merged
olevski merged 9 commits into
mainfrom
salimkayal/feat/ssh-keys
Oct 13, 2026
Merged

olevski merged 9 commits into
mainfrom
salimkayal/feat/ssh-keys

Conversation

@SalimKayal

@SalimKayal SalimKayal commented Sep 14, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds SSH public keys to data-services: a per-user key store and API, plus the internal endpoints the SSH gateway uses to turn a presented key into a Renku user and check that the user may open a session. This is the integration PR for the SSH keys stack, landing the union of the three stacked PRs (store foundation, user API, proxy endpoints).

Motivation and context

Renku sessions are reachable over SSH through a new gateway. At the first hop the gateway has no user identity yet: it derives one from the public key the user offers. That needs two things data-services did not have: somewhere to register user public keys, and a machine callable endpoint that resolves a key to a user and authorizes the session. Internal services today either run in-process or call data-services with user scoped tokens, and neither fits a caller that has no user context yet.

The deployment already anticipates this: the umbrella Helm chart mounts the Renku internal authn key into the ssh-proxy pod, the same key data-services verifies for internal JWTs. So the proxy authenticates with the existing internal token mechanism, restricted by a new service scope, instead of introducing another credential.

Behavior

  • Users can add, list, and remove their own SSH public keys through /user/ssh_keys. Each key is canonicalized (comment stripped) and fingerprinted exactly as ssh-keygen -l reports.
  • One fingerprint maps to exactly one user, and every key route is owner scoped: an admin cannot read or remove another user's key.
  • The gateway resolves a presented key with POST /internal/ssh_keys/identity, which returns the owning user id (200) or 404. Malformed and unknown keys are both a plain 404, so the proxy has one failure branch.
  • The gateway checks session access with GET /internal/sessions/{session_id}/authorize, which returns 204 for the session owner and 404 otherwise. Not-owner and not-found are indistinguishable, so session existence is never disclosed.
  • Both internal endpoints require an ssh_proxy scoped Renku internal token. User tokens and ordinary session internal tokens are rejected with 401.

Changes

  • Users: users.ssh_keys table and Alembic migration; SSHKey and UnsavedSSHKey models; OpenSSH compatible fingerprint plus key validation; SSHKeyRepository; /user/ssh_keys CRUD blueprint and spec.
  • Authn: RenkuSelfAuthenticator gains an optional required_scope. The existing internal JWT is reused, with no new credential or secret.
  • SSH proxy: a new ssh_proxy component with its own spec and blueprint for the two internal endpoints, plus the SSH_PROXY_SCOPE constant.
  • Wiring: DependencyManager and app.py register the new repository, authenticator, and blueprints; the merged OpenAPI spec includes the new component.
  • Tests: key validation and fingerprint vector tests, repository CRUD and cross user scoping, /user/ssh_keys end to end, service scope accept and reject, and internal endpoint tests (identity 200/404, authorize 401/404/422, owner versus other user).

/deploy

@coveralls

coveralls commented Sep 14, 2026 •

Copy link
Copy Markdown

Coverage Report for CI Build 38417907356

Coverage at 85.264% (no base build to compare)

Details

  • Coverage remained the same as the base build.
  • Patch coverage: 22 uncovered changes across 5 files (359 of 381 lines covered, 94.23%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
components/renku_data_services/app_config/config.py 31 18 58.06%
components/renku_data_services/notebooks/core_sessions.py 62 57 91.94%
components/renku_data_services/notebooks/config/dynamic.py 15 13 86.67%
components/renku_data_services/ssh_proxy/blueprints.py 27 26 96.3%
components/renku_data_services/users/db.py 40 39 97.5%
Total (23 files) 381 359 94.23%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 34841
Covered Lines: 29707
Line Coverage: 85.26%
Coverage Strength: 1.48 hits per line

💛 - Coveralls

@SalimKayal SalimKayal changed the title chore: set feature branch feat: add support for ssh pubkeys storing / validating / retrieving Sep 14, 2026
@SalimKayal
SalimKayal added this pull request to stack #1445 September 14, 2026 12:33
@SalimKayal
SalimKayal force-pushed the salimkayal/feat/ssh-keys branch from 020d506 to f6a35d1 Compare September 17, 2026 14:44
@SalimKayal
SalimKayal force-pushed the salimkayal/feat/ssh-keys branch from f6a35d1 to 5aa5e56 Compare September 24, 2026 09:02
@eikek
eikek force-pushed the salimkayal/feat/ssh-keys branch from 5aa5e56 to 2a06105 Compare September 25, 2026 08:37
@SalimKayal
SalimKayal force-pushed the salimkayal/feat/ssh-keys branch from 2a06105 to c4737aa Compare September 29, 2026 13:20
@SalimKayal
SalimKayal removed this pull request from stack #1445 September 29, 2026 14:59
@SalimKayal
SalimKayal added this pull request to stack #1465 September 29, 2026 15:05
@SalimKayal
SalimKayal force-pushed the salimkayal/feat/ssh-keys branch from c4737aa to b6c64b4 Compare September 29, 2026 16:49
@leafty

leafty commented Sep 30, 2026

Copy link
Copy Markdown
Member

Question:

an admin cannot read [...] another user's key.

Why? A public keys is a public key is public.

Restrictions on admin accessing public keys may make admin work more difficult, e.g. checking suspicious activity linked to a SSH key.

@olevski
olevski marked this pull request as ready for review September 30, 2026 13:49
@olevski
olevski removed this pull request from stack #1465 September 30, 2026 13:50
@olevski
olevski marked this pull request as draft September 30, 2026 13:50
@olevski
olevski force-pushed the salimkayal/feat/ssh-keys branch from 92eca04 to e04664f Compare September 30, 2026 13:57
@SalimKayal

Copy link
Copy Markdown
Collaborator Author

Why? A public keys is a public key is public.

Not really for security reasons but mainly because

  1. it simplifies the code
  2. I see no reason why an admin would actually need to see one.

If it's useful to add this, I can do it.

SalimKayal and others added 6 commits October 2, 2026 09:50
* feat(users): add ssh_keys table and models

* feat(users): validate ssh public keys

* feat(users): add SSHKeyRepository

---------

Co-authored-by: Tasko Olevski <16360283+olevski@users.noreply.github.com>
* feat: export datasets to SciCat (#1432)

* feat: add scicat in api spec

* wip: update route post_deposit

* wip: add ScicatAPIClient with create/get deposit

* wip: add scicat in ProviderKind & update post_deposit for scicat

* wip: add scicat in DepositConfig

* fix: fix bug in _request_saved_secret_creation

* wip: update deposit routes & upload job for scicat

* wip: add init-certificates container

* wip: update ScicatConfig & metadata for configmap

* chore: add __get_scicat_token method

* chore: add alembic migration for scicat

* fix: update generate_code_verifier

* wip: update scicat-cli args & add default_deposit_data

* wip: remove unset oidc_issuer_url and remove test_scicat file

* chore: remove useless ca_certs init container

* wip: small updates

* wip: update scicat upload job config & oidc_issuer_url checks

* wip: simplify ScicatAPIClient

* wip: move fetching deposit data outside of create_deposit_upload

* chore: update scicat urls to production

* chore: add test file for scicat

* fix: fix error with owner reference in _request_saved_secret_creation

* fix: handle errors in get_dc_deposit_logs

* chore: add test_env var in ScicatConfig

* chore: update ScicatAPIClient

* feat(authn): support service-scoped internal tokens for the ssh proxy

* feat(ssh_proxy): add internal identity and session authorization endpoints

* fix(ssh_proxy): use canonical ErrorResponse to satisfy apispec merge

* refactor: consistency

* revert: remove authn for service account for now

* refactor: simplify api spec

* fix(users): comment precision

* refactor(ssh-proxy): serve authorize from the internal secret service

* squashme: minor fixes

---------

Co-authored-by: boucherjeanphi <49230173+boucherjeanphi@users.noreply.github.com>
Co-authored-by: Tasko Olevski <16360283+olevski@users.noreply.github.com>
* Add frontend variant as session label

* Don't comment on the pr due to token issue

* Return frontend_variant with session details

* reactivate pr comment

* Add ssh to the frontend options

* Extend builder-frontend combination

* Add test for ssh build

* Change frontend_variant label on patch
…1458)

* Don't comment on the pr due to token issue

* reactivate pr comment

* feat(notebooks): mount proxy-to-session keys only for ssh frontends

* feat: skip the auth proxy and ingress for ssh sessions

* squashme: modify auth/ingress on patching

---------

Co-authored-by: Eike Kettner <eike.kettner@sdsc.ethz.ch>
Co-authored-by: Tasko Olevski <16360283+olevski@users.noreply.github.com>
@olevski
olevski force-pushed the salimkayal/feat/ssh-keys branch from e40dd91 to fa1a071 Compare October 2, 2026 07:51
@RenkuBot

RenkuBot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

You can access the deployment of this PR at https://renku-ci-ds-1441.dev.renku.ch

@SalimKayal
SalimKayal marked this pull request as ready for review October 2, 2026 15:19
@SalimKayal
SalimKayal requested review from a team and sgaist as code owners October 2, 2026 15:19
@olevski
olevski merged commit 677d482 into main Oct 13, 2026
58 of 61 checks passed
@olevski
olevski deleted the salimkayal/feat/ssh-keys branch October 13, 2026 16:08
@RenkuBot

Copy link
Copy Markdown
Contributor

Tearing down the temporary RenkuLab deployment for this PR.

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.

6 participants