Skip to content

Домашняя работа 8#12

Open
khitao wants to merge 6 commits into
home_work_7from
home_work_8
Open

Домашняя работа 8#12
khitao wants to merge 6 commits into
home_work_7from
home_work_8

Conversation

@khitao

@khitao khitao commented Nov 6, 2023

Copy link
Copy Markdown
Owner

Добавил в проект зависимость spring-boot-starter-security и реализовал Basic Authentication

Добавил регистрацию пользователей в приложении и организовал хранение логинов/паролей в БД.

Пользователь с ролью ADMIN создается в скрипте миграции.

Тесты минус

Тесты не получились( даже к существующим добавил @WithMockUser(roles = "ADMIN") и там где POST запросы тесты ломаются на get все хорошо (может что-то не вижу или не понимаю)(((

Добавил в проект зависимость spring-boot-starter-security и реализовал Basic Authentication

Добавил регистрацию пользователей в приложении и организовал хранение логинов/паролей в БД.

Пользователь с ролью ADMIN создается в скрипте миграции.

Тесты минус
@khitao khitao changed the title Домашняя работа 7 Домашняя работа 8 Nov 6, 2023

@madoed madoed left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Доделать и ответить на комментарии.
Включение Method security функционала, скорее всего поможет и починить тесты.

Comment on lines +38 to +39
.csrf(AbstractHttpConfigurer::disable)
.cors(AbstractHttpConfigurer::disable)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Какие аргументы в пользу отключения CSRF и CORS?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

использую только localhost поэтому, как мне кажется ограничения CORS и CSRF не нужны, так как атаки CSRF в основном угрожают приложениям, доступным в Интернете + нет важных данных + проще тестировать

}

@GetMapping("/{location}")
@PreAuthorize("hasAnyRole('USER', 'ADMIN')")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Чтобы использовать эту аннотацию нужно включить её поддержку, например через @EnableMethodSecurity

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Добавил аннотацию @EnableMethodSecurity и Тесты-mОтключил Security от прошлых тестов при помощи addFilters = false

@madoed madoed left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Нужно переделать и починить тесты


@SpringBootTest
@AutoConfigureMockMvc(printOnlyOnFailure = false)
public class AuthenticationControllerTest {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Тест не рабочий. Нет поднятия тестовой БД

Ниже используется UserRepository, который требует наличия подключения к БД

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Есть отдельный у тебя класс

SpringBootAppApplicationTests - там объявление бд - это надо использовать здесь (через наследование)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ну и остальные тесты требующие БД (а это все, которые поднимают весь контекст через @SpringBootTest) по той же причине падают

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Они у меня почему-то запустились 🤔 наверное, поэтому я и не подумал, что нужна бд
Хотя думал, что странно почему запускаются

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Снимок экрана 2023-11-13 в 13 36 51

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Снимок экрана 2023-11-13 в 17 05 01 После исправлений

@madoed madoed Nov 15, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

А покажи свои application.properties вижу они отличны, от того что в гитхабе

Есть предположение, что ты там указываешь бд (например h2) и верный api ключ. Тесты должны работать на том application.properties, который в репозитории.

Если надо изменить, то добавляешь в test application-test.properties и подвязываешь к тестам (https://www.baeldung.com/spring-tests-override-properties)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Там просто api ключ

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Снимок экрана 2023-11-15 в 11 17 53

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Я что-то почитал и почти везде пишут, что единственный способ передать ключ(это передать лично) На гит хаб нельзя же пушить

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Не наследовал от SpringBootAppApplicationTests тк не используется бд

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Используется, у тебя @SpringBootTest поднимает весь контекст и ниже используется

    @SpyBean
    private WeatherService weatherService;

Который использует реализацию сервиса без моков

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Там в самом сервисе в мапу идет сохранение ( я это имел ввиду)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Добавил тк почему-то при повторном запуске тестов в BUILD файле возникал конфликт (дублировались классы тестов) после добавления ошибка исчезла

@madoed

madoed commented Nov 16, 2023

Copy link
Copy Markdown

Все заработало кроме
getCurrentWeather_Test_status_isOk_ADMIN
getCurrentWeather_Test_status_isOk_USER

Сюда как раз надо добавить моку для внешней интеграции

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