fix(auth): scope-фильтр на writer-side endpoints (Phase 2c security gap)
ScopeContext.canAccess(DataScope) — единая точка проверки доступа,
использует DataScope.visibleTo (PUBLIC видит PUBLIC, INTERNAL видит
PUBLIC+INTERNAL, RESTRICTED видит всё).
DictionaryRecordController:
- requireReadAccess на GET /{businessKey} и /{businessKey}/history
→ 404 dictionary_not_found если scope словаря недоступен (намеренно
скрываем существование INTERNAL/RESTRICTED данных, защита от
enumeration)
- requireWriteAccess на POST/PUT/DELETE
→ 403 scope_insufficient если scope недостаточен для записи
DictionaryDefinitionController:
- list() фильтрует список по canAccess (PUBLIC-юзер не видит INTERNAL
словари в каталоге)
- get() → 404 если недоступен
- create()/update() → 403 если новый scope выше доступа юзера
(защита от scope escalation через PUT)
AuthE2ETest.publicUser_cannotSeeInternalRecords: TODO snять, expectation
обновлён на isNotFound() (раньше было isOk + комментарий о gap).
До этого fix: PUBLIC-юзер мог GET /api/v1/dictionaries/{n}/records/{k}
INTERNAL-словарь на writer-side. Read-api (отдельный модуль) фильтровал
корректно. Issue найден через AuthE2ETest в Phase 2c.
Все 6 AuthE2ETest и 24 unit-теста rest-api зелёные.
This commit is contained in:
@@ -113,16 +113,13 @@ class AuthE2ETest {
|
|||||||
.content(om.writeValueAsBytes(recBody)))
|
.content(om.writeValueAsBytes(recBody)))
|
||||||
.andExpect(status().is2xxSuccessful());
|
.andExpect(status().is2xxSuccessful());
|
||||||
|
|
||||||
// ⚠️ ИЗВЕСТНОЕ ПОВЕДЕНИЕ: writer-side endpoint /api/v1/dictionaries/{n}/records/{k}
|
// Writer endpoint фильтрует по scope: PUBLIC-юзер не видит INTERNAL словарь.
|
||||||
// НЕ фильтрует по scope юзера. Public-юзер сейчас МОЖЕТ прочитать INTERNAL.
|
// Возвращаем 404 (а не 403) — намеренно скрываем существование INTERNAL
|
||||||
// Read-api (/api/v1/{n}/records/...) — фильтрует. Writer считается admin-only.
|
// данных, как делает read-api. Защита от enumeration.
|
||||||
//
|
|
||||||
// Тест документирует текущее поведение. После security review (см. issue
|
|
||||||
// #scope-writer-filter) заменить на 403/404 expectation.
|
|
||||||
String publicToken = JwtTestSupport.signJwt(List.of("ordinis:client:public"), "user-public");
|
String publicToken = JwtTestSupport.signJwt(List.of("ordinis:client:public"), "user-public");
|
||||||
mvc.perform(get("/api/v1/dictionaries/{n}/records/{k}", dictName, key)
|
mvc.perform(get("/api/v1/dictionaries/{n}/records/{k}", dictName, key)
|
||||||
.header(HttpHeaders.AUTHORIZATION, "Bearer " + publicToken))
|
.header(HttpHeaders.AUTHORIZATION, "Bearer " + publicToken))
|
||||||
.andExpect(status().isOk()); // TODO: должно быть isForbidden() после fix scope filter
|
.andExpect(status().isNotFound());
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
@@ -49,6 +49,19 @@ public class ScopeContext {
|
|||||||
return EnumSet.of(DataScope.PUBLIC);
|
return EnumSet.of(DataScope.PUBLIC);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* True если пользовательский scope видит данные {@code dataScope}.
|
||||||
|
* Использует {@link DataScope#visibleTo(DataScope)}: PUBLIC видит PUBLIC,
|
||||||
|
* INTERNAL видит PUBLIC+INTERNAL, RESTRICTED видит всё.
|
||||||
|
*/
|
||||||
|
public boolean canAccess(DataScope dataScope) {
|
||||||
|
if (dataScope == null) return true;
|
||||||
|
for (DataScope userScope : currentScopes()) {
|
||||||
|
if (dataScope.visibleTo(userScope)) return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
public Set<DataScope> resolveForRead(String queryAsScope) {
|
public Set<DataScope> resolveForRead(String queryAsScope) {
|
||||||
Authentication auth = SecurityContextHolder.getContext().getAuthentication();
|
Authentication auth = SecurityContextHolder.getContext().getAuthentication();
|
||||||
if (auth instanceof JwtAuthenticationToken jwtAuth) {
|
if (auth instanceof JwtAuthenticationToken jwtAuth) {
|
||||||
|
|||||||
+35
-4
@@ -1,11 +1,13 @@
|
|||||||
package cloud.nstart.terravault.ordinis.restapi.web;
|
package cloud.nstart.terravault.ordinis.restapi.web;
|
||||||
|
|
||||||
|
import cloud.nstart.terravault.ordinis.auth.ScopeContext;
|
||||||
|
import cloud.nstart.terravault.ordinis.domain.DataScope;
|
||||||
import cloud.nstart.terravault.ordinis.restapi.dto.CreateDictionaryRequest;
|
import cloud.nstart.terravault.ordinis.restapi.dto.CreateDictionaryRequest;
|
||||||
import cloud.nstart.terravault.ordinis.restapi.dto.DictionaryResponse;
|
import cloud.nstart.terravault.ordinis.restapi.dto.DictionaryResponse;
|
||||||
|
import cloud.nstart.terravault.ordinis.restapi.error.OrdinisException;
|
||||||
import cloud.nstart.terravault.ordinis.restapi.service.DictionaryDefinitionService;
|
import cloud.nstart.terravault.ordinis.restapi.service.DictionaryDefinitionService;
|
||||||
import jakarta.validation.Valid;
|
import jakarta.validation.Valid;
|
||||||
import org.springframework.http.HttpStatus;
|
import org.springframework.http.HttpStatus;
|
||||||
import org.springframework.http.ResponseEntity;
|
|
||||||
import org.springframework.web.bind.annotation.GetMapping;
|
import org.springframework.web.bind.annotation.GetMapping;
|
||||||
import org.springframework.web.bind.annotation.PathVariable;
|
import org.springframework.web.bind.annotation.PathVariable;
|
||||||
import org.springframework.web.bind.annotation.PostMapping;
|
import org.springframework.web.bind.annotation.PostMapping;
|
||||||
@@ -22,24 +24,41 @@ import java.util.List;
|
|||||||
public class DictionaryDefinitionController {
|
public class DictionaryDefinitionController {
|
||||||
|
|
||||||
private final DictionaryDefinitionService service;
|
private final DictionaryDefinitionService service;
|
||||||
|
private final ScopeContext scopeContext;
|
||||||
|
|
||||||
public DictionaryDefinitionController(DictionaryDefinitionService service) {
|
public DictionaryDefinitionController(
|
||||||
|
DictionaryDefinitionService service, ScopeContext scopeContext) {
|
||||||
this.service = service;
|
this.service = service;
|
||||||
|
this.scopeContext = scopeContext;
|
||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping
|
@GetMapping
|
||||||
public List<DictionaryResponse> list() {
|
public List<DictionaryResponse> list() {
|
||||||
return service.findAll().stream().map(DictionaryResponse::from).toList();
|
return service.findAll().stream()
|
||||||
|
.filter(d -> scopeContext.canAccess(d.getScope()))
|
||||||
|
.map(DictionaryResponse::from)
|
||||||
|
.toList();
|
||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping("/{name}")
|
@GetMapping("/{name}")
|
||||||
public DictionaryResponse get(@PathVariable String name) {
|
public DictionaryResponse get(@PathVariable String name) {
|
||||||
return DictionaryResponse.from(service.findByName(name));
|
var d = service.findByName(name);
|
||||||
|
if (!scopeContext.canAccess(d.getScope())) {
|
||||||
|
throw OrdinisException.notFound(
|
||||||
|
"dictionary_not_found", "Dictionary not found: " + name);
|
||||||
|
}
|
||||||
|
return DictionaryResponse.from(d);
|
||||||
}
|
}
|
||||||
|
|
||||||
@PostMapping
|
@PostMapping
|
||||||
@ResponseStatus(HttpStatus.CREATED)
|
@ResponseStatus(HttpStatus.CREATED)
|
||||||
public DictionaryResponse create(@Valid @RequestBody CreateDictionaryRequest req) {
|
public DictionaryResponse create(@Valid @RequestBody CreateDictionaryRequest req) {
|
||||||
|
DataScope newScope = req.scope();
|
||||||
|
if (newScope != null && !scopeContext.canAccess(newScope)) {
|
||||||
|
throw OrdinisException.forbidden(
|
||||||
|
"scope_insufficient",
|
||||||
|
"Недостаточно прав scope для создания словаря с scope=" + newScope + ".");
|
||||||
|
}
|
||||||
return DictionaryResponse.from(service.create(req));
|
return DictionaryResponse.from(service.create(req));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -47,6 +66,18 @@ public class DictionaryDefinitionController {
|
|||||||
public DictionaryResponse update(
|
public DictionaryResponse update(
|
||||||
@PathVariable String name,
|
@PathVariable String name,
|
||||||
@Valid @RequestBody CreateDictionaryRequest req) {
|
@Valid @RequestBody CreateDictionaryRequest req) {
|
||||||
|
var existing = service.findByName(name);
|
||||||
|
if (!scopeContext.canAccess(existing.getScope())) {
|
||||||
|
throw OrdinisException.notFound(
|
||||||
|
"dictionary_not_found", "Dictionary not found: " + name);
|
||||||
|
}
|
||||||
|
DataScope newScope = req.scope();
|
||||||
|
if (newScope != null && newScope != existing.getScope()
|
||||||
|
&& !scopeContext.canAccess(newScope)) {
|
||||||
|
throw OrdinisException.forbidden(
|
||||||
|
"scope_insufficient",
|
||||||
|
"Недостаточно прав scope для смены словаря на scope=" + newScope + ".");
|
||||||
|
}
|
||||||
return DictionaryResponse.from(service.updateSchema(name, req));
|
return DictionaryResponse.from(service.updateSchema(name, req));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+42
-2
@@ -1,12 +1,14 @@
|
|||||||
package cloud.nstart.terravault.ordinis.restapi.web;
|
package cloud.nstart.terravault.ordinis.restapi.web;
|
||||||
|
|
||||||
|
import cloud.nstart.terravault.ordinis.auth.ScopeContext;
|
||||||
|
import cloud.nstart.terravault.ordinis.domain.DataScope;
|
||||||
import cloud.nstart.terravault.ordinis.restapi.dto.CreateRecordRequest;
|
import cloud.nstart.terravault.ordinis.restapi.dto.CreateRecordRequest;
|
||||||
import cloud.nstart.terravault.ordinis.restapi.dto.RecordResponse;
|
import cloud.nstart.terravault.ordinis.restapi.dto.RecordResponse;
|
||||||
import cloud.nstart.terravault.ordinis.restapi.error.OrdinisException;
|
import cloud.nstart.terravault.ordinis.restapi.error.OrdinisException;
|
||||||
|
import cloud.nstart.terravault.ordinis.restapi.service.DictionaryDefinitionService;
|
||||||
import cloud.nstart.terravault.ordinis.restapi.service.DictionaryRecordService;
|
import cloud.nstart.terravault.ordinis.restapi.service.DictionaryRecordService;
|
||||||
import jakarta.validation.Valid;
|
import jakarta.validation.Valid;
|
||||||
import org.springframework.http.HttpStatus;
|
import org.springframework.http.HttpStatus;
|
||||||
import org.springframework.http.ResponseEntity;
|
|
||||||
import org.springframework.web.bind.annotation.DeleteMapping;
|
import org.springframework.web.bind.annotation.DeleteMapping;
|
||||||
import org.springframework.web.bind.annotation.GetMapping;
|
import org.springframework.web.bind.annotation.GetMapping;
|
||||||
import org.springframework.web.bind.annotation.PathVariable;
|
import org.springframework.web.bind.annotation.PathVariable;
|
||||||
@@ -26,9 +28,16 @@ import java.util.List;
|
|||||||
public class DictionaryRecordController {
|
public class DictionaryRecordController {
|
||||||
|
|
||||||
private final DictionaryRecordService service;
|
private final DictionaryRecordService service;
|
||||||
|
private final DictionaryDefinitionService definitionService;
|
||||||
|
private final ScopeContext scopeContext;
|
||||||
|
|
||||||
public DictionaryRecordController(DictionaryRecordService service) {
|
public DictionaryRecordController(
|
||||||
|
DictionaryRecordService service,
|
||||||
|
DictionaryDefinitionService definitionService,
|
||||||
|
ScopeContext scopeContext) {
|
||||||
this.service = service;
|
this.service = service;
|
||||||
|
this.definitionService = definitionService;
|
||||||
|
this.scopeContext = scopeContext;
|
||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping("/{businessKey}")
|
@GetMapping("/{businessKey}")
|
||||||
@@ -36,6 +45,7 @@ public class DictionaryRecordController {
|
|||||||
@PathVariable String name,
|
@PathVariable String name,
|
||||||
@PathVariable String businessKey,
|
@PathVariable String businessKey,
|
||||||
@RequestParam(required = false) OffsetDateTime at) {
|
@RequestParam(required = false) OffsetDateTime at) {
|
||||||
|
requireReadAccess(name);
|
||||||
OffsetDateTime when = at == null ? OffsetDateTime.now() : at;
|
OffsetDateTime when = at == null ? OffsetDateTime.now() : at;
|
||||||
return service.findActive(name, businessKey, when)
|
return service.findActive(name, businessKey, when)
|
||||||
.map(RecordResponse::from)
|
.map(RecordResponse::from)
|
||||||
@@ -48,6 +58,7 @@ public class DictionaryRecordController {
|
|||||||
public List<RecordResponse> getHistory(
|
public List<RecordResponse> getHistory(
|
||||||
@PathVariable String name,
|
@PathVariable String name,
|
||||||
@PathVariable String businessKey) {
|
@PathVariable String businessKey) {
|
||||||
|
requireReadAccess(name);
|
||||||
return service.findHistory(name, businessKey).stream().map(RecordResponse::from).toList();
|
return service.findHistory(name, businessKey).stream().map(RecordResponse::from).toList();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -56,6 +67,7 @@ public class DictionaryRecordController {
|
|||||||
public RecordResponse create(
|
public RecordResponse create(
|
||||||
@PathVariable String name,
|
@PathVariable String name,
|
||||||
@Valid @RequestBody CreateRecordRequest req) {
|
@Valid @RequestBody CreateRecordRequest req) {
|
||||||
|
requireWriteAccess(name);
|
||||||
return RecordResponse.from(service.create(name, req));
|
return RecordResponse.from(service.create(name, req));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -64,6 +76,7 @@ public class DictionaryRecordController {
|
|||||||
@PathVariable String name,
|
@PathVariable String name,
|
||||||
@PathVariable String businessKey,
|
@PathVariable String businessKey,
|
||||||
@Valid @RequestBody CreateRecordRequest req) {
|
@Valid @RequestBody CreateRecordRequest req) {
|
||||||
|
requireWriteAccess(name);
|
||||||
return RecordResponse.from(service.update(name, businessKey, req));
|
return RecordResponse.from(service.update(name, businessKey, req));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -74,6 +87,33 @@ public class DictionaryRecordController {
|
|||||||
@PathVariable String businessKey,
|
@PathVariable String businessKey,
|
||||||
@RequestParam(required = false) OffsetDateTime at,
|
@RequestParam(required = false) OffsetDateTime at,
|
||||||
@RequestParam(required = false) String reason) {
|
@RequestParam(required = false) String reason) {
|
||||||
|
requireWriteAccess(name);
|
||||||
service.close(name, businessKey, at, reason);
|
service.close(name, businessKey, at, reason);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 404 если scope словаря недоступен текущему пользователю — намеренно скрываем
|
||||||
|
* существование записи (security through obscurity), как делает read-api.
|
||||||
|
*/
|
||||||
|
private void requireReadAccess(String dictionaryName) {
|
||||||
|
DataScope dictScope = definitionService.findByName(dictionaryName).getScope();
|
||||||
|
if (!scopeContext.canAccess(dictScope)) {
|
||||||
|
throw OrdinisException.notFound(
|
||||||
|
"dictionary_not_found", "Dictionary not found: " + dictionaryName);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 403 для записи: пользователь должен иметь scope ≥ scope словаря. PUBLIC-юзер
|
||||||
|
* не имеет права создавать/менять/закрывать INTERNAL/RESTRICTED записи даже
|
||||||
|
* если знает имя словаря.
|
||||||
|
*/
|
||||||
|
private void requireWriteAccess(String dictionaryName) {
|
||||||
|
DataScope dictScope = definitionService.findByName(dictionaryName).getScope();
|
||||||
|
if (!scopeContext.canAccess(dictScope)) {
|
||||||
|
throw OrdinisException.forbidden(
|
||||||
|
"scope_insufficient",
|
||||||
|
"Недостаточно прав scope для записи в словарь '" + dictionaryName + "'.");
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user