관리자만 쓸 수 있어야 하는 기능이 다른 경로로 열려 있었다.
Table of contents
Open Table of contents
증상 — 바깥에만 있던 권한 확인
관리자 화면 쪽은 이렇게 돼 있었다.
class AdminOrderController extends CI_Controller {
public function __construct() {
parent::__construct();
if (!$this->auth->isAdmin()) { show_404(); }
}
public function forceCancel($orderNo) {
$this->orderService->forceCancel($orderNo);
}
}들어오는 자리에서 isAdmin 을 보고 아니면 막는다. 여기까지는 맞다.
실제 처리를 하는 쪽에는 확인이 없었다.
class OrderService {
public function forceCancel(int $orderNo): void {
// 권한 확인 없음
$this->orders->updateState($orderNo, 'CANCELED');
$this->refund->request($orderNo);
}
}밖에서 확인했으니 안에서는 안 해도 된다고 본 것이다.
AdminOrderController 쪽에만 두는 방식에는 전제가 있다. 부르는 쪽이 전부 그 사정을 알고 있어야 성립한다. 새 경로를 만드는 사람이 forceCancel 안에 확인이 없다는 것을 알아야 한다. 그런데 안에 없으니 알 길이 없다.
다른 경로가 있었다
일반 사용자가 쓰는 API에서 같은 것을 부르고 있었다.
class OrderApiController extends CI_Controller {
public function cancel($orderNo) {
if (!$this->auth->isOwner($orderNo)) { show_404(); }
$order = $this->orders->find($orderNo);
if ($order->state === 'SHIPPED') {
$this->orderService->forceCancel($orderNo); // ← 여기
}
}
}발송된 주문을 취소하려고 강제 취소를 그대로 쓴 것이다.
isOwner 로 본인인지는 봤지만 강제 취소는 관리자만 할 수 있어야 하는 처리였다. isOwner 와 isAdmin 은 서로 다른 질문인데 앞의 것만 하고 뒤의 것은 안 한 셈이다.
구멍은 새 경로가 생긴 그날 함께 생겼다. OrderService 안에 확인이 없으니 부르는 경로가 늘어난 만큼 구멍이 생겼다. 이 코드를 쓴 사람에게는 forceCancel 을 재사용한 것이었을 뿐이다.
조치 — 안쪽에도 확인을 넣었다
누가 부르는지를 인자로 받게 했다.
public function forceCancel(int $orderNo, Actor $actor): void {
if (!$actor->can('order.force_cancel')) {
throw new Forbidden('강제 취소 권한이 없습니다');
}
...
}부르는 쪽이 Actor 를 넘겨야 하고 can 이 거짓이면 Forbidden 으로 그 자리에서 멈춘다.
고치는 일 자체는 한 줄이었다. 인자를 더해도 부르는 자리가 저절로 드러나지는 않는다. 그래서 grep 으로 forceCancel 을 부르는 자리를 전부 뽑아 하나씩 고쳤다. 그 목록에 우리가 몰랐던 경로가 하나 더 있었다.
넘길 주체가 없는 자리도 있었다.
// 배치에서 부르는 자리
$this->orderService->forceCancel($no, Actor::system('auto_cancel_batch'));auto_cancel_batch 처럼 사람이 아니라 배치가 부르는 곳이었다.
여기만 확인을 건너뛰게 하면 그 길이 다시 구멍이 된다. Actor::system 을 만들어 넘기니 누가 한 것인지가 남고 건너뛰는 길도 안 생겼다.
검증 — 기록과 지난 41건
무엇을 누가 했는지 남기게 했다.
$this->auditLog->write([
'action' => 'order.force_cancel',
'target' => $orderNo,
'actor' => $actor->id(),
'actor_type' => $actor->type(),
'at' => date('Y-m-d H:i:s'),
]);actor_type 을 따로 둔 것은 사람과 배치를 나중에 갈라 보기 위해서였다. 둘을 한 칸에 담으면 그 구분을 다시 문자열로 풀어야 한다.
기록을 넣고 나서 지난 것부터 훑었다. 일반 사용자 경로로 강제 취소가 나간 것이 41건 있었다. 그 41건을 하나씩 확인했다.
actor_type 이 사용자로 찍힌 41건 중 대부분은 실제로 취소가 필요한 건이었다. 다만 정상 절차를 안 거쳐서 정산에 반영이 안 된 것이 넷 있었고 그 넷은 따로 처리해야 했다.
설정 — 권한 목록과 역할 표
권한 이름을 한 곳에 모았다.
final class Permission {
public const ORDER_VIEW = 'order.view';
public const ORDER_CANCEL = 'order.cancel';
public const ORDER_FORCE_CANCEL = 'order.force_cancel';
public const ORDER_REFUND = 'order.refund';
}역할과 권한은 표로 이었다.
CREATE TABLE role_permission (
role varchar(30) NOT NULL,
permission varchar(50) NOT NULL,
PRIMARY KEY (role, permission)
);코드에서는 역할 이름을 안 보고 order.force_cancel 같은 권한 이름만 본다. 역할은 표에만 있고 코드에는 안 나온다.
역할이 하나 늘어도 코드는 그대로이고 role_permission 에 줄을 넣으면 된다. isAdmin 처럼 역할 이름을 코드에 적어 두면 역할이 생길 때마다 조건문이 함께 늘어난다.
확인이 빠진 곳을 찾았다
다른 자리에도 같은 구멍이 있는지 셌다.
$ grep -rn "public function" src/Service/*.php | wc -l
182
$ grep -rn "\$actor->can(" src/Service/*.php | wc -l
34182개 중 34개만 확인하고 있었다. 나머지가 전부 문제인 것은 아니다.
조회만 하는 것은 이 확인이 필요 없어서 바꾸는 것만 골라 봤다.
$ grep -rn "public function \(create\|update\|delete\|cancel\|approve\|reject\)" src/Service/*.php47개가 나왔다. 그중 확인이 있는 것이 34개라 열셋이 남았다.
열셋을 하나씩 보니 여덟은 넣어야 했고 다섯은 applyApproval 처럼 안쪽에서만 부르는 것이라 필요 없었다. 필요 없는 다섯에는 왜 없는지를 적어 뒀다.
/* 이 메서드는 approve() 안에서만 부른다. 권한은 그쪽에서 확인한다. */
private function applyApproval(int $no): void { ... }주석 한 줄이면 되는 일이었다. 적어 두지 않으면 다음 사람이 빠뜨린 것으로 보고 같은 검토를 다시 한다. 없는 것과 일부러 안 넣은 것은 코드만 봐서는 구분되지 않는다.
정리
- 권한 확인이 바깥에만 있으면 부르는 경로가 늘 때 구멍이 생긴다
- 부르는 쪽이 전부 그 사정을 알고 있어야 성립하는 방식이다
- 본인 확인과 권한 확인은 서로 다른 질문이다
- 안쪽 처리에도 확인을 넣고 누가 부르는지를 인자로 받는다
- 인자를 더해도 부르는 자리는 저절로 안 드러나므로 전부 찾아 고친다
- 배치가 부르는 것도 시스템 주체를 만들어 넘긴다
- 한 곳만 건너뛰게 하면 그 길이 다시 구멍이 된다
- 기록에 주체를 남기고 지난 41건처럼 잘못 나간 것을 확인한다
- 권한을 목록으로 만들고 역할과 표로 이으면 역할이 늘어도 코드를 안 고친다
- 확인이 빠진 곳은 바꾸는 처리만 골라 본다
- 일부러 안 넣은 곳에는 이유를 적는다