From 12b0992c5bd0605366c9e12cb2719f1442acf268 Mon Sep 17 00:00:00 2001 From: "smalruby3-editor-bot[bot]" <297607354+smalruby3-editor-bot[bot]@users.noreply.github.com> Date: Sat, 10 Oct 2026 02:46:24 +0000 Subject: [PATCH 1/2] fix(classroom): surface the real Google Classroom failure when posting an assignment Posting an assignment collapsed every non-401/403 Google failure into a bare 502 "Google Classroom API error", so a teacher (and CloudWatch) learned nothing about why the share failed. Google's error.status/message are now parsed, logged (never the access token) and passed through with a stable reason key, and the UI translates each reason into an actionable Japanese message instead of echoing the raw server string. The consent screen also lets a teacher untick classroom.coursework.students, which is required to create course work; the token is now checked with hasGrantedAllScopes and re-requested with prompt=consent, and a 403 drops the cached token. Refs #1175 Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/classroom/architecture.md | 21 +++ infra/smalruby-classroom/lambda/handler.ts | 116 +++++++++++++++-- .../lambda/tests/handler-google-error.test.ts | 102 +++++++++++++++ packages/scratch-gui/.prettierignore | 1 + .../src/containers/classroom-error-utils.js | 71 ++++++++++ .../src/containers/use-google-classroom.js | 9 +- .../src/lib/google-classroom-auth.js | 91 ++++++++++--- packages/scratch-gui/src/locales/ja-Hira.js | 9 ++ packages/scratch-gui/src/locales/ja.js | 9 ++ .../containers/classroom-error-utils.test.js | 65 ++++++++++ .../unit/lib/google-classroom-auth.test.js | 121 ++++++++++++++++++ 11 files changed, 583 insertions(+), 32 deletions(-) create mode 100644 infra/smalruby-classroom/lambda/tests/handler-google-error.test.ts create mode 100644 packages/scratch-gui/test/unit/lib/google-classroom-auth.test.js diff --git a/docs/classroom/architecture.md b/docs/classroom/architecture.md index f7902ada84..f77f446e32 100644 --- a/docs/classroom/architecture.md +++ b/docs/classroom/architecture.md @@ -249,6 +249,27 @@ One Tap が表示できない環境（Windows や Playwright では毎回）で | `POST` | `/classrooms/google-import` | コースをインポートして課題を作成（`Classrooms` に書き込む） | | `POST` | `/classrooms/{id}/google-assignment` | Google Classroom に課題を投稿 | +#### Google API 失敗時のレスポンス + +Google Classroom API の失敗は、Google が返した `error.status` / `error.message` を +そのまま透過し、UI が翻訳するための安定キー `reason` を付けて返す +（詳細は CloudWatch にも `Google Classroom API error` として記録する。アクセストークンは記録しない）。 + +| Google の HTTP | 返す HTTP | `reason` | 典型的な原因 | +|---|---|---|---| +| 401 | 401 | `googleTokenExpired` | Google のアクセストークンが失効 | +| 403 | 403 | `googlePermissionDenied` | 権限（スコープ）不足 / そのコースの先生ではない | +| 404 | 404 | `googleCourseNotFound` | ひもづけたコースが削除された / そのコースにアクセスできない | +| 400 | 400 | `googleInvalidRequest` | Google がリクエストを拒否（`FAILED_PRECONDITION` 等を含む） | +| その他 | 502 | `googleApiError` | レート超過・Google 側障害など | + +レスポンス本文: `{error, reason, googleStatus?, googleMessage?}`。 + +先生に表示するアクセストークンは `classroom.coursework.students` を含む 3 スコープを要求するが、 +Google の同意画面はスコープごとにチェックボックスを出すため**一部だけ許可される**ことがある。 +クライアントは `hasGrantedAllScopes` で検証し、不足していれば `prompt: 'consent'` で再要求する +（それでも不足するなら `googlePermissionDenied` として扱う）。 + ## データモデル ### Classrooms テーブル diff --git a/infra/smalruby-classroom/lambda/handler.ts b/infra/smalruby-classroom/lambda/handler.ts index 1bac7a69f3..0de77f74d0 100644 --- a/infra/smalruby-classroom/lambda/handler.ts +++ b/infra/smalruby-classroom/lambda/handler.ts @@ -348,10 +348,17 @@ class ConflictError extends Error { class GoogleAPIError extends Error { statusCode: number; - constructor(statusCode: number, message: string) { + // Google's own error envelope (`error.status` / `error.message`), kept so the + // response and the CloudWatch log can name the actual cause instead of the + // opaque "Google Classroom API error" the teacher used to see (issue #1175). + googleStatus: string; + googleMessage: string; + constructor(statusCode: number, message: string, googleStatus = '', googleMessage = '') { super(message); this.name = 'GoogleAPIError'; this.statusCode = statusCode; + this.googleStatus = googleStatus; + this.googleMessage = googleMessage; } } @@ -380,6 +387,86 @@ const KICK_TOMBSTONE_TTL_SECONDS = parseInt(process.env.KICK_TOMBSTONE_TTL_SECON // --- Google Classroom API proxy --- +/** Max length of a Google error message we keep (logs + API response). */ +const GOOGLE_ERROR_MESSAGE_MAX = 300; + +/** + * Pull `status` / `message` out of a Google API error envelope: + * `{"error":{"code":404,"message":"...","status":"NOT_FOUND"}}`. + * + * Google also answers with HTML (front-end proxies, 5xx) and with bodies that + * carry no envelope, so every failure mode degrades to empty strings instead of + * throwing — this runs inside an error path and must never mask the original + * failure. + */ +export function parseGoogleApiErrorBody(raw: string): { googleStatus: string; googleMessage: string } { + const empty = { googleStatus: '', googleMessage: '' }; + if (!raw) return empty; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return empty; + } + const error = (parsed as { error?: unknown } | null)?.error; + if (!error || typeof error !== 'object') return empty; + const { status, message } = error as { status?: unknown; message?: unknown }; + return { + googleStatus: typeof status === 'string' ? status : '', + googleMessage: typeof message === 'string' ? message.slice(0, GOOGLE_ERROR_MESSAGE_MAX) : '', + }; +} + +/** + * Map a Google Classroom failure onto our API response. + * + * Before issue #1175 everything that was not 401/403 collapsed into a bare 502 + * "Google Classroom API error", which told the teacher nothing and left no way + * to tell "the linked course is gone / you are not a teacher of it" (404) apart + * from a malformed request (400). The Google-provided status and message are + * passed through so the UI can show an actionable message, and `reason` gives + * the UI a stable key to translate. + */ +export function googleApiErrorResponse( + statusCode: number, + googleStatus: string, + googleMessage: string, +): { statusCode: number; body: string } { + let httpStatus: number; + let reason: string; + let error: string; + if (statusCode === 401) { + httpStatus = 401; + reason = 'googleTokenExpired'; + error = 'Google access token expired. Please re-authorize.'; + } else if (statusCode === 403) { + httpStatus = 403; + reason = 'googlePermissionDenied'; + error = 'Insufficient Google Classroom permissions.'; + } else if (statusCode === 404) { + httpStatus = 404; + reason = 'googleCourseNotFound'; + error = 'Google Classroom course not found, or you are not a teacher of it.'; + } else if (statusCode === 400) { + httpStatus = 400; + reason = 'googleInvalidRequest'; + error = 'Google Classroom rejected the request.'; + } else { + httpStatus = 502; + reason = 'googleApiError'; + error = 'Google Classroom API error'; + } + return { + statusCode: httpStatus, + body: JSON.stringify({ + error, + reason, + ...(googleStatus ? { googleStatus } : {}), + ...(googleMessage ? { googleMessage } : {}), + }), + }; +} + async function callGoogleClassroomAPI( accessToken: string, path: string, @@ -396,7 +483,23 @@ async function callGoogleClassroomAPI( body: body ? JSON.stringify(body) : undefined, }); if (!response.ok) { - throw new GoogleAPIError(response.status, `Google Classroom API error: ${response.status}`); + const raw = await response.text().catch(() => ''); + const { googleStatus, googleMessage } = parseGoogleApiErrorBody(raw); + // Logged (never the access token) so a teacher report like issue #1175 can + // be diagnosed from CloudWatch without reproducing it. + console.error('Google Classroom API error', { + method, + path, + httpStatus: response.status, + googleStatus, + googleMessage, + }); + throw new GoogleAPIError( + response.status, + `Google Classroom API error: ${response.status}`, + googleStatus, + googleMessage, + ); } return response.json(); } @@ -5070,13 +5173,8 @@ export const handler = async (event: APIGatewayProxyEventV2): Promise { + test('extracts status and message from a Google error envelope', () => { + const raw = JSON.stringify({ + error: { + code: 404, + message: 'Requested entity was not found.', + status: 'NOT_FOUND', + }, + }); + expect(parseGoogleApiErrorBody(raw)).toEqual({ + googleStatus: 'NOT_FOUND', + googleMessage: 'Requested entity was not found.', + }); + }); + + test('returns empty fields for a non-JSON body', () => { + expect(parseGoogleApiErrorBody('502 Bad Gateway')).toEqual({ + googleStatus: '', + googleMessage: '', + }); + }); + + test('returns empty fields for an empty body', () => { + expect(parseGoogleApiErrorBody('')).toEqual({ googleStatus: '', googleMessage: '' }); + }); + + test('tolerates a JSON body without the error envelope', () => { + expect(parseGoogleApiErrorBody(JSON.stringify({ foo: 'bar' }))).toEqual({ + googleStatus: '', + googleMessage: '', + }); + }); + + test('ignores non-string status / message fields', () => { + const raw = JSON.stringify({ error: { status: 42, message: { nested: true } } }); + expect(parseGoogleApiErrorBody(raw)).toEqual({ googleStatus: '', googleMessage: '' }); + }); + + test('truncates an overly long message', () => { + const raw = JSON.stringify({ error: { status: 'INVALID_ARGUMENT', message: 'あ'.repeat(600) } }); + const parsed = parseGoogleApiErrorBody(raw); + expect(parsed.googleStatus).toBe('INVALID_ARGUMENT'); + expect(parsed.googleMessage.length).toBeLessThanOrEqual(300); + }); +}); + +describe('googleApiErrorResponse', () => { + const parse = (res: { statusCode: number; body: string }) => JSON.parse(res.body); + + test('401 keeps the re-authorization hint and flags googleTokenExpired', () => { + const res = googleApiErrorResponse(401, 'UNAUTHENTICATED', 'Invalid Credentials'); + expect(res.statusCode).toBe(401); + expect(parse(res).reason).toBe('googleTokenExpired'); + }); + + test('403 flags googlePermissionDenied', () => { + const res = googleApiErrorResponse(403, 'PERMISSION_DENIED', 'The caller does not have permission'); + expect(res.statusCode).toBe(403); + expect(parse(res).reason).toBe('googlePermissionDenied'); + }); + + test('404 is surfaced as 404 googleCourseNotFound, not a generic 502', () => { + const res = googleApiErrorResponse(404, 'NOT_FOUND', 'Requested entity was not found.'); + expect(res.statusCode).toBe(404); + const body = parse(res); + expect(body.reason).toBe('googleCourseNotFound'); + expect(body.googleStatus).toBe('NOT_FOUND'); + expect(body.googleMessage).toBe('Requested entity was not found.'); + }); + + test('400 is surfaced as 400 googleInvalidRequest with Google detail', () => { + const res = googleApiErrorResponse(400, 'INVALID_ARGUMENT', 'Request contains an invalid argument.'); + expect(res.statusCode).toBe(400); + const body = parse(res); + expect(body.reason).toBe('googleInvalidRequest'); + expect(body.googleMessage).toBe('Request contains an invalid argument.'); + }); + + test('FAILED_PRECONDITION (e.g. an archived course) is surfaced as 400', () => { + const res = googleApiErrorResponse(400, 'FAILED_PRECONDITION', 'CourseNotModifiable'); + expect(res.statusCode).toBe(400); + expect(parse(res).googleStatus).toBe('FAILED_PRECONDITION'); + }); + + test('other statuses stay a 502 but still carry the Google detail', () => { + const res = googleApiErrorResponse(429, 'RESOURCE_EXHAUSTED', 'Quota exceeded'); + expect(res.statusCode).toBe(502); + const body = parse(res); + expect(body.reason).toBe('googleApiError'); + expect(body.googleStatus).toBe('RESOURCE_EXHAUSTED'); + expect(body.googleMessage).toBe('Quota exceeded'); + }); + + test('omits empty Google detail fields', () => { + const body = parse(googleApiErrorResponse(500, '', '')); + expect(body.googleStatus).toBeUndefined(); + expect(body.googleMessage).toBeUndefined(); + expect(body.reason).toBe('googleApiError'); + }); +}); diff --git a/packages/scratch-gui/.prettierignore b/packages/scratch-gui/.prettierignore index 0a95352c5f..7a6fa74bc2 100644 --- a/packages/scratch-gui/.prettierignore +++ b/packages/scratch-gui/.prettierignore @@ -397,6 +397,7 @@ test/unit/lib/* !test/unit/lib/classroom-assignment-utils.test.js !test/unit/lib/classroom-download-utils.test.js !test/unit/lib/classroom-group-utils.test.js +!test/unit/lib/google-classroom-auth.test.js !test/unit/lib/classroom-retention.test.js !test/unit/lib/shared-assignment-taxonomy.test.js !test/unit/lib/shared-author-profile.test.js diff --git a/packages/scratch-gui/src/containers/classroom-error-utils.js b/packages/scratch-gui/src/containers/classroom-error-utils.js index c330f2f100..291d8a46a3 100644 --- a/packages/scratch-gui/src/containers/classroom-error-utils.js +++ b/packages/scratch-gui/src/containers/classroom-error-utils.js @@ -1,3 +1,67 @@ +/** + * Translate a Google Classroom proxy failure (`reason` starting with `google`) + * into a localized, actionable message. Returns null when the error did not + * come from the Google Classroom proxy so the caller can fall through to the + * regular classroom branches. + * @param {object} intl - react-intl intl object + * @param {Error} err - Error from a classroom API call + * @returns {?string} Localized message, or null when not a Google failure. + */ +const translateGoogleClassroomError = (intl, err) => { + // `body.reason` comes from the API proxy; `googleReason` is set client-side + // (e.g. the teacher unticked a scope on the Google consent screen). + const body = (err && err.body) || {}; + const reason = body.reason || (err && err.googleReason); + if (typeof reason !== 'string' || !reason.startsWith('google')) return null; + + if (reason === 'googleTokenExpired') { + return intl.formatMessage({ + defaultMessage: + 'Your Google authorization has expired. Please try again and allow access to Google Classroom.', + description: 'Error when the Google Classroom access token is no longer valid', + id: 'gui.classroom.error.googleTokenExpired', + }); + } + if (reason === 'googlePermissionDenied') { + return intl.formatMessage({ + defaultMessage: + 'Smalruby is not allowed to post to this Google Classroom class. Sign in again and allow ' + + 'every requested permission, and check that you are a teacher of the class.', + description: 'Error when Google Classroom denies the post (missing scope or not a teacher)', + id: 'gui.classroom.error.googlePermissionDenied', + }); + } + if (reason === 'googleCourseNotFound') { + return intl.formatMessage({ + defaultMessage: + 'The linked Google Classroom class could not be found. It may have been deleted, or the ' + + 'Google account you signed in with may not be a teacher of that class.', + description: 'Error when the linked Google Classroom course is missing or inaccessible', + id: 'gui.classroom.error.googleCourseNotFound', + }); + } + + // googleInvalidRequest / googleApiError: Google rejected the request for a + // reason we cannot map. Show its own status and message so the teacher can + // report something actionable. + const detail = [body.googleStatus, body.googleMessage].filter(Boolean).join(': '); + if (detail) { + return intl.formatMessage( + { + defaultMessage: 'Could not post to Google Classroom. (Google: {detail})', + description: 'Error when Google Classroom rejects the post, with the Google-provided detail', + id: 'gui.classroom.error.googleApiFailedDetail', + }, + { detail }, + ); + } + return intl.formatMessage({ + defaultMessage: 'Could not post to Google Classroom. Please wait a moment and try again.', + description: 'Error when posting to Google Classroom fails for an unknown reason', + id: 'gui.classroom.error.googleApiFailed', + }); +}; + /** * Translate known API error messages to localized user-friendly messages. * @param {object} intl - react-intl intl object @@ -26,6 +90,13 @@ const translateError = (intl, err, context = 'general') => { ); } + // Google Classroom proxy failures. Checked before the generic HTTP branches + // because their statuses overlap (a 401 here is an expired *Google* token, + // not an expired classroom session) and because the raw server message + // ("Google Classroom API error") is useless to a teacher (issue #1175). + const googleMessage = translateGoogleClassroomError(intl, err); + if (googleMessage) return googleMessage; + if (context === 'join' && (status === 404 || msg.includes('Invalid join code'))) { return intl.formatMessage({ defaultMessage: 'Could not join the classroom. Please check the join code and try again.', diff --git a/packages/scratch-gui/src/containers/use-google-classroom.js b/packages/scratch-gui/src/containers/use-google-classroom.js index bcbccad83c..880c096db2 100644 --- a/packages/scratch-gui/src/containers/use-google-classroom.js +++ b/packages/scratch-gui/src/containers/use-google-classroom.js @@ -55,8 +55,9 @@ const useGoogleClassroom = ({ setGoogleCourses(data.courses || []); setSelectedGoogleCourse(null); } catch (err) { - if (err.status === 401) { + if (err.status === 401 || err.status === 403) { clearClassroomAccessToken(); + setGoogleAccessToken(null); } showError(translateError(intl, err)); } finally { @@ -118,7 +119,11 @@ const useGoogleClassroom = ({ } return result; } catch (err) { - if (err.status === 401) { + // 401 = the Google token expired; 403 = Google refused (most + // often a scope the teacher did not grant). Either way the + // cached token is useless, so drop it and let the next attempt + // re-run consent instead of replaying the same failure (#1175). + if (err.status === 401 || err.status === 403) { clearClassroomAccessToken(); setGoogleAccessToken(null); } diff --git a/packages/scratch-gui/src/lib/google-classroom-auth.js b/packages/scratch-gui/src/lib/google-classroom-auth.js index 261ff6b75e..d3338e49f7 100644 --- a/packages/scratch-gui/src/lib/google-classroom-auth.js +++ b/packages/scratch-gui/src/lib/google-classroom-auth.js @@ -7,11 +7,16 @@ */ import { loadAllGoogleScripts } from './google-script-loader'; -const CLASSROOM_SCOPES = [ +export const CLASSROOM_SCOPE_LIST = [ 'https://www.googleapis.com/auth/classroom.courses.readonly', 'https://www.googleapis.com/auth/classroom.rosters.readonly', + // Required to create course work (= post an assignment). Google shows one + // checkbox per scope, so a teacher can grant the read-only ones and leave + // this one off — the posting call then fails inside Google (issue #1175). 'https://www.googleapis.com/auth/classroom.coursework.students', -].join(' '); +]; + +const CLASSROOM_SCOPES = CLASSROOM_SCOPE_LIST.join(' '); const CLIENT_ID = process.env.GOOGLE_CLIENT_ID; @@ -67,18 +72,12 @@ const _initialize = () => { }; /** - * Request an access token with Google Classroom scopes. - * Returns cached token if still valid, otherwise triggers OAuth consent. - * @returns {Promise} Access token string. + * Ask Google Identity Services for a token. + * @param {string} prompt - GIS prompt mode ('' = silent when already granted). + * @returns {Promise} The GIS token response. */ -export const requestClassroomAccessToken = async () => { - await _initialize(); - - if (_isTokenValid()) { - return _accessToken; - } - - return new Promise((resolve, reject) => { +const _requestToken = (prompt) => + new Promise((resolve, reject) => { const expectedState = _generateState(); _tokenClient.callback = (response) => { @@ -90,17 +89,67 @@ export const requestClassroomAccessToken = async () => { reject(new Error('OAuth state mismatch: potential CSRF attack detected')); return; } - _accessToken = response.access_token; - // expires_in is in seconds - _tokenExpiresAt = Date.now() + (response.expires_in || 3600) * 1000; - resolve(response.access_token); + resolve(response); }; - _tokenClient.requestAccessToken({ - prompt: '', - state: expectedState, - }); + _tokenClient.requestAccessToken({ prompt, state: expectedState }); }); + +/** + * Check that the token response carries every scope we need. Older GIS builds + * may not expose the helper; assume granted then so we never block a teacher on + * a check we cannot perform. + * @param {object} response - GIS token response. + * @returns {boolean} True when all classroom scopes were granted. + */ +const _hasAllScopes = (response) => { + const oauth2 = window.google && window.google.accounts && window.google.accounts.oauth2; + if (!oauth2 || typeof oauth2.hasGrantedAllScopes !== 'function') return true; + return oauth2.hasGrantedAllScopes(response, ...CLASSROOM_SCOPE_LIST); +}; + +/** + * Store a fully-granted token for reuse. + * @param {object} response - GIS token response. + * @returns {string} The access token. + */ +const _cacheToken = (response) => { + _accessToken = response.access_token; + // expires_in is in seconds + _tokenExpiresAt = Date.now() + (response.expires_in || 3600) * 1000; + return _accessToken; +}; + +/** + * Request an access token with Google Classroom scopes. + * Returns cached token if still valid, otherwise triggers OAuth consent. + * @returns {Promise} Access token string. + */ +export const requestClassroomAccessToken = async () => { + await _initialize(); + + if (_isTokenValid()) { + return _accessToken; + } + + const response = await _requestToken(''); + if (_hasAllScopes(response)) { + return _cacheToken(response); + } + + // The teacher unticked at least one scope on the consent screen. Ask again + // with an explicit consent prompt so they can grant the missing one; a + // silent re-request would just hand back the same partial grant. + const retry = await _requestToken('consent'); + if (_hasAllScopes(retry)) { + return _cacheToken(retry); + } + + const err = new Error('Google Classroom permission was not granted. Please allow every requested permission.'); + // Same key the backend uses for a Google 403 so the UI translates both the + // client-side (unticked scope) and server-side denial identically. + err.googleReason = 'googlePermissionDenied'; + throw err; }; /** diff --git a/packages/scratch-gui/src/locales/ja-Hira.js b/packages/scratch-gui/src/locales/ja-Hira.js index d18a3c8a74..13ebc7fec9 100644 --- a/packages/scratch-gui/src/locales/ja-Hira.js +++ b/packages/scratch-gui/src/locales/ja-Hira.js @@ -162,6 +162,15 @@ export default { 'gui.classroom.error.classroomInactive': 'このクラスはげんざいりようできません。', 'gui.classroom.error.networkUnreachable': 'サーバーにせつぞくできませんでした。ネットワークやファイアウォールのせっていで {host}（HTTPS / 443）へのつうしんがきょかされているかごかくにんください。', + 'gui.classroom.error.googleTokenExpired': + 'Google のにんしょうのゆうこうきげんがきれました。もういちどおためしいただき、Google Classroom へのアクセスをきょかしてください。', + 'gui.classroom.error.googlePermissionDenied': + 'このクラスの Google Classroom にかだいをはいしんするけんげんがありません。もういちどログインしてすべてのこうもくをきょかし、そのクラスのせんせいとしてさんかしているかごかくにんください。', + 'gui.classroom.error.googleCourseNotFound': + 'ひもづいている Google Classroom のクラスがみつかりませんでした。クラスがさくじょされたか、ログインちゅうの Google アカウントがそのクラスのせんせいではないかのうせいがあります。', + 'gui.classroom.error.googleApiFailedDetail': 'Google Classroom にはいしんできませんでした。（Google: {detail}）', + 'gui.classroom.error.googleApiFailed': + 'Google Classroom にはいしんできませんでした。しばらくまってからもういちどおためしください。', 'gui.classroom.error.generic': 'よきしないエラーがはっせいしました。もういちどおためしください。', 'gui.classroom.error.goToLogin': 'ログインがめんをひょうじ', 'gui.classroom.error.goToJoin': 'さんかがめんをひょうじ', diff --git a/packages/scratch-gui/src/locales/ja.js b/packages/scratch-gui/src/locales/ja.js index e7fa04ffa6..82d4c10a2d 100644 --- a/packages/scratch-gui/src/locales/ja.js +++ b/packages/scratch-gui/src/locales/ja.js @@ -158,6 +158,15 @@ export default { 'gui.classroom.error.classroomInactive': 'このクラスは現在利用できません。', 'gui.classroom.error.networkUnreachable': 'サーバーに接続できませんでした。ネットワークやファイアウォールの設定で {host}（HTTPS / 443）への通信が許可されているかご確認ください。', + 'gui.classroom.error.googleTokenExpired': + 'Google の認証の有効期限が切れました。もう一度お試しいただき、Google Classroom へのアクセスを許可してください。', + 'gui.classroom.error.googlePermissionDenied': + 'このクラスの Google Classroom に課題を配信する権限がありません。もう一度ログインしてすべての項目を許可し、そのクラスの先生として参加しているかご確認ください。', + 'gui.classroom.error.googleCourseNotFound': + 'ひもづいている Google Classroom のクラスが見つかりませんでした。クラスが削除されたか、ログイン中の Google アカウントがそのクラスの先生ではない可能性があります。', + 'gui.classroom.error.googleApiFailedDetail': 'Google Classroom に配信できませんでした。（Google: {detail}）', + 'gui.classroom.error.googleApiFailed': + 'Google Classroom に配信できませんでした。しばらく待ってからもう一度お試しください。', 'gui.classroom.error.generic': '予期しないエラーが発生しました。もう一度お試しください。', 'gui.classroom.error.goToLogin': 'ログイン画面を表示', 'gui.classroom.error.goToJoin': '参加画面を表示', diff --git a/packages/scratch-gui/test/unit/containers/classroom-error-utils.test.js b/packages/scratch-gui/test/unit/containers/classroom-error-utils.test.js index cd1e498dd7..721ed7e1ac 100644 --- a/packages/scratch-gui/test/unit/containers/classroom-error-utils.test.js +++ b/packages/scratch-gui/test/unit/containers/classroom-error-utils.test.js @@ -29,6 +29,71 @@ describe('translateError network failure', () => { }); }); +describe('translateError Google Classroom failures', () => { + const googleError = (status, reason, extra = {}) => { + const err = new Error('Google Classroom API error'); + err.status = status; + err.body = { error: 'Google Classroom API error', reason, ...extra }; + return err; + }; + + test('a missing/forbidden course explains that the teacher must own the GC class', () => { + const msg = translateError(intlMock, googleError(404, 'googleCourseNotFound', { googleStatus: 'NOT_FOUND' })); + expect(msg).toContain('Google Classroom'); + expect(msg).not.toBe('Google Classroom API error'); + }); + + test('insufficient permissions get their own message', () => { + const msg = translateError( + intlMock, + googleError(403, 'googlePermissionDenied', { googleStatus: 'PERMISSION_DENIED' }), + ); + expect(msg).toContain('Google Classroom'); + expect(msg).not.toBe('Google Classroom API error'); + }); + + test('an expired Google token is not reported as an expired classroom session', () => { + const msg = translateError( + intlMock, + googleError(401, 'googleTokenExpired', { googleStatus: 'UNAUTHENTICATED' }), + ); + expect(msg).toContain('Google'); + expect(msg).not.toContain('rejoin the classroom'); + }); + + test('an unexpected Google failure surfaces the Google-provided detail', () => { + const msg = translateError( + intlMock, + googleError(502, 'googleApiError', { + googleStatus: 'RESOURCE_EXHAUSTED', + googleMessage: 'Quota exceeded', + }), + ); + expect(msg).toContain('RESOURCE_EXHAUSTED'); + expect(msg).toContain('Quota exceeded'); + }); + + test('an unexpected Google failure without detail still gives an actionable message', () => { + const msg = translateError(intlMock, googleError(502, 'googleApiError')); + expect(msg).toContain('Google Classroom'); + expect(msg).not.toBe('Google Classroom API error'); + }); + + test('a client-side scope denial (googleReason) is translated too', () => { + const err = new Error('Google Classroom permission was not granted.'); + err.googleReason = 'googlePermissionDenied'; + const msg = translateError(intlMock, err); + expect(msg).toContain('Google Classroom'); + expect(msg).not.toBe('Google Classroom permission was not granted.'); + }); + + test('non-Google errors are unaffected', () => { + const err = new Error('Invalid or expired session token'); + err.status = 401; + expect(translateError(intlMock, err)).toContain('rejoin the classroom'); + }); +}); + describe('extractKickReason', () => { test('returns null for a non-410 error', () => { const err = new Error('Invalid or expired session token'); diff --git a/packages/scratch-gui/test/unit/lib/google-classroom-auth.test.js b/packages/scratch-gui/test/unit/lib/google-classroom-auth.test.js new file mode 100644 index 0000000000..6e4d7fe904 --- /dev/null +++ b/packages/scratch-gui/test/unit/lib/google-classroom-auth.test.js @@ -0,0 +1,121 @@ +/** + * Google Classroom OAuth token acquisition. + * + * The consent screen lets a teacher untick individual scopes, so a token can + * come back without `classroom.coursework.students` — the posting call then + * fails inside Google with no hint of why (issue #1175). These tests pin the + * "verify what was actually granted" behaviour. + */ +jest.mock('../../../src/lib/google-script-loader', () => ({ + loadAllGoogleScripts: jest.fn(() => Promise.resolve()), +})); + +const COURSEWORK_SCOPE = 'https://www.googleapis.com/auth/classroom.coursework.students'; + +/** + * Install a fake Google Identity Services token client. + * @param {Array} responses - queued token responses, one per request + * @param {Function} hasGrantedAllScopes - GIS scope check stub + * @returns {object} The recorded requests (`requests` array). + */ +const installGis = (responses, hasGrantedAllScopes) => { + const requests = []; + let client = null; + window.google = { + accounts: { + oauth2: { + initTokenClient: (config) => { + client = { + ...config, + requestAccessToken: (options) => { + requests.push(options); + const response = responses.shift(); + // GIS echoes the state back on success. + client.callback({ state: options.state, ...response }); + }, + }; + return client; + }, + hasGrantedAllScopes, + }, + }, + }; + return { requests }; +}; + +describe('requestClassroomAccessToken', () => { + beforeEach(() => { + jest.resetModules(); + process.env.GOOGLE_CLIENT_ID = 'test-client-id.apps.googleusercontent.com'; + }); + + afterEach(() => { + delete window.google; + }); + + test('resolves with the token when every scope is granted', async () => { + const { requests } = installGis([{ access_token: 'tok-1', expires_in: 3600 }], () => true); + const { requestClassroomAccessToken } = require('../../../src/lib/google-classroom-auth'); + await expect(requestClassroomAccessToken()).resolves.toBe('tok-1'); + expect(requests).toHaveLength(1); + expect(requests[0].prompt).toBe(''); + }); + + test('re-asks with an explicit consent prompt when a scope was unticked', async () => { + let call = 0; + const { requests } = installGis( + [ + { access_token: 'partial', expires_in: 3600 }, + { access_token: 'full', expires_in: 3600 }, + ], + () => ++call > 1, + ); + const { requestClassroomAccessToken } = require('../../../src/lib/google-classroom-auth'); + await expect(requestClassroomAccessToken()).resolves.toBe('full'); + expect(requests).toHaveLength(2); + expect(requests[1].prompt).toBe('consent'); + }); + + test('rejects with an actionable error when the scope is still missing', async () => { + installGis( + [ + { access_token: 'partial-1', expires_in: 3600 }, + { access_token: 'partial-2', expires_in: 3600 }, + ], + () => false, + ); + const { requestClassroomAccessToken } = require('../../../src/lib/google-classroom-auth'); + await expect(requestClassroomAccessToken()).rejects.toThrow(/permission/i); + }); + + test('does not cache a token that is missing scopes', async () => { + installGis( + [ + { access_token: 'partial-1', expires_in: 3600 }, + { access_token: 'partial-2', expires_in: 3600 }, + { access_token: 'partial-3', expires_in: 3600 }, + { access_token: 'partial-4', expires_in: 3600 }, + ], + () => false, + ); + const { requestClassroomAccessToken } = require('../../../src/lib/google-classroom-auth'); + await expect(requestClassroomAccessToken()).rejects.toThrow(); + // A second attempt must hit Google again rather than reuse the bad token. + await expect(requestClassroomAccessToken()).rejects.toThrow(); + }); + + test('still resolves when the GIS build has no hasGrantedAllScopes helper', async () => { + installGis([{ access_token: 'tok-legacy', expires_in: 3600 }], undefined); + const { requestClassroomAccessToken } = require('../../../src/lib/google-classroom-auth'); + await expect(requestClassroomAccessToken()).resolves.toBe('tok-legacy'); + }); +}); + +describe('CLASSROOM_SCOPES', () => { + test('includes the coursework.students scope', () => { + jest.resetModules(); + process.env.GOOGLE_CLIENT_ID = 'test-client-id.apps.googleusercontent.com'; + const { CLASSROOM_SCOPE_LIST } = require('../../../src/lib/google-classroom-auth'); + expect(CLASSROOM_SCOPE_LIST).toContain(COURSEWORK_SCOPE); + }); +}); From 491b156f1c04dc82977b0b8f76c6ce0529601ef0 Mon Sep 17 00:00:00 2001 From: "smalruby3-editor-bot[bot]" <297607354+smalruby3-editor-bot[bot]@users.noreply.github.com> Date: Sat, 10 Oct 2026 02:54:46 +0000 Subject: [PATCH 2/2] fix(classroom): treat a blocked consent retry as a permission denial MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 再同意のポップアップは最初のクリックから時間が経ってから開くため、ブラウザに ブロックされたり先生が閉じたりしうる。その拒否を googlePermissionDenied として 扱い、GIS の生の英語エラーが UI に出るのを防ぐ。state 不一致 (CSRF の疑い) は この扱いから除外する。 Co-Authored-By: Claude Opus 5 --- docs/classroom/architecture.md | 9 ++-- docs/classroom/source-code.md | 2 + .../src/lib/google-classroom-auth.js | 41 +++++++++++++++---- .../unit/lib/google-classroom-auth.test.js | 23 +++++++++++ 4 files changed, 65 insertions(+), 10 deletions(-) diff --git a/docs/classroom/architecture.md b/docs/classroom/architecture.md index f77f446e32..e5e3934c5e 100644 --- a/docs/classroom/architecture.md +++ b/docs/classroom/architecture.md @@ -265,10 +265,13 @@ Google Classroom API の失敗は、Google が返した `error.status` / `error. レスポンス本文: `{error, reason, googleStatus?, googleMessage?}`。 -先生に表示するアクセストークンは `classroom.coursework.students` を含む 3 スコープを要求するが、 +クライアントが取得するアクセストークンは `classroom.coursework.students` を含む 3 スコープを要求するが、 Google の同意画面はスコープごとにチェックボックスを出すため**一部だけ許可される**ことがある。 -クライアントは `hasGrantedAllScopes` で検証し、不足していれば `prompt: 'consent'` で再要求する -（それでも不足するなら `googlePermissionDenied` として扱う）。 +クライアントは `hasGrantedAllScopes` で検証し、不足していれば `prompt: 'consent'` で再要求する。 +再要求してもなお不足する場合に加え、**再要求のポップアップがブラウザにブロックされた / 先生が +閉じた・拒否した場合**も `googlePermissionDenied` として扱う（再要求は最初のクリックから時間が +経っているため、ブラウザがポップアップを開かないことがある）。ただし OAuth の state 不一致 +（CSRF の疑い）はこの扱いに含めず、そのままエラーにする。 ## データモデル diff --git a/docs/classroom/source-code.md b/docs/classroom/source-code.md index 4d5113b664..bd5e47243c 100644 --- a/docs/classroom/source-code.md +++ b/docs/classroom/source-code.md @@ -120,6 +120,8 @@ CDK スタック定義。以下のリソースを作成: | `verifyTeacherToken()` | Google ID Token 検証 | | `verifySessionToken()` | Session Token 検証 | | `callGoogleClassroomAPI()` | Google Classroom API プロキシ | +| `parseGoogleApiErrorBody()` | Google のエラー本文から `status` / `message` を取り出す（失敗時は空文字） | +| `googleApiErrorResponse()` | Google の HTTP ステータスを自前のレスポンス（`reason` 付き）へ写像 | | `getCorsHeaders()` | CORS ヘッダー生成 | | `checkJoinRateLimit()` | レート制限チェック | | `generateJoinCode()` | 参加コード生成 (6文字英数字) | diff --git a/packages/scratch-gui/src/lib/google-classroom-auth.js b/packages/scratch-gui/src/lib/google-classroom-auth.js index d3338e49f7..c250240bfa 100644 --- a/packages/scratch-gui/src/lib/google-classroom-auth.js +++ b/packages/scratch-gui/src/lib/google-classroom-auth.js @@ -86,7 +86,11 @@ const _requestToken = (prompt) => return; } if (response.state !== expectedState) { - reject(new Error('OAuth state mismatch: potential CSRF attack detected')); + const mismatch = new Error('OAuth state mismatch: potential CSRF attack detected'); + // Flagged so callers never downgrade a possible CSRF into a + // routine "permission was not granted" message. + mismatch.isStateMismatch = true; + reject(mismatch); return; } resolve(response); @@ -120,6 +124,20 @@ const _cacheToken = (response) => { return _accessToken; }; +/** + * Build the error raised when the teacher did not grant every scope. + * @param {Error} [cause] - Underlying GIS failure, when there was one. + * @returns {Error} Error carrying the googlePermissionDenied reason. + */ +const _permissionDeniedError = (cause) => { + const err = new Error('Google Classroom permission was not granted. Please allow every requested permission.'); + // Same key the backend uses for a Google 403 so the UI translates both the + // client-side (unticked scope) and server-side denial identically. + err.googleReason = 'googlePermissionDenied'; + if (cause) err.cause = cause; + return err; +}; + /** * Request an access token with Google Classroom scopes. * Returns cached token if still valid, otherwise triggers OAuth consent. @@ -140,16 +158,25 @@ export const requestClassroomAccessToken = async () => { // The teacher unticked at least one scope on the consent screen. Ask again // with an explicit consent prompt so they can grant the missing one; a // silent re-request would just hand back the same partial grant. - const retry = await _requestToken('consent'); + let retry; + try { + retry = await _requestToken('consent'); + } catch (err) { + // This second popup opens after the first consent round trip, so the + // original click no longer counts as user activation and the browser + // may refuse to open it; the teacher can also close or decline it. + // All of those are still "the permission was not granted" — report + // them as such instead of surfacing GIS's raw English error, which is + // exactly the opaque message issue #1175 is about. A state mismatch is + // a possible CSRF and must never be downgraded. + if (err && err.isStateMismatch) throw err; + throw _permissionDeniedError(err); + } if (_hasAllScopes(retry)) { return _cacheToken(retry); } - const err = new Error('Google Classroom permission was not granted. Please allow every requested permission.'); - // Same key the backend uses for a Google 403 so the UI translates both the - // client-side (unticked scope) and server-side denial identically. - err.googleReason = 'googlePermissionDenied'; - throw err; + throw _permissionDeniedError(); }; /** diff --git a/packages/scratch-gui/test/unit/lib/google-classroom-auth.test.js b/packages/scratch-gui/test/unit/lib/google-classroom-auth.test.js index 6e4d7fe904..0b868e0d90 100644 --- a/packages/scratch-gui/test/unit/lib/google-classroom-auth.test.js +++ b/packages/scratch-gui/test/unit/lib/google-classroom-auth.test.js @@ -104,6 +104,29 @@ describe('requestClassroomAccessToken', () => { await expect(requestClassroomAccessToken()).rejects.toThrow(); }); + test('reports a blocked or declined consent retry as a permission problem', async () => { + // The retry popup opens without a fresh user gesture, so the browser + // can refuse it ('popup_failed_to_open') — the teacher must still get + // the translated permission message, not GIS's raw English error. + installGis([{ access_token: 'partial', expires_in: 3600 }, { error: 'popup_failed_to_open' }], () => false); + const { requestClassroomAccessToken } = require('../../../src/lib/google-classroom-auth'); + await expect(requestClassroomAccessToken()).rejects.toMatchObject({ + googleReason: 'googlePermissionDenied', + }); + }); + + test('does not downgrade a state mismatch on the retry to a permission problem', async () => { + installGis( + [ + { access_token: 'partial', expires_in: 3600 }, + { access_token: 'spoofed', expires_in: 3600, state: 'not-the-expected-state' }, + ], + () => false, + ); + const { requestClassroomAccessToken } = require('../../../src/lib/google-classroom-auth'); + await expect(requestClassroomAccessToken()).rejects.toThrow(/state mismatch/i); + }); + test('still resolves when the GIS build has no hasGrantedAllScopes helper', async () => { installGis([{ access_token: 'tok-legacy', expires_in: 3600 }], undefined); const { requestClassroomAccessToken } = require('../../../src/lib/google-classroom-auth');