Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Interaction.js.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script>
function createInteractionModule(app) {

Check warning on line 2 in Interaction.js.html

View workflow job for this annotation

GitHub Actions / validate

'createInteractionModule' is defined but never used
const elements = AppElements; // Shortcut to elements

const interactionModule = {
Expand Down Expand Up @@ -31,7 +31,7 @@
els.undoBtn.addEventListener('click', () => app.historyModule.undo());
els.redoBtn.addEventListener('click', () => app.historyModule.redo());

const adminOnlyMessage = '此功能需要管理員或課表建立者權限。<br>若有需求,請聯繫系統管理員。';
const adminOnlyMessage = '在「所有課表」檢視模式下無法使用此功能。<br>請先切換到特定課表再進行操作。';

const setupAdminModal = (triggerBtn, modal, closeBtn, onOpen) => {
app.modals.setupModalListeners(triggerBtn, modal, closeBtn, () => {
Expand Down
12 changes: 3 additions & 9 deletions ScheduleManager.js.html
Original file line number Diff line number Diff line change
Expand Up @@ -345,17 +345,11 @@
};

App.canManageCurrentScheduleSettings = function() {
if (App.isCurrentUserAdmin()) {
return true;
}
// Ref: #152 — All logged-in users can manage schedules
if (App.activeScheduleId === AppConfig.ALL_SCHEDULES_ID) {
return false; // Cannot manage settings in "All Schedules" view
}
const schedule = App.schedules[App.activeScheduleId];
if (!schedule || !schedule.createdBy) {
return false; // No schedule or creator info
return false; // Cannot manage in "All Schedules" view
}
return App.currentUserEmail === App.getShortUserName(schedule.createdBy);
return true;
};

// --- Drag and drop ---
Expand Down
31 changes: 16 additions & 15 deletions tests/unit/backend.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,13 @@ describe('_checkPermission', () => {
expect(() => gas._checkPermission('creator@school.com')).not.toThrow();
});

it('rejects unauthorized user', () => {
// Ref: #152 — All logged-in users can now edit/delete/copy/rename
it('allows any logged-in user (not just admin/creator)', () => {
const gas = createGasEnv({
userEmail: 'hacker@school.com',
scriptProps: { ADMIN_EMAIL: 'admin@school.com' },
});
expect(() => gas._checkPermission('creator@school.com')).toThrow('權限不足');
expect(() => gas._checkPermission('creator@school.com')).not.toThrow();
});

it('case-insensitive email comparison', () => {
Expand All @@ -208,7 +209,8 @@ describe('_checkPermission', () => {
// ─── copySchedule (#41 permission check) ─────────────────────────────────

describe('copySchedule', () => {
it('rejects copy by unauthorized user (Ref: #41)', () => {
// Ref: #152 — All logged-in users can copy
it('allows copy by any logged-in user (Ref: #152)', () => {
const dataSheet = createMockSheet('Data', {
A1: 'ID', B1: 'Name', C1: 'Modified', D1: 'CreatedBy',
F1: '2024-01-01T00:00:00.000Z',
Expand All @@ -229,8 +231,7 @@ describe('copySchedule', () => {
metadataTimestamp: '2024-01-01T00:00:00.000Z',
});

expect(result.success).toBe(false);
expect(result.error).toContain('權限不足');
expect(result.success).toBe(true);
});

it('allows copy by owner', () => {
Expand Down Expand Up @@ -465,15 +466,15 @@ describe('saveData', () => {
expect(result.error).toContain('找不到');
});

it('rejects unauthorized user', () => {
// Ref: #152 — All logged-in users can save
it('allows save by any logged-in user (Ref: #152)', () => {
const gas = createSaveEnv({ userEmail: 'hacker@test.com' });
const result = gas.saveData({
scheduleId: 'schedule_1',
scheduleData: { scheduleData: {}, classrooms: [], tags: [] },
lastModified: '2024-06-15T10:30:00.000Z',
});
expect(result.success).toBe(false);
expect(result.error).toContain('權限不足');
expect(result.success).toBe(true);
});

it('returns error on invalid payload (missing fields)', () => {
Expand Down Expand Up @@ -521,14 +522,14 @@ describe('deleteSchedule', () => {
expect(result.success).toBe(true);
});

it('rejects unauthorized user', () => {
// Ref: #152 — All logged-in users can delete
it('allows delete by any logged-in user (Ref: #152)', () => {
const gas = createDeleteEnv({ userEmail: 'hacker@test.com' });
const result = gas.deleteSchedule({
id: 'schedule_1',
metadataTimestamp: '2024-06-15T10:30:00.000Z',
});
expect(result.success).toBe(false);
expect(result.error).toContain('權限不足');
expect(result.success).toBe(true);
});
});

Expand Down Expand Up @@ -609,15 +610,15 @@ describe('updateScheduleMetadata', () => {
expect(result.success).toBe(true);
});

it('rejects unauthorized user', () => {
// Ref: #152 — All logged-in users can rename
it('allows rename by any logged-in user (Ref: #152)', () => {
const gas = createRenameEnv({ userEmail: 'hacker@test.com' });
const result = gas.updateScheduleMetadata({
id: 'schedule_1',
newName: 'Hacked',
newName: 'Renamed',
metadataTimestamp: '2024-06-15T10:30:00.000Z',
});
expect(result.success).toBe(false);
expect(result.error).toContain('權限不足');
expect(result.success).toBe(true);
});
});

Expand Down
100 changes: 23 additions & 77 deletions tests/unit/businessLogicEdgeCases.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,24 +309,26 @@ describe('_checkPermission — edge cases (#138)', () => {
});

// --- Creator access ---
it('allows creator to manage their own schedule', () => {
// Ref: #152 — All logged-in users can manage, not just creator
it('allows any logged-in user, not just creator', () => {
const gas = createGasEnv({
userEmail: 'teacher@school.com',
userEmail: 'anyone@school.com',
scriptProps: { ADMIN_EMAIL: 'admin@school.com' },
});
expect(() => gas._checkPermission('teacher@school.com')).not.toThrow();
expect(() => gas._checkPermission('creator@school.com')).not.toThrow();
});

// --- Unauthorized access ---
it('rejects user who is neither admin nor creator', () => {
// --- Previously unauthorized access now allowed ---
// Ref: #152 — Permission model simplified
it('allows user who is neither admin nor creator (#152)', () => {
const gas = createGasEnv({
userEmail: 'other@school.com',
scriptProps: { ADMIN_EMAIL: 'admin@school.com' },
});
expect(() => gas._checkPermission('creator@school.com')).toThrow('權限不足');
expect(() => gas._checkPermission('creator@school.com')).not.toThrow();
});

// --- Case sensitivity: admin email ---
// --- Case sensitivity: admin email (still valid — admin passes) ---
it('admin check is case-insensitive (upper vs lower)', () => {
const gas = createGasEnv({
userEmail: 'ADMIN@SCHOOL.COM',
Expand All @@ -343,24 +345,16 @@ describe('_checkPermission — edge cases (#138)', () => {
expect(() => gas._checkPermission('creator@school.com')).not.toThrow();
});

// --- Case sensitivity: creator email ---
it('creator check is case-insensitive', () => {
// --- Case sensitivity: creator email (still valid — any logged-in user passes) ---
it('any logged-in user passes regardless of email case', () => {
const gas = createGasEnv({
userEmail: 'Teacher@School.COM',
scriptProps: { ADMIN_EMAIL: 'admin@school.com' },
});
expect(() => gas._checkPermission('teacher@school.com')).not.toThrow();
});

it('creator check is case-insensitive (reversed case)', () => {
const gas = createGasEnv({
userEmail: 'teacher@school.com',
scriptProps: { ADMIN_EMAIL: 'admin@school.com' },
});
expect(() => gas._checkPermission('TEACHER@SCHOOL.COM')).not.toThrow();
});

// --- Empty email guard (Ref: #62) ---
// --- Empty email guard (Ref: #62) — still enforced ---
it('throws "未登入" when current user email is empty', () => {
const gas = createGasEnv({
userEmail: '',
Expand All @@ -369,86 +363,38 @@ describe('_checkPermission — edge cases (#138)', () => {
expect(() => gas._checkPermission('creator@school.com')).toThrow('未登入');
});

// --- Missing ADMIN_EMAIL config ---
it('non-admin user passes when ADMIN_EMAIL config is missing', () => {
// getConfig('ADMIN_EMAIL') returns null/undefined
// (null || '').toLowerCase() = '' which won't match anyone
// So only creator match matters
// --- Missing ADMIN_EMAIL config — all logged-in users still pass (#152) ---
it('allows any logged-in user when ADMIN_EMAIL config is missing', () => {
const gas = createGasEnv({
userEmail: 'creator@school.com',
userEmail: 'anyone@school.com',
scriptProps: {}, // no ADMIN_EMAIL
});
expect(() => gas._checkPermission('creator@school.com')).not.toThrow();
});

it('rejects non-creator when ADMIN_EMAIL config is missing', () => {
const gas = createGasEnv({
userEmail: 'other@school.com',
scriptProps: {}, // no ADMIN_EMAIL
});
expect(() => gas._checkPermission('creator@school.com')).toThrow('權限不足');
});

it('no one is admin when ADMIN_EMAIL config is missing', () => {
// Even if user's email is something, without config it can't match
const gas = createGasEnv({
userEmail: 'admin@school.com',
scriptProps: {}, // no ADMIN_EMAIL set
});
// admin@school.com is NOT admin because config is empty
// But admin@school.com !== creator@school.com → reject
expect(() => gas._checkPermission('creator@school.com')).toThrow('權限不足');
});

// --- ADMIN_EMAIL config is empty string ---
it('empty ADMIN_EMAIL config means no one is admin', () => {
const gas = createGasEnv({
userEmail: 'user@school.com',
scriptProps: { ADMIN_EMAIL: '' },
});
// ''.toLowerCase() = '' — user@school.com !== '' so not admin
// user@school.com !== creator@school.com → reject
expect(() => gas._checkPermission('creator@school.com')).toThrow('權限不足');
});

// --- createdBy is null/undefined ---
it('throws when createdBy is null (null.toLowerCase throws)', () => {
// --- createdBy is null/undefined — no longer throws (#152, createdBy not accessed) ---
it('does not throw when createdBy is null (#152 — createdBy unused)', () => {
const gas = createGasEnv({
userEmail: 'user@school.com',
scriptProps: { ADMIN_EMAIL: 'admin@school.com' },
});
// createdBy.toLowerCase() will throw TypeError on null
expect(() => gas._checkPermission(null)).toThrow();
expect(() => gas._checkPermission(null)).not.toThrow();
});

it('throws when createdBy is undefined', () => {
it('does not throw when createdBy is undefined (#152 — createdBy unused)', () => {
const gas = createGasEnv({
userEmail: 'user@school.com',
scriptProps: { ADMIN_EMAIL: 'admin@school.com' },
});
expect(() => gas._checkPermission(undefined)).toThrow();
expect(() => gas._checkPermission(undefined)).not.toThrow();
});

// --- Admin with null createdBy should still pass (admin bypass) ---
it('admin can pass even with null createdBy (admin check comes first)', () => {
const gas = createGasEnv({
userEmail: 'admin@school.com',
scriptProps: { ADMIN_EMAIL: 'admin@school.com' },
});
// isAdmin=true → short-circuits before createdBy.toLowerCase()
// Wait: the code does `!isAdmin && currentUser.toLowerCase() !== createdBy.toLowerCase()`
// isAdmin=true → !isAdmin=false → AND short-circuits → no throw
expect(() => gas._checkPermission(null)).not.toThrow();
});

// --- Whitespace in emails ---
it('does not trim whitespace in emails (documents behavior)', () => {
// --- Whitespace in emails — still passes (#152, only login check matters) ---
it('allows user with whitespace in email (#152 — only login check)', () => {
const gas = createGasEnv({
userEmail: ' admin@school.com ',
scriptProps: { ADMIN_EMAIL: 'admin@school.com' },
});
// ' admin@school.com '.toLowerCase() !== 'admin@school.com' (whitespace matters)
// Not admin, not creator → reject
expect(() => gas._checkPermission('creator@school.com')).toThrow('權限不足');
expect(() => gas._checkPermission('creator@school.com')).not.toThrow();
});
});
5 changes: 1 addition & 4 deletions 程式碼.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,11 @@
* Throws an error if permission is denied.
* @param {string} createdBy The email of the user who created the schedule.
*/
function _checkPermission(createdBy) {

Check warning on line 43 in 程式碼.js

View workflow job for this annotation

GitHub Actions / validate

'createdBy' is defined but never used
const currentUser = Session.getActiveUser().getEmail();
// Ref: #62 — Guard against empty email (e.g. time-driven triggers return '')
if (!currentUser) throw new Error('未登入,無法執行此操作');
const isAdmin = currentUser.toLowerCase() === (getConfig('ADMIN_EMAIL') || '').toLowerCase();
if (!isAdmin && currentUser.toLowerCase() !== createdBy.toLowerCase()) {
throw new Error("權限不足。只有管理員或建立者才能執行此操作。");
}
// Ref: #152 — All logged-in users can edit/delete/copy/rename
}

/**
Expand Down Expand Up @@ -85,7 +82,7 @@
* Handles HTTP GET requests to serve the web app.
* @returns {GoogleAppsScript.HTML.HtmlOutput} The HTML output for the web app.
*/
function doGet() {

Check warning on line 85 in 程式碼.js

View workflow job for this annotation

GitHub Actions / validate

'doGet' is defined but never used
const template = HtmlService.createTemplateFromFile('Index');
const currentUser = Session.getActiveUser().getEmail();
const adminEmail = getConfig('ADMIN_EMAIL') || '';
Expand All @@ -103,7 +100,7 @@
* Fine-grained read ACL is not needed for this use case.
* @returns {object} An object containing schedules, and lastModified time, or an error object.
*/
function getData() {

Check warning on line 103 in 程式碼.js

View workflow job for this annotation

GitHub Actions / validate

'getData' is defined but never used
try {
Logger.log("開始獲取數據");
const ss = _getSs();
Expand Down Expand Up @@ -185,7 +182,7 @@
* @param {object} payload The data object to save, containing scheduleId and the schedule's data.
* @returns {object} A success object with the new lastModified time, or an error object.
*/
function saveData(payload) {

Check warning on line 185 in 程式碼.js

View workflow job for this annotation

GitHub Actions / validate

'saveData' is defined but never used
const lock = LockService.getScriptLock();
try {
lock.waitLock(30000);
Expand All @@ -200,7 +197,7 @@
throw new Error("無效的數據格式。數據必須包含 scheduleId, scheduleData, 和 lastModified 時間戳。");
}

const ss = _getSs();

Check warning on line 200 in 程式碼.js

View workflow job for this annotation

GitHub Actions / validate

'ss' is assigned a value but never used
const dataSheet = getOrCreateSheet(SHEET_DATA);

const { index: rowIndex, values: rowValues } = _findScheduleRowInfo(scheduleId, dataSheet);
Expand Down Expand Up @@ -306,7 +303,7 @@
* @param {object} scheduleInfo Object containing id, name, and metadataTimestamp.
* @returns {object} A success object or an error object.
*/
function addSchedule(scheduleInfo) {

Check warning on line 306 in 程式碼.js

View workflow job for this annotation

GitHub Actions / validate

'addSchedule' is defined but never used
const lock = LockService.getScriptLock();
try {
lock.waitLock(30000);
Expand Down Expand Up @@ -354,7 +351,7 @@
* @param {object} scheduleInfo Object containing id, newName, isDraft, and metadataTimestamp.
* @returns {object} A success object or an error object.
*/
function updateScheduleMetadata(scheduleInfo) { // Ref: #67.5 — Renamed from renameSchedule

Check warning on line 354 in 程式碼.js

View workflow job for this annotation

GitHub Actions / validate

'updateScheduleMetadata' is defined but never used
const lock = LockService.getScriptLock();
try {
lock.waitLock(30000);
Expand Down Expand Up @@ -401,7 +398,7 @@
* @param {object} scheduleInfo Object containing id and metadataTimestamp.
* @returns {object} A success object or an error object.
*/
function deleteSchedule(scheduleInfo) {

Check warning on line 401 in 程式碼.js

View workflow job for this annotation

GitHub Actions / validate

'deleteSchedule' is defined but never used
const lock = LockService.getScriptLock();
try {
lock.waitLock(30000);
Expand Down Expand Up @@ -446,7 +443,7 @@
* @param {object} copyInfo Object containing sourceId, newName, and metadataTimestamp.
* @returns {object} A success object with the new schedule's ID and owner.
*/
function copySchedule(copyInfo) {

Check warning on line 446 in 程式碼.js

View workflow job for this annotation

GitHub Actions / validate

'copySchedule' is defined but never used
const lock = LockService.getScriptLock();
try {
lock.waitLock(30000);
Expand Down Expand Up @@ -502,7 +499,7 @@
* @param {string} scheduleId The ID of the schedule to get versions for.
* @returns {Array<object>|object} An array of version objects or an error object.
*/
function getVersions(scheduleId) {

Check warning on line 502 in 程式碼.js

View workflow job for this annotation

GitHub Actions / validate

'getVersions' is defined but never used
try {
if (!scheduleId) {
throw new Error("必須提供課表 ID 以獲取版本紀錄。");
Expand Down
Loading