Skip to content

Restrict page sections API to admins [issue 1055]#1067

Open
Levasey wants to merge 5 commits intohexlet-volunteers:mainfrom
Levasey:iss1055-page-section-authorization
Open

Restrict page sections API to admins [issue 1055]#1067
Levasey wants to merge 5 commits intohexlet-volunteers:mainfrom
Levasey:iss1055-page-section-authorization

Conversation

@Levasey
Copy link
Copy Markdown

@Levasey Levasey commented Mar 28, 2026

Summary
Restricts /api/pages/sections to users with the ADMIN role so unauthenticated and non-admin clients cannot call these endpoints.

Changes

  1. PageSectionController: class-level @PreAuthorize("hasRole('ADMIN')"), same pattern as other admin controllers.
  2. SecurityConfig: hasRole("ADMIN") for /api/pages/sections and /api/pages/sections/** so OAuth2 Resource Server returns 401 when there is no JWT (method security alone would often yield 403 for anonymous requests on permitAll() routes).
  3. PageSectionControllerTest: JWT cookie for admin on successful flows; tests for 401 (no token) and 403 (candidate user) on POST.

.cookie(new Cookie("access_token", candidateToken))
.header("X-Inertia", "true")
.contentType(MediaType.APPLICATION_JSON)
.content(om.writeValueAsString(dto)))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Переименуй, плз, om в objectMapper

@Test
public void testPostUnauthorizedReturns401() throws Exception {

var dto = new PageSectionCreateDTO();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

создание dto дублируется в двух тестах, можно вынести в отдельный метод. И заменить поля на более осмысленные, например dto.setPageKey("somePageKey") - так будет понятно, что в данном контексте неважно, что в этих полях, и при этом глаз не будет цепляться за "x", "y" и прочее.

public void setUp() {

userRepository.deleteAll();
var admin = User.builder()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше готовить тестовые данные внутри теста и только те, которые действительно нужны для выполнения этого теста

private PageSection section2;

@BeforeEach
public void setUp() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вижу, что часть логики про подготовку тестовых данных ты вынес в тесты, но паттерн Arrange-Act-Assert все еще нарушен. Исправь, плз. Также каждый тест предлагаю разбить на секции
//given
//when
//then

.andExpect(status().isOk())
.andReturn()
.getResponse();
// given
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нужно, чтобы каждый тест был изолированным, для этого делают Arrange-часть каждого теста явной, а не спрятанной в общем setUp(). В данном случае метод setUp() нужно удалить и перенести создание тестовых данныхв каждый тест, соблюдая условие, что тест готовит тоько те тестовые данные, которые ему действиетльно нужны.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants