같은 기능인데 메뉴가 둘이고 로그인이 달랐다

수강생 목록을 보는 화면이 두 곳에 있었는데 들어가는 로그인 방식이 서로 달랐다.

Open Table of contents

증상 — 입구가 둘이었다
원인 — 상세에 조건이 없었다
조치 — 같은 검사를 거치게
배경 — 복사로 만든 화면
검증 — 같은 방식의 화면들
재발 방지 — 확인 장치
정리

증상 — 입구가 둘이었다

들어가는 자리가 둘이었다.

/admin/student      관리자 로그인
/teacher/student    강사 로그인

teacher 는 자기 과정 수강생만 보고 admin 은 전체를 보는 구조였다.

코드도 따로 있었다.

controllers/admin/StudentController.php
controllers/teacher/StudentController.php

거의 같은 것을 보여 주는 입구가 둘이면 두 입구의 검사가 같은지부터 확인해야 한다.

원인 — 상세에 조건이 없었다

index 쪽은 제대로 걸러지고 있었다.

// Teacher
public function index() {
    $courseIds = $this->user->courseIds();
    return $this->render('teacher/student/index', [
        'students' => $this->students->inCourses($courseIds),
    ]);
}

inCourses 에 courseIds 를 걸어 자기 과정만 본다.

문제는 show 쪽이었다.

// Teacher
public function show($id) {
    return $this->render('teacher/student/show', [
        'student' => $this->students->find($id),   // 확인이 없다
    ]);
}

find 에 범위 조건이 없어 주소에 다른 번호를 넣으면 남의 과정 수강생이 보였다.

index 에서 거른 것이 show 에서 거른 것은 아니었다. index 와 show 가 각각 자기 몫의 검사를 따로 해야 했다.

조치 — 같은 검사를 거치게

show 에 조건을 하나 더 붙이는 것과 범위를 한 번만 정의하는 것 두 가지 방법 중 뒤쪽이었다. 볼 수 있는 범위를 scope 하나로 한 번만 정의했다.

abstract class StudentController extends BaseController {
    abstract protected function scope(): array;   // 볼 수 있는 범위 조건

    public function index() {
        return $this->render('index', ['students' => $this->students->all($this->scope())]);
    }

    public function show($id) {
        $student = $this->students->one($id, $this->scope());   // 같은 범위에서 찾는다
        if (!$student) return $this->notFound();
        return $this->render('show', ['student' => $student]);
    }
}

scope 하나를 목록과 상세가 함께 쓴다.

class TeacherStudentController extends StudentController {
    protected function scope(): array {
        return ['course_id' => $this->user->courseIds()];
    }
}

강사용은 scope 만 구현하면 나머지가 따라온다.

index 와 show 에 같은 조건을 따로 적어 두면 언젠가 한쪽만 고쳐진다. 정의가 scope 한 곳이면 상세에서만 빠지는 일이 없어진다.

배경 — 복사로 만든 화면

Teacher 쪽 화면이 나중에 생긴 것이었다.

관리자 화면을 복사해서 만들고 조건을 붙였는데 목록에는 붙이고 상세는 빠뜨렸다.

복사해서 만들면 원본의 구조를 그대로 갖고 온다.
붙여야 할 조건을 전부 붙였는지 확인할 방법이 없다.

복사가 아니라 물려받는 구조로 바꾸니 이 문제가 없어졌다.

검증 — 같은 방식의 화면들

같은 식으로 만들어진 화면이 더 있었다.

과정 목록/상세
평가 목록/상세
수료증 목록/상세

셋 다 봤고 평가 상세 에 같은 문제가 있었다.

$ grep -rn "\->find(\|\->one(" --include=*.php controllers/teacher/

범위 없이 찾는 자리를 전부 찾아 고쳤다.

재발 방지 — 확인 장치

남의 것을 열 수 없는지 확인하는 테스트를 만들었다.

public function testTeacherCannotSeeOtherCourseStudent(): void {
    $teacher = $this->makeTeacherWithCourse(1);
    $other   = $this->makeStudentInCourse(2);

    $res = $this->loginAs($teacher)->get("/teacher/student/{$other->id}");
    $this->assertEquals(404, $res->status);
}

403 대신 404 쪽을 골랐다. 이유는 있는데 권한이 없다는 것도 알려 주지 않아서다.

index 쪽에도 같은 것을 넣었다.

public function testTeacherListOnlyOwnStudents(): void {
    ...
    $this->assertCount(3, $res['students']);
    $this->assertNotContains($otherStudent->id, array_column($res['students'], 'id'));
}

assertNotContains 로 남의 것이 목록에 안 들어가는지도 본다.

show 만 보면 목록에서 새는 것을 놓친다. 목록에 개수만 나와도 그쪽에 얼마나 있는지가 그대로 새어 나간다.

정리

같은 기능에 입구가 둘이면 한쪽이 느슨할 수 있다
두 입구의 검사를 나란히 놓고 본다
목록에서 거른 것이 상세에서 거른 것은 아니다
범위를 한 번 정의하고 목록과 상세가 같은 것을 쓴다
두 곳에 같은 조건을 적으면 한쪽만 고쳐진다
복사해서 만들면 붙여야 할 조건을 빠뜨린다
물려받는 구조로 바꾼다
같은 방식으로 만든 화면을 전부 찾아본다
남의 것을 볼 수 없는지 확인하는 것을 만든다
있는지도 안 알려 주는 편이 낫다
상세만이 아니라 목록도 확인한다