mirror of
https://github.com/ARUP-CAS/aiscr-qgis-amcr-viewer.git
synced 2026-10-09 20:37:37 +02:00
* fix: před stahováním ověřit přihlášení přes islogged Server při vypršelé session nevrací chybu, ale tiše odpoví jako anonymnímu uživateli (jen přístupnost A). Plugin proto před stahováním volá /api/user/islogged; při 'nologged' se jednou znovu přihlásí z uložených údajů, a když to nejde, varuje v liště, že stahuje anonymně. - amcr_tools: _check_islogged, _ensure_logged_in, volání v load_amcr_data - smoke test: 7 offline scénářů stavu přihlášení - README; changelog v2.2.0 v metadata.txt - openspec/changes/fix-session-expiry-islogged: návrh, spec, design, úkoly Closes #72. Připraveno s pomocí AI (Claude), ručně zkontrolováno. * feat: odebrání přihlašovacích údajů uživatele i odhlásí Dosud zůstala přihlášená session v paměti až do restartu QGIS, takže se po „Odebrat uložené přihlašovací údaje“ dál stahovalo jako přihlášený. Nově se session odhlásí na serveru (GET /api/user/logout) a zahodí; když server neodpoví, zahodí se aspoň lokálně. - amcr_tools: logout_from_api; amcr_dialog: volání v _forget_credentials - smoke test: odhlášení, chyba sítě, bez session - README, changelog v2.2.0, OpenSpec (požadavek + úkoly 2b; 3.3 ověřeno) Připraveno s pomocí AI (Claude), ručně zkontrolováno. * chore: archivovat OpenSpec změnu fix-session-expiry-islogged Všechny úkoly hotové (2b.2 – odhlášení – ověřeno ručně v QGIS), archivováno s --skip-specs (stupeň change-tracked, bez openspec/specs/). Připraveno s pomocí AI (Claude), ručně zkontrolováno.
This commit is contained in:
1 parent
e5b0716fd6
commit
74090a80c6
10 files changed
+614
-10
No files matched your search
@@ -0,0 +1,2 @@
|
||||
schema: spec-driven
|
||||
created: 2026-10-02
|
||||
@@ -0,0 +1,81 @@
|
||||
# Design
|
||||
|
||||
## Context
|
||||
|
||||
- The session lives only in memory (`amcr_tools.AMCR_SESSION`, a
|
||||
`requests.Session` with the `JSESSIONID` cookie). `_get_session()` logs in
|
||||
from stored credentials only when no session object exists, so after QGIS
|
||||
start the first download always logs in; an expired session object is
|
||||
reused forever.
|
||||
- All data requests of a download go through `_api_get_json()` (main query
|
||||
pages and PIAN batches). `_is_auth_error()` there reacts to HTTP 401 or
|
||||
error text – neither occurs on expiry (see proposal.md – Why).
|
||||
- Server behaviour (verified 2026-10-02, live API):
|
||||
`GET /api/user/islogged` → `{"remaining": <s>}` when logged in,
|
||||
`{"error": "nologged"}` otherwise, both HTTP 200. It does not extend the
|
||||
session; any `search/query` does.
|
||||
- Codelists (`amcr_codelists.py`) use plain `requests.get` without the
|
||||
session – unaffected by login state.
|
||||
|
||||
## Goals / Non-Goals
|
||||
|
||||
**Goals:**
|
||||
- One check at the start of `load_amcr_data`, before the first data request.
|
||||
- Reuse existing login code (`login_to_api`, `LoginDialog.get_credentials`).
|
||||
- Testable offline: the check takes its HTTP behaviour from the session
|
||||
object so the smoke test can inject a fake.
|
||||
|
||||
**Non-Goals:**
|
||||
- Checking before every page / PIAN batch (a download takes seconds to
|
||||
minutes and every data request renews the sliding timeout).
|
||||
- Refactoring session handling into a class.
|
||||
|
||||
## Decisions
|
||||
|
||||
1. **New helper `_ensure_logged_in() -> str`** in `amcr_tools.py`, returning
|
||||
one of `"anonymous"` (no session, no credentials – nothing to check),
|
||||
`"logged_in"`, `"relogged"`, `"fallback"` (expected login, ended
|
||||
anonymous), `"unknown"` (check failed, proceeding).
|
||||
Flow: get session via `_get_session()` (logs in if needed); if none and no
|
||||
credentials → `anonymous`; if none but credentials → login failed →
|
||||
`fallback`; otherwise call `islogged`; `remaining` → `logged_in`;
|
||||
`nologged` → drop session, re-login once, verify again → `relogged` or
|
||||
`fallback`; exception / non-JSON → `unknown`.
|
||||
*Alternative:* re-login unconditionally before each download – simpler,
|
||||
but one POST with the password per download and no way to distinguish a
|
||||
real failure; rejected.
|
||||
*Alternative:* compare `remaining` with a local timestamp of last request
|
||||
– fragile (server timeout may change); rejected.
|
||||
2. **Caller decides UI.** `load_amcr_data` pushes the message bar warning on
|
||||
`fallback`; the helper only logs (keeps it free of `iface` for the test).
|
||||
3. **Interpretation of the response:** logged in iff the body is a dict with
|
||||
key `remaining`. Anything else with an `error` key → not logged in. Unknown
|
||||
shape → `unknown` (do not trigger a re-login loop on a format change).
|
||||
4. **Keep `_is_auth_error`** as a fallback, with a comment that the current
|
||||
server never triggers it; removing it brings no benefit and it still
|
||||
covers a possible future 401.
|
||||
5. **Never log the response of `islogged?wantsUser=true`** – we do not use
|
||||
that parameter at all; only `remaining` (number) is logged.
|
||||
|
||||
6. **Logout on credential removal** – new `logout_from_api()` in
|
||||
`amcr_tools.py` called from `LoginDialog._forget_credentials`. The local
|
||||
session is dropped first and unconditionally; the server call is best
|
||||
effort (a failure is logged and reported in the dialog text). Without
|
||||
it, the in-memory session would keep downloading logged-in data until
|
||||
QGIS restarts even though the user believes he is "forgotten".
|
||||
|
||||
## Risks / Trade-offs
|
||||
|
||||
- [Extra request per download] → only for logged-in / credential users; cost
|
||||
~100 ms.
|
||||
- [Session expires during a very long download] → practically impossible:
|
||||
each page request renews the 1 h sliding timeout.
|
||||
- [Re-login prompts for the QGIS master password] → `get_credentials()` is
|
||||
already called on first download after start; behaviour unchanged.
|
||||
- [`islogged` endpoint changes shape] → `unknown`, logged warning, download
|
||||
proceeds as today (no regression).
|
||||
|
||||
## Migration Plan
|
||||
|
||||
Plain plugin update; no settings or data migration. Rollback = previous
|
||||
release.
|
||||
@@ -0,0 +1,59 @@
|
||||
# Proposal
|
||||
|
||||
## Why
|
||||
|
||||
Login to digiarchiv expires after 1 h of inactivity (`sessionTimeout: 3600`)
|
||||
and the server then silently treats the request as anonymous: HTTP 200, no
|
||||
`error`, only `pristupnost=A` data. The plugin detects expiry only by HTTP 401
|
||||
or error text, which never arrives, so a logged-in user who downloads again
|
||||
after a pause gets incomplete data without any warning (issue #72, verified
|
||||
manually in QGIS and against the live API).
|
||||
|
||||
## What Changes
|
||||
|
||||
- Before each download the plugin checks the login state with
|
||||
`GET /api/user/islogged` whenever the user is (or should be) logged in –
|
||||
i.e. an in-memory session exists or credentials are stored.
|
||||
- When the server answers `{"error": "nologged"}` and credentials are stored,
|
||||
the plugin logs in again and continues the download with the new session.
|
||||
- When the re-login fails (or credentials are missing), the plugin warns in
|
||||
the QGIS message bar that the download runs anonymously and returns only
|
||||
records with access level A – not only in the log.
|
||||
- Removing the stored credentials (*Odebrat uložené přihlašovací údaje*)
|
||||
also logs the session out on the server (`GET /api/user/logout`) and
|
||||
drops it from memory; today it stays logged in until QGIS restarts.
|
||||
- When the check itself cannot be completed (network error, invalid JSON),
|
||||
the download is not blocked; the plugin logs a warning and proceeds.
|
||||
- The existing error-text based detection (`_is_auth_error`) stays as
|
||||
a fallback; it is documented as not triggered by the current server.
|
||||
- Version bump + changelog (`metadata.txt`, `CITATION.cff`).
|
||||
|
||||
Out of scope:
|
||||
|
||||
- Keeping the session alive in the background (polling `islogged` does not
|
||||
extend it anyway).
|
||||
- Showing the user's access level in the UI (`islogged?wantsUser=true`).
|
||||
- Codelist updates: `amcr_codelists` calls the API with plain `requests`
|
||||
without the session, so login state does not affect them today.
|
||||
|
||||
## Capabilities
|
||||
|
||||
### New Capabilities
|
||||
|
||||
- `amcr-session`: login session against digiarchiv – validating the session
|
||||
before a download, transparent re-login and informing the user when data
|
||||
are downloaded anonymously.
|
||||
|
||||
### Modified Capabilities
|
||||
|
||||
<!-- none – openspec/specs/ is empty -->
|
||||
|
||||
## Impact
|
||||
|
||||
- Code: `amcr_viewer/amcr_dialog.py` (logout when credentials are
|
||||
removed); `amcr_viewer/amcr_tools.py` (logout helper, new login-state check, call at the start
|
||||
of `load_amcr_data`, message bar warning); `tests/smoke_test.py` (offline
|
||||
test of the check with a mocked HTTP session).
|
||||
- API: one extra `GET /api/user/islogged` per download, only when the user is
|
||||
logged in or has stored credentials; anonymous users are unaffected.
|
||||
- No new dependencies; Qt5/Qt6 compatibility rules from `AGENTS.md` apply.
|
||||
+70
@@ -0,0 +1,70 @@
|
||||
# Spec Delta
|
||||
|
||||
## Purpose
|
||||
|
||||
Keeps a logged-in user's data downloads from digiarchiv running under a valid
|
||||
login, and makes it visible when a download falls back to anonymous access.
|
||||
|
||||
## ADDED Requirements
|
||||
|
||||
### Requirement: Login state is verified before a download
|
||||
Before starting a data download, the plugin SHALL ask the server whether the
|
||||
current session is logged in, whenever an in-memory session exists or login
|
||||
credentials are stored. Users with neither SHALL download anonymously without
|
||||
this check.
|
||||
|
||||
#### Scenario: Valid session
|
||||
- **WHEN** a session exists and the server reports it as logged in
|
||||
- **THEN** the download proceeds with that session and no re-login happens
|
||||
|
||||
#### Scenario: Anonymous user without stored credentials
|
||||
- **WHEN** no session exists and no credentials are stored
|
||||
- **THEN** no login-state request is sent and the download proceeds anonymously without a warning
|
||||
|
||||
### Requirement: Expired login is renewed transparently
|
||||
When the server reports the session as not logged in and credentials are
|
||||
stored, the plugin SHALL log in again and run the whole download with the new
|
||||
session.
|
||||
|
||||
#### Scenario: Session expired after inactivity
|
||||
- **WHEN** the user downloads data more than one hour after the previous download within the same QGIS run
|
||||
- **THEN** the plugin logs in again with the stored credentials and the download returns the same records as for a fresh login
|
||||
|
||||
#### Scenario: No session yet, credentials stored
|
||||
- **WHEN** the first download after QGIS start is requested and credentials are stored
|
||||
- **THEN** the plugin logs in and verifies that the new session is logged in before downloading
|
||||
|
||||
### Requirement: Anonymous fallback is reported to the user
|
||||
When the plugin expected to be logged in but cannot obtain a logged-in
|
||||
session, it SHALL show a warning in the QGIS message bar stating that the
|
||||
download runs anonymously and contains only records with access level A.
|
||||
|
||||
#### Scenario: Re-login fails
|
||||
- **WHEN** the session has expired and logging in again with stored credentials fails
|
||||
- **THEN** a warning appears in the message bar and the download continues anonymously
|
||||
|
||||
#### Scenario: Session expired and credentials removed
|
||||
- **WHEN** an in-memory session has expired and no credentials are stored any more
|
||||
- **THEN** a warning appears in the message bar and the download continues anonymously
|
||||
|
||||
### Requirement: Failed state check does not block the download
|
||||
If the login-state check cannot be completed (network error or a response
|
||||
that is not valid JSON), the plugin SHALL log a warning and proceed with the
|
||||
download using the current session.
|
||||
|
||||
#### Scenario: Login-state endpoint unreachable
|
||||
- **WHEN** the login-state request fails with a network error
|
||||
- **THEN** a warning is written to the log and the download is attempted as usual
|
||||
|
||||
### Requirement: Removing stored credentials logs the user out
|
||||
When the user removes the stored credentials, the plugin SHALL log the
|
||||
current session out on the server and discard it, so that later downloads
|
||||
run anonymously without restarting QGIS.
|
||||
|
||||
#### Scenario: Credentials removed while logged in
|
||||
- **WHEN** the user removes the stored credentials while a logged-in session exists
|
||||
- **THEN** the session is logged out on the server and the next download is anonymous without a warning
|
||||
|
||||
#### Scenario: Server unreachable during logout
|
||||
- **WHEN** the logout request fails with a network error
|
||||
- **THEN** the session is still discarded locally and the user is told the next download will be anonymous
|
||||
@@ -0,0 +1,53 @@
|
||||
# Tasks
|
||||
|
||||
## 1. Login-state check
|
||||
|
||||
- [x] 1.1 Add `_ensure_logged_in()` to `amcr_viewer/amcr_tools.py` per
|
||||
design.md (statuses `anonymous` / `logged_in` / `relogged` / `fallback` /
|
||||
`unknown`, `GET /api/user/islogged` with the current session, one re-login
|
||||
on `nologged`); verify with `python3 tests/check_sources.py` and
|
||||
`ruff check .`
|
||||
- [x] 1.2 Add a comment to `_is_auth_error` that the current server never
|
||||
returns such an error on expiry and the check is kept as a fallback;
|
||||
verify by reading the diff
|
||||
- [x] 1.3 Extend `tests/smoke_test.py` with offline cases using a fake
|
||||
session object (valid session, `nologged` + successful re-login,
|
||||
`nologged` + failed re-login, no credentials, network error); verify the
|
||||
smoke test passes in `qgis/qgis:ltr` and `qgis/qgis:stable`
|
||||
|
||||
## 2. Integration into the download
|
||||
|
||||
- [x] 2.1 Call `_ensure_logged_in()` in `load_amcr_data` after the
|
||||
re-entrancy guard, before the first query; on `fallback` push a message
|
||||
bar warning (Czech, scoped `Qgis.MessageLevel.Warning`) that the download
|
||||
runs anonymously and contains only access level A; verify by smoke test
|
||||
and code review
|
||||
- [x] 2.2 Live check without credentials: anonymous download path sends no
|
||||
`islogged` request and a made-up `JSESSIONID` yields `nologged`
|
||||
(curl / probe script in scratch); verify outputs recorded in the PR
|
||||
- [x] 2.3 Update `README.md` if it describes login/session behaviour; verify
|
||||
the text matches the new behaviour (or note that nothing needed changing)
|
||||
|
||||
## 2b. Logout when credentials are removed
|
||||
|
||||
- [x] 2b.1 Add `logout_from_api()` to `amcr_tools.py` and call it from
|
||||
`LoginDialog._forget_credentials`; extend the smoke test (session
|
||||
logged out + dropped, network error still drops it, no session = no
|
||||
request); update README and changelog; verify smoke test ltr + stable
|
||||
- [x] 2b.2 Manual test in QGIS: log in, download, remove the stored
|
||||
credentials, download again; verify the log shows "Uživatel odhlášen"
|
||||
and the count drops to the anonymous one
|
||||
|
||||
## 3. Release preparation and verification
|
||||
|
||||
- [x] 3.1 Add changelog entries under v2.2.0 in `amcr_viewer/metadata.txt`
|
||||
(the fix ships with 2.2.0; `CITATION.cff` already says 2.2.0 and
|
||||
`date-released` moves on release day); verify both versions match
|
||||
- [x] 3.2 Run the full local check set from `AGENTS.md` (check_sources,
|
||||
bandit, detect-secrets `--all-files`, flake8 `--isolated`, ruff,
|
||||
pyqgis4-checker log empty, smoke test ltr + stable); verify all clean
|
||||
- [x] 3.3 Manual test in QGIS with a researcher account: download SN for
|
||||
whole CZ, simulate expiry in the Python console with
|
||||
`amcr_tools.AMCR_SESSION.get("https://digiarchiv.aiscr.cz/api/user/logout")`,
|
||||
download again; verify log shows re-login and the count matches the
|
||||
logged-in count (not the anonymous one)
|
||||
Reference in new issue
Block a user