Skip to content

Conversation

MNassimM
Copy link
Collaborator

@MNassimM MNassimM commented Apr 29, 2025

  • Kind: feature / refactor / BREAKING CHANGE

Description

This PR introduces Learn-OCaml v2.0 session-based authentication while preserving backward compatibility for existing CLI users:

  • adds a full set of “*_ssession endpoints on both the server and the client;
  • all token → session relationships are stored in a session.json file within the sync directory
  • keeps the original token endpoints, but marks them Upto v"2.0" so old (≤ 1.x) CLI clients continue to use them;
  • the front side now stores only sync-session in the local storage;
  • provides a one-shot migrate_from_legacy_token helper in the front-end that detects an old sync-token, logs in via the new /login route, stores the returned session, fetches the save file, then deletes the legacy sync-token – all with a small “connection preserved” alert.

Checklist

Note to maintainers

  • Read this wiki page.
  • Make sure the PR has a milestone.
  • Assign yourself before merging.
  • Either do a regular merge:
    • for PRs containing several commits following conventional-commits,
    • or for PRs containing 1 commit shared with a later PR (to preserve the SHA1)
  • Or do a squash-merge:
    • for PRs containing only 1 commit (not shared with a later PR),
    • or for PRs containing several commits that need not be kept in the history;
    • Update the commit message header with a conventional-commit type,
    • Add a footer Close #… if a related issue exists.

@erikmd erikmd added the kind: feature New user-facing feature. label Jun 2, 2025
@erikmd
Copy link
Collaborator

erikmd commented Jun 2, 2025

Just FYI @MNassimM, I added three small commits following our recent discussions on the migration to decompress 1.5.3:

  • avoiding the use of De.io_buffer_size that looks unneeded (and local testing is still fine)
  • removing the inotify dependency from the *.locked file as it just doesn't work on macOS,
    see also https://opam.ocaml.org/packages/irmin-watcher/ that says "inotify" {os = "linux"}
  • and adding the cryptokit dependency that was missing in the branch (it was certainly the cause of the CI failure).

Feel free to add other commits in the branch, and I propose that we'll interactively rebase your branch during our meeting tomorrow (in case you have Magit questions!)

MNassimM and others added 13 commits June 3, 2025 11:02
All endpoints previously requiring a token now expect a session.

BREAKING CHANGE: Endpoints and clients using token-based authentication must be updated to use session-based authentication.
All API calls and local_storage interactions in the app/ folder now use session for authentication instead of token.
Add a new  API route that returns the token associated with a given session, enabling clients to retrieve their token server-side without storing it in the frontend.
…or CLI)

Update supported_versions so:
 - legacy token routes are valid Upto v2.0
 - new session routes are valid Since v2.0

Clients ≥ 2.0 negotiate the new API; CLI clients continue to work.

Front-end now uses only the “_s” routes.
This change needed to refactor the decompress-based deflate encoding
Replace the JSON-based storage of session-token-date entries with an Irmin Git-backed key-value store.

Sessions are no longer stored in 'sessions.json' but in an Irmin Git repository ('session_store.git' by default).

Credits: this patch reuses code from oauth-moodle-dev by the following authors.
Co-authored-by: Louis Tariot <[email protected]>
erikmd added 4 commits June 3, 2025 17:23
Motivation:
- Learnocaml_store contained Json_codec
- learnocaml_client.ml relies on Json_codec
- Learnocaml_store depends on Learnocaml_api
- Learnocaml_store pulls irmin-git.unix and cryptokit
  - unlike Learnocaml_api
  - and these two dependencies are unneeded for compiling learnocaml_client.ml
@erikmd erikmd force-pushed the temporary-token branch from baaea6b to 7ce615a Compare June 3, 2025 21:54
Otherwise we'd get:

```
Error loading shared library libgmp.so.10: No such file or directory (needed by /usr/bin/learn-ocaml)
```

cf. https://github.com/ocaml-sf/learn-ocaml/actions/runs/15428646832/job/43421680304?pr=610
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: feature New user-facing feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants