diff --git a/app/github/routes.py b/app/github/routes.py index c5aae29..3d55661 100644 --- a/app/github/routes.py +++ b/app/github/routes.py @@ -69,9 +69,39 @@ @bp.route("/") @login_required def index(): - """Dashboard showing the user's GitHub connection status.""" + """Dashboard showing the user's GitHub connection status. + + Displays the scopes GitHub actually *granted* (issue #57), falling back to + the scopes captured at connect time, and warns when the ``repo`` scope is + missing (private repositories will not load). + """ account = GithubAccount.query.filter_by(user_id=current_user.id).first() - return render_template("github/index.html", account=account) + granted_scopes = _granted_scopes(account) + repo_scope_missing = bool(granted_scopes) and "repo" not in granted_scopes + return render_template( + "github/index.html", + account=account, + granted_scopes=granted_scopes, + repo_scope_missing=repo_scope_missing, + ) + + +def _granted_scopes(account: GithubAccount | None) -> list[str]: + """Return the scopes to display for the dashboard. + + Prefers a live ``GET /user`` read (the token's current grants) and falls + back to the scopes stored at connect time. Best-effort: a failed read never + breaks the dashboard, and the value is used for display only — never for an + authorization decision (issue #57). + """ + if account is None: + return [] + stored = [scope.strip() for scope in (account.scopes or "").split(",") if scope.strip()] + try: + live = _client().get_granted_scopes() + except GitHubError: + live = [] + return live or stored @bp.route("/connect") @@ -154,7 +184,13 @@ def callback(): db.session.add(account) account.github_user_id = user["id"] account.github_username = user.get("login", "") - account.scopes = token_data.get("scope", "") + # Prefer the scopes GitHub reports via the X-OAuth-Scopes header, falling + # back to the token-exchange `scope` value. Informational only (issue #57). + try: + granted = client.get_granted_scopes() + except GitHubError: + granted = [] + account.scopes = ",".join(granted) if granted else token_data.get("scope", "") account.token_type = token_data.get("token_type", "bearer") account.set_access_token(token) account.set_refresh_token(token_data.get("refresh_token")) @@ -216,6 +252,8 @@ def status(): } if account is not None: payload["rate_limit"] = _rate_limit_summary() + scopes = [scope.strip() for scope in (account.scopes or "").split(",") if scope.strip()] + payload["repo_scope_missing"] = bool(scopes) and "repo" not in scopes return jsonify(payload) diff --git a/app/services/github.py b/app/services/github.py index 7e17d20..709bfe1 100644 --- a/app/services/github.py +++ b/app/services/github.py @@ -356,6 +356,23 @@ def _get_page( def get_user(self) -> dict: return self._get("/user") + def get_granted_scopes(self) -> list[str]: + """Return the OAuth scopes GitHub actually granted this token (issue #57). + + GitHub reports the granted scopes in the ``X-OAuth-Scopes`` response + header of an authenticated request (the token exchange response also + carries them, but the header reflects the token's *current* grants). + The result is informational only: it is never used to make + authorization decisions, which always depend on the token itself. + """ + response = self.session.get(f"{self.api_url}/user", timeout=self.timeout) + if response.status_code >= 400: + # Re-issue through _request so failures become the typed errors the + # rest of the app expects (auth, rate limit, not found, ...). + self._request("GET", "/user") + raw = response.headers.get("X-OAuth-Scopes", "") + return [scope.strip() for scope in raw.split(",") if scope.strip()] + def get_rate_limit(self) -> dict | None: """Return the caller's core rate-limit budget, or ``None`` if unknown. diff --git a/app/templates/github/index.html b/app/templates/github/index.html index 9080aa3..80605d0 100644 --- a/app/templates/github/index.html +++ b/app/templates/github/index.html @@ -12,7 +12,11 @@

GitHub Integration

Connected as @{{ account.github_username }} - {% if account.scopes %}(scopes: {{ account.scopes }}){% endif %} + {% if granted_scopes %} + (granted scopes: {{ granted_scopes|join(', ') }}) + {% else %} + (granted scopes not reported by GitHub) + {% endif %}
Browse repositories @@ -23,6 +27,14 @@

GitHub Integration

+ {% if repo_scope_missing %} + + {% endif %} +
diff --git a/tests/test_github_routes.py b/tests/test_github_routes.py index 92dc84d..8a3eeb6 100644 --- a/tests/test_github_routes.py +++ b/tests/test_github_routes.py @@ -754,3 +754,97 @@ def test_token_never_serialized(self, client, app, monkeypatch): def _last_session_state(client): with client.session_transaction() as sess: return sess.get("github_oauth_state") + + +class TestDashboardGrantedScopes: + """The dashboard shows the granted scopes and warns when repo is missing (#57).""" + + def test_shows_granted_scopes_without_warning_when_repo_present(self, client, app, monkeypatch): + _logged_in_client(client) + _create_account(app) + monkeypatch.setattr( + "app.services.github.requests.Session", + lambda: _make_fake_session( + [ + ( + "GET", + "/user", + 200, + {"id": 42, "login": "ghuser"}, + {"X-OAuth-Scopes": "read:user, repo"}, + ) + ] + ), + ) + body = client.get("/github/").get_data(as_text=True) + assert "granted scopes" in body + assert "read:user" in body + assert "repo" in body + assert "Missing the" not in body + + def test_warns_when_repo_scope_missing(self, client, app, monkeypatch): + _logged_in_client(client) + _create_account(app) + monkeypatch.setattr( + "app.services.github.requests.Session", + lambda: _make_fake_session( + [ + ( + "GET", + "/user", + 200, + {"id": 42, "login": "ghuser"}, + {"X-OAuth-Scopes": "read:user, gist"}, + ) + ] + ), + ) + body = client.get("/github/").get_data(as_text=True) + assert "Missing the" in body + + def test_falls_back_to_stored_scopes_when_header_unavailable(self, client, app, monkeypatch): + _logged_in_client(client) + account = _create_account(app) + account.scopes = "read:user, repo" + db.session.commit() + monkeypatch.setattr( + "app.services.github.requests.Session", + lambda: _make_fake_session([("GET", "/user", 200, {"id": 42, "login": "ghuser"})]), + ) + body = client.get("/github/").get_data(as_text=True) + assert "read:user, repo" in body + + +class TestStatusScopes: + """The status API reports whether the repo scope is missing (issue #57).""" + + _RATE_LIMIT = ( + "GET", + "/rate_limit", + 200, + {"resources": {"core": {"limit": 5000, "remaining": 4999, "reset": 1700000000, "used": 1}}}, + ) + + def _patch(self, monkeypatch): + monkeypatch.setattr( + "app.services.github.requests.Session", + lambda: _make_fake_session([self._RATE_LIMIT]), + ) + + def test_status_flags_missing_repo_scope(self, client, app, monkeypatch): + _logged_in_client(client) + account = _create_account(app) + account.scopes = "read:user" + db.session.commit() + self._patch(monkeypatch) + data = client.get("/github/api/status").get_json() + assert data["repo_scope_missing"] is True + + def test_status_repo_scope_present(self, client, app, monkeypatch): + _logged_in_client(client) + account = _create_account(app) + account.scopes = "read:user,repo" + db.session.commit() + self._patch(monkeypatch) + data = client.get("/github/api/status").get_json() + assert data["repo_scope_missing"] is False diff --git a/tests/test_github_service.py b/tests/test_github_service.py index cca1ee7..fb4ec6b 100644 --- a/tests/test_github_service.py +++ b/tests/test_github_service.py @@ -372,3 +372,26 @@ def test_validate_path_rejects_traversal(self): for bad in ["../secret", "foo/../../etc/passwd", "..", "a/../b"]: with pytest.raises(GitHubError): validate_path(bad) + + +class TestGrantedScopes: + """get_granted_scopes reads the X-OAuth-Scopes header (issue #57).""" + + def test_reads_and_splits_the_header(self, ok_client): + client, session = ok_client + session.responses = [ + FakeResponse(200, data={"id": 1}, headers={"X-OAuth-Scopes": "read:user, repo, gist"}) + ] + assert client.get_granted_scopes() == ["read:user", "repo", "gist"] + + def test_trims_whitespace_and_ignores_blanks(self, ok_client): + client, session = ok_client + session.responses = [ + FakeResponse(200, data={"id": 1}, headers={"X-OAuth-Scopes": " repo ,, read:user "}) + ] + assert client.get_granted_scopes() == ["repo", "read:user"] + + def test_empty_when_header_absent(self, ok_client): + client, session = ok_client + session.responses = [FakeResponse(200, data={"id": 1})] + assert client.get_granted_scopes() == []