* feat(auth): display Google Workspace groups on /profile
- Request cloud-identity.groups.readonly scope in Google OAuth
- Fetch groups via Cloud Identity API after callback; tolerate 4xx
(non-Workspace tenants) and network errors — never break login
- Store result in Starlette session as google_groups
- Replace /profile redirect with a real profile page rendering
account details (email, name, role) and the group list; show a
friendly empty state when no groups are available
- Tests: helper parsing + 403 + exception paths; profile page
smoke test; updated the old redirect test
* test: remove stale /profile redirect tests
Cherry-pick of Zdeněk's 4f7e4cd ("display Google Workspace groups on
/profile") replaces the /profile redirect with a real profile page —
but only updated one of three tests that expected the old behaviour.
These two tests in test_admin_tokens_ui.py and test_pat.py were left
asserting `/profile → 302 /tokens`, which now returns
`/profile → 302 /login?next=%2Fprofile` for unauth users (the standard
auth guard) or `/profile → 200 HTML` for authenticated users.
Removed both rather than patched — coverage for the new behaviour
already exists in tests/test_auth_providers.py (added by the same
commit). The /tokens render assertions in the deleted test_pat.py case
are redundant with test_admin_tokens_ui.py's own /tokens UI tests.
* fix(auth): Google groups search query needs parent + labels predicates
Cloud Identity Groups Search API returns 400 INVALID_ARGUMENT when the
CEL query lacks the required `parent == 'customers/<id>'` predicate AND
a `'<label>' in labels` membership predicate. Zdeněk's original 4f7e4cd
query had only `member_key_id == '<email>'` — every fetch silently
returned [] and the /profile groups list was always empty.
Fix: build the query with all three required pieces:
parent == 'customers/my_customer' (alias = caller's own Workspace
org; no need to look up customer ID)
member_key_id == '<email>' (filter to this user's memberships)
'cloudidentity.googleapis.com/groups.discussion_forum' in labels
(Workspace mailing-list groups —
the common case; security-group
coverage is a follow-up)
Also: log the full error body (not truncated to 200 chars) and the
query string so the next time Google rejects something we can diagnose
in one log line instead of a re-deploy.
Caught when first agnes-dev login completed normally (HTTP 302) but app
log showed `Google groups fetch returned 400 for petr@keboola.com:
{"error":{"code":400,"message":"Request contains an invalid argument."}}`
on the same VM (kids-ai-data-analysis / agnes-dev.keboola.com).
Reference: https://cloud.google.com/identity/docs/reference/rest/v1/groups/search
* feat(web): add Profile link to user dropdown menu
The /profile page (Zdeněk's 4f7e4cd cherry-pick) renders a real profile
view including Google Workspace groups, but had no entry point in the
UI — users could only reach it by typing the URL manually. Add a
"Profile" menu item between the user header (email + role) and
"My tokens" so the page is discoverable.
Side effect: cleaned up the leftover `or _path.startswith('/profile')`
condition on the "My tokens" active class, which dated from the old
/profile → /tokens redirect (removed in c789617). Now each menu item
owns its own active state.
* fix: profile-link tests + .env quoting for CADDY_TLS
Two issues caught by Keboola's first agnes-dev deploy + agnes-auto-upgrade
cron run:
1. tests/test_web_ui.py — two negative assertions ("href=/profile" NOT in
body) date from when /profile was a redirect-only stub. Now /profile
is a real page (groups display) AND has a dropdown menu link, so the
negative assertions flip to positive. Same for ">Profile<" text in
the non-admin nav test.
2. startup-script.sh.tpl — CADDY_TLS line must be QUOTED in .env, because
agnes-auto-upgrade.sh sources .env via `set -a; . .env; set +a` and
bash treats `KEY=value with spaces` as `KEY=value` followed by `with`
and `spaces` exec attempts. Symptom: cron log spam
`/opt/agnes/.env: line 14: petr@keboola.com: command not found`,
the cron exits non-zero, and no auto-upgrade ever happens. Caddy
itself reads the value fine because docker-compose env_file=.env
parses key=value properly without shell-evaluating the rest.
Fix: emit `CADDY_TLS="tls <email>"` instead of `CADDY_TLS=tls <email>`.
Both the cron source and docker-compose env_file accept the quoted
form; cron stops failing.
* fix(auth): use searchTransitiveGroups + security label for non-admin user
Three bugs in the original cherry-pick + my prior fix attempt, all caught
by a stdlib probe script (scripts/debug/probe_google_groups.py) run
locally with a Playground-issued OAuth token:
1. Wrong endpoint. `groups:search` is the admin "find groups in org"
endpoint and 400s for non-admin users regardless of query. Switched
to `groups/-/memberships:searchTransitiveGroups` which is the
user-perspective "what groups am I in" endpoint.
2. Wrong label. Querying with `cloudidentity.googleapis.com/groups.discussion_forum`
returns 403 "Insufficient permissions to retrieve memberships" even
on the new endpoint — Workspace policy denies non-admin reads of
discussion-forum groups. Switching to `groups.security` returns 200
with the actual membership list. Empirically every Workspace group
at Keboola carries BOTH labels, so the security filter sees the full
set anyway. Confirmed with the probe script.
3. Wrong response shape. `searchTransitiveGroups` returns
{"memberships": [...]}, not {"groups": [...]}. Parser updated
accordingly.
Also adds scripts/debug/probe_google_groups.py — stdlib-only standalone
probe that hits 6 candidate endpoints with a user OAuth token. Saved a
deploy cycle (~10 min) per query iteration; future API-syntax debugging
should start there.
Verified end-to-end: petr@keboola.com login on agnes-dev returns 5
groups (LIC-1PASSWORD, ROLE_ATLASSIAN_*, etc.) via the probe; once
deployed, the same will populate session["google_groups"] and render
on /profile.
* test(auth): update Google groups parser fixture to match searchTransitiveGroups shape
Mock payload was `{"groups": [...]}` (the shape `groups:search` returns).
After switching to `groups/-/memberships:searchTransitiveGroups` in the
prior commit, the actual response is `{"memberships": [...]}` and the
parser iterates that key. Test now mirrors the real shape.
The per-item structure (groupKey.id + displayName) is unchanged, so the
expected output dict stays the same: [{"id": "...", "name": "..."}].
* docs(auth): add docs/auth-groups.md — Google Workspace groups runbook
Captures the non-obvious bits: the GCP-side setup checklist (Cloud
Identity API + scope on consent screen + Internal user type), the
`security` vs `discussion_forum` label trap (the latter 403s for
non-admins, the former 200s — one of those is a 4-iteration debug
session and shouldn't have to be repeated), where groups are stored
(session, not DB) and how to refresh (re-login), plus how to use the
probe script for future API-syntax issues.
Deliberately stops short of explaining "what is Cloud Identity" or
"what is OAuth scope" — those belong in Google's own docs, not ours.
* docs(claude): document release workflows + module versioning + recreate trick
New "Release & deploy workflows" section in CLAUDE.md covers what didn't
exist anywhere in the repo before:
- Distinction between release.yml (auto-build per push) vs the new
keboola-deploy.yml (tag-triggered, explicit deploy only) — plus when
to use which (per-developer convenience vs shared dev VM safety)
- Module versioning (infra-vX.Y.Z) and the bump-after-merge dance
- The lifecycle.ignore_changes [metadata_startup_script] gotcha and how
to force a recreate via workflow_dispatch's recreate_targets input
All generic — no customer hostnames, project IDs, IPs. Customer-specific
deploy steps belong in the consuming infra repo's README.
Also: cross-reference docs/auth-groups.md from the Authentication
section so future Claude sessions find the Workspace-groups runbook
without grepping.
---------
Co-authored-by: ZdenekSrotyr <zdenek.srotyr@keboola.com>
413 lines
14 KiB
Python
413 lines
14 KiB
Python
"""Tests for the split /tokens (own) and /admin/tokens (all) UI.
|
|
|
|
The two routes render distinct templates:
|
|
- /tokens → my_tokens.html (any signed-in user, own PATs, create modal)
|
|
- /admin/tokens → admin_tokens.html (admin-only, all users, stat strip,
|
|
owner search, sort-by-owner)
|
|
|
|
/profile 302-redirects to /tokens for back-compat.
|
|
"""
|
|
|
|
import hashlib
|
|
import tempfile
|
|
import uuid
|
|
from datetime import datetime, timezone, timedelta
|
|
|
|
import pytest
|
|
|
|
|
|
@pytest.fixture
|
|
def fresh_db(monkeypatch):
|
|
with tempfile.TemporaryDirectory() as tmp:
|
|
monkeypatch.setenv("DATA_DIR", tmp)
|
|
monkeypatch.setenv("TESTING", "1")
|
|
monkeypatch.setenv("JWT_SECRET_KEY", "test-jwt-secret-key-minimum-32-chars!!")
|
|
yield tmp
|
|
|
|
|
|
def _make_user_and_session(conn, email: str, role: str):
|
|
"""Create a user and return (uid, session_jwt)."""
|
|
from src.repositories.users import UserRepository
|
|
from app.auth.jwt import create_access_token
|
|
|
|
uid = str(uuid.uuid4())
|
|
UserRepository(conn).create(id=uid, email=email, name=email.split("@")[0], role=role)
|
|
token = create_access_token(user_id=uid, email=email, role=role)
|
|
return uid, token
|
|
|
|
|
|
def _make_pat_row(conn, user_id: str, name: str = "ci",
|
|
expires_in_days: int = 30, revoked: bool = False,
|
|
last_used_ip: str | None = None,
|
|
last_used_ago_days: int | None = None):
|
|
from src.repositories.access_tokens import AccessTokenRepository
|
|
repo = AccessTokenRepository(conn)
|
|
tid = str(uuid.uuid4())
|
|
raw = "r" * 40
|
|
exp = datetime.now(timezone.utc) + timedelta(days=expires_in_days) if expires_in_days is not None else None
|
|
repo.create(
|
|
id=tid, user_id=user_id, name=name,
|
|
token_hash=hashlib.sha256(raw.encode()).hexdigest(),
|
|
prefix=tid.replace("-", "")[:8],
|
|
expires_at=exp,
|
|
)
|
|
if last_used_ago_days is not None:
|
|
ts = datetime.now(timezone.utc) - timedelta(days=last_used_ago_days)
|
|
conn.execute(
|
|
"UPDATE personal_access_tokens SET last_used_at = ?, last_used_ip = ? WHERE id = ?",
|
|
[ts, last_used_ip, tid],
|
|
)
|
|
if revoked:
|
|
repo.revoke(tid)
|
|
return tid
|
|
|
|
|
|
# ── /tokens — "My tokens" (own PATs) — every signed-in user ────────────────
|
|
|
|
def test_non_admin_sees_my_tokens_page(fresh_db):
|
|
"""Non-admin GET /tokens: personal body, New-token CTA, create modal."""
|
|
from fastapi.testclient import TestClient
|
|
from src.db import get_system_db, close_system_db
|
|
from app.main import app
|
|
|
|
conn = get_system_db()
|
|
try:
|
|
_, sess = _make_user_and_session(conn, "user@t", "analyst")
|
|
finally:
|
|
conn.close()
|
|
close_system_db()
|
|
|
|
client = TestClient(app)
|
|
resp = client.get(
|
|
"/tokens",
|
|
headers={"Accept": "text/html"},
|
|
cookies={"access_token": sess},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
body = resp.text
|
|
# Non-admin title + eyebrow
|
|
assert "My tokens" in body
|
|
assert "Your account" in body
|
|
assert "Long-lived tokens for CLI" in body
|
|
# Role-awareness marker stays on the page root
|
|
assert 'data-is-admin="false"' in body
|
|
assert 'data-view="my"' in body
|
|
# New-token CTA + create modal are rendered
|
|
assert 'id="new-token-btn"' in body
|
|
assert 'id="create-modal"' in body
|
|
assert 'id="reveal-banner"' in body
|
|
# Admin-only stat strip is NOT rendered
|
|
assert 'id="tokens-counts"' not in body
|
|
assert 'id="count-active"' not in body
|
|
# Owner search (admin-only) is NOT rendered
|
|
assert 'placeholder="Search by owner email' not in body
|
|
# Admin title must not bleed in
|
|
assert "Access tokens" not in body
|
|
assert "Administration" not in body
|
|
|
|
|
|
def test_admin_sees_my_tokens_on_tokens_path(fresh_db):
|
|
"""Admin GET /tokens renders the SAME "My tokens" page as non-admins.
|
|
|
|
/tokens is always the personal view — admins use /admin/tokens for the
|
|
org-wide list."""
|
|
from fastapi.testclient import TestClient
|
|
from src.db import get_system_db, close_system_db
|
|
from app.main import app
|
|
|
|
conn = get_system_db()
|
|
try:
|
|
_, admin_sess = _make_user_and_session(conn, "admin@t", "admin")
|
|
finally:
|
|
conn.close()
|
|
close_system_db()
|
|
|
|
client = TestClient(app)
|
|
resp = client.get(
|
|
"/tokens",
|
|
headers={"Accept": "text/html"},
|
|
cookies={"access_token": admin_sess},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
body = resp.text
|
|
# Personal view markers (same as non-admin)
|
|
assert "My tokens" in body
|
|
assert "Your account" in body
|
|
assert 'id="new-token-btn"' in body
|
|
assert 'id="create-modal"' in body
|
|
assert 'data-is-admin="false"' in body
|
|
# Admin-only UI must NOT show on /tokens, even for an admin
|
|
assert 'id="tokens-counts"' not in body
|
|
assert "Access tokens" not in body # admin hero title
|
|
assert "Administration" not in body
|
|
|
|
|
|
def test_unauthenticated_redirects_from_tokens_page(fresh_db):
|
|
from fastapi.testclient import TestClient
|
|
from app.main import app
|
|
|
|
client = TestClient(app)
|
|
resp = client.get(
|
|
"/tokens",
|
|
headers={"Accept": "text/html"},
|
|
follow_redirects=False,
|
|
)
|
|
assert resp.status_code in (302, 303, 401), resp.text
|
|
|
|
|
|
# ── /admin/tokens — admin-only list of ALL tokens ──────────────────────────
|
|
|
|
def test_admin_can_render_admin_tokens_page(fresh_db):
|
|
"""Admin GET /admin/tokens: the org-wide list with stat strip + owner
|
|
search + sort-by-owner chip."""
|
|
from fastapi.testclient import TestClient
|
|
from src.db import get_system_db, close_system_db
|
|
from app.main import app
|
|
|
|
conn = get_system_db()
|
|
try:
|
|
_, admin_sess = _make_user_and_session(conn, "admin@t", "admin")
|
|
finally:
|
|
conn.close()
|
|
close_system_db()
|
|
|
|
client = TestClient(app)
|
|
resp = client.get(
|
|
"/admin/tokens",
|
|
headers={"Accept": "text/html"},
|
|
cookies={"access_token": admin_sess},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
body = resp.text
|
|
# Admin-specific title + eyebrow + subtitle
|
|
assert "Access tokens" in body
|
|
assert "Administration" in body
|
|
assert "incident response and offboarding" in body
|
|
# Role-awareness marker
|
|
assert 'data-is-admin="true"' in body
|
|
assert 'data-view="admin"' in body
|
|
# Filter controls
|
|
assert 'id="flt-status"' in body
|
|
assert 'id="flt-user"' in body
|
|
assert 'id="flt-last-used"' in body
|
|
# Stat strip (admin-only)
|
|
assert 'id="tokens-counts"' in body
|
|
assert 'id="count-active"' in body
|
|
assert 'id="count-expiring"' in body
|
|
# Sort-by-owner chip is only on admin page
|
|
assert 'data-sort-key="user_email"' in body
|
|
# Owner search input
|
|
assert 'placeholder="Search by owner email' in body
|
|
# Revoke hook is in JS template
|
|
assert "data-revoke" in body
|
|
# Admin page must NOT have the "New token" CTA or create modal
|
|
assert 'id="new-token-btn"' not in body
|
|
assert 'id="create-modal"' not in body
|
|
assert 'id="reveal-banner"' not in body
|
|
# Admin page must NOT use the "My tokens" title in its main content.
|
|
# (The shared user-menu in the header shows a "My tokens" link for
|
|
# every signed-in user — scope the check to the page body only.)
|
|
page_start = body.find('class="tokens-page"')
|
|
assert page_start != -1, "admin tokens page body marker not found"
|
|
assert "My tokens" not in body[page_start:]
|
|
|
|
|
|
def test_non_admin_cannot_access_admin_tokens_page(fresh_db):
|
|
"""Non-admin GET /admin/tokens: 403 (or redirect) — admin-only route."""
|
|
from fastapi.testclient import TestClient
|
|
from src.db import get_system_db, close_system_db
|
|
from app.main import app
|
|
|
|
conn = get_system_db()
|
|
try:
|
|
_, sess = _make_user_and_session(conn, "user@t", "analyst")
|
|
finally:
|
|
conn.close()
|
|
close_system_db()
|
|
|
|
client = TestClient(app)
|
|
resp = client.get(
|
|
"/admin/tokens",
|
|
headers={"Accept": "text/html"},
|
|
cookies={"access_token": sess},
|
|
follow_redirects=False,
|
|
)
|
|
# require_role(Role.ADMIN) denies with 403 for non-admin
|
|
assert resp.status_code in (302, 401, 403), resp.text
|
|
|
|
|
|
def test_admin_tokens_deeplink_preserves_user_query(fresh_db):
|
|
"""/admin/users deep-links with ?user=<email>; page should still render
|
|
and contain the owner search input (JS pre-fills it)."""
|
|
from fastapi.testclient import TestClient
|
|
from src.db import get_system_db, close_system_db
|
|
from app.main import app
|
|
|
|
conn = get_system_db()
|
|
try:
|
|
_, admin_sess = _make_user_and_session(conn, "admin@t", "admin")
|
|
finally:
|
|
conn.close()
|
|
close_system_db()
|
|
|
|
client = TestClient(app)
|
|
resp = client.get(
|
|
"/admin/tokens?user=alice%40example.com",
|
|
headers={"Accept": "text/html"},
|
|
cookies={"access_token": admin_sess},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
# Owner search input is present; JS reads ?user from window.location.
|
|
assert 'id="flt-user"' in resp.text
|
|
|
|
|
|
# NOTE: test_profile_redirects_to_tokens removed — /profile no longer
|
|
# redirects to /tokens; it renders a real profile page including Google
|
|
# Workspace groups (cherry-pick of Zdeněk's 4f7e4cd). Current /profile
|
|
# behaviour is covered by tests/test_auth_providers.py.
|
|
|
|
|
|
# ── Admin list API — expanded fields ───────────────────────────────────────
|
|
|
|
def test_admin_list_includes_user_email_and_last_used_ip(fresh_db):
|
|
from fastapi.testclient import TestClient
|
|
from src.db import get_system_db, close_system_db
|
|
from app.main import app
|
|
|
|
conn = get_system_db()
|
|
try:
|
|
admin_uid, admin_sess = _make_user_and_session(conn, "admin@t", "admin")
|
|
other_uid, _ = _make_user_and_session(conn, "victim@t", "analyst")
|
|
_make_pat_row(conn, other_uid, name="laptop", last_used_ip="9.9.9.9",
|
|
last_used_ago_days=2)
|
|
finally:
|
|
conn.close()
|
|
close_system_db()
|
|
|
|
client = TestClient(app)
|
|
resp = client.get(
|
|
"/auth/admin/tokens",
|
|
headers={"Authorization": f"Bearer {admin_sess}"},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
items = resp.json()
|
|
assert len(items) >= 1
|
|
row = [r for r in items if r["name"] == "laptop"][0]
|
|
assert row["user_id"] == other_uid
|
|
assert row["user_email"] == "victim@t"
|
|
assert row["last_used_ip"] == "9.9.9.9"
|
|
assert row["last_used_at"] # not None
|
|
|
|
|
|
def test_non_admin_cannot_list_admin_tokens(fresh_db):
|
|
from fastapi.testclient import TestClient
|
|
from src.db import get_system_db, close_system_db
|
|
from app.main import app
|
|
|
|
conn = get_system_db()
|
|
try:
|
|
_, analyst_sess = _make_user_and_session(conn, "u@t", "analyst")
|
|
finally:
|
|
conn.close()
|
|
close_system_db()
|
|
|
|
client = TestClient(app)
|
|
resp = client.get(
|
|
"/auth/admin/tokens",
|
|
headers={"Authorization": f"Bearer {analyst_sess}"},
|
|
)
|
|
assert resp.status_code == 403
|
|
|
|
|
|
# ── Admin revoke ──────────────────────────────────────────────────────────
|
|
|
|
def test_admin_can_revoke_another_users_token(fresh_db):
|
|
from fastapi.testclient import TestClient
|
|
from src.db import get_system_db, close_system_db
|
|
from src.repositories.access_tokens import AccessTokenRepository
|
|
from app.main import app
|
|
|
|
conn = get_system_db()
|
|
try:
|
|
admin_uid, admin_sess = _make_user_and_session(conn, "admin@t", "admin")
|
|
other_uid, _ = _make_user_and_session(conn, "victim@t", "analyst")
|
|
tid = _make_pat_row(conn, other_uid, name="to-kill")
|
|
finally:
|
|
conn.close()
|
|
close_system_db()
|
|
|
|
client = TestClient(app)
|
|
resp = client.delete(
|
|
f"/auth/admin/tokens/{tid}",
|
|
headers={"Authorization": f"Bearer {admin_sess}"},
|
|
)
|
|
assert resp.status_code == 204
|
|
|
|
conn = get_system_db()
|
|
try:
|
|
row = AccessTokenRepository(conn).get_by_id(tid)
|
|
assert row is not None
|
|
assert row["revoked_at"] is not None
|
|
finally:
|
|
conn.close()
|
|
close_system_db()
|
|
|
|
|
|
def test_non_admin_can_create_pat_via_tokens_page_api(fresh_db):
|
|
"""The /tokens create-modal submits POST /auth/tokens (name + expires)."""
|
|
from fastapi.testclient import TestClient
|
|
from src.db import get_system_db, close_system_db
|
|
from src.repositories.access_tokens import AccessTokenRepository
|
|
from app.main import app
|
|
|
|
conn = get_system_db()
|
|
try:
|
|
uid, sess = _make_user_and_session(conn, "user@t", "analyst")
|
|
finally:
|
|
conn.close()
|
|
close_system_db()
|
|
|
|
client = TestClient(app)
|
|
resp = client.post(
|
|
"/auth/tokens",
|
|
headers={"Authorization": f"Bearer {sess}"},
|
|
json={"name": "laptop", "expires_in_days": 30},
|
|
)
|
|
assert resp.status_code == 201, resp.text
|
|
data = resp.json()
|
|
assert data["name"] == "laptop"
|
|
assert data["token"] # raw JWT returned exactly once
|
|
assert data["prefix"]
|
|
|
|
# It must be owned by the creator
|
|
conn = get_system_db()
|
|
try:
|
|
row = AccessTokenRepository(conn).get_by_id(data["id"])
|
|
finally:
|
|
conn.close()
|
|
close_system_db()
|
|
assert row is not None
|
|
assert row["user_id"] == uid
|
|
assert row["name"] == "laptop"
|
|
|
|
|
|
def test_non_admin_cannot_admin_revoke(fresh_db):
|
|
from fastapi.testclient import TestClient
|
|
from src.db import get_system_db, close_system_db
|
|
from app.main import app
|
|
|
|
conn = get_system_db()
|
|
try:
|
|
_, analyst_sess = _make_user_and_session(conn, "u@t", "analyst")
|
|
other_uid, _ = _make_user_and_session(conn, "other@t", "analyst")
|
|
tid = _make_pat_row(conn, other_uid, name="keep")
|
|
finally:
|
|
conn.close()
|
|
close_system_db()
|
|
|
|
client = TestClient(app)
|
|
resp = client.delete(
|
|
f"/auth/admin/tokens/{tid}",
|
|
headers={"Authorization": f"Bearer {analyst_sess}"},
|
|
)
|
|
assert resp.status_code == 403
|